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

[csharp] ctor params should always be camelCase #7519

Merged
merged 5 commits into from
Feb 6, 2018

Conversation

jimschubert
Copy link
Contributor

@jimschubert jimschubert commented Jan 28, 2018

After PR #6305, var names defaulted to PascalCase results in constructor
arguments also being PacalCase. Model properties and constructor
arguments have no reason to be the same case, and in fact may cause
issues (name = name will result in a compilation error).

This commit forces all constructor params in models to lowerCase.

This is a necessary change, for instance, if client SDK consumers assign
using named args:

var a = new Model(first = "", second = "")

The PacalCase default and update to constructor arg casing will break
existing consumers of the client.

See #7070 for more details and discussion.

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

/cc @mandrean

After PR swagger-api#6305, var names defaulted to PascalCase results in constructor
arguments also being PacalCase. Model properties and constructor
arguments have no reason to be the same case, and in fact may cause
issues (`name = name` will result in a compilation error).

This commit forces all constructor params in models to lowerCase.

This is a necessary change, for instance, if client SDK consumers assign
using named args:

var a = new Model(first = "", second = "")

The PacalCase default and update to constructor arg casing will break
existing consumers of the client.

See swagger-api#7070 for more details and discussion.
@jimschubert
Copy link
Contributor Author

NOTE: This change may be a breaking change for consumers using named constructor parameter assignments for 2.3.x releases. This PR reverts behavior to pre-2.3.x camel case args for constructors, regardless of model property naming strategy.

@jimschubert
Copy link
Contributor Author

AppVeyor failed because I'll need to account for escaping reserved word. But the approach can be reviewed. I'll comment here when it's fixed.

@jimschubert
Copy link
Contributor Author

I've updated with a parameter-specific lambda for camel casing words. This lambda accounts for rules in escaping reserved words as well as applying some additional logic related to parameter names (e.g. names can't start with numbers). These are all provided by the calling generator.

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

Successfully merging this pull request may close these issues.

2 participants