diff --git a/.changes/unreleased/added-20261004-oneof-struct-field-attribute.yaml b/.changes/unreleased/added-20261004-oneof-struct-field-attribute.yaml new file mode 100644 index 00000000..980e7de8 --- /dev/null +++ b/.changes/unreleased/added-20261004-oneof-struct-field-attribute.yaml @@ -0,0 +1,4 @@ +kind: Added +body: |- + **`Config::oneof_struct_field_attribute` / `CodeGenConfig::oneof_struct_field_attributes`** (#561) put a custom attribute on the message struct's field holding a oneof (the `Option` member), matched against the oneof's path (`.pkg.Message.oneof_name`). `field_attribute` never reached that field: on the oneof's path it matches only the variants, where an attribute such as `#[serde(skip_serializing_if = "...")]` is rejected. prost-build puts such a `field_attribute` on the struct field as well. A `#[deprecated]` given through `oneof_struct_field_attribute` marks the owned struct's field only, and the generated items that visit it carry `#[allow(deprecated)]`. +time: 2026-10-04T21:00:00.000000000+00:00 diff --git a/buffa-build/src/lib.rs b/buffa-build/src/lib.rs index 31273c98..c5c9b691 100644 --- a/buffa-build/src/lib.rs +++ b/buffa-build/src/lib.rs @@ -1812,7 +1812,10 @@ impl Config { /// Prefix matching respects proto-segment boundaries. /// /// Also applies to oneof variants when `path` matches - /// `".pkg.Msg.my_oneof.variant_name"`. + /// `".pkg.Msg.my_oneof.variant_name"`, but not to the struct field holding + /// the oneof; use + /// [`oneof_struct_field_attribute`](Self::oneof_struct_field_attribute) for + /// that. /// /// A `#[deprecated]` supplied here wins over the one codegen derives from /// the field's `[deprecated = true]` option — rustc permits only one @@ -1931,7 +1934,9 @@ impl Config { /// order. The match key is the oneof's fully-qualified path /// (`.my.pkg.MyMessage.my_oneof`) — the whole-enum path has no variant /// segment; to target a single variant's field, append `.variant_name` - /// and use [`field_attribute`](Self::field_attribute) instead. A + /// and use [`field_attribute`](Self::field_attribute) instead, and for the + /// message struct's field holding the oneof, use + /// [`oneof_struct_field_attribute`](Self::oneof_struct_field_attribute). A /// malformed attribute produces a compile-time error in the generated /// code. Useful when a oneof needs a different attribute set than the /// surrounding types — for example to keep `#[derive(serde::Serialize)]` @@ -1976,6 +1981,100 @@ impl Config { self } + /// Add a custom attribute to the message struct's field holding a oneof + /// (the `Option` member), not to the oneof's variants. + /// + /// The match key is the oneof's fully-qualified path + /// (`.my.pkg.MyMessage.my_oneof`), with the same path-matching semantics + /// as [`type_attribute`](Self::type_attribute): `".my.pkg"` reaches the + /// oneof field of every message in the package, nested ones included. + /// Attributes accumulate in insertion order, after buffa's own attributes + /// on the field. A malformed attribute produces a compile-time error. + /// + /// A rule on a variant's path (`.my.pkg.MyMessage.my_oneof.variant_name`) + /// does not match the oneof, so it has no effect, and the build does not + /// warn about it. Variants take their attributes from + /// [`field_attribute`](Self::field_attribute). + /// + /// Each of these lines of the generated code for a oneof takes its + /// attributes from a different method: + /// + /// ```rust,ignore + /// pub struct Event { + /// // oneof_struct_field_attribute(".pkg.Event.payload", ..) + /// pub payload: Option, + /// } + /// + /// // oneof_attribute(".pkg.Event.payload", ..), or type_attribute on the same path + /// pub enum Payload { + /// // field_attribute(".pkg.Event.payload.text", ..) + /// Text(String), + /// } + /// ``` + /// + /// `field_attribute` on the oneof's path (`.pkg.Event.payload`) matches + /// the variants only, where an attribute + /// such as `#[serde(skip_serializing_if = "...")]` is rejected. prost-build + /// puts such a `field_attribute` on the struct field as well; use this + /// method for the struct field. + /// + /// Applies to the owned message struct only; the view structs do not get + /// the attribute. + /// + /// A `#[deprecated]` given here marks the owned struct's field only. The + /// same field on the view structs stays unmarked, so reading the oneof + /// through a view does not warn, unlike a field deprecated through + /// `field_attribute`. The generated items that visit the owned field + /// carry `#[allow(deprecated)]`. + /// + /// # Pitfalls + /// + /// With [`generate_json(true)`](Self::generate_json) the field already + /// carries buffa's `#[serde(flatten)]` for the derived `Serialize`, and + /// buffa generates the `Deserialize` impl of a message with a oneof + /// instead of deriving it. So a serde attribute given here changes the + /// owned message's `Serialize` output only: `Deserialize` and the views' + /// `Serialize` ignore it. A second `flatten` is a compile error in the + /// generated code. Serde attributes here are for a serde derive that you + /// attach yourself, with `generate_json` off. + /// + /// With + /// [`gate_impls_on_crate_features(true)`](Self::gate_impls_on_crate_features) + /// buffa's serde derive is compiled only under the JSON feature. Write a + /// serde attribute for it as `#[cfg_attr(feature = "json", serde(...))]`, + /// with your JSON feature's name if you renamed it, so that it is compiled + /// under the same feature. + /// + /// # Example + /// + /// ```rust,ignore + /// buffa_build::Config::new() + /// // `UnknownFields` does not implement `serde::Serialize`. With this + /// // off, unknown fields are dropped on decode. + /// .preserve_unknown_fields(false) + /// .message_attribute(".my.pkg.MyMessage", "#[derive(serde::Serialize)]") + /// .oneof_attribute(".my.pkg.MyMessage.my_oneof", "#[derive(serde::Serialize)]") + /// .oneof_struct_field_attribute( + /// ".my.pkg.MyMessage.my_oneof", + /// "#[serde(skip_serializing_if = \"Option::is_none\")]", + /// ) + /// .files(&["proto/my_service.proto"]) + /// .includes(&["proto/"]) + /// .compile() + /// .unwrap(); + /// ``` + #[must_use] + pub fn oneof_struct_field_attribute( + mut self, + path: impl Into, + attribute: impl Into, + ) -> Self { + self.codegen_config + .oneof_struct_field_attributes + .push((normalize_attr_path(path.into()), attribute.into())); + self + } + /// Use `buf build` instead of `protoc` for descriptor generation. /// /// `buf` is often easier to install and keep current than `protoc` @@ -3716,6 +3815,24 @@ mod tests { assert!(cfg.codegen_config.field_attributes.is_empty()); } + #[test] + fn oneof_struct_field_attribute_forwards_normalized_path() { + let cfg = + Config::new().oneof_struct_field_attribute("my.pkg.Msg.payload.", "#[serde(skip)]"); + assert_eq!( + cfg.codegen_config.oneof_struct_field_attributes, + vec![( + ".my.pkg.Msg.payload".to_string(), + "#[serde(skip)]".to_string() + )] + ); + assert!(cfg.codegen_config.oneof_attributes.is_empty()); + assert!(cfg.codegen_config.field_attributes.is_empty()); + assert!(cfg.codegen_config.type_attributes.is_empty()); + assert!(cfg.codegen_config.message_attributes.is_empty()); + assert!(cfg.codegen_config.enum_attributes.is_empty()); + } + #[test] fn oneof_attribute_forwards_normalized_path() { let cfg = Config::new().oneof_attribute("my.pkg.Msg.payload.", "#[derive(Hash)]"); @@ -3731,6 +3848,7 @@ mod tests { assert!(cfg.codegen_config.enum_attributes.is_empty()); assert!(cfg.codegen_config.message_attributes.is_empty()); assert!(cfg.codegen_config.field_attributes.is_empty()); + assert!(cfg.codegen_config.oneof_struct_field_attributes.is_empty()); } #[test] diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index cdfe07ee..3cd0a64a 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -1505,7 +1505,10 @@ pub struct CodeGenConfig { /// /// Each entry is `(proto_path, attribute)`. The `proto_path` is matched /// as a prefix against the fully-qualified field path (e.g., - /// `".my.pkg.MyMessage.my_field"`). `"."` applies to all fields. + /// `".my.pkg.MyMessage.my_field"`). `"."` applies to all fields. Oneof + /// variants are matched as `".my.pkg.MyMessage.my_oneof.variant"`; the + /// struct field holding the oneof is not reached, see + /// `oneof_struct_field_attributes`. pub field_attributes: Vec<(String, String)>, /// Custom attributes to inject on generated message structs only (not enums). /// @@ -1530,6 +1533,18 @@ pub struct CodeGenConfig { /// separate `enum_attributes` entry puts a different serde derive on the /// regular enums. pub oneof_attributes: Vec<(String, String)>, + /// Custom attributes to inject on a message struct's field holding a + /// oneof (the `Option` member), not on the oneof's variants or + /// the oneof enum. + /// + /// Same path-matching semantics as `type_attributes`, matched against the + /// oneof's fully-qualified path (`.pkg.Message.oneof_name`). + /// `field_attributes` never reaches this field: on the oneof's path it + /// matches only the variants (`.pkg.Message.oneof_name.variant`). + /// + /// Applies to the owned message struct only; the view structs do not get + /// the attribute. + pub oneof_struct_field_attributes: Vec<(String, String)>, /// Wrap generated `impl`s in `#[cfg(feature = "...")]` instead of /// emitting them unconditionally. /// @@ -1985,6 +2000,7 @@ impl Default for CodeGenConfig { message_attributes: Vec::new(), enum_attributes: Vec::new(), oneof_attributes: Vec::new(), + oneof_struct_field_attributes: Vec::new(), gate_impls_on_crate_features: false, generate_with_setters: true, generate_reflection: false, @@ -5372,8 +5388,9 @@ pub enum CodeGenError { MessageSetNotSupported { message_name: String }, /// A custom attribute string configured via [`CodeGenConfig::type_attributes`], /// [`CodeGenConfig::field_attributes`], [`CodeGenConfig::message_attributes`], - /// [`CodeGenConfig::enum_attributes`], or [`CodeGenConfig::oneof_attributes`] - /// could not be parsed as a Rust attribute. + /// [`CodeGenConfig::enum_attributes`], [`CodeGenConfig::oneof_attributes`], + /// or [`CodeGenConfig::oneof_struct_field_attributes`] could not be parsed + /// as a Rust attribute. #[error( "invalid custom attribute for path '{path}': '{attribute}' is not a valid \ Rust attribute ({detail})" diff --git a/buffa-codegen/src/message.rs b/buffa-codegen/src/message.rs index 31a96188..542a12c3 100644 --- a/buffa-codegen/src/message.rs +++ b/buffa-codegen/src/message.rs @@ -276,26 +276,29 @@ fn generate_message_with_nesting( } else { quote! {} }; - let oneof_generated: Vec<(TokenStream, Ident)> = msg - .oneof_decl - .iter() - .enumerate() - .filter_map(|(idx, oneof)| { - let enum_ident = oneof_idents.get(&idx)?; - let oneof_name = oneof.name.as_deref()?; - let field_ident = ctx.oneof_ident(oneof_name); - let opt = resolver.option_at(ctx, nesting); - let rename_note = ctx - .oneof_rename_note(oneof_name) - .map(|note| quote! { #[doc = #note] }); - let tokens = quote! { - #rename_note - #oneof_serde_attr - pub #field_ident: #opt<#oneof_prefix #enum_ident>, - }; - Some((tokens, field_ident)) - }) - .collect(); + let mut oneof_generated: Vec<(TokenStream, Ident)> = Vec::new(); + for (idx, oneof) in msg.oneof_decl.iter().enumerate() { + let (Some(enum_ident), Some(oneof_name)) = (oneof_idents.get(&idx), oneof.name.as_deref()) + else { + continue; + }; + let field_ident = ctx.oneof_ident(oneof_name); + let opt = resolver.option_at(ctx, nesting); + let rename_note = ctx + .oneof_rename_note(oneof_name) + .map(|note| quote! { #[doc = #note] }); + let custom_attrs = CodeGenContext::matching_attributes( + &ctx.config.oneof_struct_field_attributes, + &format!("{proto_fqn}.{oneof_name}"), + )?; + let tokens = quote! { + #rename_note + #oneof_serde_attr + #custom_attrs + pub #field_ident: #opt<#oneof_prefix #enum_ident>, + }; + oneof_generated.push((tokens, field_ident)); + } let oneof_struct_fields: Vec<&TokenStream> = oneof_generated.iter().map(|(t, _)| t).collect(); // Redaction of oneof payloads is handled by the oneof enum's own Debug impl. debug_fields.extend(oneof_generated.iter().map(|(_, id)| (id, false))); @@ -2158,11 +2161,17 @@ pub(crate) fn is_deprecated(field: &crate::generated::descriptor::FieldDescripto /// of it. Unparseable attribute strings are ignored here — they are reported /// by [`CodeGenContext::matching_attributes`] on the way to the same field. pub(crate) fn caller_deprecated_attr(ctx: &CodeGenContext, fqn: &str) -> bool { - if ctx.config.field_attributes.is_empty() { + rules_deprecate(&ctx.config.field_attributes, fqn) +} + +/// True when one of the caller's `(path, attribute)` rules matches `fqn` and +/// carries a `#[deprecated]` marker. +fn rules_deprecate(rules: &[(String, String)], fqn: &str) -> bool { + if rules.is_empty() { return false; } let fqn_dotted = format!(".{fqn}"); - ctx.config.field_attributes.iter().any(|(prefix, attr)| { + rules.iter().any(|(prefix, attr)| { crate::context::matches_proto_prefix(prefix, &fqn_dotted) && is_deprecated_attr_str(attr) }) } @@ -2268,11 +2277,21 @@ pub(crate) fn default_names_deprecated_value( /// Whether generated code for `msg` touches anything marked deprecated: one of /// its own non-oneof fields (from the option or from a caller's -/// `field_attribute`), or an enum variant named by a field's `[default = …]`. +/// `field_attribute`), a struct field holding a oneof (from a caller's +/// `oneof_struct_field_attribute`), or an enum variant named by a field's +/// `[default = …]`. fn references_deprecated(ctx: &CodeGenContext, msg: &DescriptorProto, proto_fqn: &str) -> bool { + let oneof_rules = &ctx.config.oneof_struct_field_attributes; msg.field.iter().any(|f| { if crate::impl_message::is_real_oneof_member(f) { - return false; + // The member is a variant; the struct field is its oneof's. + return !oneof_rules.is_empty() + && f.oneof_index + .and_then(|idx| msg.oneof_decl.get(usize::try_from(idx).ok()?)) + .and_then(|oneof| oneof.name.as_deref()) + .is_some_and(|name| { + rules_deprecate(oneof_rules, &format!("{proto_fqn}.{name}")) + }); } let field_fqn = format!("{proto_fqn}.{}", f.name.as_deref().unwrap_or_default()); field_is_deprecated(ctx, f, &field_fqn) || default_names_deprecated_value(ctx, f) diff --git a/buffa-codegen/src/tests/custom_attributes.rs b/buffa-codegen/src/tests/custom_attributes.rs index fbb440de..5f8d0af3 100644 --- a/buffa-codegen/src/tests/custom_attributes.rs +++ b/buffa-codegen/src/tests/custom_attributes.rs @@ -320,8 +320,16 @@ fn test_field_attribute_catchall() { // ── oneof coverage ────────────────────────────────────────────────── +/// A message with a plain `id` field ahead of one oneof, so an attribute on the +/// struct and an attribute on the struct's first field are different places +/// from the oneof field. fn oneof_message(name: &str, oneof_name: &str, variant_names: &[&str]) -> DescriptorProto { - let mut fields = Vec::new(); + let mut fields = vec![make_field( + "id", + 100, + Label::LABEL_OPTIONAL, + Type::TYPE_INT32, + )]; for (i, v) in variant_names.iter().enumerate() { let mut f = make_field(v, (i + 1) as i32, Label::LABEL_OPTIONAL, Type::TYPE_STRING); f.oneof_index = Some(0); @@ -410,6 +418,268 @@ fn test_oneof_attribute_on_oneof_not_message_or_enum() { ); } +// ── oneof_struct_field_attribute tests ────────────────────────────── + +const MARKER: &str = "#[allow(clippy::pedantic)]"; +const SECOND_MARKER: &str = "#[allow(clippy::nursery)]"; + +/// Every item of the generated files, including the items of inline modules. +fn all_items(files: &[GeneratedFile]) -> Vec { + fn walk(items: Vec, out: &mut Vec) { + for item in items { + if let syn::Item::Mod(module) = &item { + if let Some((_, inner)) = &module.content { + walk(inner.clone(), out); + } + } + out.push(item); + } + } + let mut out = Vec::new(); + for f in files { + let parsed = syn::parse_file(&f.content) + .unwrap_or_else(|e| panic!("{} must parse: {e}\n{}", f.name, f.content)); + walk(parsed.items, &mut out); + } + out +} + +/// The attributes as source text without whitespace, in declaration order. +fn rendered(attrs: &[syn::Attribute]) -> Vec { + use quote::ToTokens; + attrs + .iter() + .map(|a| { + let mut text = a.to_token_stream().to_string(); + text.retain(|c| !c.is_whitespace()); + text + }) + .collect() +} + +fn find_struct<'a>(items: &'a [syn::Item], name: &str) -> &'a syn::ItemStruct { + let mut found = items.iter().filter_map(|item| match item { + syn::Item::Struct(s) if s.ident == name => Some(s), + _ => None, + }); + let first = found + .next() + .unwrap_or_else(|| panic!("struct {name} is generated")); + assert!(found.next().is_none(), "one struct is named {name}"); + first +} + +/// The rendered attributes of the named field of `item`. +fn field_attrs(item: &syn::ItemStruct, field: &str) -> Vec { + let field = item + .fields + .iter() + .find(|f| f.ident.as_ref().is_some_and(|ident| ident == field)) + .unwrap_or_else(|| panic!("{} has a field {field}", item.ident)); + rendered(&field.attrs) +} + +/// Generated code as parsed items and as source text without whitespace. +struct Generated { + items: Vec, + source: String, +} + +/// Asserts that [`MARKER`] is on the field `oneof_field` of each struct in +/// `on_structs` and nowhere else in the generated code. +fn assert_marker_exactly_on_fields(generated: &Generated, oneof_field: &str, on_structs: &[&str]) { + let has_marker = |attrs: &[syn::Attribute]| rendered(attrs).iter().any(|a| a == MARKER); + for item in &generated.items { + let syn::Item::Struct(s) = item else { + continue; + }; + for f in &s.fields { + let name = f.ident.as_ref().map(ToString::to_string); + let expected = + name.as_deref() == Some(oneof_field) && on_structs.iter().any(|n| s.ident == n); + assert_eq!(has_marker(&f.attrs), expected, "{}.{name:?}", s.ident); + } + } + // The fields above account for every occurrence, so a struct, an enum, a + // variant, an impl and a method are all unmarked. + assert_eq!( + generated.source.matches(MARKER).count(), + on_structs.len(), + "{}", + generated.source + ); +} + +fn oneof_field_config(rules: &[(&str, &str)]) -> CodeGenConfig { + CodeGenConfig { + oneof_struct_field_attributes: rules + .iter() + .map(|(p, a)| (p.to_string(), a.to_string())) + .collect(), + ..CodeGenConfig::default() + } +} + +/// `pkg.Msg` and `pkg.Outer.Inner`, each with an `id` field and a oneof +/// `payload`. +fn two_oneof_messages_file() -> FileDescriptorProto { + let mut file = proto3_file("oneofs.proto"); + file.package = Some("pkg".to_string()); + file.message_type + .push(oneof_message("Msg", "payload", &["a", "b"])); + file.message_type.push(DescriptorProto { + name: Some("Outer".to_string()), + nested_type: vec![oneof_message("Inner", "payload", &["a", "b"])], + ..Default::default() + }); + file +} + +fn generate_items(file: FileDescriptorProto, config: &CodeGenConfig) -> Generated { + let name = file.name.clone().unwrap_or_default(); + let files = generate(&[file], &[name], config).expect("should generate"); + let mut source = joined(&files); + source.retain(|c| !c.is_whitespace()); + Generated { + items: all_items(&files), + source, + } +} + +#[test] +fn test_oneof_struct_field_attribute_on_oneof_field_only() { + // Views are on: the view struct has a `payload` field too, and stays bare. + let generated = generate_items( + two_oneof_messages_file(), + &oneof_field_config(&[(".pkg.Msg.payload", MARKER)]), + ); + assert_eq!( + field_attrs(find_struct(&generated.items, "Msg"), "payload"), + [MARKER], + "the oneof field carries exactly the custom attribute" + ); + assert!(field_attrs(find_struct(&generated.items, "MsgView"), "payload").is_empty()); + assert_marker_exactly_on_fields(&generated, "payload", &["Msg"]); +} + +/// With JSON on, buffa's own `serde(flatten)` comes first on the oneof field +/// and the custom attribute follows it. +#[test] +fn test_oneof_struct_field_attribute_nested_with_json_and_views() { + let config = CodeGenConfig { + generate_json: true, + ..oneof_field_config(&[(".pkg.Outer.Inner.payload", MARKER)]) + }; + let generated = generate_items(two_oneof_messages_file(), &config); + assert_eq!( + field_attrs(find_struct(&generated.items, "Inner"), "payload"), + ["#[serde(flatten)]", MARKER] + ); + assert_eq!( + field_attrs(find_struct(&generated.items, "Msg"), "payload"), + ["#[serde(flatten)]"] + ); + assert_marker_exactly_on_fields(&generated, "payload", &["Inner"]); +} + +#[test] +fn test_oneof_struct_field_attribute_prefix_and_catch_all() { + for rule in [".pkg", "."] { + let generated = generate_items( + two_oneof_messages_file(), + &oneof_field_config(&[(rule, MARKER)]), + ); + assert_marker_exactly_on_fields(&generated, "payload", &["Msg", "Inner"]); + } + // A prefix ends on a path segment, and a variant's path is longer than the + // oneof's, so these rules match nothing. + for rule in [".pkg.Msg.pay", ".pkg.Msg.payload.a", ".pk"] { + let generated = generate_items( + two_oneof_messages_file(), + &oneof_field_config(&[(rule, MARKER)]), + ); + assert_marker_exactly_on_fields(&generated, "payload", &[]); + } +} + +#[test] +fn test_oneof_struct_field_attribute_skips_synthetic_oneof() { + // `optional string note` is a member of the synthetic oneof `_note`, which + // has no struct field of its own. + let mut msg = oneof_message("Msg", "payload", &["a", "b"]); + let mut note = make_field("note", 50, Label::LABEL_OPTIONAL, Type::TYPE_STRING); + note.oneof_index = Some(1); + note.proto3_optional = Some(true); + msg.field.push(note); + msg.oneof_decl.push(OneofDescriptorProto { + name: Some("_note".to_string()), + ..Default::default() + }); + let mut file = proto3_file("synthetic.proto"); + file.package = Some("pkg".to_string()); + file.message_type.push(msg); + let generated = generate_items(file, &oneof_field_config(&[(".", MARKER)])); + assert!(field_attrs(find_struct(&generated.items, "Msg"), "note") + .iter() + .all(|a| a != MARKER)); + assert_marker_exactly_on_fields(&generated, "payload", &["Msg"]); +} + +#[test] +fn test_oneof_struct_field_attribute_follows_rename_note() { + // The oneof `self` would be `self_`, which the field has, so the oneof + // field is `self__` and carries a doc note about the rename. + let mut msg = oneof_message("Msg", "self", &["a"]); + msg.field[0].name = Some("self_".to_string()); + let mut file = proto3_file("renamed.proto"); + file.package = Some("pkg".to_string()); + file.message_type.push(msg); + let generated = generate_items(file, &oneof_field_config(&[(".pkg.Msg.self", MARKER)])); + let attrs = field_attrs(find_struct(&generated.items, "Msg"), "self__"); + assert_eq!(attrs.len(), 2, "{attrs:?}"); + assert!( + attrs[0].starts_with("#[doc=") && attrs[0].contains("`self__`"), + "the rename note comes first: {attrs:?}" + ); + assert_eq!(attrs[1], MARKER); +} + +#[test] +fn test_oneof_struct_field_attributes_accumulate_in_insertion_order() { + for (rules, expected) in [ + ( + [(".", MARKER), (".pkg.Msg.payload", SECOND_MARKER)], + [MARKER, SECOND_MARKER], + ), + ( + [(".pkg.Msg.payload", SECOND_MARKER), (".", MARKER)], + [SECOND_MARKER, MARKER], + ), + ] { + let generated = generate_items(two_oneof_messages_file(), &oneof_field_config(&rules)); + assert_eq!( + field_attrs(find_struct(&generated.items, "Msg"), "payload"), + expected + ); + } +} + +#[test] +fn test_oneof_struct_field_attribute_invalid_attribute_errors() { + let file = two_oneof_messages_file(); + let config = oneof_field_config(&[(".pkg.Msg.payload", "not a valid #[attribute")]); + let err = generate(&[file], &["oneofs.proto".to_string()], &config) + .expect_err("malformed attribute should error"); + assert!( + matches!( + &err, + CodeGenError::InvalidCustomAttribute { path, attribute, .. } + if path == ".pkg.Msg.payload" && attribute == "not a valid #[attribute" + ), + "{err:?}" + ); +} + #[test] fn test_oneof_attribute_specific_path_matches_one_oneof() { let mut file = proto3_file("two.proto"); diff --git a/buffa-codegen/src/tests/deprecation.rs b/buffa-codegen/src/tests/deprecation.rs index 1539669f..3c97f757 100644 --- a/buffa-codegen/src/tests/deprecation.rs +++ b/buffa-codegen/src/tests/deprecation.rs @@ -613,6 +613,69 @@ fn oneof_only_deprecation_is_unmarked_and_unguarded() { } } +#[test] +fn caller_deprecated_oneof_struct_field_guards_the_impls_that_visit_it() { + // `Widget.choice` is the only deprecated member, and only through the + // caller's `oneof_struct_field_attribute`. The `[deprecated = true]` on + // the oneof's member plays no part. + let rule = |path: &str| CodeGenConfig { + oneof_struct_field_attributes: vec![( + path.to_string(), + "#[deprecated(note = \"hand\")]".to_string(), + )], + ..Default::default() + }; + let impl_heads = [ + "impl::buffa::MessageforWidget", + "impl::core::fmt::DebugforWidget", + "impl<'a>::buffa::MessageView<'a>forWidgetView<'a>", + ]; + + let content = generate_squashed(oneof_only_file(), &rule(".deprecate.test.Widget.choice")); + assert!( + content.contains(r#"#[deprecated(note="hand")]pubchoice:"#), + "the owned struct's oneof field takes the attribute: {content}" + ); + assert_eq!(content.matches("#[deprecated").count(), 1, "{content}"); + for impl_head in impl_heads { + assert!( + content.contains(&format!("#[allow(deprecated)]{impl_head}")), + "`{impl_head}` visits the deprecated oneof field: {content}" + ); + } + + // The rule is matched against oneof paths, so one that names the plain + // field `label` deprecates nothing and guards nothing. + let content = generate_squashed(oneof_only_file(), &rule(".deprecate.test.Widget.label")); + assert!(!content.contains("#[deprecated"), "{content}"); + assert!(!content.contains("#[allow(deprecated)]"), "{content}"); + + // Nor does one that reaches only the synthetic oneof of an `optional` + // field, which has no struct field of its own. + let mut note = make_field("note", 1, Label::LABEL_OPTIONAL, Type::TYPE_STRING); + note.oneof_index = Some(0); + note.proto3_optional = Some(true); + let mut file = proto3_file("synthetic.proto"); + file.package = Some("deprecate.test".to_string()); + file.message_type.push(DescriptorProto { + name: Some("Widget".to_string()), + field: vec![note], + oneof_decl: vec![OneofDescriptorProto { + name: Some("_note".to_string()), + ..Default::default() + }], + ..Default::default() + }); + for path in [".", ".deprecate.test.Widget._note"] { + let content = generate_squashed(file.clone(), &rule(path)); + assert!(!content.contains("#[deprecated"), "{path}: {content}"); + assert!( + !content.contains("#[allow(deprecated)]"), + "{path}: {content}" + ); + } +} + // ── views ─────────────────────────────────────────────────────────── #[test] diff --git a/buffa-test/build.rs b/buffa-test/build.rs index 74a3c1e7..deec130d 100644 --- a/buffa-test/build.rs +++ b/buffa-test/build.rs @@ -848,7 +848,9 @@ fn main() { // `-D warnings`. `generate_arbitrary` adds the `Arbitrary` impls, which // only a build with the `arbitrary` feature compiles. One field takes its // `#[deprecated]` from `field_attribute`, which marks and guards the same - // items as the option. + // items as the option. `HandMarkedGroup.body` takes one from + // `oneof_struct_field_attribute`, which guards the impls that visit the + // oneof's struct field. let mut deprecated = buffa_build::Config::new() .files(&["protos/deprecated.proto"]) .includes(&["protos/"]) @@ -856,6 +858,10 @@ fn main() { ".deprecated.LegacyProfile.hand_marked", "#[deprecated(note = \"marked in build.rs\")]", ) + .oneof_struct_field_attribute( + ".deprecated.HandMarkedGroup.body", + "#[deprecated(note = \"marked in build.rs\")]", + ) .generate_views(true) .generate_text(true) .generate_json(true) @@ -888,6 +894,28 @@ fn main() { .compile() .expect("buffa_build failed for deprecated_proto2.proto"); + // `oneof_struct_field_attribute` — a `Serialize` that the caller derives, + // with `generate_json` off. `UnknownFields` does not implement + // `Serialize`, so the derive needs unknown-field preservation off. + buffa_build::Config::new() + .files(&["protos/oneof_struct_field_attr.proto"]) + .includes(&["protos/"]) + .preserve_unknown_fields(false) + .message_attribute( + ".oneof_struct_field_attr.Event", + "#[derive(serde::Serialize)]", + ) + .oneof_attribute( + ".oneof_struct_field_attr.Event.payload", + "#[derive(serde::Serialize)]", + ) + .oneof_struct_field_attribute( + ".oneof_struct_field_attr.Event.payload", + "#[serde(skip_serializing_if = \"Option::is_none\")]", + ) + .compile() + .expect("buffa_build failed for oneof_struct_field_attr.proto"); + // `skip_debug` — the hand-written `Debug` impls in `src/lib.rs` compile // only if the generated ones are omitted. Views enabled so the view of a // matched message compiles too. diff --git a/buffa-test/protos/deprecated.proto b/buffa-test/protos/deprecated.proto index a257b1cd..117b09c5 100644 --- a/buffa-test/protos/deprecated.proto +++ b/buffa-test/protos/deprecated.proto @@ -48,6 +48,17 @@ message Audit { string legacy_action = 2 [deprecated = true]; } +// Live in the schema; build.rs deprecates the struct field holding `body` with +// `oneof_struct_field_attribute`. The message has no other deprecated member, +// so that attribute alone has to put the guard on the impls that visit the +// field. +message HandMarkedGroup { + oneof body { + string text = 1; + int32 code = 2; + } +} + enum Routing { option allow_alias = true; diff --git a/buffa-test/protos/oneof_struct_field_attr.proto b/buffa-test/protos/oneof_struct_field_attr.proto new file mode 100644 index 00000000..0ea256a4 --- /dev/null +++ b/buffa-test/protos/oneof_struct_field_attr.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +package oneof_struct_field_attr; + +// Compiled with `generate_json` off. build.rs derives `serde::Serialize` on +// the message and on the oneof enum, and gives the struct field holding the +// oneof a `skip_serializing_if` with `oneof_struct_field_attribute`. +message Event { + string id = 1; + oneof payload { + string text = 2; + int32 code = 3; + } +} diff --git a/buffa-test/src/lib.rs b/buffa-test/src/lib.rs index 489508ca..1239ab34 100644 --- a/buffa-test/src/lib.rs +++ b/buffa-test/src/lib.rs @@ -31,6 +31,13 @@ pub mod deprecated_proto2 { buffa::include_proto!("deprecated_proto2"); } +/// `oneof_struct_field_attribute` — `build.rs` derives `serde::Serialize` and +/// puts a serde attribute on the struct field holding the oneof. +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +pub mod oneof_struct_field_attr { + buffa::include_proto!("oneof_struct_field_attr"); +} + /// `skip_debug` — hand-written `Debug` impls for the types `build.rs` names /// in its rules. #[allow(clippy::derivable_impls, clippy::match_single_binding)] diff --git a/buffa-test/src/tests/mod.rs b/buffa-test/src/tests/mod.rs index 3a04c06e..1b10d135 100644 --- a/buffa-test/src/tests/mod.rs +++ b/buffa-test/src/tests/mod.rs @@ -104,6 +104,7 @@ mod message_set; mod mod_collision; mod nesting; mod nestpkg; +mod oneof_struct_field_attr; mod open_enums; mod owned_view; mod proto2; diff --git a/buffa-test/src/tests/oneof_struct_field_attr.rs b/buffa-test/src/tests/oneof_struct_field_attr.rs new file mode 100644 index 00000000..e461cb72 --- /dev/null +++ b/buffa-test/src/tests/oneof_struct_field_attr.rs @@ -0,0 +1,30 @@ +//! `oneof_struct_field_attribute`: a serde attribute on the struct field +//! holding a oneof, read by a `Serialize` that build.rs derives itself. That +//! the module compiles shows the attribute is on the field and not on a +//! variant, where serde rejects `skip_serializing_if`. + +use crate::oneof_struct_field_attr::{event, Event}; + +#[test] +fn unset_oneof_is_skipped_by_the_derived_serialize() { + let event = Event { + id: "e1".to_string(), + ..Default::default() + }; + assert_eq!( + serde_json::to_value(&event).unwrap(), + serde_json::json!({ "id": "e1" }) + ); +} + +#[test] +fn set_oneof_is_serialized_by_the_derived_serialize() { + let event = Event { + id: "e1".to_string(), + payload: Some(event::Payload::Text("hi".to_string())), + }; + assert_eq!( + serde_json::to_value(&event).unwrap(), + serde_json::json!({ "id": "e1", "payload": { "Text": "hi" } }) + ); +} diff --git a/docs/guide.md b/docs/guide.md index df73be19..8b9d5fec 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -231,7 +231,7 @@ The macro pulls in `OUT_DIR/.mod.rs`, which in turn includes the per | `.file_per_package(bool)` | `false` | Emit one `.rs` per package instead of per-proto-file content + a stitcher | | `.idiomatic_imports(bool)` | `false` | **Experimental.** Emit `use`-backed short type names at the package root (struct fields read `MessageField` instead of fully-qualified paths). Requires `.file_per_package(true)`. Only type declarations are shortened — impl bodies and nested modules stay fully qualified — and the generated file must keep its `#[allow]` wrapper (the short names coexist with qualified impl-body paths, which `unused_qualifications` would otherwise flag) | | `.type_attribute(path, attr)` / `.message_attribute` / `.enum_attribute` / `.oneof_attribute` | — | Attach a Rust attribute (e.g. an extra `#[derive(...)]`) to generated types matching a proto path prefix (`oneof_attribute` matches the oneof's own path, `.pkg.Msg.oneof_name`) | -| `.field_attribute(path, attr)` | — | Attach a Rust attribute to generated fields matching a proto path prefix | +| `.field_attribute(path, attr)` / `.oneof_struct_field_attribute` | — | Attach a Rust attribute to generated fields matching a proto path prefix (`field_attribute` reaches oneof variants as `.pkg.Msg.oneof_name.variant`; `oneof_struct_field_attribute` reaches the struct field holding the oneof, `.pkg.Msg.oneof_name`) | | `.use_buf()` | — | Use `buf build` instead of `protoc` for descriptor generation | | `.include_file(name)` | — | Generate a module tree file for `include!` (recommended) | | `.descriptor_set(path)` | — | Use a pre-compiled `FileDescriptorSet` file | @@ -1107,6 +1107,10 @@ Two things are not marked: attach to the variant with `field_attribute` goes on the owned oneof enum only. The generated impls that match on it are not guarded, so `examples/addressbook` keeps a module-level `#[allow(deprecated)]`. + The struct field holding the oneof is a separate item. A `#[deprecated]` that + you attach to it with `oneof_struct_field_attribute` goes on the owned struct's + field only (the view's field stays unmarked), and the generated impls that + visit the field are guarded. - **Whole-message and whole-enum deprecation is not emitted**, matching prost. A derive that you attach with `enum_attribute` or `type_attribute` can name a diff --git a/docs/migration-from-prost.md b/docs/migration-from-prost.md index 75120ca7..72e856ad 100644 --- a/docs/migration-from-prost.md +++ b/docs/migration-from-prost.md @@ -295,7 +295,7 @@ The buffa equivalent of each prost-build feature, or a note that there is none: | `bytes(&[...])` | Supported. `.use_bytes_type()` for all, or `.use_bytes_type_in(&[...])` for specific fields. Paths are matched as for `map_type_in`. | | `extern_path(proto, rust)` | Supported. Same API, both package-level (`.extern_path(".pkg", "::crate")`) and per-type (`.extern_path(".google.protobuf.Timestamp", "::pbjson_types::Timestamp")`) mappings. A per-type mapping to a non-buffa crate requires `.generate_views(false)`, or map to a buffa-generated crate instead — see [External type paths](guide.md#external-type-paths). | | `type_attribute(path, attr)` | Supported. Same API, plus `message_attribute` / `enum_attribute` / `oneof_attribute` for narrower targeting. (For serde, prefer `generate_json(true)`, which emits the proto3-canonical JSON impls.) | -| `field_attribute(path, attr)` | Supported. Same API. | +| `field_attribute(path, attr)` | Supported. Same API, with one difference: on a oneof's path (`".pkg.Msg.my_oneof"`), prost-build puts the attribute on the struct field holding the oneof and on every variant, buffa on the variants only. Use `oneof_struct_field_attribute` for the struct field. | | `skip_debug(&[...])` | Supported. `buffa_build::Config::skip_debug(&[...])` omits the generated `Debug` for matching messages and their oneof enums, so you can write your own. Four differences from prost-build. Paths must be fully qualified (`".my_pkg.Uuid4"`): a prost suffix path (`"Uuid4"`) matches nothing and produces a build warning. Repeated calls accumulate, where prost-build's replace the list. An enum loses its `Debug` only when a path is its exact name, and then needs a hand-written impl, because `buffa::Enumeration` requires `Debug`. View types keep their generated `Debug`, so `skip_debug(&["."])` removes the impls of owned messages and their oneof enums only. | | `service_generator(...)` | Not supported. Services codegen is planned. | | `#[derive(prost::Message)]` | No derive; generate from `.proto` or use `extern_path` for an existing Rust type. See [Where is `#[derive(Message)]`?](#where-is-derivemessage). |