Skip to content

feat(buffa-codegen): gate generated json/views/text impls on crate features#117

Merged
iainmcgin merged 3 commits into
mainfrom
feature-gated-codegen
May 14, 2026
Merged

feat(buffa-codegen): gate generated json/views/text impls on crate features#117
iainmcgin merged 3 commits into
mainfrom
feature-gated-codegen

Conversation

@iainmcgin
Copy link
Copy Markdown
Collaborator

@iainmcgin iainmcgin commented May 13, 2026

Summary

Adds the codegen mechanism that lets buffa-descriptor (and, if we choose to later, buffa-types) ship every generated impl while keeping the codegen toolchain (buffa-codegen / buffa-build / protoc-gen-buffa) lean. #118 (stacked on this PR) does the buffa-descriptor regen and dep cleanup. Tracked in #113.

What

CodeGenConfig::gate_impls_on_crate_features: bool (default false). When true, generated impls controlled by generate_json, generate_views, and generate_text are wrapped in #[cfg(feature = "json" | "views" | "text")] (or #[cfg_attr(...)] for derives and field attributes) instead of being emitted unconditionally. The consuming crate defines matching Cargo features:

[features]
json  = ["buffa/json", "dep:serde", "dep:serde_json"]
views = []
text  = ["buffa/text"]

The generate_* flags still control whether an impl kind is emitted; the new flag only controls how. generate_arbitrary is unchanged — it was always cfg_attr-gated. Default false is a no-op: task gen-bootstrap-types / task gen-wkt-types produce byte-identical output.

Mechanism

feature_gates.rs adds a FeatureGates struct (resolved once per config from gate_impls_on_crate_featuresgenerate_*) and four pub(crate) helpers:

  • cfg_block(tokens, gate)#[cfg(feature = "x")] <item> for a single item or statement, with a debug_assert against multi-item misuse (a #[cfg] outer attribute attaches only to the next item, so trailing siblings would silently leak ungated).
  • cfg_const_block(tokens, gate)#[cfg(feature = "x")] const _: () = { <impls> }; for sibling impls (the anonymous const is itself a single item; trait impls inside register globally).
  • cfg_block_any(tokens, &[gates])#[cfg(any(feature = "a", feature = "b"))] for items that exist iff any of a set of modes is enabled. Used by register_types, which registers both JSON and text entries.
  • cfg_attr(attr_body, gate)#[cfg_attr(feature = "x", <attr>)] or #[<attr>] for derives and helper attributes that must only apply when the feature is on (a #[serde(...)] field attribute on a struct that doesn't #[derive(Serialize)] is a hard compile error).

What's gated

Surface Wrapped as Where
#[derive(::serde::Serialize, ::serde::Deserialize)], #[serde(default)] cfg_attr(feature = "json", ...) message.rs
Field #[serde(rename = ...)], oneof #[serde(flatten)], unknown-fields #[serde(skip)] cfg_attr message.rs
Custom impl Deserialize, impl ProtoElemJson, oneof impl Serialize, view impl Serialize cfg(feature = "json") message.rs, oneof.rs, view.rs
Enum impl Serialize/Deserialize/ProtoElemJson cfg(feature = "json") on a const _: () = { ... }; enumeration.rs
__FooExtJson wrapper's Serialize/Deserialize cfg(feature = "json"); the struct + Deref/DerefMut/From stay unconditional — encode/decode reach the inner UnknownFields through DerefMut and a struct field's type can't change behind a cfg message.rs
JSON/Text Any consts (__*_JSON_ANY / __*_TEXT_ANY), JSON/Text extension consts cfg message.rs, extension.rs
impl ::buffa::text::TextFormat (including the MessageSet early return) cfg(feature = "text") impl_text.rs
__buffa::view module in the stitcher, natural-path pub use ... FooView; cfg(feature = "views") lib.rs, message.rs
register_types fn body statements, the fn declaration, and its package-root re-export per-statement cfg; cfg(any(feature = "json", feature = "text")) on the fn and re-export lib.rs

Tests

  • feature_gates.rs helper unit tests: for_config resolution, cfg_block/cfg_const_block/cfg_attr/cfg_block_any shapes, the debug_assert against multi-item cfg_block misuse.
  • tests/feature_gating.rs integration tests against generated output: cfg attributes appear in the right places; ungated output has zero feature = "json"|"views"|"text" references; disabling a generate_* flag suppresses both the impl and its gate; the __ExtJson wrapper struct is unconditional but its serde impls are gated; register_types carries any(json, text) gates on both the fn and its re-export; syn::parse_file smoke test for structural validity.

Notes for review

rust-code-reviewer ran. Findings addressed:

  • HIGH: the MessageSet early return in impl_text.rs bypassed the gate — fixed.
  • HIGH: register_types referenced TypeRegistry unconditionally when its body could be all-cfg'd-out — gated the fn and re-export on any(json, text).
  • MEDIUM: doc comments incorrectly described an inner #![cfg] (it's an outer #[cfg] on the const) — fixed.
  • MEDIUM: string-match assertions were brittle to prettyplease line-wrapping — added a squash() helper and normalised them.
  • MEDIUM: cfg_block had no enforcement against multi-item misuse — debug_assert added.
  • MEDIUM: no test for generate_json = false + gating on — added.
  • HIGH (acknowledged, deferred): no compile coverage with each feature combination. Added a syn::parse_file smoke test as a structural floor; this PR adds a feature_gating_compile matrix test against the buffa-test/protos/ fixture set, and feat(buffa-descriptor): regen with views/json/text/arbitrary behind crate features #118 (the buffa-descriptor regen) is verified end-to-end against bufbuild/registry/bufbuild/buf with each feature combo.

Follow-ups (separate PRs)

…atures

Add `CodeGenConfig::gate_impls_on_crate_features: bool` (default `false`).
When `true`, generated impls controlled by `generate_json`,
`generate_views`, and `generate_text` are wrapped in
`#[cfg(feature = "json" | "views" | "text")]` (or `#[cfg_attr(...)]` for
derives and field attributes) instead of being emitted unconditionally.
The consuming crate defines matching Cargo features and enables the
corresponding runtime support behind them.

The `generate_*` flags still control *whether* an impl kind is emitted;
the new flag only controls *how*. `generate_arbitrary` is unchanged — it
was always `cfg_attr`-gated on `feature = "arbitrary"`.

Mechanism: `feature_gates.rs` adds a `FeatureGates` struct (resolved once
per config) and four helpers — `cfg_block` for a single item/statement
(with a `debug_assert` against multi-item misuse), `cfg_const_block` for
sibling impls (`#[cfg(...)] const _: () = { ... };`), `cfg_block_any` for
items that exist iff *any* of a set of modes is on (e.g.
`register_types`, gated on `any(json, text)`), and `cfg_attr` for derives
and helper attributes.

Wired through:
- `message.rs`: struct serde derives + `#[serde(default)]`, field
  `#[serde(...)]` attrs, oneof `#[serde(flatten)]`, unknown-fields
  `#[serde(skip)]`, custom `Deserialize` impl, `ProtoElemJson` impl,
  JSON/Text Any consts, view re-exports. The `__FooExtJson` wrapper
  struct (and Deref/DerefMut/From) become unconditional — a struct
  field's type can't change behind a `cfg`, and encode/decode reach the
  inner `UnknownFields` through `DerefMut` either way; only its
  `Serialize`/`Deserialize` impls are gated.
- `enumeration.rs`: enum serde impls in a `const _: () = { ... };` block.
- `oneof.rs`: oneof `Serialize` impl.
- `view.rs`: `impl Serialize for FooView`.
- `impl_text.rs`: `impl TextFormat`, including the `MessageSet` early
  return.
- `extension.rs`: JSON/Text extension consts.
- `lib.rs`: `register_types` (per-statement gates plus an
  `any(json, text)` gate on the fn and its package-root re-export), the
  `__buffa::view` module in the stitcher.

`task gen-bootstrap-types` and `task gen-wkt-types` are no-ops — the
default produces byte-identical output.

Tests: helper-fn unit tests in `feature_gates.rs`; integration tests in
`tests/feature_gating.rs` asserting cfg attributes appear in the right
places, that ungated output has zero `feature = ...` cfgs, that disabling
a `generate_*` flag suppresses both the impl and its gate, and a
`syn::parse_file` smoke test on the gated output. Compile coverage with
each feature combination is deferred to the `buffa-descriptor` /
`buffa-types` regen PR (#113), which adds the actual feature definitions.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

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

…trix

The string-match assertions in `tests/feature_gating.rs` and the
`syn::parse_file` smoke test verify the *shape* of gated output but not
that it resolves. A `cfg`-gated item referenced from an ungated context
is valid Rust syntax — only the compiler catches it.

Add `tests/feature_gating_compile.rs`: generate all 24 protoc-buildable
`buffa-test/protos/*.proto` files (editions, oneofs, extensions, maps,
groups, MessageSet, cross-package, prelude shadows, …) with
`gate_impls_on_crate_features = true`, lay them out in a temp crate that
defines `json` / `views` / `text` features, and `cargo check` each of the
8 `{json, views, text}` subsets. Marked `#[ignore]` so the default
workspace test stays fast; CI runs it explicitly via a new
`Feature-gating compile matrix` step.

The test caught a real bug: top-level message view re-exports
(`pub use self::__buffa::view::FooView;`, emitted in `lib.rs` rather than
`message.rs` where the nested-message and view-oneof re-exports live)
were not `cfg`-gated. The string assertions and syntax check missed it
because the re-export *is* valid Rust — the failure is a name-resolution
error against a `view` module that's been cfg'd out. Fixed and pinned
with `gated_view_reexports_are_cfg_blocked`.
@iainmcgin
Copy link
Copy Markdown
Collaborator Author

iainmcgin commented May 14, 2026

Update: compile-matrix coverage

Pushed a second commit adding tests/feature_gating_compile.rs. The string-match assertions and syn::parse_file smoke test verify the shape of gated output but not that it resolves — a cfg-gated item referenced from an ungated context is valid Rust syntax and only fails at name resolution.

The new test generates all 24 protoc-buildable buffa-test/protos/*.proto files with gate_impls_on_crate_features = true, lays them out in a temp crate that defines json/views/text features, and cargo checks each of the 8 {json, views, text} subsets. #[ignore]d (slow ~30s warm); CI runs it explicitly via a new Feature-gating compile matrix step.

It caught a real bug the existing tests missed: top-level message view re-exports (pub use self::__buffa::view::FooView;, emitted in lib.rs rather than message.rs where the nested-message and view-oneof re-exports live) were not cfg-gated. With --no-default-features, the view module is cfg'd out but the re-export survives and the build fails with could not find view in __buffa. Fixed and pinned with a regression test.

Coverage now:

@iainmcgin iainmcgin marked this pull request as ready for review May 14, 2026 18:53
@iainmcgin iainmcgin requested a review from rpb-ant May 14, 2026 18:53
@iainmcgin iainmcgin enabled auto-merge (squash) May 14, 2026 18:53
@iainmcgin iainmcgin merged commit 86d5efe into main May 14, 2026
7 checks passed
@iainmcgin iainmcgin deleted the feature-gated-codegen branch May 14, 2026 19:00
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 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.

Feature-gated codegen impls + buffa-descriptor regen for descriptor message types as fields

2 participants