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

[BUG][C++] Reserved keywords are escaped via underscore prefix, resulting in compile failure on MSVC 14 #5268

Closed
5 of 6 tasks
alexweav opened this issue Feb 10, 2020 · 1 comment · Fixed by #5269
Closed
5 of 6 tasks

Comments

@alexweav
Copy link
Contributor

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix (example)
Description

In code generated by the cpprest client generator, we use the keyword inline is a query parameter, which is reserved in C++. By default, reserved keywords are escaped by prefixing them with an underscore.

This is documented as bad practice in C++, as there may still be conflicts with the global namespace:

Also, all identifiers that contain a double underscore __ in any position and each identifier that begins with an underscore followed by an uppercase letter is always reserved and all identifiers that begin with an underscore are reserved for use as names in the global namespace.

See the sample OpenAPI file below. When generating the cpprest client for this file, the signature for the generated GET /things call in DefaultApi.h looks like

    pplx::task<std::vector<utility::string_t>> thingsGet(
        boost::optional<bool> _inline
    );

On MSVC 14, this actually causes a compile failure:

DefaultApi.h(60): error C2735: 'inline' keyword is not permitted in formal parameter type specifier

openapi-generator version

4.2.2

OpenAPI declaration file content or url
    openapi: 3.0.0
    info:
      title: Inline API
      version: 0.0.1
    paths:
      /things:
        get:
          summary: Returns a list of things.
          parameters:
            - in: query
              name: inline
              schema:
                type: boolean
              description: Matches a keyword in C++.
          responses:
            '200':    # status code
              description: A JSON array of things.
              content:
                application/json:
                  schema: 
                    type: array
                    items: 
                      type: string
Command line used for generation
path/to/java.exe -cp openapi-generator-cli.jar org.openapitools.codegen.OpenAPIGenerator generate -g cpp-restsdk --skip-validate-spec -i test.yml -o output
Steps to reproduce
  1. Generate the cpp-restsdk client for the file given above.
  2. Notice the underscores in the generated source.
  3. Compile using msvc-14. The argument _inline is rejected.
Related issues/PRs

I found various language-specific escaping issues, but none which were directly relevant to this.

Suggest a fix

We were able to work around this using templates and the preprocessor. We added:

#define _inline r_inline

to the api-header.mustache template file, effectively replacing the keyword via the preprocessor before it gets compiled.

However, I believe the generator itself should be modified so that it escapes reserved words in a manner which does not conflict with C++ conventions. I opted for r_, but anything similar can be used.

I will post a PR immediately following this issue with this fix, although if you think there are any better alternatives, please let me know. I would be happy to adjust as needed. Thanks!

@auto-labeler
Copy link

auto-labeler bot commented Feb 10, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant