fix: qualify Option in nested scopes to prevent prelude shadowing (#36)#39
Merged
iainmcgin merged 3 commits intoanthropics:mainfrom Apr 23, 2026
Merged
Conversation
th0114nd
force-pushed
the
fix/option-prelude-shadow
branch
from
3dc06f5 to
a19bc77
Compare
th0114nd
force-pushed
the
fix/option-prelude-shadow
branch
from
a19bc77 to
71bc6ea
Compare
Contributor
Author
|
Hi @iainmcgin, addressed the lint failures from https://github.com/anthropics/buffa/actions/runs/24587343683/job/72158628513 with 42122b0 |
…thropics#36) When a proto message named `Option` is nested inside another message, the generated `pub struct Option` shadows `core::option::Option` in the parent module scope. This caused struct fields like `pub value: Option<option::Value>` to resolve to the proto struct (0 generic args) instead of the standard library type. Fix: extend ImportResolver with `child_for_message()` to propagate blocked prelude names through the `use super::*` module chain. Each recursive `generate_message` call now receives a child resolver that accounts for sibling types in its module scope. Also qualify bare `Option` in custom deserialize temporaries and `skip_serializing_if` predicates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Made-with: Cursor
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The unit tests in tests/naming.rs only checked codegen output substrings. This adds: - buffa-test/protos/prelude_shadow.proto built with views + JSON, covering the gh#36 nested case, a sibling-propagation case (Outer.Option + Outer.Middle.Inner where Inner must qualify Option via two `use super::*` hops), and the top-level case under JSON. Compilation is the assertion. - Two runtime tests in tests/collision.rs that round-trip the generated types over binary and JSON. - A unit test in tests/naming.rs for the sibling-propagation case, verifying child_for_message inherits the parent blocked set. Audited all codegen modules: view.rs, enumeration.rs, impl_message.rs, extension.rs, impl_text.rs already emit ::core::option::Option uniformly, and Result/Default/Debug are universally ::core::-qualified, so no additional qualification sites were needed. Co-authored-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
iainmcgin
force-pushed
the
fix/option-prelude-shadow
branch
from
42122b0 to
6964025
Compare
iainmcgin
approved these changes
Apr 23, 2026
Collaborator
iainmcgin
left a comment
iainmcgin
left a comment
There was a problem hiding this comment.
[claude code] Audited all Option<...> emission sites across the codegen crate — view.rs/enumeration.rs/impl_message.rs/extension.rs/impl_text.rs already use fully-qualified ::core::option::Option, so the resolver threading in message.rs is sufficient. Result/Default/Debug are universally ::core::-qualified already, so PRELUDE_NAMES = ["Option"] is correct.
Added an end-to-end regression proto (buffa-test/protos/prelude_shadow.proto, built with views + JSON) covering top-level, nested (gh#36 reproducer), and sibling-propagation (Outer.Option + Outer.Middle.Inner) cases, plus a unit test verifying child_for_message inherits the parent blocked set. Rebased onto current main.
LGTM, thanks for the fix.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Summary
Fixes #36 — a proto message named
Optionshadowscore::option::Option, causing type errors on oneof fields in nested scopes.ImportResolverwithchild_for_message()to propagate blocked prelude names through theuse super::*module chaingenerate_messagecall now receives a child resolver that accounts for sibling types in its module scopeOptionin custom deserialize temporaries andskip_serializing_ifpredicatesOptionmessage scenariosTest plan
cargo test -p buffa-codegen(239 tests pass)cargo test --workspace(all pass)cargo clippy --workspace --all-targets(clean)Made with Cursor