fix(go,python/java): bad code for members with same name with different casing #2699
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 13 commits
March 14, 2021 12:34
Go pacmak will reimplement methods and properties in case the class has more than a single "base" (interface/class). The Go code generator failed to include these members when determining which imports to generate. Add a calc test fixture to verify (failed without this change). Fixes #2647
When a foreign type is referenced within a complex type (array/list), it wasn't included in the import rendering. Fixes #2689
…ae/fix-go-imports-from-complex-types
…s-from-complex-types
…same base name If a struct has two parent structs with the same base name (but different packages), the emitted conversion function will have the same name (`ToParentStruct` and `ToParentStruct`). Since we are not even sure that these base conversion functions are required, omit them for now and we can decide to restore them at a later stage if the use case is clearer. Fixes #2692
It is common in TypeScript to use `_` as an argument name if the argument is not used. This is an invalid name in Go, so replace it with `_arg`. Fixes #2530
…pitalization In TypeScript, it is possible to give two members (methods/properties) the same name but with different capitalization. This results in duplicate members in Go and Python. In Go, where the member is expected to be PascalCase, use the same conversion used in .NET, in which we only capitalize the first letter (assuming TypeScript uses camelCase). This offers more tolerance. In Python, where members use snake_case, we implemented a different heuristic in which we only render the non-deprecated member and fail if there is more than one deprecated member. Fixes #2508
eladb
requested review from
RomainMuller and
MrArnoldPalmer
and removed request for
RomainMuller
Base automatically changed from
benisrae/replace_underscore
to
benisrae/fix-duplicate-conversions
March 15, 2021 12:52
RomainMuller
approved these changes
Mar 15, 2021
iliapolo
reviewed
Mar 15, 2021
added 2 commits
March 16, 2021 11:43
eladb
changed the title
iliapolo
reviewed
Mar 16, 2021
iliapolo
approved these changes
Mar 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In TypeScript, it is possible to give two members (methods/properties) the same name but with different capitalization. This results in duplicate members in Go and Python.
In Go, where the member is expected to be PascalCase, use the same conversion used in .NET, in which we only capitalize the first letter (assuming TypeScript uses camelCase). This offers more tolerance.
In Python, where members use snake_case, we implemented a different heuristic in which we only render the non-deprecated member and fail if there is more than one deprecated member.
In Java, this issue existed only for property names.
Fixes #2508
BREAKING CHANGE: if multiple members have the same name with different capitalization, only one is allowed to be non-deprecated. This will currently only manifest when producing python bindings, but will be added as a jsii compiler error in the future.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.