diff --git a/CHANGELOG.md b/CHANGELOG.md index 09eb9b1..0166230 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,29 @@ and `__private::arbitrary_bytes`, none of which exist in `buffa` 0.4.0. ### Added +- Generated message structs now include `with_(value) -> Self` builder + methods for every explicit-presence scalar, bytes, and enum field (proto3 + `optional`, proto2 `optional`, and editions fields with + `field_presence = EXPLICIT`). This allows chained construction without + `Some(...)` wrapping: + + ```rust + let req = GetSecretRequest::default() + .with_name("alice") + .with_timeout_ms(30_000) + .with_enabled(true); + ``` + + String fields accept `impl Into` (`&str` works directly); bytes + fields accept `impl Into>` or `impl Into` (byte + array literals like `b"data"` work directly); enum fields accept + `impl Into>` (bare variant works directly, no + `EnumValue::Known(...)` wrapper needed); plain scalars take the bare + type. Message fields (`MessageField`), repeated fields, map fields, + oneof variants, proto2 `required` fields, and implicit-presence fields + are unaffected. To clear a field, assign `None` directly. + Controlled by `CodeGenConfig::generate_with_setters` (default `true`). + ([#30](https://github.com/anthropics/buffa/issues/30)) - `buffa_codegen::GeneratedFileKind::Companion` and `apply_companions` let downstream code generators (e.g. connect-rust) supply extra per-proto files that buffa wires into the per-package stitcher, instead of having diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index fc6af88..0b45cca 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -300,6 +300,30 @@ pub struct CodeGenConfig { /// `#[derive(strum::EnumIter)]` when the user does not want to apply the /// same attribute to every message in the matched scope. pub enum_attributes: Vec<(String, String)>, + /// Generate `with_*` builder-style setter methods for explicit-presence fields. + /// + /// Each explicit-presence scalar, bytes, or enum field gets a + /// `pub fn with_(mut self, value: T) -> Self` method that wraps the + /// value in `Some` and returns `self`, enabling chained construction: + /// + /// ```ignore + /// let req = MyRequest::default() + /// .with_name("alice") + /// .with_timeout_ms(30_000); + /// ``` + /// + /// **Fields that receive a setter:** proto3 `optional`, proto2 `optional`, + /// and editions fields with `field_presence = EXPLICIT`. + /// + /// **Fields that do not receive a setter:** message fields + /// (`MessageField`), repeated fields, map fields, oneof variant fields, + /// proto2 `required` fields, and any implicit-presence field. + /// + /// There is no `clear_` companion — to clear a field, assign `None` + /// directly: `msg.name = None;`. + /// + /// Defaults to `true`. + pub generate_with_setters: bool, } impl Default for CodeGenConfig { @@ -320,6 +344,7 @@ impl Default for CodeGenConfig { field_attributes: Vec::new(), message_attributes: Vec::new(), enum_attributes: Vec::new(), + generate_with_setters: true, } } } diff --git a/buffa-codegen/src/message.rs b/buffa-codegen/src/message.rs index 4ee4198..939ab11 100644 --- a/buffa-codegen/src/message.rs +++ b/buffa-codegen/src/message.rs @@ -184,6 +184,34 @@ fn generate_message_with_nesting( // Collect field identifiers for the manual Debug impl (excludes __buffa_ internals). let mut debug_field_idents: Vec<&Ident> = generated_fields.iter().map(|f| &f.ident).collect(); + let setter_methods: TokenStream = generated_fields + .iter() + .filter_map(|f| f.setter.as_ref().map(|s| (f, s))) + .map(|(f, s)| { + let field_ident = &f.ident; + let setter_ident = &s.ident; + let field_name = field_ident.to_string(); + let doc = format!( + "Sets [`Self::{field_name}`] to `Some(value)`, consuming and returning `self`." + ); + let body = if s.use_into { + quote! { Some(value.into()) } + } else { + quote! { Some(value) } + }; + let param = &s.param_type; + quote! { + #[must_use = "with_* setters return `self` by value; assign or chain the result"] + #[inline] + #[doc = #doc] + pub fn #setter_ident(mut self, value: #param) -> Self { + self.#field_ident = #body; + self + } + } + }) + .collect(); + // Module name for this message (snake_case of proto name). let proto_name = msg.name.as_deref().unwrap_or(rust_name); let mod_name_str = crate::oneof::to_snake_case(proto_name); @@ -649,6 +677,16 @@ fn generate_message_with_nesting( let message_doc = crate::comments::doc_attrs_resolved(ctx.comment(proto_fqn), proto_fqn, &ctx.type_map); + let with_setters_impl = if ctx.config.generate_with_setters && !setter_methods.is_empty() { + quote! { + impl #name_ident { + #setter_methods + } + } + } else { + quote! {} + }; + let top_level = quote! { #message_doc #[derive(Clone, PartialEq, #derive_default)] @@ -674,6 +712,8 @@ fn generate_message_with_nesting( pub const TYPE_URL: &'static str = #type_url; } + #with_setters_impl + #message_impl #text_impl @@ -1228,6 +1268,9 @@ struct FieldInfo { /// features — that field is TYPE_MESSAGE so the overlay doesn't fire. /// See `map_serde_module`. map_value_enum_closed: Option, + /// The bare inner type `T` when `is_optional = true` (`rust_type` is `Option`). + /// `None` for all non-optional fields. + inner_opt_type: Option, } /// Resolve the Rust type and map-entry metadata for a single field. @@ -1272,6 +1315,7 @@ fn classify_field( quote! { #vec } }; + let mut inner_opt_type: Option = None; let rust_type = if let Some(entry) = map_entry { map_rust_type_from_entry(scope, entry, resolver)? } else if is_repeated { @@ -1298,6 +1342,7 @@ fn classify_field( } else { scalar_rust_type(field_type, resolver)? }; + inner_opt_type = Some(inner.clone()); { let opt = resolver.option(); quote! { #opt<#inner> } @@ -1360,9 +1405,19 @@ fn classify_field( map_key_type, map_value_type, map_value_enum_closed, + inner_opt_type, }) } +/// Setter method info for a single explicit-presence field. +struct SetterInfo { + ident: Ident, + param_type: TokenStream, + /// `true` → emit `Some(value.into())` (String, bytes::Bytes) + /// `false` → emit `Some(value)` (scalars, enums, Vec) + use_into: bool, +} + /// Generate a single field declaration. /// /// Returns `None` for fields that belong to a real oneof — those are @@ -1372,6 +1427,7 @@ fn classify_field( struct GeneratedField { tokens: TokenStream, ident: Ident, + setter: Option, } fn generate_field( @@ -1436,9 +1492,36 @@ fn generate_field( #custom_field_attrs pub #rust_name: #rust_type, }; + + // Use inner_opt_type as the gate: it is set only when classify_field + // actually took the is_optional branch (i.e. the struct field is Option). + // is_optional alone is not sufficient — proto2 repeated fields can have + // is_optional=true (explicit-presence default) while is_repeated=true. + let setter = if let Some(inner) = &info.inner_opt_type { + let field_type = crate::impl_message::effective_type(ctx, field, features); + let setter_ident = format_ident!("with_{}", field_name); + // impl Into where a common conversion exists: + // String: &str. Vec: &[u8; N] (From<&[T; N]> stable since Rust 1.74). + // bytes::Bytes: Vec. EnumValue: E (From impl on EnumValue). + let (param_type, use_into) = match field_type { + Type::TYPE_STRING | Type::TYPE_BYTES | Type::TYPE_ENUM => { + (quote! { impl Into<#inner> }, true) + } + _ => (quote! { #inner }, false), + }; + Some(SetterInfo { + ident: setter_ident, + param_type, + use_into, + }) + } else { + None + }; + Ok(Some(GeneratedField { tokens, ident: rust_name, + setter, })) } diff --git a/buffa-codegen/tests/codegen_integration.rs b/buffa-codegen/tests/codegen_integration.rs index 098df12..78c2fdc 100644 --- a/buffa-codegen/tests/codegen_integration.rs +++ b/buffa-codegen/tests/codegen_integration.rs @@ -853,3 +853,171 @@ fn inline_empty_message_no_unknown_fields() { assert!(content.contains("pub struct Empty")); assert!(!content.contains("__buffa_unknown_fields")); } + +// ── with_* setter tests ─────────────────────────────────────────────────────── + +#[test] +fn with_setters_emitted_for_explicit_presence_fields() { + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + enum Color { COLOR_UNSPECIFIED = 0; RED = 1; } + message Msg { + optional int32 count = 1; + optional string name = 2; + optional bytes data = 3; + optional Color color = 4; + // implicit-presence fields — no setter + int32 implicit_int = 5; + string implicit_str = 6; + // repeated — no setter + repeated string tags = 7; + } + "#, + &no_views(), + ); + + // Explicit-presence fields get setters. + assert!( + content.contains("pub fn with_count"), + "with_count missing: {content}" + ); + assert!( + content.contains("pub fn with_name"), + "with_name missing: {content}" + ); + assert!( + content.contains("pub fn with_data"), + "with_data missing: {content}" + ); + assert!( + content.contains("pub fn with_color"), + "with_color missing: {content}" + ); + + // String setter uses impl Into<...> for &str ergonomics. + assert!( + content.contains("impl Into"), + "string setter should use impl Into: {content}" + ); + + // Implicit-presence and repeated fields must NOT get setters. + assert!( + !content.contains("with_implicit_int"), + "implicit int should not get setter: {content}" + ); + assert!( + !content.contains("with_implicit_str"), + "implicit string should not get setter: {content}" + ); + assert!( + !content.contains("with_tags"), + "repeated field should not get setter: {content}" + ); +} + +#[test] +fn with_setters_disabled_by_config() { + let mut config = no_views(); + config.generate_with_setters = false; + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Msg { optional int32 count = 1; } + "#, + &config, + ); + assert!( + !content.contains("pub fn with_count"), + "setter should be absent when generate_with_setters=false: {content}" + ); +} + +#[test] +fn with_setters_bytes_type_uses_into() { + let mut config = no_views(); + config.bytes_fields.push(".test.Msg.data".into()); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Msg { optional bytes data = 1; } + "#, + &config, + ); + // bytes::Bytes field should use impl Into for ergonomics. + assert!( + content.contains("pub fn with_data"), + "with_data missing: {content}" + ); + assert!( + content.contains("impl Into"), + "bytes::Bytes setter should use impl Into: {content}" + ); +} + +#[test] +fn with_setters_vec_u8_uses_into() { + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Msg { optional bytes data = 1; } + "#, + &no_views(), + ); + // Vec bytes field uses impl Into: From<&[T; N]> for Vec is stable + // since Rust 1.74, so b"hello" works directly without .to_vec(). + assert!( + content.contains("pub fn with_data"), + "with_data missing: {content}" + ); + assert!( + content.contains("impl Into"), + "Vec setter should use impl Into: {content}" + ); +} + +#[test] +fn with_setters_enum_uses_into() { + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + enum Color { COLOR_UNSPECIFIED = 0; RED = 1; } + message Msg { optional Color color = 1; } + "#, + &no_views(), + ); + // EnumValue: From, so impl Into> lets callers pass the + // enum variant directly without wrapping in EnumValue::Known(...). + assert!( + content.contains("pub fn with_color"), + "with_color missing: {content}" + ); + assert!( + content.contains("impl Into"), + "enum setter should use impl Into: {content}" + ); +} + +#[test] +fn with_setters_proto2_repeated_no_setter() { + // Regression: proto2 repeated fields have is_explicit_presence=true due to + // proto2's EXPLICIT presence default, but their struct field is Vec, + // not Option. They must not receive a setter. + let content = generate_proto( + r#" + syntax = "proto2"; + package test; + message Msg { repeated string items = 1; } + "#, + &no_views(), + ); + assert!( + !content.contains("with_items"), + "proto2 repeated field should not get setter: {content}" + ); +} diff --git a/buffa-test/build.rs b/buffa-test/build.rs index 17259e0..20a47c0 100644 --- a/buffa-test/build.rs +++ b/buffa-test/build.rs @@ -250,6 +250,14 @@ fn main() { .compile() .expect("buffa_build failed for messageset.proto"); + // with_* setter methods (issue #30) — explicit-presence scalars/enum/bytes. + buffa_build::Config::new() + .files(&["protos/with_setters.proto"]) + .includes(&["protos/"]) + .generate_views(false) + .compile() + .expect("buffa_build failed for with_setters.proto"); + // Edition 2024 — requires protoc v30+ (stabilized edition 2024). // Older protoc rejects it with "later than the maximum supported edition". // Skip gracefully on older protoc so the crate still builds; tests are diff --git a/buffa-test/protos/with_setters.proto b/buffa-test/protos/with_setters.proto new file mode 100644 index 0000000..ae02844 --- /dev/null +++ b/buffa-test/protos/with_setters.proto @@ -0,0 +1,29 @@ +// End-to-end regression for with_* setter methods (issue #30). +// Exercises every field kind that should get a setter and verifies that +// kinds that must not (message, repeated, implicit-presence) don't. + +syntax = "proto3"; + +package test.setters; + +enum Priority { + PRIORITY_UNSPECIFIED = 0; + LOW = 1; + HIGH = 2; +} + +message Request { + // Explicit-presence fields — each gets a with_* setter. + optional int32 count = 1; + optional string name = 2; + optional bytes payload = 3; + optional bool enabled = 4; + optional Priority priority = 5; + + // Implicit-presence fields — no setter. + int32 implicit_count = 6; + string implicit_name = 7; + + // Repeated — no setter. + repeated string tags = 8; +} diff --git a/buffa-test/src/lib.rs b/buffa-test/src/lib.rs index 7c5afc8..c5886df 100644 --- a/buffa-test/src/lib.rs +++ b/buffa-test/src/lib.rs @@ -176,6 +176,15 @@ pub mod msgset { buffa::include_proto!("buffa.test.messageset"); } +#[allow( + clippy::derivable_impls, + clippy::match_single_binding, + non_camel_case_types +)] +pub mod with_setters { + buffa::include_proto!("test.setters"); +} + #[cfg(has_edition_2024)] #[allow( clippy::derivable_impls, diff --git a/buffa-test/src/tests/mod.rs b/buffa-test/src/tests/mod.rs index fec3ad1..b159532 100644 --- a/buffa-test/src/tests/mod.rs +++ b/buffa-test/src/tests/mod.rs @@ -46,4 +46,5 @@ mod proto3_semantics; mod textproto; mod utf8_validation; mod view; +mod with_setters; mod wkt; diff --git a/buffa-test/src/tests/with_setters.rs b/buffa-test/src/tests/with_setters.rs new file mode 100644 index 0000000..bb1879b --- /dev/null +++ b/buffa-test/src/tests/with_setters.rs @@ -0,0 +1,74 @@ +//! End-to-end tests for `with_*` setter methods (issue #30). + +use crate::with_setters::{Priority, Request}; +use buffa::{EnumValue, Message}; // EnumValue used in assertions + +fn round_trip(req: &Request) -> Request { + Request::decode(&mut req.encode_to_vec().as_slice()).expect("decode") +} + +#[test] +fn chained_setters_produce_correct_values() { + let req = Request::default() + .with_count(42) + .with_name("alice") + .with_enabled(true) + .with_priority(Priority::HIGH); // EnumValue: From — no wrapper needed + + assert_eq!(req.count, Some(42)); + assert_eq!(req.name, Some("alice".to_string())); + assert_eq!(req.enabled, Some(true)); + assert_eq!(req.priority, Some(EnumValue::Known(Priority::HIGH))); +} + +#[test] +fn setter_overwrites_prior_value() { + let req = Request::default().with_count(1).with_count(99); + assert_eq!(req.count, Some(99)); +} + +#[test] +fn unset_fields_remain_none() { + let req = Request::default().with_count(7); + assert_eq!(req.count, Some(7)); + assert_eq!(req.name, None); + assert_eq!(req.enabled, None); + assert_eq!(req.payload, None); +} + +#[test] +fn setters_round_trip() { + let req = Request::default() + .with_count(5) + .with_name("bob") + .with_payload(b"hello".to_vec()) + .with_enabled(false); + + let rt = round_trip(&req); + assert_eq!(rt.count, Some(5)); + assert_eq!(rt.name, Some("bob".to_string())); + assert_eq!(rt.payload, Some(b"hello".to_vec())); + assert_eq!(rt.enabled, Some(false)); +} + +#[test] +fn string_setter_accepts_str_ref() { + // with_name takes impl Into, so &str should compile directly. + let req = Request::default().with_name("charlie"); + assert_eq!(req.name, Some("charlie".to_string())); +} + +#[test] +fn bytes_setter_accepts_byte_array_literal() { + // Vec uses impl Into>; From<&[u8; N]> for Vec is stable + // since Rust 1.74, so b"..." literals work without .to_vec(). + let req = Request::default().with_payload(b"world"); + assert_eq!(req.payload, Some(b"world".to_vec())); +} + +#[test] +fn enum_setter_accepts_bare_variant() { + // EnumValue: From lets callers pass the variant directly. + let req = Request::default().with_priority(Priority::LOW); + assert_eq!(req.priority, Some(EnumValue::Known(Priority::LOW))); +}