fix(arbitrary): add Bytes shims so bytes_fields + generate_arbitrary compiles (#88)#90
Conversation
…compiles bytes::Bytes has no Arbitrary impl, so any struct with a bytes_fields-typed field and generate_arbitrary = true failed to compile with --features arbitrary (broken since anthropics#51 changed Any.value from Vec<u8> to Bytes in 0.4.0). Add three helpers to buffa::__private: - arbitrary_bytes — singular Bytes field - arbitrary_bytes_opt — Option<Bytes> (proto3 explicit-presence) - arbitrary_bytes_vec — Vec<Bytes> (repeated) Codegen emits #[cfg_attr(feature = "arbitrary", arbitrary(with = ...))] on every bytes_fields-typed struct field (selecting the right helper by shape) and on the inner field of bytes_fields-typed oneof variants. Map values are always Vec<u8> regardless of bytes_fields and need no shim. The fix covers Any.value directly (WKT regen) and generalises to all user protos that combine bytes_fields with generate_arbitrary = true. Regression tests: any_arbitrary_with_bytes_value pins Any; the new basic_arbitrary_bytes build block (basic.proto × use_bytes_type × generate_arbitrary) exercises all four shapes at compile time, and bytes_contexts_arbitrary_all_shapes verifies them at runtime under --features arbitrary. Closes anthropics#88.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
- buffa-build: document the buffa/arbitrary feature-forwarding requirement and the literal "arbitrary" feature-name constraint on Config::generate_arbitrary, where consumers actually see it (the buffa-codegen doc covers a much smaller hand-driving audience). Calls out the cannot-find-function symptom when the forward is forgotten so the error is searchable. - buffa: helper-level equivalence tests against Vec<u8>/Option<Vec<u8>>/ Vec<Vec<u8>> with a non-zero seed, replacing the all-zero type-compat-only test. Catches a regression that always returns empty Bytes without depending on arbitrary's internal byte-consumption layout. - buffa: helper rustdoc notes oneof variants reuse the singular arbitrary_bytes helper. - buffa-codegen/oneof.rs: debug_assert that boxed variants are never bytes_fields-typed, locking the invariant that they don't need (and silently lose) the shim attribute. - buffa-test: keep the integration test as a type-compat smoke test (compilation is the primary assertion); content non-emptiness is pinned at the helper level instead. - CHANGELOG: state that the fix covers singular, optional, and repeated fields plus oneof variants, not just singular. - CI: add a `cargo check --workspace --all-features` step to lint-and-test so feature-flag-only compile breaks like anthropics#88 can't recur silently.
iainmcgin
left a comment
There was a problem hiding this comment.
[claude code] Approving with a review-feedback commit pushed directly to your branch (maintainerCanModify is on; let me know if you'd rather I revert and have you take it). The fix design is exactly right — option (2) from #88, three __private helpers covering all field shapes, with codegen selecting by shape and skipping map values. The threading of use_bytes through FieldInfo/VariantInfo reuses the existing classification cleanly, and the BytesContexts test message exercising all four shapes is targeted.
What I pushed in b4e0e6db (after merging main to pick up #94/#95/#96):
buffa-build::Config::generate_arbitrarydoc — the constraint that the downstream feature must be named exactly"arbitrary"and forward tobuffa/arbitrarywas documented onbuffa_codegen::CodeGenConfig, but almost everyone callsbuffa_build::Configfrombuild.rs. Duplicated the guidance there with a[features]snippet and the symptom (cannot find function 'arbitrary_bytes' in module '__private') so the error is searchable.- Helper-level equivalence tests in
buffa/src/lib.rs— replaced the all-zero-buffer test with three tests that compare each helper's output against the underlyingVec<u8>/Option<Vec<u8>>/Vec<Vec<u8>>impl from an identicalUnstructured. An all-zero seed deterministically yields empty vectors, which would let a regression that drops the innerArbitrarycall slip through. The integration test inbuffa-teststays as a type-compat smoke test (compilation is the primary assertion there). debug_assert!(!v.use_bytes)on the boxed-variant branch inoneof.rs— boxed variants are message/group types, never bytes, so the shim is structurally unreachable. The assert locks the invariant in caseis_boxed_variantever broadens.- CHANGELOG — noted that singular, optional, and repeated fields plus oneof variants are covered (the original wording said "singular" only, which would lead a user with
repeated bytesto think they're not fixed). - CI — added a
cargo check --workspace --all-featuresstep. CI didn't exercise--features arbitraryanywhere, which is exactly how this regression escaped in the first place.checkrather thantestto keep the time delta small. - Helper rustdoc — note that oneof variants reuse the singular
arbitrary_byteshelper.
Verified locally on the merged head: cargo build -p buffa-types --features arbitrary (the originally-reported failure), cargo test -p buffa-test --features arbitrary (292 pass), cargo test --workspace (1560 pass), cargo clippy --workspace --all-features --all-targets clean, RUSTDOCFLAGS=-D warnings cargo doc --workspace --all-features clean, cargo +1.95 fmt --check clean, task gen-wkt-types no drift.
Thanks for the contribution — this unblocks --all-features and the arbitrary-based benchmarks.
Description:
Closes #88
Root cause
Since #51 changed
Any.valuefromVec<u8>tobytes::Bytesin 0.4.0,buffa-types --features arbitraryfails:error[E0277]: the trait bound buffa::bytes::Bytes: Arbitrary<'_> is not satisfied
The same failure hits any user crate that combines
bytes_fieldswithgenerate_arbitrary = true— the struct-level#[derive(Arbitrary)]cannotsatisfy the bound for
bytes::Bytesfields regardless of cardinality.Fix
Add three helpers to
buffa::__private(gated onfeature = "arbitrary"):arbitrary_bytesbytes::Bytesarbitrary_bytes_optOption<bytes::Bytes>arbitrary_bytes_vecVec<bytes::Bytes>Codegen now emits
#[cfg_attr(feature = "arbitrary", arbitrary(with = ...))]on every
bytes_fields-typed struct field (selecting the right helper byshape) and on the inner field of
bytes_fields-typed oneof variants. Mapvalues remain
Vec<u8>regardless ofbytes_fieldsand need no shim.Tests
any_arbitrary_with_bytes_valueinbuffa-types/src/any_ext.rs— regressionpin for
Anydirectly (the reported failure).basic_arbitrary_bytesbuild block inbuffa-testcompilesbasic.protowith
use_bytes_type() + generate_arbitrary(true).BytesContextshas allfour shapes (singular, optional, repeated, oneof variant), so compilation is
the primary assertion.
bytes_contexts_arbitrary_all_shapesexercises all four shapes at runtimeunder
--features arbitrary, callingslice(..)on each result to assertthe type is concretely
bytes::Bytes.Checklist
task lintcleantask test— 1 500+ tests, 0 failurescargo test -p buffa-test --features arbitrarypassestask gen-wkt-typesrun; onlygoogle.protobuf.any.rschanged