Emit a clear error instead of crashing on duplicate generated schema names#907
Open
adityasingh2400 wants to merge 1 commit into
Open
Emit a clear error instead of crashing on duplicate generated schema names#907adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
…names
When the idiomatic naming strategy maps two distinct OpenAPI schema names
(for example NullTime and nullTime) to the same Swift type name, the
generator trapped while boxing recursive types. The boxing code built a
name-to-node lookup with Dictionary(uniqueKeysWithValues:), which calls
fatalError on duplicate keys, surfacing as
Fatal error: Duplicate values for key: 'NullTime'
with no indication of which document or strategy caused it.
Detect the collision before constructing the lookup and emit an error
diagnostic that names the conflicting type and points at the documented
workarounds (switch namingStrategy to defensive, or add a nameOverrides
entry). The lookup now also tolerates duplicate keys so the diagnostic is
the failure path rather than a trap, regardless of the diagnostic
collector in use.
Adds a regression test that translates two schemas which collide under the
idiomatic strategy and asserts the error is emitted; it crashed before this
change. Also covers that distinct idiomatic names emit no error.
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
1 participant
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.
When the idiomatic naming strategy maps two distinct OpenAPI schema names to the same generated Swift type name, the generator crashes instead of reporting the problem. This is the crash tracked in #854: a document with both
NullTimeandnullTimeunder#/components/schemasproducesRoot cause is in
boxRecursiveTypes, which builds a name-to-node lookup for recursion detection withDictionary(uniqueKeysWithValues:). That initializer traps on duplicate keys, and the idiomatic strategy can produce two declarations sharing a name even though the OpenAPI names were distinct. The defensive strategy keeps them distinct, which is why the crash only shows up with idiomatic naming.The fix detects the collision before building the lookup and emits an error diagnostic that names the conflicting type and points at the documented workarounds (switch
namingStrategytodefensive, or add anameOverridesentry). TheConfiguring-the-generatorarticle already notes that idiomatic naming "might produce name conflicts (in that case, switch back to defensive)", so this turns an undiagnosed trap into the guidance that already exists. The lookup construction now also tolerates duplicate keys, so the error diagnostic is the failure path rather than a fatal trap, independent of which diagnostic collector is in use.Verification: running the generator with
namingStrategy: idiomaticon a spec containingNullTimeandnullTimepreviously crashed; it now exits non-zero withI added a regression test in
Test_translateSchemasthat translates two schemas colliding under the idiomatic strategy and asserts the error is emitted. With the previous code that test aborts with theDuplicate values for keytrap (signal 5); with the fix it passes. A second test confirms that distinct idiomatic names emit no error. The fullOpenAPIGeneratorCoreTeststarget (115 tests) and the reference tests (114 tests) pass, so generated output for valid documents is unchanged, including recursive-type boxing.