From 96ad361a39017f3574dfee9756c4c325dc098b20 Mon Sep 17 00:00:00 2001 From: RA <70325462+RAprogramm@users.noreply.github.com> Date: Sat, 20 Sep 2025 07:10:04 +0700 Subject: [PATCH] Resolve format arguments in derive Display --- CHANGELOG.md | 8 +- Cargo.lock | 4 +- Cargo.toml | 4 +- README.md | 14 +- masterror-derive/Cargo.toml | 2 +- masterror-derive/src/display.rs | 248 ++++++++++++++++++++++++++------ masterror-derive/src/input.rs | 6 +- tests/error_derive.rs | 78 ++++++++++ 8 files changed, 305 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6b07fc..6ddfbf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,13 @@ All notable changes to this project will be documented in this file. ## [Unreleased] ### Added -- _Nothing yet._ +- Resolve `#[error("...")]` format arguments when generating `Display` + implementations, supporting named bindings, explicit indices and implicit + placeholders via a shared argument environment. + +### Tests +- Cover named format argument expressions, implicit placeholder ordering and + enum variants using format arguments. ## [0.6.0] - 2025-10-08 diff --git a/Cargo.lock b/Cargo.lock index a5f5d3e..9fb8554 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1527,7 +1527,7 @@ dependencies = [ [[package]] name = "masterror" -version = "0.6.0" +version = "0.6.1" dependencies = [ "actix-web", "axum", @@ -1557,7 +1557,7 @@ dependencies = [ [[package]] name = "masterror-derive" -version = "0.2.0" +version = "0.2.1" dependencies = [ "masterror-template", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 824820c..2d148b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "masterror" -version = "0.6.0" +version = "0.6.1" rust-version = "1.90" edition = "2024" license = "MIT OR Apache-2.0" @@ -49,7 +49,7 @@ turnkey = [] openapi = ["dep:utoipa"] [workspace.dependencies] -masterror-derive = { version = "0.2.0", path = "masterror-derive" } +masterror-derive = { version = "0.2.1", path = "masterror-derive" } masterror-template = { version = "0.2.0", path = "masterror-template" } [dependencies] diff --git a/README.md b/README.md index 85674db..f4a2887 100644 --- a/README.md +++ b/README.md @@ -29,9 +29,9 @@ Stable categories, conservative HTTP mapping, no `unsafe`. ~~~toml [dependencies] -masterror = { version = "0.6.0", default-features = false } +masterror = { version = "0.6.1", default-features = false } # or with features: -# masterror = { version = "0.6.0", features = [ +# masterror = { version = "0.6.1", features = [ # "axum", "actix", "openapi", "serde_json", # "sqlx", "sqlx-migrate", "reqwest", "redis", # "validator", "config", "tokio", "multipart", @@ -66,10 +66,10 @@ masterror = { version = "0.6.0", default-features = false } ~~~toml [dependencies] # lean core -masterror = { version = "0.6.0", default-features = false } +masterror = { version = "0.6.1", default-features = false } # with Axum/Actix + JSON + integrations -# masterror = { version = "0.6.0", features = [ +# masterror = { version = "0.6.1", features = [ # "axum", "actix", "openapi", "serde_json", # "sqlx", "sqlx-migrate", "reqwest", "redis", # "validator", "config", "tokio", "multipart", @@ -383,13 +383,13 @@ assert_eq!(resp.status, 401); Minimal core: ~~~toml -masterror = { version = "0.6.0", default-features = false } +masterror = { version = "0.6.1", default-features = false } ~~~ API (Axum + JSON + deps): ~~~toml -masterror = { version = "0.6.0", features = [ +masterror = { version = "0.6.1", features = [ "axum", "serde_json", "openapi", "sqlx", "reqwest", "redis", "validator", "config", "tokio" ] } @@ -398,7 +398,7 @@ masterror = { version = "0.6.0", features = [ API (Actix + JSON + deps): ~~~toml -masterror = { version = "0.6.0", features = [ +masterror = { version = "0.6.1", features = [ "actix", "serde_json", "openapi", "sqlx", "reqwest", "redis", "validator", "config", "tokio" ] } diff --git a/masterror-derive/Cargo.toml b/masterror-derive/Cargo.toml index 80a1c5d..ee65b78 100644 --- a/masterror-derive/Cargo.toml +++ b/masterror-derive/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "masterror-derive" rust-version = "1.90" -version = "0.2.0" +version = "0.2.1" edition = "2024" license = "MIT OR Apache-2.0" repository = "https://github.com/RAprogramm/masterror" diff --git a/masterror-derive/src/display.rs b/masterror-derive/src/display.rs index 6bf327a..05663d7 100644 --- a/masterror-derive/src/display.rs +++ b/masterror-derive/src/display.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use masterror_template::template::{TemplateFormatter, TemplateFormatterKind}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; @@ -5,8 +7,8 @@ use syn::{Error, spanned::Spanned}; use crate::{ input::{ - DisplaySpec, ErrorData, ErrorInput, Field, Fields, StructData, VariantData, - placeholder_error + DisplaySpec, ErrorData, ErrorInput, Field, Fields, FormatArg, FormatArgsSpec, + FormatBindingKind, StructData, VariantData, placeholder_error }, template_support::{ DisplayTemplate, TemplateIdentifierSpec, TemplatePlaceholderSpec, TemplateSegmentSpec @@ -25,12 +27,19 @@ fn expand_struct(input: &ErrorInput, data: &StructData) -> Result render_struct_transparent(&data.fields), - DisplaySpec::Template(template) - | DisplaySpec::TemplateWithArgs { - template, .. - } => render_template(template, |placeholder| { - struct_placeholder_expr(&data.fields, placeholder) + DisplaySpec::Template(template) => render_template(template, Vec::new(), |placeholder| { + struct_placeholder_expr(&data.fields, placeholder, None) })?, + DisplaySpec::TemplateWithArgs { + template, + args + } => { + let mut env = FormatArgumentsEnv::new(args); + let preludes = env.prelude_tokens(); + render_template(template, preludes, |placeholder| { + struct_placeholder_expr(&data.fields, placeholder, Some(&mut env)) + })? + } DisplaySpec::FormatterPath { path, .. } => { @@ -89,10 +98,11 @@ fn render_variant(variant: &VariantData) -> Result { DisplaySpec::Transparent { .. } => render_variant_transparent(variant), - DisplaySpec::Template(template) - | DisplaySpec::TemplateWithArgs { - template, .. - } => render_variant_template(variant, template), + DisplaySpec::Template(template) => render_variant_template(variant, template, None), + DisplaySpec::TemplateWithArgs { + template, + args + } => render_variant_template(variant, template, Some(args)), DisplaySpec::FormatterPath { path, .. } => Err(Error::new(path.span(), "`fmt = ...` is not supported yet")) @@ -122,6 +132,100 @@ impl ResolvedPlaceholderExpr { } } +#[derive(Debug)] +struct FormatArgumentsEnv<'a> { + args: Vec>, + named: HashMap, + positional: HashMap, + implicit: Vec> +} + +#[derive(Debug)] +struct EnvFormatArg<'a> { + binding: Ident, + arg: &'a FormatArg +} + +impl<'a> FormatArgumentsEnv<'a> { + fn new(spec: &'a FormatArgsSpec) -> Self { + let mut env = Self { + args: Vec::new(), + named: HashMap::new(), + positional: HashMap::new(), + implicit: Vec::new() + }; + + for (index, arg) in spec.args.iter().enumerate() { + let binding = format_ident!("__masterror_format_arg_{}", index); + let arg_index = env.args.len(); + env.args.push(EnvFormatArg { + binding, + arg + }); + + match &arg.kind { + FormatBindingKind::Named(ident) => { + env.named.insert(ident.to_string(), arg_index); + } + FormatBindingKind::Positional(pos_index) => { + env.positional.insert(*pos_index, arg_index); + env.implicit.push(Some(arg_index)); + } + FormatBindingKind::Implicit(implicit_index) => { + env.register_implicit(*implicit_index, arg_index); + } + } + } + + env + } + + fn prelude_tokens(&self) -> Vec { + self.args.iter().map(EnvFormatArg::prelude_tokens).collect() + } + + fn resolve_placeholder( + &mut self, + placeholder: &TemplatePlaceholderSpec + ) -> Option { + let arg_index = match &placeholder.identifier { + TemplateIdentifierSpec::Named(name) => self.named.get(name).copied(), + TemplateIdentifierSpec::Positional(index) => self.positional.get(index).copied(), + TemplateIdentifierSpec::Implicit(index) => { + self.implicit.get(*index).and_then(|slot| *slot) + } + }?; + + Some(self.args[arg_index].resolved_expr(placeholder.formatter)) + } + + fn register_implicit(&mut self, index: usize, arg_index: usize) { + if self.implicit.len() <= index { + self.implicit.resize(index + 1, None); + } + self.implicit[index] = Some(arg_index); + } +} + +impl<'a> EnvFormatArg<'a> { + fn prelude_tokens(&self) -> TokenStream { + let binding = &self.binding; + let expr = &self.arg.expr; + quote! { + let #binding = #expr; + } + } + + fn resolved_expr(&self, formatter: TemplateFormatter) -> ResolvedPlaceholderExpr { + let binding = &self.binding; + if needs_pointer_value(formatter) { + ResolvedPlaceholderExpr::with(quote!(#binding), true) + } else { + ResolvedPlaceholderExpr::new(quote!(&#binding)) + } + } +} + fn render_variant_transparent(variant: &VariantData) -> Result { let variant_ident = &variant.ident; @@ -159,17 +263,26 @@ fn render_variant_transparent(variant: &VariantData) -> Result ) -> Result { let variant_ident = &variant.ident; + let mut env = format_args.map(FormatArgumentsEnv::new); match &variant.fields { Fields::Unit => { - let body = render_template(template, |_placeholder| { - Err(Error::new( - variant.span, - "unit variants cannot reference fields" - )) + let preludes = env + .as_ref() + .map(|env| env.prelude_tokens()) + .unwrap_or_default(); + let span = variant.span; + let body = render_template(template, preludes, |placeholder| { + if let Some(env) = env.as_mut() + && let Some(resolved) = env.resolve_placeholder(placeholder) + { + return Ok(resolved); + } + Err(Error::new(span, "unit variants cannot reference fields")) })?; Ok(quote! { Self::#variant_ident => { @@ -180,8 +293,12 @@ fn render_variant_template( Fields::Unnamed(fields) => { let bindings: Vec<_> = fields.iter().map(binding_ident).collect(); let pattern = quote!(Self::#variant_ident(#(#bindings),*)); - let body = render_template(template, |placeholder| { - variant_tuple_placeholder(&bindings, placeholder) + let preludes = env + .as_ref() + .map(|env| env.prelude_tokens()) + .unwrap_or_default(); + let body = render_template(template, preludes, |placeholder| { + variant_tuple_placeholder(&bindings, placeholder, env.as_mut()) })?; Ok(quote! { #pattern => { @@ -195,8 +312,12 @@ fn render_variant_template( .map(|field| field.ident.clone().expect("named field")) .collect(); let pattern = quote!(Self::#variant_ident { #(#bindings),* }); - let body = render_template(template, |placeholder| { - variant_named_placeholder(fields, &bindings, placeholder) + let preludes = env + .as_ref() + .map(|env| env.prelude_tokens()) + .unwrap_or_default(); + let body = render_template(template, preludes, |placeholder| { + variant_named_placeholder(fields, &bindings, placeholder, env.as_mut()) })?; Ok(quote! { #pattern => { @@ -207,11 +328,15 @@ fn render_variant_template( } } -fn render_template(template: &DisplayTemplate, resolver: F) -> Result +fn render_template( + template: &DisplayTemplate, + preludes: Vec, + mut resolver: F +) -> Result where - F: Fn(&TemplatePlaceholderSpec) -> Result + F: FnMut(&TemplatePlaceholderSpec) -> Result { - let mut pieces = Vec::new(); + let mut pieces = preludes; for segment in &template.segments { match segment { TemplateSegmentSpec::Literal(text) => { @@ -232,15 +357,26 @@ where fn struct_placeholder_expr( fields: &Fields, - placeholder: &TemplatePlaceholderSpec + placeholder: &TemplatePlaceholderSpec, + env: Option<&mut FormatArgumentsEnv<'_>> ) -> Result { + if matches!( + &placeholder.identifier, + TemplateIdentifierSpec::Named(name) if name == "self" + ) { + return Ok(ResolvedPlaceholderExpr::with( + quote!(self), + needs_pointer_value(placeholder.formatter) + )); + } + + if let Some(env) = env + && let Some(resolved) = env.resolve_placeholder(placeholder) + { + return Ok(resolved); + } + match &placeholder.identifier { - TemplateIdentifierSpec::Named(name) if name == "self" => { - Ok(ResolvedPlaceholderExpr::with( - quote!(self), - needs_pointer_value(placeholder.formatter) - )) - } TemplateIdentifierSpec::Named(name) => { if let Some(field) = fields.get_named(name) { Ok(struct_field_expr(field, placeholder.formatter)) @@ -289,15 +425,26 @@ fn pointer_prefers_value(ty: &syn::Type) -> bool { fn variant_tuple_placeholder( bindings: &[Ident], - placeholder: &TemplatePlaceholderSpec + placeholder: &TemplatePlaceholderSpec, + env: Option<&mut FormatArgumentsEnv<'_>> ) -> Result { + if matches!( + &placeholder.identifier, + TemplateIdentifierSpec::Named(name) if name == "self" + ) { + return Ok(ResolvedPlaceholderExpr::with( + quote!(self), + needs_pointer_value(placeholder.formatter) + )); + } + + if let Some(env) = env + && let Some(resolved) = env.resolve_placeholder(placeholder) + { + return Ok(resolved); + } + match &placeholder.identifier { - TemplateIdentifierSpec::Named(name) if name == "self" => { - Ok(ResolvedPlaceholderExpr::with( - quote!(self), - needs_pointer_value(placeholder.formatter) - )) - } TemplateIdentifierSpec::Named(_) => { Err(placeholder_error(placeholder.span, &placeholder.identifier)) } @@ -325,15 +472,26 @@ fn variant_tuple_placeholder( fn variant_named_placeholder( fields: &[Field], bindings: &[Ident], - placeholder: &TemplatePlaceholderSpec + placeholder: &TemplatePlaceholderSpec, + env: Option<&mut FormatArgumentsEnv<'_>> ) -> Result { + if matches!( + &placeholder.identifier, + TemplateIdentifierSpec::Named(name) if name == "self" + ) { + return Ok(ResolvedPlaceholderExpr::with( + quote!(self), + needs_pointer_value(placeholder.formatter) + )); + } + + if let Some(env) = env + && let Some(resolved) = env.resolve_placeholder(placeholder) + { + return Ok(resolved); + } + match &placeholder.identifier { - TemplateIdentifierSpec::Named(name) if name == "self" => { - Ok(ResolvedPlaceholderExpr::with( - quote!(self), - needs_pointer_value(placeholder.formatter) - )) - } TemplateIdentifierSpec::Named(name) => { if let Some(index) = fields .iter() diff --git a/masterror-derive/src/input.rs b/masterror-derive/src/input.rs index 86cf612..b628073 100644 --- a/masterror-derive/src/input.rs +++ b/masterror-derive/src/input.rs @@ -529,7 +529,9 @@ fn parse_format_args(input: ParseStream) -> Result { let mut seen_named = HashSet::new(); - for (index, raw) in parsed.into_iter().enumerate() { + let mut positional_index = 0usize; + + for raw in parsed { match raw { RawFormatArg::Named { ident, @@ -556,6 +558,8 @@ fn parse_format_args(input: ParseStream) -> Result { expr, span } => { + let index = positional_index; + positional_index += 1; let tokens = expr.to_token_stream(); args.args.push(FormatArg { tokens, diff --git a/tests/error_derive.rs b/tests/error_derive.rs index 665a055..6708f65 100644 --- a/tests/error_derive.rs +++ b/tests/error_derive.rs @@ -215,6 +215,40 @@ struct FormatterDebugShowcase { tuple: (&'static str, u8) } +#[derive(Debug, Error)] +#[error("{formatted}", formatted = self.message.to_uppercase())] +struct FormatArgExpressionError { + message: &'static str +} + +#[derive(Debug, Error)] +#[error("{}, {label}, {}", label = self.label, self.first, self.second)] +struct MixedImplicitArgsError { + label: &'static str, + first: &'static str, + second: &'static str +} + +#[derive(Debug, Error)] +enum FormatArgEnum { + #[error("{detail}", detail = detail.to_uppercase())] + Upper { detail: String } +} + +#[derive(Debug, Error)] +#[error("{1}::{0}", self.first, self.second)] +struct ExplicitIndexArgsError { + first: &'static str, + second: &'static str +} + +#[derive(Debug, Error)] +#[error("{0}::{label}", label = self.label, self.value)] +struct MixedNamedPositionalArgsError { + label: &'static str, + value: &'static str +} + #[derive(Debug, Error)] #[error("{value}")] struct DisplayFormatterError { @@ -325,6 +359,50 @@ fn enum_variants_cover_display_and_source() { assert_eq!(StdError::source(&pair).unwrap().to_string(), "leaf failure"); } +#[test] +fn named_format_arg_expression_is_used() { + let err = FormatArgExpressionError { + message: "value" + }; + assert_eq!(err.to_string(), "VALUE"); +} + +#[test] +fn implicit_format_args_follow_positional_ordering() { + let err = MixedImplicitArgsError { + label: "tag", + first: "one", + second: "two" + }; + assert_eq!(err.to_string(), "one, tag, two"); +} + +#[test] +fn explicit_format_arg_indices_resolve() { + let err = ExplicitIndexArgsError { + first: "left", + second: "right" + }; + assert_eq!(err.to_string(), "right::left"); +} + +#[test] +fn mixed_named_and_positional_indices_resolve() { + let err = MixedNamedPositionalArgsError { + label: "tag", + value: "item" + }; + assert_eq!(err.to_string(), "item::tag"); +} + +#[test] +fn enum_variant_format_args_resolve_bindings() { + let err = FormatArgEnum::Upper { + detail: String::from("variant") + }; + assert_eq!(err.to_string(), "VARIANT"); +} + #[test] fn tuple_struct_from_wraps_source() { let err = TupleWrapper::from(LeafError);