diff --git a/impl/src/attr.rs b/impl/src/attr.rs index 7ad83e0..59ba6bf 100644 --- a/impl/src/attr.rs +++ b/impl/src/attr.rs @@ -27,6 +27,11 @@ pub struct Display<'a> { pub infinite_recursive: bool, pub implied_bounds: Set<(usize, Trait)>, pub bindings: Vec<(Ident, TokenStream)>, + /// Names of the fields used in the format string. + /// For named fields: the field name (e.g., "host") + /// For unnamed fields: the binding name (e.g., "_0", "_1") + /// Used for selective variable binding to avoid unused_variables warnings. + pub used_field_names: Set, } #[derive(Copy, Clone)] @@ -181,6 +186,7 @@ fn parse_error_attribute<'a>(attrs: &mut Attrs<'a>, attr: &'a Attribute) -> Resu infinite_recursive: false, implied_bounds: Set::new(), bindings: Vec::new(), + used_field_names: Set::new(), }; if attrs.display.is_some() { return Err(Error::new_spanned( diff --git a/impl/src/expand.rs b/impl/src/expand.rs index 4e11ff8..df86abd 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -1,5 +1,5 @@ use crate::ast::{Enum, Field, Input, Struct}; -use crate::attr::Trait; +use crate::attr::{Display, Trait}; use crate::fallback; use crate::generics::InferredBounds; use crate::private; @@ -133,10 +133,12 @@ fn impl_struct(input: Struct) -> TokenStream { } else if let Some(display) = &input.attrs.display { display_implied_bounds.clone_from(&display.implied_bounds); let use_as_display = use_as_display(display.has_bonus_display); - let pat = fields_pat(&input.fields); + // Extract fields used in format string for selective binding + let used = used_fields(display); + let pat = fields_pat(&input.fields, &used); Some(quote! { #use_as_display - #[allow(unused_variables, deprecated)] + #[allow(deprecated)] let Self #pat = self; #display }) @@ -395,23 +397,34 @@ fn impl_enum(input: Enum) -> TokenStream { }; let arms = input.variants.iter().map(|variant| { let mut display_implied_bounds = Set::new(); - let display = if let Some(display) = &variant.attrs.display { + let (display, used) = if let Some(display) = &variant.attrs.display { display_implied_bounds.clone_from(&display.implied_bounds); - display.to_token_stream() + let used = used_fields(display); + (display.to_token_stream(), used) } else if let Some(fmt) = &variant.attrs.fmt { + // For #[error(fmt = path)], all fields are passed to the formatter function, + // so all fields are considered "used" let fmt_path = &fmt.path; let vars = variant.fields.iter().map(|field| match &field.member { MemberUnraw::Named(ident) => ident.to_local(), MemberUnraw::Unnamed(index) => format_ident!("_{}", index), }); - quote!(#fmt_path(#(#vars,)* __formatter)) + // Collect binding names in the format used by fields_pat + let used: Set = variant.fields.iter().map(|f| match &f.member { + MemberUnraw::Named(_) => f.member.to_string(), + MemberUnraw::Unnamed(index) => format_ident!("_{}", index).to_string(), + }).collect(); + (quote!(#fmt_path(#(#vars,)* __formatter)), used) } else { + // For transparent display (only first field is used) let only_field = match &variant.fields[0].member { MemberUnraw::Named(ident) => ident.to_local(), MemberUnraw::Unnamed(index) => format_ident!("_{}", index), }; display_implied_bounds.insert((0, Trait::Display)); - quote!(::core::fmt::Display::fmt(#only_field, __formatter)) + let mut used = Set::new(); + used.insert(only_field.to_string()); + (quote!(::core::fmt::Display::fmt(#only_field, __formatter)), used) }; for (field, bound) in display_implied_bounds { let field = &variant.fields[field]; @@ -420,7 +433,7 @@ fn impl_enum(input: Enum) -> TokenStream { } } let ident = &variant.ident; - let pat = fields_pat(&variant.fields); + let pat = fields_pat(&variant.fields, &used); quote! { #ty::#ident #pat => #display } @@ -433,7 +446,7 @@ fn impl_enum(input: Enum) -> TokenStream { impl #impl_generics ::core::fmt::Display for #ty #ty_generics #display_where_clause { fn fmt(&self, __formatter: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { #use_as_display - #[allow(unused_variables, deprecated, clippy::used_underscore_binding)] + #[allow(deprecated, clippy::used_underscore_binding)] match #void_deref self { #(#arms,)* } @@ -508,13 +521,53 @@ pub(crate) fn call_site_ident(ident: &Ident) -> Ident { ident } -fn fields_pat(fields: &[Field]) -> TokenStream { +/// Extracts the set of field binding names used in Display. +/// For named fields: returns the local binding name (e.g., "host" or "r#fn" for raw identifiers) +/// For unnamed fields: returns the binding name (e.g., "_0", "_1") +/// This is needed for selective variable binding to avoid unused_variables warnings +/// on fields that are not referenced in the #[error(...)] format string. +fn used_fields(display: &Display) -> Set { + display.used_field_names.clone() +} + +/// Generates the pattern for matching fields in a struct or enum variant. +/// For fields that are not used in the Display format string, we generate +/// `field: _` to avoid triggering unused_variables warnings. +/// This implements selective variable binding as per issue #446. +fn fields_pat(fields: &[Field], used_fields: &Set) -> TokenStream { let mut members = fields.iter().map(|field| &field.member).peekable(); match members.peek() { - Some(MemberUnraw::Named(_)) => quote!({ #(#members),* }), + Some(MemberUnraw::Named(_)) => { + let patterns = fields.iter().map(|field| { + let member = &field.member; + // Get the local binding name (same as ToTokens uses) + let binding_name = match member { + MemberUnraw::Named(ident) => ident.to_local().to_string(), + MemberUnraw::Unnamed(_) => unreachable!(), + }; + if used_fields.contains(&binding_name) { + // Field is used in format string - bind normally + quote! { #member } + } else { + // Field is NOT used in format string - bind to _ to avoid warning + quote! { #member: _ } + } + }); + quote!({ #(#patterns),* }) + } Some(MemberUnraw::Unnamed(_)) => { - let vars = members.map(|member| match member { - MemberUnraw::Unnamed(index) => format_ident!("_{}", index), + let vars = fields.iter().map(|field| match &field.member { + MemberUnraw::Unnamed(index) => { + // For unnamed fields, the binding name is "_0", "_1", etc. + let var_name = format_ident!("_{}", index); + if used_fields.contains(&var_name.to_string()) { + // Field is used in format string - bind normally + quote! { #var_name } + } else { + // Field is NOT used in format string - use _ to avoid warning + quote! { _ } + } + } MemberUnraw::Named(_) => unreachable!(), }); quote!((#(#vars),*)) diff --git a/impl/src/fmt.rs b/impl/src/fmt.rs index 2988ca3..6e7f59c 100644 --- a/impl/src/fmt.rs +++ b/impl/src/fmt.rs @@ -36,6 +36,8 @@ impl Display<'_> { let mut implied_bounds = BTreeSet::new(); let mut bindings = Vec::new(); let mut macro_named_args = BTreeSet::new(); + // Track which field names are used in the format string for selective binding + let mut used_field_names = BTreeSet::new(); self.requires_fmt_machinery = self.requires_fmt_machinery || fmt.contains('}'); @@ -143,6 +145,8 @@ impl Display<'_> { MemberUnraw::Named(ident) => ident.to_local(), }; binding_value.set_span(span.resolved_at(fields[field].member.span())); + // Track the original binding name for selective variable binding + used_field_names.insert(binding_value.to_string()); let wrapped_binding_value = if bonus_display { quote_spanned!(span=> #binding_value.as_display()) } else if bound == Trait::Pointer { @@ -159,6 +163,12 @@ impl Display<'_> { self.infinite_recursive = infinite_recursive; self.implied_bounds = implied_bounds; self.bindings = bindings; + self.used_field_names = used_field_names; + + // Also scan args for field references like .field or .0 + // These are used directly in expressions and need to be bound in the pattern + self.used_field_names.extend(scan_field_refs(&self.args, fields)); + Ok(()) } } @@ -321,3 +331,42 @@ fn between<'a>(begin: ParseStream<'a>, end: ParseStream<'a>) -> TokenStream { tokens } + +/// Scans a TokenStream for field binding references that were already transformed. +/// The `parse_token_expr` function in `attr.rs` transforms `.field` to `field` and `.0` to `_0`. +/// So we look for these already-transformed identifiers. +/// Returns the set of binding names that need to be available in the pattern. +fn scan_field_refs(tokens: &TokenStream, fields: &[Field]) -> BTreeSet { + let mut used = BTreeSet::new(); + let mut tokens = tokens.clone().into_iter().peekable(); + + // Build sets of valid binding names for validation + // For named fields: binding name is the local binding name (via to_local()) + // For unnamed fields: binding name is "_0", "_1", etc. + let valid_binding_names: BTreeSet = fields + .iter() + .map(|f| match &f.member { + MemberUnraw::Named(ident) => ident.to_local().to_string(), + MemberUnraw::Unnamed(index) => format_ident!("_{}", index).to_string(), + }) + .collect(); + + while let Some(token) = tokens.next() { + // Look for Ident tokens that match field binding names + if let TokenTree::Ident(ident) = &token { + let name = ident.to_string(); + // Check if this is a field binding name + if valid_binding_names.contains(&name) { + used.insert(name); + } + } + + // Recursively scan inside groups (parentheses, brackets, braces) + if let TokenTree::Group(group) = &token { + let inner_used = scan_field_refs(&group.stream(), fields); + used.extend(inner_used); + } + } + + used +} diff --git a/tests/ui/unused_variables.rs b/tests/ui/unused_variables.rs new file mode 100644 index 0000000..b95ac2d --- /dev/null +++ b/tests/ui/unused_variables.rs @@ -0,0 +1,26 @@ +#![deny(unused_variables)] + +pub enum ServerError { + #[error("Connection timeout to {host}")] + Timeout { + host: String, + // The `port` variable intentionally is not unsed in the macros string. + // So, the compiler must throw unused_variables error + port: u16, + }, +} + +#[error("Failed to connect to {host}")] +pub struct ConnectionError { + host: String, + // Not used variable in this structure + port: u16, +} + +pub enum TupleError { + #[error("First: {0}")] + // The second element (u32) of the tuple is not used + FirstOnly(String, u32), +} + +fn main() {} diff --git a/tests/ui/unused_variables.stderr b/tests/ui/unused_variables.stderr new file mode 100644 index 0000000..5d80cb3 --- /dev/null +++ b/tests/ui/unused_variables.stderr @@ -0,0 +1,17 @@ +error: cannot find attribute `error` in this scope + --> tests/ui/unused_variables.rs:21:7 + | +21 | #[error("First: {0}")] + | ^^^^^ + +error: cannot find attribute `error` in this scope + --> tests/ui/unused_variables.rs:13:3 + | +13 | #[error("Failed to connect to {host}")] + | ^^^^^ + +error: cannot find attribute `error` in this scope + --> tests/ui/unused_variables.rs:4:7 + | +4 | #[error("Connection timeout to {host}")] + | ^^^^^