From c34450f41dbbb5e7466b3f63469b95f097111a7f Mon Sep 17 00:00:00 2001 From: Dmitry Voropaev Date: Wed, 28 Oct 2026 11:57:59 +0300 Subject: [PATCH] codegen: closed-enum extension without a default reads as its first value extension_or_default fell back to i32::default() for a singular enum extension with no [default = ...], so a proto2 extension of enum Level { LOW = 1; HIGH = 2; } read as 0, a number the enum does not declare. The same happened for an editions enum with enum_type = CLOSED. An unset field of that enum already reads as LOW, through the generated Default impl, reflection and the map codec. Codegen now passes Extension::with_default a fn returning the enum's first value via its Default impl, so extern_path enums work too. Open enums start at 0 and repeated extensions have no default, so neither changes. --- .../unreleased/fixed-20261028-115331.yaml | 4 + buffa-codegen/src/extension.rs | 145 +++++++++++++++++- buffa-test/protos/custom_options.proto | 7 + buffa-test/protos/group_ext.proto | 16 ++ buffa-test/src/tests/extensions.rs | 35 ++++- buffa/src/extension.rs | 13 +- 6 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 .changes/unreleased/fixed-20261028-115331.yaml diff --git a/.changes/unreleased/fixed-20261028-115331.yaml b/.changes/unreleased/fixed-20261028-115331.yaml new file mode 100644 index 000000000..e3dd1139b --- /dev/null +++ b/.changes/unreleased/fixed-20261028-115331.yaml @@ -0,0 +1,4 @@ +kind: Fixed +body: |- + **`extension_or_default` returns a closed enum's first value for an extension that declares no default** (#801). A singular extension of a closed enum (any proto2 enum, or an editions enum with `enum_type = CLOSED`) without `[default = ...]` read as 0 when absent, and 0 need not be a value of the enum: for `enum Level { LOW = 1; HIGH = 2; }` it is not. Generated code now gives such an extension the enum's first declared value as its default, the number an unset field of that enum already reads as. Extensions of open enums, whose first value is always 0, and repeated extensions are unchanged, and `extension()` still returns `None` for an absent extension. +time: 2026-10-28T11:53:31.504699+03:00 diff --git a/buffa-codegen/src/extension.rs b/buffa-codegen/src/extension.rs index 526a20e4a..52d35f902 100644 --- a/buffa-codegen/src/extension.rs +++ b/buffa-codegen/src/extension.rs @@ -101,6 +101,8 @@ fn generate_one( // Proto2 `[default = ...]`. The proto spec only allows defaults on singular // scalar/enum fields; protoc rejects message/group/repeated at parse time. // We check defensively because a hand-crafted descriptor could slip through. + // A singular closed-enum extension without one defaults to the enum's + // first declared value, as a field of that enum does. let default_fn = if field .default_value .as_deref() @@ -127,6 +129,17 @@ fn generate_one( nesting, features, )?) + } else if !repeated + && ty == Type::TYPE_ENUM + && crate::message::is_closed_enum(&crate::features::resolve_field(ctx, field, features)) + { + Some(closed_enum_default_fn_tokens( + ctx, + field, + &const_ident, + current_package, + nesting, + )?) } else { None }; @@ -361,6 +374,35 @@ fn default_fn_tokens( )) } +/// Build the default fn of a singular closed-enum extension that declares no +/// `[default = ...]`: the enum's first declared value, which is what an unset +/// field of that enum reads as. Without it `extension_or_default` would return +/// `i32::default()`, and 0 need not be a value of a closed enum +/// (`enum Level { LOW = 1; HIGH = 2; }`). An open enum's first value is +/// always 0, so it needs none. +/// +/// The fn is not `const`: it goes through the enum's `Default` impl, which +/// also covers an `extern_path` enum whose values this build cannot see. +fn closed_enum_default_fn_tokens( + ctx: &CodeGenContext, + field: &FieldDescriptorProto, + const_ident: &Ident, + current_package: &str, + nesting: usize, +) -> Result<(Ident, TokenStream), CodeGenError> { + let fn_ident = format_ident!("__{}_default", const_ident.to_string().to_lowercase()); + let first = crate::reflect_view::enum_default_number(ctx, field, current_package, nesting)?; + Ok(( + fn_ident.clone(), + quote! { + #[doc(hidden)] + fn #fn_ident() -> i32 { + #first + } + }, + )) +} + /// Map a proto scalar/enum type to its `ExtensionCodec::Value` Rust type. /// /// Only called for types that can carry a `[default = ...]` — message/group @@ -767,10 +809,10 @@ mod tests { #[test] fn enum_extension_doc_mentions_type() { let mut field = ext_field("level", 50006, Type::TYPE_ENUM); - field.type_name = Some(".my.pkg.LogLevel".to_string()); - let tokens = gen(&field).unwrap(); + field.type_name = Some(".my.pkg.Color".to_string()); + let (tokens, _) = gen_in_pkg(&field).unwrap(); assert!(tokens.contains("EnumI32"), "{tokens}"); - assert!(tokens.contains("my.pkg.LogLevel"), "{tokens}"); + assert!(tokens.contains("my.pkg.Color"), "{tokens}"); } #[test] @@ -1112,6 +1154,103 @@ mod tests { assert!(tokens.contains("Color :: RED as i32"), "{tokens}"); } + /// Run `generate_one` for extension `level` of enum `.my.pkg.Level`, + /// declared in `file` (package `my.pkg`), under the file's features. + fn gen_enum_ext(file: FileDescriptorProto, label: Label) -> String { + let files = [file]; + let config = CodeGenConfig::default(); + let ctx = CodeGenContext::new(&files, &config, &[]); + let features = crate::features::for_file(&files[0]); + let mut field = ext_field("level", 50040, Type::TYPE_ENUM); + field.type_name = Some(".my.pkg.Level".into()); + field.label = Some(label); + generate_one(&ctx, &field, "my.pkg", 0, &features, "my.pkg") + .unwrap() + .unwrap() + .0 + .to_string() + } + + fn level_file(syntax: &str, level: EnumDescriptorProto) -> FileDescriptorProto { + FileDescriptorProto { + name: Some("level.proto".into()), + package: Some("my.pkg".into()), + syntax: Some(syntax.into()), + edition: (syntax == "editions") + .then_some(crate::generated::descriptor::Edition::EDITION_2023), + enum_type: vec![level], + ..Default::default() + } + } + + fn level_enum( + enum_type: Option, + ) -> EnumDescriptorProto { + use crate::generated::descriptor::{EnumOptions, FeatureSet}; + EnumDescriptorProto { + name: Some("Level".into()), + options: match enum_type { + Some(t) => buffa::MessageField::some(EnumOptions { + features: buffa::MessageField::some(FeatureSet { + enum_type: Some(t), + ..Default::default() + }), + ..Default::default() + }), + None => buffa::MessageField::none(), + }, + ..Default::default() + } + } + + const FIRST_VALUE: &str = + ":: buffa :: Enumeration :: to_i32 (& < Level as :: core :: default :: Default > :: default ())"; + + /// A closed enum need not declare 0, so a singular extension of one with no + /// `[default]` reads absent as the enum's first value, through a non-const + /// fn, in proto2 and under editions `enum_type = CLOSED`. + #[test] + fn closed_enum_without_default_uses_first_value() { + use crate::generated::descriptor::feature_set::EnumType; + let files = [ + level_file("proto2", level_enum(None)), + level_file("editions", level_enum(Some(EnumType::CLOSED))), + ]; + for file in files { + let tokens = gen_enum_ext(file, Label::LABEL_OPTIONAL); + assert!(tokens.contains(":: with_default"), "{tokens}"); + assert!(tokens.contains("fn __level_default () -> i32"), "{tokens}"); + assert!(!tokens.contains("const fn __level_default"), "{tokens}"); + assert!(tokens.contains(FIRST_VALUE), "{tokens}"); + } + } + + /// An open enum's first value is 0, which `i32::default()` already gives, + /// and a repeated extension has no default, so neither gets a fn. + #[test] + fn open_or_repeated_enum_without_default_emits_plain_new() { + let cases = [ + ( + level_file("proto3", level_enum(None)), + Label::LABEL_OPTIONAL, + ), + ( + level_file("editions", level_enum(None)), + Label::LABEL_OPTIONAL, + ), + ( + level_file("proto2", level_enum(None)), + Label::LABEL_REPEATED, + ), + ]; + for (file, label) in cases { + let tokens = gen_enum_ext(file, label); + assert!(tokens.contains(":: new"), "{tokens}"); + assert!(!tokens.contains("with_default"), "{tokens}"); + assert!(!tokens.contains("__level_default"), "{tokens}"); + } + } + #[test] fn default_value_absent_emits_plain_new() { let f = ext_field("weight", 50002, Type::TYPE_SINT32); diff --git a/buffa-test/protos/custom_options.proto b/buffa-test/protos/custom_options.proto index f8b7c2086..361b3f170 100644 --- a/buffa-test/protos/custom_options.proto +++ b/buffa-test/protos/custom_options.proto @@ -59,4 +59,11 @@ extend Carrier { optional int32 c_priority = 110 [default = 7]; optional string c_label = 111 [default = "none"]; optional bool c_active = 112 [default = true]; + optional Level c_level = 113; // closed enum, no [default] +} + +// A closed enum that does not declare 0: absent `c_level` reads as LOW. +enum Level { + LOW = 1; + HIGH = 2; } diff --git a/buffa-test/protos/group_ext.proto b/buffa-test/protos/group_ext.proto index 943851ffc..192504ba6 100644 --- a/buffa-test/protos/group_ext.proto +++ b/buffa-test/protos/group_ext.proto @@ -24,4 +24,20 @@ message Carrier { extend Carrier { Inner delim_inner = 100; repeated Inner delim_repeated = 101; + Level level = 102; + Mode mode = 103; +} + +// Enum extensions without a default read absent as the first value: LOW for +// the closed enum, which does not declare 0, and 0 for the open one. +enum Level { + option features.enum_type = CLOSED; + + LOW = 3; + HIGH = 4; +} + +enum Mode { + MODE_UNSPECIFIED = 0; + MODE_ON = 1; } diff --git a/buffa-test/src/tests/extensions.rs b/buffa-test/src/tests/extensions.rs index e07cc94a2..af56f5b0b 100644 --- a/buffa-test/src/tests/extensions.rs +++ b/buffa-test/src/tests/extensions.rs @@ -1,11 +1,11 @@ //! Integration tests for `extend` codegen and the `ExtensionSet` runtime API. use crate::custopts::__buffa::ext::{ - ACTIVE, ANN, C_ACTIVE, C_ANN, C_FLAG, C_LABEL, C_MARKER, C_PRIORITY, C_TAGS, C_WEIGHT, + ACTIVE, ANN, C_ACTIVE, C_ANN, C_FLAG, C_LABEL, C_LEVEL, C_MARKER, C_PRIORITY, C_TAGS, C_WEIGHT, IS_INTERNAL, LABEL, PRIORITY, TAGS, WEIGHT, }; -use crate::custopts::{carrier, Annotation, Carrier}; -use buffa::{Extension, ExtensionSet, Message}; +use crate::custopts::{carrier, Annotation, Carrier, Level}; +use buffa::{Enumeration, Extension, ExtensionSet, Message}; /// Sanity: field numbers, extendees, and codec types match the proto source. /// Codec types are enforced by the `const` binding itself — a mismatch is a @@ -253,6 +253,20 @@ fn extension_or_default_no_declared_default_uses_type_default() { assert!(!c.extension_or_default(&C_FLAG)); } +/// A closed enum need not declare 0, so a closed-enum extension without a +/// `[default]` reads absent as the enum's first value, the same number an +/// unset field of that enum reads as. +#[test] +fn closed_enum_extension_or_default_is_first_value() { + let mut c = Carrier::default(); + assert_eq!(c.extension(&C_LEVEL), None); + assert_eq!(c.extension_or_default(&C_LEVEL), Level::LOW.to_i32()); + assert_eq!(Level::default(), Level::LOW); + + c.set_extension(&C_LEVEL, Level::HIGH.to_i32()); + assert_eq!(c.extension_or_default(&C_LEVEL), Level::HIGH.to_i32()); +} + /// The declared default survives encode/decode: a freshly-decoded carrier /// with no extension data still returns the proto default. (The default lives /// in the generated `const`, not on the wire.) @@ -273,10 +287,21 @@ fn proto2_default_after_roundtrip() { // Group-encoded extensions (editions DELIMITED) // ──────────────────────────────────────────────────────────────────────────── -use crate::groupext::__buffa::ext::{DELIM_INNER, DELIM_REPEATED}; -use crate::groupext::{Carrier as GroupCarrier, Inner}; +use crate::groupext::__buffa::ext::{DELIM_INNER, DELIM_REPEATED, LEVEL, MODE}; +use crate::groupext::{Carrier as GroupCarrier, Inner, Level as EditionsLevel}; use buffa::extension::codecs::{GroupCodec, Repeated}; +/// Editions: an extension of an `enum_type = CLOSED` enum without a default +/// reads absent as the first value; one of an open enum still reads 0. +#[test] +fn editions_enum_extension_or_default() { + let c = GroupCarrier::default(); + assert_eq!(c.extension(&LEVEL), None); + assert_eq!(c.extension_or_default(&LEVEL), EditionsLevel::LOW.to_i32()); + assert_eq!(c.extension(&MODE), None); + assert_eq!(c.extension_or_default(&MODE), 0); +} + /// Codec type check: DELIMITED extension gets `GroupCodec`, not `MessageCodec`. /// A type mismatch here is a compile error. #[test] diff --git a/buffa/src/extension.rs b/buffa/src/extension.rs index d733e9f61..68aacbac8 100644 --- a/buffa/src/extension.rs +++ b/buffa/src/extension.rs @@ -48,7 +48,9 @@ //! # Defaults //! //! Proto2 `[default = ...]` values are surfaced by [`extension_or_default`], -//! which returns the declared default when the extension is absent. Presence +//! which returns the declared default when the extension is absent. A +//! closed-enum extension with no declared default returns the enum's first +//! value, as an unset field of that enum does. Presence //! is still distinguishable via [`extension`] (returns `None`) or //! [`has_extension`]. //! @@ -168,7 +170,9 @@ impl Extension { /// The function pointer is called lazily on each `extension_or_default` /// call. For `Copy` scalars codegen emits a `const fn`; for `String` and /// `bytes` a regular `fn` (allocates on each call — same cost as a - /// hand-written `.unwrap_or_else(|| "x".into())`). + /// hand-written `.unwrap_or_else(|| "x".into())`). Codegen also passes a + /// regular `fn` for a closed-enum extension that declares no default, + /// returning the enum's first value. /// /// # Panics /// @@ -451,6 +455,11 @@ pub trait ExtensionSet { /// Read a singular extension value, returning the proto2 `[default = ...]` /// value if absent, or the type's `Default` if no proto default was declared. /// + /// Generated code gives a closed-enum extension without a declared default + /// the enum's first value, which need not be 0, so this returns the same + /// number an unset field of that enum reads as. An open enum's first value + /// is always 0. + /// /// Only meaningful for singular codecs (`Output = Option`). For /// repeated codecs use [`extension`](Self::extension) — there is no such /// thing as a repeated default.