Add buffa-remote-derive: ProtoString/ProtoBytes/ProtoList/ProtoBox/MapStorage for remote types

[rsd-darshan]

Closes #212 — a #[derive(...)] #[buffa(remote = ...)] macro that generates the newtype boilerplate (Deref, AsRef/DerefMut, the relevant From impls, and the buffa trait itself) needed to use a foreign type as a custom owned field type.

Two tiers, one crate

ProtoString, ProtoBytes, ProtoList — the remote type's existing supertrait bounds (From<&str>, From<Vec<u8>>, FromIterator<T>, etc.) give the macro everything it needs; no extra configuration beyond naming the remote type.

#[derive(Clone, PartialEq, Default, Debug, buffa_remote_derive::ProtoString)]
#[buffa(remote = ecow::EcoString)]
pub struct MyEcoString(pub ecow::EcoString);

ProtoBox, MapStorage — their reference newtypes (examples/custom-types/src/types/small_box.rs, index_map.rs) call inherent methods on the remote type (smallbox::SmallBox::into_inner(), indexmap::IndexMap::insert()), not trait methods, so the macro can't synthesize the call purely from bounds. These default to the conventional method names (new/into_inner for pointers; len/insert/clear/iter for maps) with an attribute override for a remote type that names them differently:

#[derive(buffa_remote_derive::ProtoBox)]
#[buffa(remote = smallbox::SmallBox<T, smallbox::space::S4>)]
pub struct SmallBox<T>(pub smallbox::SmallBox<T, smallbox::space::S4>);

// A map whose insert method is named `put` instead:
#[derive(Clone, PartialEq, Debug, buffa_remote_derive::MapStorage)]
#[buffa(remote = ..., insert = MyMap::put)]
pub struct MyMapStorage<K, V>(pub MyMap<K, V>);

The override and the default are both rendered as UFCS-style calls (Type::method(&mut self.0, args...)), so a differently-named method with the same receiver shape drops in directly.

Notes

• The #[buffa(remote = ...)] attribute is documentation, not codegen input — the macro always reads the field's actual type. Comparing the attribute's value against the field's type isn't possible from within a derive macro without resolving use imports, so the attribute is required (and must parse as a type, catching typos) but isn't checked for equality against the field.
• ProtoList's generated clear reinitializes via Default::default() rather than retaining capacity — documented trade-off, acceptable per the trait's "where the underlying type allows" contract.
• MapStorage's Default/FromIterator are required by the message codec, not by this derive (a derived impl would wrongly force K: Default/V: Default) — must be hand-written, same reasoning as ProtoList.

Testing

Integration tests against real foreign types (ecow::EcoString, smallvec::SmallVec, smallbox::SmallBox, indexmap::IndexMap, plus hand-rolled types with non-conventional method names to exercise the override paths), covering both tuple-struct and named-field newtype shapes, decode/encode round-trips, and generic element types without Eq/Ord.

task lint and task test pass clean across the workspace.

[github-actions]

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

[iainmcgin]

[claude code] Thanks for taking on #212 — this is a faithful implementation of the design sketched there: the supertrait-driven derives for ProtoString/ProtoBytes/ProtoList, the inherent-method override attributes for ProtoBox/MapStorage (a clean solution to a real asymmetry, and well explained in the crate docs), idiomatic proc-macro error handling, and no_std-clean ::buffa::alloc paths throughout. Review verified the generated impls against the real trait surfaces and found them correct. Three things before it can land — the first two blocking:

1. Feature-gated forwarders are missing. #212 scopes the derive output to include the (feature-gated) serde / Arbitrary / reflection forwarders, and the trait docs require them: a JSON build needs Serialize/Deserialize on the storage type, a reflection/vtable build needs ReflectList/ReflectMap, and fuzzing needs Arbitrary. Without them, a derived newtype used as a message field in a json- or reflect-enabled build fails to compile deep inside generated message code (MyType: Serialize is not satisfied), far from the derive that should have provided it. Either generate them behind feature flags, or — at minimum — document prominently that the derives currently cover the binary codec only and the user must hand-write the rest, with follow-up issues filed for the gaps.

2. No end-to-end proof. Every test calls the trait methods directly (from_wire, push, storage_insert, …); none wires a derived type into a buffa_build-generated message and round-trips it through encode/decode, which is the entire point of #212. The natural proof is migrating at least one newtype in examples/custom-types to the derive — which would also cross-link the example to the new crate (right now a reader of the example would never learn the derive exists).

3. Unqualified .as_ref() in the generated Deref/AsRef bodies (string.rs, bytes.rs, list.rs) breaks for remote types with more than one AsRef impl — method resolution doesn't use the return type to disambiguate, so e.g. a type with both AsRef<str> and AsRef<[u8]> produces a "multiple applicable items" error in generated code. Emit the UFCS form (::core::convert::AsRef::<str>::as_ref(&self.0) and the [u8]/[T] equivalents).

Smaller, non-blocking notes: nothing exercises the generated output under no_std (the dev-dependencies pull in std features — worth a compile check given the workspace's no_std guarantee); ProtoBytes::from_wire always copies via to_vec, worth a one-line doc caveat alongside the existing ProtoList::clear note; and parse/parse_with_overrides in remote_field.rs share enough structure to fold into one helper.

The test breadth at the trait-method layer (tuple and named shapes, override paths, last-write-wins, non-Eq elements) is genuinely good. Looking forward to the next revision.

[rsd-darshan]

Thanks for the thorough review — all three addressed:

1. Feature-gated forwarders: added a "Scope: the binary codec only" section near the top of the crate docs, naming exactly what's absent (serde/Arbitrary/reflection), the failure mode (a trait-bound error in generated message code, not at the derive site), and the fix (hand-write the extra #[derive(..)]s, same as the reference newtypes already do).

2. End-to-end proof: migrated FlexStr in examples/custom-types to #[derive(buffa_remote_derive::ProtoString)] + #[buffa(remote = flexstr::SharedStr)], replacing the hand-written Deref/AsRef/From/ProtoString impls. Ran it through buffa_build codegen, binary encode/decode, and JSON round-trip — all pass (cargo run -p example-custom-types).

3. Unqualified .as_ref() ambiguity: confirmed and fixed in string.rs/bytes.rs/list.rs — now <Remote as AsRef<Target>>::as_ref(&self.0). Added a regression test (DualAsRef, a type implementing both AsRef<str> and AsRef<[u8]>) that would fail to compile under the old form.

Smaller notes also done: folded parse/parse_with_overrides into one helper, and documented the to_vec() copy in ProtoBytes's generated from_wire.

Didn't touch the no_std compile-check suggestion — flagging it as a possible follow-up rather than silently skipping it.

[iainmcgin]

[claude code] Re-review after 5254c41. All three blockers from the previous round are genuinely resolved, verified independently:

1. Codec scope — the new crate-doc section is the documentation route the previous review allowed, and it names the failure mode precisely. (Two accuracy issues in it, flagged as inline comments.)
2. End-to-end proof — the FlexStr migration is the real thing: ran the example through buffa_build codegen, binary and JSON round-trips, all assertions pass. I also verified the derived from_wire is semantically identical to the removed hand-written impl: WirePayload::to_str maps to the same DecodeError::InvalidUtf8, and flexstr's From<&str> delegates to from_ref, so the inline-vs-Arc allocation behavior is preserved with no intermediate String.
3. AsRef ambiguity — the UFCS fix is applied consistently across Deref/AsRef/From/FromIterator in all three trait derives, and the DualAsRef regression test locks it in.

One more small round before merge — everything below is a one-or-two-line fix (details in the inline comments):

• The single remaining unqualified method call on a remote type: push's .extend(...) in list.rs, plus a dropped struct where-clause on the same impl. Same class as the as_ref blocker, so it should go out with this PR.
• Two accuracy fixes in the new scope docs and one in flex_str.rs's doc comment.
• map_storage.rs test writes the wrong type into remote =.
• Changelog fragment: CONTRIBUTING.md asks for a .changes/unreleased/ fragment per user-visible change, and a new publishable crate qualifies. task changelog-new -- -k Added -b '...' — the check-changelog CI job doesn't enforce presence, so it passed without one.

Non-blocking, fine as a follow-up (say which you prefer): the README "Workspace layout" table and docs/guide.md's custom-types section don't mention buffa-remote-derive yet — the guide still teaches the hand-written boilerplate this crate exists to remove, so one sentence plus a link there would help readers find it. Also purely cosmetic: the [WirePayload::into_bytes] link joins three pre-existing [buffa::*] links that can't resolve (the crate doesn't depend on buffa) — plain backticks without brackets would drop the rustdoc warnings; and examples/custom-types/Cargo.toml still says src/types.rs where the module is now src/types/.

The forwarder gaps (serde/Arbitrary/reflection generation) stay tracked in #212, which remains open for that scope.

[iainmcgin]

[claude code] This is the one remaining unqualified method call on a remote type in the generated output — same ambiguity/hijack class as the as_ref fix this commit makes, and it sits eight lines below the comment explaining why plain method-call syntax isn't used. An inherent fn extend(&mut self, ...) on the remote collection would shadow Extend::extend (inherent methods win in method resolution). The where-clause already guarantees the bound, so:

<#field_ty as ::core::iter::Extend<#element_ty>>::extend(&mut #accessor, ::core::iter::once(value));

While you're in this impl: it's also the only generated impl that replaces the struct's own where-clause instead of appending to it (compare the Deref/FromIterator/From impls above, and map.rs/box_ptr.rs, which all thread #where_clause through). A newtype declared as struct Foo<T>(SmallVec<[T; 4]>) where T: Copy; loses T: Copy on the ProtoList impl and fails well-formedness. Appending the struct's predicates to the hand-written where block fixes it.

[iainmcgin]

[claude code] The #[serde(transparent)] advice is right for string and repeated newtypes but wrong for bytes (and map-of-bytes): codegen emits #[serde(with = <base64 module>)] on bytes fields, so serde places no Serialize bound on the storage type — a bytes newtype needs no serde impls at all. SmallBytes in examples/custom-types demonstrates this (it derives no serde and documents why). Worse, a consumer who follows this advice and adds #[serde(transparent)] to a bytes newtype gets an impl that serializes as a JSON array of numbers rather than base64 if it's ever used standalone. Worth splitting the claim by kind: string/repeated need the hand-written serde derives; bytes route through the base64 with helpers and don't.

[iainmcgin]

[claude code] WirePayload::into_bytes won't deliver what this sentence implies: per its own docs it's zero-copy only for an Owned payload, and a single-chunk source — the overwhelmingly common decode path — arrives as Borrowed and is copied anyway. Only the built-in bytes::Bytes field representation gets guaranteed zero-copy. Suggest stating that plainly: a custom bytes type can't currently reach the zero-copy decode path, and the built-in Bytes representation is the remedy when the copy matters — rather than pointing at an escape hatch that still copies.

[iainmcgin]

[claude code] "traits the derive crate doesn't yet cover" isn't the real reason — ProtoBytes/ProtoList cover exactly the traits small_bytes.rs/small_vec.rs implement, and both wrapped types satisfy the derive bounds, so the derive would compile for them. What they'd actually lose is behavior: SmallBytes::from_wire uses SmallVec::from_slice (no intermediate Vec) where the derived path goes to_vec() → From<Vec<u8>>, and small_vec.rs's clear retains capacity where the derive's drops it. Stating the true reason makes these files a better teaching pair: derive for the common case, hand-write when the generic path can't express your allocation behavior.

[iainmcgin]

[claude code] remote = std::collections::BTreeMap<K, V> but the wrapped field is OddMap<K, V>. The attribute is documentation-only (as remote_field.rs says), so nothing catches this — which makes the crate's own test the counterexample to the "makes the newtype's purpose legible" rationale. Should be remote = OddMap<K, V>. Longer-term it may be worth validating the attribute against the field type at derive time (they must be the same type in the newtype pattern, so a token-level comparison would catch exactly this), but that can be a follow-up.

[iainmcgin]

[claude code] Since the remaining punch list was small and mechanical, I've applied it directly to your branch rather than asking for another round — pushed as 6a6ee56 (maintainerCanModify allows this). What it covers:

• Every item from the re-review's inline comments: the UFCS extend in list.rs (plus a regression test with a booby-trapped inherent extend), the struct where-clause propagation on the ProtoList impl (done via make_where_clause on a cloned Generics rather than hand-splicing predicates, plus a where T: Display regression test), the scope-doc and flex_str.rs accuracy fixes, the map_storage.rs remote = value, and the changelog fragment.
• The doc ripple flagged as optional: examples/custom-types/README.md (it still showed the hand-written FlexStr impl this PR deleted), the root README workspace table, and a docs/guide.md pointer to the new crate.
• One correction found while fixing the docs: the serde guidance is now stated by codegen routing — singular/oneof strings and all bytes positions go through buffa's with-modules and need no serde impls on the newtype; repeated elements, optional strings, and map values serialize through the newtype's own serde. (The first draft of my own suggested wording got optional wrong, so the review comment's phrasing shouldn't be taken as gospel either — optional_serde_module in buffa-codegen/src/message.rs is the source of truth.)

Verified end-to-end: a scratch consumer crate wiring repeated_type_custom to a ProtoList-derived newtype (with the panicking inherent extend and a struct where-clause) round-trips through a real buffa_build-generated message, and the generated decoder demonstrably calls the derived push. cargo test -p buffa-remote-derive, clippy, rustdoc (now warning-free), markdownlint, and the custom-types example all pass locally.

Once CI is green this should be ready to land. Thanks again for the contribution and the responsive first round — the derive crate is a genuinely nice addition.

[iainmcgin]

[claude code] The CI failure on 6a6ee56 was a semantic conflict with main, not this branch's changes: #247 made WirePayload opaque after this branch was cut, and the direct-construction tests here used WirePayload::Borrowed(...). Merged main into the branch and switched the three test sites to the WirePayload::borrowed() constructor (0596c4f + 569e8b4). Full workspace clippy and test suite pass locally on the merge result.

[iainmcgin]

[claude code] All items from both review rounds are addressed (the last round applied maintainer-side, per the comments above), CI is fully green on the merge with main, and the derive was verified end-to-end through a real buffa_build-generated message. Ready to land.

[iainmcgin]

[claude code] Merged as 976f66d — thanks @rsd-darshan for the contribution and the responsive review round. The derives are a real quality-of-life improvement for custom owned types.