Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated schema files can include C++ reserved keywords #209

Closed
AndrewLipscomb opened this issue Feb 2, 2022 · 1 comment
Closed

Generated schema files can include C++ reserved keywords #209

AndrewLipscomb opened this issue Feb 2, 2022 · 1 comment
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@AndrewLipscomb
Copy link
Contributor

AndrewLipscomb commented Feb 2, 2022

For a schema file

input DocumentFields {
    display: String
    default: Boolean
}

You end up with the generated member for DocumentFields

	std::optional<bool> default {};

Which is a no-go due to the default keyword.

While in our case we have the opportunity to change this schema - obviously thats not available to everyone.

I've seen other libs work around the keywords issue by renaming keys to be default_ or similar - boost::hana does this for default_ and case_

I've only dug into the library just now so unsure how the generator works under the hood - this might be a hard one to work around in the generator code.

@wravery
Copy link
Contributor

wravery commented Feb 2, 2022

Good catch. There is a mechanism for safe C++ names in the generator, it handles the leading __ or _ followed by a capital letter: SchemaLoader::getSafeCppName.

Once it makes a replacement, it stores the replacement in a static map called safeNames, so it doesn't redo the regex_replace. In retrospect, it might make sense to store the misses for regex_search as well, since an unordered_map lookup should be much faster, and the number of unique names in the GraphQL schema is not that large compared to the parsed AST.

One simple way to handle additional keywords would be pre-fill the safeNames map with entries for all known C++ keywords and their replacements if they appear in the GraphQL schema.

@wravery wravery added bug Something isn't working good first issue Good for newcomers labels Feb 2, 2022
@wravery wravery self-assigned this Feb 6, 2022
@wravery wravery closed this as completed in 29cf498 Feb 6, 2022
wravery added a commit that referenced this issue Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants