protoc-gen-go: fix generation of public imports #549
Merged
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
(*Generator).DefaultPackageName returns either a qualified ("pkg.name") or
unqualified ("name") name.
Prior to commit 9d4962b, it based this decision on whether the
name was in the package we're generating code for. After this
commit, it bases this on whether the name is in the file we're
currently generating code for. i.e., what was once an invariant
across all files is no longer.
This is mostly fine, except for one case: We run the generator
across files we do not generate code for, in order to build a list
of symbols used to generate code for public imports. This change
caused some of those symbols to no longer be qualified.
In particular, this causes bad generation for wrapper types of
publicly imported messages that contain a field whose type is
another message in the same publicly imported package.
Fix this. Clean up some effectively dead code in SetPackageNames
at the same time. Fix tests to catch this case. (It wouldn't have
caught it anyway, but imp.proto is supposed to contain a public import.
This seems to have been lost when exporting code to git. Fixed.)
dsnet
approved these changes
Mar 13, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
(*Generator).DefaultPackageName returns either a qualified ("pkg.name") or
unqualified ("name") name.
Prior to commit 9d4962b, it based this decision on whether the
name was in the package we're generating code for. After this
commit, it bases this on whether the name is in the file we're
currently generating code for. i.e., what was once an invariant
across all files is no longer.
This is mostly fine, except for one case: We run the generator
across files we do not generate code for, in order to build a list
of symbols used to generate code for public imports. This change
caused some of those symbols to no longer be qualified.
In particular, this causes bad generation for wrapper types of
publicly imported messages that contain a field whose type is
another message in the same publicly imported package.
Fix this. Clean up some effectively dead code in SetPackageNames
at the same time. Fix tests to catch this case. (It wouldn't have
caught it anyway, but imp.proto is supposed to contain a public import.
This seems to have been lost when exporting code to git. Fixed.)