Qualify nested-type refs shadowed by a sibling of the same simple name#118
Closed
ghostdogpr wants to merge 1 commit into
Closed
Qualify nested-type refs shadowed by a sibling of the same simple name#118ghostdogpr wants to merge 1 commit into
ghostdogpr wants to merge 1 commit into
Conversation
qualifyReferences emitted a bare simple name for any nested-type reference whose target parent enclosed the current scope, ignoring proto3's innermost-scope-first name resolution. When the current message had its own nested type of the same simple name, the bare name bound to that sibling instead of the intended type, so the rendered .proto silently pointed the field at the wrong message. Resolve the bare name the way proto3 would (scope outward to root) and only keep it when it lands on the intended target; otherwise fall back to the qualified path.
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.
Problem
qualifyReferences(used byProtobufCodec.renderandService.render) rendered a reference to a nested type as just its bare simple name whenever the referencing scope was enclosed by the target's parent:This ignores proto3's innermost-scope-first name resolution. If the current message has its own nested type of the same simple name, that inner type shadows the intended one, so the emitted bare name binds to the wrong message. The
.protostill parses, but the field now points at a different type — a silently corrupted schema.Concrete case
Outerhas nestedOuter.Innerand nestedOuter.Deep;Outer.Deephas its own nestedOuter.Deep.Inner. A field inOuter.Deepreferencing the outerOuter.Innerrendered as bareInner, which under proto3 rebinds toOuter.Deep.Inner.Fix
Resolve the bare name the way proto3 would — from
scopeoutward to the root, first same-named declared type wins — and keep the bare name only when it lands on the intended target. Otherwise fall back to the qualified path (the existing out-of-scope rendering).Test
ProtobufRenderSpec: a sibling-shadowing scenario asserts the ref is qualified toOuter.Inner. Red before the fix (rendered bareInner), green after. Full core suite (138) and grpcRenderSpec/FileDescriptorSpec(43) remain green.