Skip to content

protoc-gen-buffa: add unbox_oneof option - #392

Merged
iainmcgin merged 3 commits into
anthropics:mainfrom
fallintoplace:fix/protoc-gen-unbox-oneof-option
Sep 2, 2026
Merged

protoc-gen-buffa: add unbox_oneof option#392
iainmcgin merged 3 commits into
anthropics:mainfrom
fallintoplace:fix/protoc-gen-unbox-oneof-option

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Add repeatable unbox_oneof=<path> support to protoc-gen-buffa.
  • Normalize paths without a leading . like Config::unbox_oneof_in.
  • Support unbox_oneof=. for all non-recursive message/group variants.
  • Keep recursive-variant handling in the existing codegen validation.

Tests

  • cargo test -p protoc-gen-buffa unbox_oneof
  • task lint
  • task test
  • cargo test --workspace --all-features

No generated files or CI changes.

@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code]

The normalization matches buffa-build::unbox_oneof_in character for character and the guide row is accurate, but the option diverges from every sibling in this parser in ways that bite:

  • unbox_oneof= (empty value) silently means "unbox everything". Prepending . to an empty value yields the blanket rule. normalize_override_path exists to reject exactly this ("use '.' explicitly to match everything"), and exclude_package rejects it too; a trailing comma in opt: should not change the storage layout of every oneof in the schema. Use normalize_override_path(value.trim())? here, which also fixes the two smaller ones: the value is not trimmed (unbox_oneof= .a.B becomes ". .a.B" and matches nothing) and a trailing dot is not stripped (.my.pkg. never matches).
  • Name. buffa-build spells the path form unbox_oneof_in(&[..]) and reserves unbox_oneof() for the blanket, and the plugin's other path-scoped options are all *_in=. Most rows in the guide table are foo=true, so unbox_oneof=true is the natural guess, and it parses as the path .true and silently matches nothing. unbox_oneof_in=<path> (with unbox_oneof=true as the blanket, mirroring buffa-build) avoids a breaking rename later.
  • The recursion error at oneof.rs:427-432 tells a plugin user to remove the path from unbox_oneof_in or call unbox_oneof(), buffa-build vocabulary a buf.gen.yaml author never wrote; the shared_descriptor_pool rejections in main.rs show the plugin-side wording.
  • docs/guide.md:925 still says message and group variants are always boxed, and the guide has no section on unboxing to point the new row at.

Tests should cover the empty, whitespace, and trailing-dot inputs. Re-review on push; on the 0.9.2 list.

@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code]

The rename and the normalize_override_path sharing land exactly as asked; one thing the push added undid part of it. The unbox_oneof=<path> "compatibility alias" preserved a spelling that only existed on this branch (no tag, no main commit, no release carries it), and it routed near-miss booleans — unbox_oneof=TRUE, =1, =yes — into the path parser, where they became .TRUE / .1, matched nothing, and generated boxed code with no diagnostic: the same silent-misparse case the send-back was about, one arm over. Pushed 35c55e7 rather than a third round: unbox_oneof is boolean-only through parse_bool like its siblings (with a test that TRUE/1/a path all error), the empty-path error names unbox_oneof_in instead of unbox_oneof, an all-dots case joins the rejection test, the codegen recursion error says "the blanket . rule" instead of citing the builder's unbox_oneof() to plugin users, the guide section notes that view oneofs stay boxed, and the fragment drops the alias sentence. Ready to queue once Iain stamps it.

fallintoplace and others added 3 commits September 2, 2026 09:52
…rrors by name

The path-valued unbox_oneof=<path> alias preserved a spelling that only
ever existed on this branch, and it routed near-miss booleans
(unbox_oneof=TRUE, =1) into the path parser, where they normalize to a
path that matches nothing and generate boxed code with no diagnostic.
unbox_oneof now goes through parse_bool like every other boolean option;
unbox_oneof_in=<path> is the only path spelling. Its empty-path error
names unbox_oneof_in instead of unbox_oneof, the codegen recursion error
no longer cites the builder's unbox_oneof() to plugin users, the guide
section is one flowing paragraph and notes that views stay boxed, and
the fragment drops the alias sentence.
@iainmcgin
iainmcgin force-pushed the fix/protoc-gen-unbox-oneof-option branch from 7f86b63 to 84c4f33 Compare September 2, 2026 16:53
@iainmcgin
iainmcgin added this pull request to the merge queue Sep 2, 2026
Merged via the queue into anthropics:main with commit 5898e95 Sep 2, 2026
11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants