feat: add AddPackageNamespaces option to disambiguate same-name types across packages - #194
Open
AZERDSQ131 wants to merge 1 commit into
Open
feat: add AddPackageNamespaces option to disambiguate same-name types across packages#194AZERDSQ131 wants to merge 1 commit into
AZERDSQ131 wants to merge 1 commit into
Conversation
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.
Fixes the case described in #42: two unrelated types with the same name (e.g.
http.Configandtcp.Config) declared in different packages collide on the same$defs/$refentry when both end up referenced in the same schema, silently producing an incorrect schema for one of them.As suggested in the issue thread, this adds an opt-in
Reflector.AddPackageNamespaces boolfield. When set,typeNameprefixes the type's name with the last segment of its package path (e.g.httpconf.Config,tcpconf.Config) instead of the bare type name, so colliding names get distinct$defsentries. It only applies whenNamerdoesn't already return a name for the type, and is fully opt-in (defaultfalse), so existing output is unaffected unless explicitly enabled.Note: this repo previously had a similar
FullyQualifyTypeNamesoption that was removed in 3330865 in favor ofNamer/DoNotReference. That option always used the full package import path though; this one is scoped to the package's base name only (as the issue's proposed direction describes), and is documented as an additional option alongsideNamer/DoNotReferencerather than a replacement for either — updated the README's historical note and added a new "Type Naming and Conflicts" section accordingly.Changes
reflect.go: newAddPackageNamespacesfield onReflector;typeNameprefixes withpath.Base(t.PkgPath())when enabled andNamerdidn't provide a name.internal/nsfixture/{httpconf,tcpconf}: two fixture packages, each with a collidingConfigtype, used to reproduce the exact scenario from if we have same struct name, then we will got the same$ref#42 in a test.reflect_test.go:TestAddPackageNamespacesDisambiguatesSameNameStructs— verifies distinct$defs/$refentries with the option enabled, and confirms the pre-existing collision still happens with it disabled (default), as a regression guard.README.md: documents the new option and cross-references it from the historical note about the removedFullyQualifyTypeNames.Testing
go build ./...,go vet ./...,gofmt -l .(clean) andgo test ./...all pass.Fixes #42