From 0c796cde9f181341045c8bf82cd1ba7606d2ded9 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Sun, 6 Sep 2026 04:44:31 -0400 Subject: [PATCH 1/3] Add docs for new type enum variants to generated settings `#[settings]` on an enum emitted an empty `Settings` impl, so `add_docs` fell back to the no-op default in foundations/src/settings/mod.rs. A new type variant is the one variant kind that gets a key of its own in the serialized YAML, and everything under that key was written without its doc comments. `TracingSettings::output` defaults to `TracesOutput::JaegerThriftUdp` (foundations/src/telemetry/settings/tracing.rs:80), so a config written by `Cli`'s `--generate` listed `server_addr`, `reporter_bind_addr`, `num_tasks` and `max_batch_size` with no documentation at all. Same for `UserTracesOutput::OtlpUds` and `SamplingStrategy::Active`. `expand_enum` now builds an `add_docs` that matches on the variants and, for each new type variant serde actually serializes, pushes the variant's key and forwards to the wrapped value before recording the variant's own doc comment. Unit variants serialize as a bare value and have no key to document. `serde(skip)` variants are left out so the type they wrap does not have to implement `Settings`, which `LogOutput::Custom` (an `Arc`) does not. --- foundations-macros/src/settings.rs | 305 +++++++++++++++++- ...settings_enum_new_type_variant_fields.yaml | 8 + ...settings_enum_new_type_variant_fields.yaml | 9 + foundations/tests/settings.rs | 36 +++ 4 files changed, 351 insertions(+), 7 deletions(-) create mode 100644 foundations/tests/data/serde-saphyr/settings_enum_new_type_variant_fields.yaml create mode 100644 foundations/tests/data/serde-yaml/settings_enum_new_type_variant_fields.yaml diff --git a/foundations-macros/src/settings.rs b/foundations-macros/src/settings.rs index 9efdc3b2..d74884e4 100644 --- a/foundations-macros/src/settings.rs +++ b/foundations-macros/src/settings.rs @@ -4,10 +4,11 @@ use darling::util::{Flag, Override}; use proc_macro::TokenStream; use quote::{ToTokens, TokenStreamExt, quote, quote_spanned}; use syn::parse::{Parse, ParseStream}; +use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::{ Attribute, Expr, ExprLit, Field, Fields, Ident, Item, ItemEnum, ItemStruct, Lit, LitStr, Meta, - MetaNameValue, Path, Type, parse_macro_input, parse_quote, + MetaNameValue, Path, Token, Type, Variant, parse_macro_input, parse_quote, }; const ERR_NOT_STRUCT_OR_ENUM: &str = "Settings should be either structure or enum."; @@ -127,13 +128,12 @@ fn expand_enum(options: Options, item: &mut ItemEnum) -> Result proc_macro2::TokenStream { + let ident = item.ident.clone(); + let crate_path = &options.crate_path; + + // Nothing under a variant can be documented, so keep the default no-op `add_docs`. + if !item.variants.iter().any(is_documented_variant) { + return quote! { + impl #crate_path::settings::Settings for #ident { } + }; + } + + let mut doc_comments_impl = quote! {}; + + for variant in &item.variants { + doc_comments_impl.append_all(impl_settings_trait_for_variant(options, variant)); + } + + quote! { + impl #crate_path::settings::Settings for #ident { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + #doc_comments_impl + } + } + } + } +} + +/// Returns whether a variant gets a key of its own in the serialized settings. +/// +/// A unit variant serializes as a bare value, so there is no line to document. A skipped +/// variant never reaches the config at all, and the type it wraps doesn't have to implement +/// [`Settings`]. +fn is_documented_variant(variant: &Variant) -> bool { + matches!(variant.fields, Fields::Unnamed(_)) && !is_serde_skipped(&variant.attrs) +} + +fn impl_settings_trait_for_variant( + options: &Options, + variant: &Variant, +) -> proc_macro2::TokenStream { + let ident = &variant.ident; + let span = variant.fields.span(); + + let cfg_attrs = variant + .attrs + .iter() + .filter(|a| a.path().is_ident("cfg")) + .collect::>(); + + if !is_documented_variant(variant) { + let pattern = match variant.fields { + Fields::Unnamed(_) => quote_spanned! { span=> Self::#ident(..) }, + _ => quote_spanned! { span=> Self::#ident }, + }; + + return quote! { + #(#cfg_attrs)* + #pattern => {} + }; + } + + let crate_path = &options.crate_path; + let key_str = serde_variant_name(variant); + let docs = extract_doc_comments(&variant.attrs); + + let mut impl_for_variant = quote_spanned! { span=> + let mut key = parent_key.to_vec(); + key.push(#key_str.into()); + #crate_path::settings::Settings::add_docs(value, &key, docs); + }; + + if !docs.is_empty() { + impl_for_variant.append_all(quote! { + docs.insert(key, &[#(#docs,)*][..]); + }); + } + + quote_spanned! { span=> + #(#cfg_attrs)* + Self::#ident(value) => { + #impl_for_variant + } + } +} + fn extract_doc_comments(attrs: &[Attribute]) -> Vec { let mut comments = vec![]; @@ -351,6 +441,87 @@ fn is_serde_flattened(attrs: &[Attribute]) -> bool { ) } +/// Returns the arguments of every `serde` attribute in `attrs`, including the ones written as +/// `cfg_attr(, serde(...))`, which reach an attribute macro unexpanded. +fn serde_args(attrs: &[Attribute]) -> Vec { + let mut args = Vec::new(); + + for attr in attrs { + let meta = if attr.path().is_ident("serde") { + attr.meta.clone() + } else if attr.path().is_ident("cfg_attr") { + let Ok(nested) = attr.parse_args_with(Punctuated::::parse_terminated) + else { + continue; + }; + + // The first argument is the `cfg` predicate, the rest are the attributes it guards. + let Some(meta) = nested + .into_iter() + .skip(1) + .find(|meta| meta.path().is_ident("serde")) + else { + continue; + }; + + meta + } else { + continue; + }; + + let Meta::List(list) = meta else { + continue; + }; + + if let Ok(nested) = list.parse_args_with(Punctuated::::parse_terminated) { + args.extend(nested); + } + } + + args +} + +/// Returns whether `attrs` contains `serde(skip)`. +fn is_serde_skipped(attrs: &[Attribute]) -> bool { + serde_args(attrs) + .iter() + .any(|arg| arg.path().is_ident("skip")) +} + +/// Returns the key an enum variant serializes under. +fn serde_variant_name(variant: &Variant) -> String { + for arg in serde_args(&variant.attrs) { + if let Meta::NameValue(MetaNameValue { + path, + value: + Expr::Lit(ExprLit { + lit: Lit::Str(name), + .. + }), + .. + }) = &arg + && path.is_ident("rename") + { + return name.value(); + } + } + + // `expand_enum` puts `serde(rename_all = "snake_case")` on every settings enum, so an + // un-renamed variant serializes under the snake case of its identifier. + let ident = variant.ident.to_string(); + let mut name = String::with_capacity(ident.len()); + + for (index, character) in ident.char_indices() { + if index > 0 && character.is_uppercase() { + name.push('_'); + } + + name.push(character.to_ascii_lowercase()); + } + + name +} + fn impl_serde_aware_default(item: &ItemStruct) -> Result { let name = &item.ident; let (impl_generics, ty_generics, where_clause) = item.generics.split_for_impl(); @@ -904,7 +1075,22 @@ mod tests { NewTypeVariant(String) } - impl ::foundations::settings::Settings for TestEnum { } + impl ::foundations::settings::Settings for TestEnum { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + Self::UnitVariant => {} + Self::NewTypeVariant(value) => { + let mut key = parent_key.to_vec(); + key.push("new_type_variant".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + } + } + } + } }; assert_eq!(actual, expected); @@ -940,7 +1126,22 @@ mod tests { NewTypeVariant(String) } - impl ::foundations::settings::Settings for TestEnum { } + impl ::foundations::settings::Settings for TestEnum { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + Self::UnitVariant => {} + Self::NewTypeVariant(value) => { + let mut key = parent_key.to_vec(); + key.push("new_type_variant".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + } + } + } + } }; assert_eq!(actual, expected); @@ -978,7 +1179,97 @@ mod tests { NewTypeVariant(String) } - impl ::foundations::settings::Settings for TestEnum { } + impl ::foundations::settings::Settings for TestEnum { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + Self::UnitVariant => {} + Self::NewTypeVariant(value) => { + let mut key = parent_key.to_vec(); + key.push("new_type_variant".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + } + } + } + } + }; + + assert_eq!(actual, expected); + } + + #[test] + fn expand_enum_with_variant_docs() { + let options = parse_attr! { + #[settings(impl_default = false)] + }; + + let src = parse_quote! { + enum TestEnum { + /// Nested settings. + Nested(NestedStruct), + /// Renamed variant. + #[serde(rename = "RENAMED")] + Renamed(NestedStruct), + /// Not serialized. + #[serde(skip)] + Skipped(NotSettings), + /// A unit variant. + UnitVariant + } + }; + + let actual = expand_from_parsed(options, src).unwrap().to_string(); + + let expected = code_str! { + #[derive( + Clone, + ::foundations::reexports_for_macros::serde::Serialize, + ::foundations::reexports_for_macros::serde::Deserialize, + )] + #[derive(Debug)] + #[serde(crate = ":: foundations :: reexports_for_macros :: serde")] + #[serde(deny_unknown_fields)] + #[serde(rename_all="snake_case")] + enum TestEnum { + #[doc = r" Nested settings."] + Nested(NestedStruct), + #[doc = r" Renamed variant."] + #[serde(rename = "RENAMED")] + Renamed(NestedStruct), + #[doc = r" Not serialized."] + #[serde(skip)] + Skipped(NotSettings), + #[doc = r" A unit variant."] + UnitVariant + } + + impl ::foundations::settings::Settings for TestEnum { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + Self::Nested(value) => { + let mut key = parent_key.to_vec(); + key.push("nested".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + docs.insert(key, &[r" Nested settings.",][..]); + } + Self::Renamed(value) => { + let mut key = parent_key.to_vec(); + key.push("RENAMED".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + docs.insert(key, &[r" Renamed variant.",][..]); + } + Self::Skipped(..) => {} + Self::UnitVariant => {} + } + } + } }; assert_eq!(actual, expected); diff --git a/foundations/tests/data/serde-saphyr/settings_enum_new_type_variant_fields.yaml b/foundations/tests/data/serde-saphyr/settings_enum_new_type_variant_fields.yaml new file mode 100644 index 00000000..51c019ff --- /dev/null +++ b/foundations/tests/data/serde-saphyr/settings_enum_new_type_variant_fields.yaml @@ -0,0 +1,8 @@ +# Where the output is written +output: + # Writes the output to a file. + file: + # Path of the output file. + path: "" + # Whether an existing file is truncated. + truncate: false diff --git a/foundations/tests/data/serde-yaml/settings_enum_new_type_variant_fields.yaml b/foundations/tests/data/serde-yaml/settings_enum_new_type_variant_fields.yaml new file mode 100644 index 00000000..64143116 --- /dev/null +++ b/foundations/tests/data/serde-yaml/settings_enum_new_type_variant_fields.yaml @@ -0,0 +1,9 @@ +--- +# Where the output is written +output: + # Writes the output to a file. + file: + # Path of the output file. + path: "" + # Whether an existing file is truncated. + truncate: false diff --git a/foundations/tests/settings.rs b/foundations/tests/settings.rs index d9bf0d46..92b7f26c 100644 --- a/foundations/tests/settings.rs +++ b/foundations/tests/settings.rs @@ -61,6 +61,34 @@ struct StructWithEnumField { field: SomeEnum, } +#[settings(impl_default = false)] +enum SomeOutput { + /// Writes the output to a file. + File(FileOutputSettings), + /// Writes the output to the terminal. + Terminal, +} + +impl Default for SomeOutput { + fn default() -> Self { + Self::File(Default::default()) + } +} + +#[settings] +struct FileOutputSettings { + /// Path of the output file. + path: String, + /// Whether an existing file is truncated. + truncate: bool, +} + +#[settings] +struct StructWithNewTypeEnumField { + /// Where the output is written + output: SomeOutput, +} + #[settings] struct ProxySettings { /// Proxy address. @@ -220,6 +248,14 @@ fn enum_fields() { assert_ser_eq!(StructWithEnumField::default(), "settings_enum_fields.yaml"); } +#[test] +fn enum_new_type_variant_fields() { + assert_ser_eq!( + StructWithNewTypeEnumField::default(), + "settings_enum_new_type_variant_fields.yaml" + ); +} + #[test] fn complex_settings() { assert_ser_eq!(ProxySettings::default(), "settings_complex.yaml"); From 332ad7fba9d87bfbbb7fa79b802f9e133956a5e6 Mon Sep 17 00:00:00 2001 From: Max Pollard Date: Wed, 9 Sep 2026 23:13:18 -0400 Subject: [PATCH 2/3] Gate new type variant docs behind foundations_unstable Documenting a new type variant makes the wrapped type reachable through add_docs, so that type now has to implement Settings. That is a new requirement on existing code, so the generated impl stays behind --cfg foundations_unstable until the next breaking release. Without the flag the macro emits the same empty impl as before. --- foundations-macros/src/settings.rs | 103 +++++++++++++---------------- foundations/src/lib.rs | 1 + foundations/tests/settings.rs | 7 ++ 3 files changed, 55 insertions(+), 56 deletions(-) diff --git a/foundations-macros/src/settings.rs b/foundations-macros/src/settings.rs index d74884e4..651d554a 100644 --- a/foundations-macros/src/settings.rs +++ b/foundations-macros/src/settings.rs @@ -299,8 +299,12 @@ fn impl_settings_trait_for_enum(options: &Options, item: &ItemEnum) -> proc_macr let ident = item.ident.clone(); let crate_path = &options.crate_path; - // Nothing under a variant can be documented, so keep the default no-op `add_docs`. - if !item.variants.iter().any(is_documented_variant) { + // Documenting a new type variant makes the type it wraps reachable through `add_docs`, so + // that type has to implement `Settings`. That is a new requirement on existing code, so it + // stays behind `--cfg foundations_unstable` until the next breaking release. + // + // Nothing under a variant can be documented otherwise, so keep the default no-op `add_docs`. + if !cfg!(foundations_unstable) || !item.variants.iter().any(is_documented_variant) { return quote! { impl #crate_path::settings::Settings for #ident { } }; @@ -565,6 +569,37 @@ mod tests { use crate::common::test_utils::{code_str, parse_attr}; use syn::parse_quote; + /// The `Settings` impl expected for an enum with a new type variant. + /// + /// Documenting a new type variant is behind `--cfg foundations_unstable`, see + /// `impl_settings_trait_for_enum`. Without it the enum keeps the default no-op `add_docs`. + fn test_enum_settings_impl() -> String { + if cfg!(foundations_unstable) { + code_str! { + impl ::foundations::settings::Settings for TestEnum { + fn add_docs( + &self, + parent_key: &[String], + docs: &mut ::std::collections::HashMap, &'static [&'static str]>) + { + match self { + Self::UnitVariant => {} + Self::NewTypeVariant(value) => { + let mut key = parent_key.to_vec(); + key.push("new_type_variant".into()); + ::foundations::settings::Settings::add_docs(value, &key, docs); + } + } + } + } + } + } else { + code_str! { + impl ::foundations::settings::Settings for TestEnum { } + } + } + } + #[test] fn expand_structure() { let options = parse_attr! { @@ -1058,7 +1093,7 @@ mod tests { let actual = expand_from_parsed(options, src).unwrap().to_string(); - let expected = code_str! { + let mut expected = code_str! { #[derive(Default)] #[derive( Clone, @@ -1074,24 +1109,9 @@ mod tests { UnitVariant, NewTypeVariant(String) } - - impl ::foundations::settings::Settings for TestEnum { - fn add_docs( - &self, - parent_key: &[String], - docs: &mut ::std::collections::HashMap, &'static [&'static str]>) - { - match self { - Self::UnitVariant => {} - Self::NewTypeVariant(value) => { - let mut key = parent_key.to_vec(); - key.push("new_type_variant".into()); - ::foundations::settings::Settings::add_docs(value, &key, docs); - } - } - } - } }; + expected.push(' '); + expected.push_str(&test_enum_settings_impl()); assert_eq!(actual, expected); } @@ -1111,7 +1131,7 @@ mod tests { let actual = expand_from_parsed(options, src).unwrap().to_string(); - let expected = code_str! { + let mut expected = code_str! { #[derive( Clone, ::foundations::reexports_for_macros::serde::Serialize, @@ -1125,24 +1145,9 @@ mod tests { UnitVariant, NewTypeVariant(String) } - - impl ::foundations::settings::Settings for TestEnum { - fn add_docs( - &self, - parent_key: &[String], - docs: &mut ::std::collections::HashMap, &'static [&'static str]>) - { - match self { - Self::UnitVariant => {} - Self::NewTypeVariant(value) => { - let mut key = parent_key.to_vec(); - key.push("new_type_variant".into()); - ::foundations::settings::Settings::add_docs(value, &key, docs); - } - } - } - } }; + expected.push(' '); + expected.push_str(&test_enum_settings_impl()); assert_eq!(actual, expected); } @@ -1163,7 +1168,7 @@ mod tests { let actual = expand_from_parsed(options, src).unwrap().to_string(); - let expected = code_str! { + let mut expected = code_str! { #[derive(Default)] #[derive( Clone, @@ -1178,28 +1183,14 @@ mod tests { UnitVariant, NewTypeVariant(String) } - - impl ::foundations::settings::Settings for TestEnum { - fn add_docs( - &self, - parent_key: &[String], - docs: &mut ::std::collections::HashMap, &'static [&'static str]>) - { - match self { - Self::UnitVariant => {} - Self::NewTypeVariant(value) => { - let mut key = parent_key.to_vec(); - key.push("new_type_variant".into()); - ::foundations::settings::Settings::add_docs(value, &key, docs); - } - } - } - } }; + expected.push(' '); + expected.push_str(&test_enum_settings_impl()); assert_eq!(actual, expected); } + #[cfg(foundations_unstable)] #[test] fn expand_enum_with_variant_docs() { let options = parse_attr! { diff --git a/foundations/src/lib.rs b/foundations/src/lib.rs index c42d14af..d597b9cb 100644 --- a/foundations/src/lib.rs +++ b/foundations/src/lib.rs @@ -51,6 +51,7 @@ //! Foundations has unstable features which are gated behind `--cfg foundations_unstable`: //! //! - **tokio-runtime-metrics**: Enables runtime metrics for Tokio runtimes. Implicitly enables the **metrics** feature. [Also requires tokio_unstable](https://docs.rs/tokio/latest/tokio/#unstable-features). +//! - **settings-new-type-variant-docs**: Documents the fields of a new type enum variant in generated settings. Requires the type the variant wraps to implement [`Settings`](crate::settings::Settings). //! //! To enable these, you must add `--cfg foundations_unstable` to your RUSTFLAGS environment variable. //! diff --git a/foundations/tests/settings.rs b/foundations/tests/settings.rs index 92b7f26c..788dd67f 100644 --- a/foundations/tests/settings.rs +++ b/foundations/tests/settings.rs @@ -61,6 +61,9 @@ struct StructWithEnumField { field: SomeEnum, } +// Documenting a new type variant is behind `--cfg foundations_unstable`, since it requires the +// type the variant wraps to implement `Settings`. +#[cfg(foundations_unstable)] #[settings(impl_default = false)] enum SomeOutput { /// Writes the output to a file. @@ -69,12 +72,14 @@ enum SomeOutput { Terminal, } +#[cfg(foundations_unstable)] impl Default for SomeOutput { fn default() -> Self { Self::File(Default::default()) } } +#[cfg(foundations_unstable)] #[settings] struct FileOutputSettings { /// Path of the output file. @@ -83,6 +88,7 @@ struct FileOutputSettings { truncate: bool, } +#[cfg(foundations_unstable)] #[settings] struct StructWithNewTypeEnumField { /// Where the output is written @@ -248,6 +254,7 @@ fn enum_fields() { assert_ser_eq!(StructWithEnumField::default(), "settings_enum_fields.yaml"); } +#[cfg(foundations_unstable)] #[test] fn enum_new_type_variant_fields() { assert_ser_eq!( From 08599a75655b97c38deb07aa5db663df44c052ee Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Thu, 10 Sep 2026 07:33:38 -0400 Subject: [PATCH 3/3] Document new type variant docs on the settings macro The crate level Unstable Features list names cargo features, and there is no feature for this one: the generated code is switched by the cfg alone, so an entry there sends readers to a feature that does not exist. Describe the behaviour on `#[settings]` instead, the way `#[with_removal]` is documented in the metrics macro. --- foundations/src/lib.rs | 1 - foundations/src/settings/mod.rs | 9 +++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/foundations/src/lib.rs b/foundations/src/lib.rs index d597b9cb..c42d14af 100644 --- a/foundations/src/lib.rs +++ b/foundations/src/lib.rs @@ -51,7 +51,6 @@ //! Foundations has unstable features which are gated behind `--cfg foundations_unstable`: //! //! - **tokio-runtime-metrics**: Enables runtime metrics for Tokio runtimes. Implicitly enables the **metrics** feature. [Also requires tokio_unstable](https://docs.rs/tokio/latest/tokio/#unstable-features). -//! - **settings-new-type-variant-docs**: Documents the fields of a new type enum variant in generated settings. Requires the type the variant wraps to implement [`Settings`](crate::settings::Settings). //! //! To enable these, you must add `--cfg foundations_unstable` to your RUSTFLAGS environment variable. //! diff --git a/foundations/src/settings/mod.rs b/foundations/src/settings/mod.rs index c9f85569..69f3de2b 100644 --- a/foundations/src/settings/mod.rs +++ b/foundations/src/settings/mod.rs @@ -334,6 +334,15 @@ use std::path::Path; /// } /// ``` /// +/// # New type variant documentation (unstable) +/// +/// **This feature is unstable and becomes a noop without `cfg(foundations_unstable)`.** +/// +/// A new type enum variant is serialized under a key of its own. With the flag, that key gets +/// the doc comment of the variant, and the value under it gets the documentation of the type +/// the variant wraps. This requires that type to implement [`Settings`], so variants annotated +/// with `#[serde(skip)]` are left out. +/// /// [`Settings`]: crate::settings::Settings pub use foundations_macros::settings;