Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Member
|
thanks for the PR cc @OpenAPITools/generator-core-team |
This branch has not been deployed
This file contains hidden or 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
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.
postProcessEnumVarsrenames duplicate enum names by appending a counter, but new name can already be taken by another entry.Etc/GMT-1becomesEtcGmt12andEtc/GMT+12already has that name after sanitizing, so full timezone enum gives TS1117 in typescript-* and duplicate constantETC_GMT_122in javaNow dedupe repeats while names are still not unqiue. Second pass runs only when duplicates are left, so output that was valid before is not changed
Added unit test in
DefaultCodegenTestwith names ofEtc/GMT+12,Etc/GMT+1,Etc/GMT0,Etc/GMT-1andEtc/GMT-12, it fails on master and passes with the fix. Generated typescript-angular and java enums from the full timezone list in #17277 have no duplicate namesRan all configs with
./bin/generate-samples.sh ./bin/configs/*.yaml, no sample changes../mvnw clean packagepasses except Kotlin generator tests, those fail on JDK 25 on my machine (embedded Kotlin compiler)On earlier attempt #22087 answer was
enumPropertyNamingReplaceSpecialChar. That option is typescript only, so java and every other generator has no workaround exceptenumNameMappingsorx-enum-varnames. This change doesn't rename anything that was valid, so it is not breaking and needs no optionFixes #22074
Fixes #17277
PR checklist
Summary by cubic
Fixes enum variable name deduplication so renamed entries can no longer collide with existing names.
postProcessEnumVarspreviously appended a counter on collision, but the new name could already be taken — e.g.,Etc/GMT-1becomesEtcGmt12, which collides with the sanitized name ofEtc/GMT+12— causing TS1117 errors in TypeScript generators and duplicate constants in Java. The pass now repeats until names are unique. Output that was already valid is unchanged, so this isn't breaking and needs no new option. Fixes #22074 and #17277.Written for commit 991fa3b. Summary will update on new commits.