-
Notifications
You must be signed in to change notification settings - Fork 245
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
jsii should reject a member name with the same name as the class #1880
Labels
bug
This issue is a bug.
effort/small
Small work item – less than a day of effort
needs-triage
This issue or PR still needs to be triaged.
p1
Comments
rix0rrr
added
bug
This issue is a bug.
p1
needs-triage
This issue or PR still needs to be triaged.
labels
Aug 12, 2020
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this issue
Aug 12, 2020
The addition of the new `construct` member leads to problems in the C# code generation, where it would properly be called `Construct.Construct`: a member may not have the same name as the class, because that is the name of the class constructor (see aws/jsii#1880). Our build is currently broken because of this. Revert the renames to unblock the build, giving us the opportunity to tackle this problem afresh next week. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
RomainMuller
added a commit
that referenced
this issue
Aug 17, 2020
It is illegal in C# to name a class' member (method, property) with the same name as the class itself (even if the member is static). Since both class and member names are pascal-cased in C#, this means a class' members cannot share the same PascalCased representation. This adds a compile-time validation for this condition, effectively prohibiting member names to have the same PascalCased representation as their declaring class' name. Fixes #1880
RomainMuller
added a commit
that referenced
this issue
Aug 21, 2020
It is illegal in C# to name a class' member (method, property) with the same name as the class itself (even if the member is static). Since both class and member names are pascal-cased in C#, this means a class' members cannot share the same PascalCased representation. This adds a compile-time validation for this condition, effectively prohibiting member names to have the same PascalCased representation as their declaring class' name. Fixes #1880
RomainMuller
added a commit
that referenced
this issue
Aug 24, 2020
…1903) It is illegal in C# to name a class' member (method, property) with the same name as the class itself (even if the member is static). Since both class and member names are pascal-cased in C#, this means a class' members cannot share the same PascalCased representation. This adds a compile-time validation for this condition, effectively calling out member names to have the same PascalCased representation as their declaring class' name. This is currently only a warning, as this is "made to work" by altering the type name by appending `_` at the end of it, which is ugly and dangerous but works, and is currently done in several places). As all warnings, this turns into an error when operating under `--strict`, and future work (i.e: #1931) will allow more granular configuration of these errors, so we can hopefully opt all new codebases into the strict behavior and eventually drop the slugification. Fixes #1880
Closed
2 tasks
eladb
pushed a commit
to cdklabs/decdk
that referenced
this issue
Jan 18, 2022
The addition of the new `construct` member leads to problems in the C# code generation, where it would properly be called `Construct.Construct`: a member may not have the same name as the class, because that is the name of the class constructor (see aws/jsii#1880). Our build is currently broken because of this. Revert the renames to unblock the build, giving us the opportunity to tackle this problem afresh next week. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
This issue is a bug.
effort/small
Small work item – less than a day of effort
needs-triage
This issue or PR still needs to be triaged.
p1
🐛 Bug Report
This has just blocked our pipelines and wasted a lot of Elad's work.
In aws/aws-cdk#9557 and aws/aws-cdk#9584 we added a property called
construct
to the classConstruct
.In C#, this property name gets renamed to
Construct
, so that the member is now calledConstruct::Construct
. However, that name is not allowed because it is reserved for the constructor (which this is not).In Java this might be a theoretical issue, but it's not a practical one because class names are always pascal-cased while member names are always camel-cased.
The C# generator thought about this and tries to do something "smart": it renames the class to
Construct_
, which makes the property name allowed again. However, we now broke backwards compatibility with existing code by renaming a class, which is definitely not the intended behavior.The only solution is to completely disallow this occurrence in the first place:
This also means that there is a danger in specifying interfaces -- any of the member names in an interface cannot be used as the class name of the implementing class. Yes, there is a way in C# to still properly implement that interface, but it comes with other undesirable consequences attached:
It's probably just pragmatic to disallow this occurrence, regardless of whether the member comes from an interface. It will simplify our analysis to not have to deal with the special case, and devs can always work around any potential future limitations on their own (which should be rare anyway).
Affected Languages
TypeScript
orJavascript
Python
Java
C#
,F#
, ...)The text was updated successfully, but these errors were encountered: