diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index 207ae8ad844bb..dda466b026d91 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -137,6 +137,20 @@ builtin_macros_format_positional_after_named = positional arguments cannot follo .label = positional arguments must be before named arguments .named_args = named argument +builtin_macros_format_redundant_args = redundant {$n -> + [one] argument + *[more] arguments + } + .help = {$n -> + [one] the formatting string already captures the binding directly, it doesn't need to be included in the argument list + *[more] the formatting strings already captures the bindings directly, they don't need to be included in the argument list + } + .note = {$n -> + [one] the formatting specifier is referencing the binding already + *[more] the formatting specifiers are referencing the bindings already + } + .suggestion = this can be removed + builtin_macros_format_remove_raw_ident = remove the `r#` builtin_macros_format_requires_string = requires at least a format string argument diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 1238773d58b55..fde4270334b67 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -646,6 +646,27 @@ pub(crate) struct FormatPositionalMismatch { pub(crate) highlight: SingleLabelManySpans, } +#[derive(Diagnostic)] +#[diag(builtin_macros_format_redundant_args)] +pub(crate) struct FormatRedundantArgs { + #[primary_span] + pub(crate) span: MultiSpan, + pub(crate) n: usize, + + #[note] + pub(crate) note: MultiSpan, + + #[subdiagnostic] + pub(crate) sugg: Option, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(builtin_macros_suggestion, applicability = "machine-applicable")] +pub(crate) struct FormatRedundantArgsSugg { + #[suggestion_part(code = "")] + pub(crate) spans: Vec, +} + #[derive(Diagnostic)] #[diag(builtin_macros_test_case_non_item)] pub(crate) struct TestCaseNonItem { diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 4b5c777f4ad64..214fed8e2d827 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1,3 +1,4 @@ +use parse::Position::ArgumentNamed; use rustc_ast::ptr::P; use rustc_ast::tokenstream::TokenStream; use rustc_ast::{token, StmtKind}; @@ -7,7 +8,9 @@ use rustc_ast::{ FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait, }; use rustc_data_structures::fx::FxHashSet; -use rustc_errors::{Applicability, MultiSpan, PResult, SingleLabelManySpans}; +use rustc_errors::{ + Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, PResult, SingleLabelManySpans, +}; use rustc_expand::base::{self, *}; use rustc_parse_format as parse; use rustc_span::symbol::{Ident, Symbol}; @@ -364,8 +367,8 @@ fn make_format_args( let mut unfinished_literal = String::new(); let mut placeholder_index = 0; - for piece in pieces { - match piece { + for piece in &pieces { + match *piece { parse::Piece::String(s) => { unfinished_literal.push_str(s); } @@ -513,7 +516,17 @@ fn make_format_args( // If there's a lot of unused arguments, // let's check if this format arguments looks like another syntax (printf / shell). let detect_foreign_fmt = unused.len() > args.explicit_args().len() / 2; - report_missing_placeholders(ecx, unused, detect_foreign_fmt, str_style, fmt_str, fmt_span); + report_missing_placeholders( + ecx, + unused, + &used, + &args, + &pieces, + detect_foreign_fmt, + str_style, + fmt_str, + fmt_span, + ); } // Only check for unused named argument names if there are no other errors to avoid causing @@ -580,6 +593,9 @@ fn invalid_placeholder_type_error( fn report_missing_placeholders( ecx: &mut ExtCtxt<'_>, unused: Vec<(Span, bool)>, + used: &[bool], + args: &FormatArguments, + pieces: &[parse::Piece<'_>], detect_foreign_fmt: bool, str_style: Option, fmt_str: &str, @@ -598,6 +614,26 @@ fn report_missing_placeholders( }) }; + let placeholders = pieces + .iter() + .filter_map(|piece| { + if let parse::Piece::NextArgument(argument) = piece && let ArgumentNamed(binding) = argument.position { + let span = fmt_span.from_inner(InnerSpan::new(argument.position_span.start, argument.position_span.end)); + Some((span, binding)) + } else { None } + }) + .collect::>(); + + if !placeholders.is_empty() { + if let Some(mut new_diag) = + report_redundant_format_arguments(ecx, &args, used, placeholders) + { + diag.cancel(); + new_diag.emit(); + return; + } + } + // Used to ensure we only report translations for *one* kind of foreign format. let mut found_foreign = false; @@ -685,6 +721,76 @@ fn report_missing_placeholders( diag.emit(); } +/// This function detects and reports unused format!() arguments that are +/// redundant due to implicit captures (e.g. `format!("{x}", x)`). +fn report_redundant_format_arguments<'a>( + ecx: &mut ExtCtxt<'a>, + args: &FormatArguments, + used: &[bool], + placeholders: Vec<(Span, &str)>, +) -> Option> { + let mut fmt_arg_indices = vec![]; + let mut args_spans = vec![]; + let mut fmt_spans = vec![]; + + for (i, unnamed_arg) in args.unnamed_args().iter().enumerate().rev() { + let Some(ty) = unnamed_arg.expr.to_ty() else { continue }; + let Some(argument_binding) = ty.kind.is_simple_path() else { continue }; + let argument_binding = argument_binding.as_str(); + + if used[i] { + continue; + } + + let matching_placeholders = placeholders + .iter() + .filter(|(_, inline_binding)| argument_binding == *inline_binding) + .map(|(span, _)| span) + .collect::>(); + + if !matching_placeholders.is_empty() { + fmt_arg_indices.push(i); + args_spans.push(unnamed_arg.expr.span); + for span in &matching_placeholders { + if fmt_spans.contains(*span) { + continue; + } + fmt_spans.push(**span); + } + } + } + + if !args_spans.is_empty() { + let multispan = MultiSpan::from(fmt_spans); + let mut suggestion_spans = vec![]; + + for (arg_span, fmt_arg_idx) in args_spans.iter().zip(fmt_arg_indices.iter()) { + let span = if fmt_arg_idx + 1 == args.explicit_args().len() { + *arg_span + } else { + arg_span.until(args.explicit_args()[*fmt_arg_idx + 1].expr.span) + }; + + suggestion_spans.push(span); + } + + let sugg = if args.named_args().len() == 0 { + Some(errors::FormatRedundantArgsSugg { spans: suggestion_spans }) + } else { + None + }; + + return Some(ecx.create_err(errors::FormatRedundantArgs { + n: args_spans.len(), + span: MultiSpan::from(args_spans), + note: multispan, + sugg, + })); + } + + None +} + /// Handle invalid references to positional arguments. Output different /// errors for the case where all arguments are positional and for when /// there are named arguments or numbered positional arguments in the diff --git a/tests/ui/did_you_mean/issue-105225-named-args.rs b/tests/ui/did_you_mean/issue-105225-named-args.rs new file mode 100644 index 0000000000000..38e817765769f --- /dev/null +++ b/tests/ui/did_you_mean/issue-105225-named-args.rs @@ -0,0 +1,10 @@ +fn main() { + let x = "x"; + let y = "y"; + + println!("{x}", x, x = y); + //~^ ERROR: redundant argument + + println!("{x}", x = y, x = y); + //~^ ERROR: duplicate argument named `x` +} diff --git a/tests/ui/did_you_mean/issue-105225-named-args.stderr b/tests/ui/did_you_mean/issue-105225-named-args.stderr new file mode 100644 index 0000000000000..72204102ef6cc --- /dev/null +++ b/tests/ui/did_you_mean/issue-105225-named-args.stderr @@ -0,0 +1,22 @@ +error: redundant argument + --> $DIR/issue-105225-named-args.rs:5:21 + | +LL | println!("{x}", x, x = y); + | ^ + | +note: the formatting specifier is referencing the binding already + --> $DIR/issue-105225-named-args.rs:5:16 + | +LL | println!("{x}", x, x = y); + | ^ + +error: duplicate argument named `x` + --> $DIR/issue-105225-named-args.rs:8:28 + | +LL | println!("{x}", x = y, x = y); + | - ^ duplicate argument + | | + | previously here + +error: aborting due to 2 previous errors + diff --git a/tests/ui/did_you_mean/issue-105225.fixed b/tests/ui/did_you_mean/issue-105225.fixed new file mode 100644 index 0000000000000..f756be615a1bb --- /dev/null +++ b/tests/ui/did_you_mean/issue-105225.fixed @@ -0,0 +1,21 @@ +// run-rustfix + +fn main() { + let x = "x"; + let y = "y"; + + println!("{x}", ); + //~^ ERROR: redundant argument + + println!("{x} {}", x, ); + //~^ ERROR: redundant argument + + println!("{} {x}", x, ); + //~^ ERROR: redundant argument + + println!("{x} {y}", ); + //~^ ERROR: redundant arguments + + println!("{} {} {x} {y} {}", x, x, x, ); + //~^ ERROR: redundant arguments +} diff --git a/tests/ui/did_you_mean/issue-105225.rs b/tests/ui/did_you_mean/issue-105225.rs new file mode 100644 index 0000000000000..91cdf0eb28f6f --- /dev/null +++ b/tests/ui/did_you_mean/issue-105225.rs @@ -0,0 +1,21 @@ +// run-rustfix + +fn main() { + let x = "x"; + let y = "y"; + + println!("{x}", x); + //~^ ERROR: redundant argument + + println!("{x} {}", x, x); + //~^ ERROR: redundant argument + + println!("{} {x}", x, x); + //~^ ERROR: redundant argument + + println!("{x} {y}", x, y); + //~^ ERROR: redundant arguments + + println!("{} {} {x} {y} {}", x, x, x, y, y); + //~^ ERROR: redundant arguments +} diff --git a/tests/ui/did_you_mean/issue-105225.stderr b/tests/ui/did_you_mean/issue-105225.stderr new file mode 100644 index 0000000000000..5fb46222bee59 --- /dev/null +++ b/tests/ui/did_you_mean/issue-105225.stderr @@ -0,0 +1,72 @@ +error: redundant argument + --> $DIR/issue-105225.rs:7:21 + | +LL | println!("{x}", x); + | ^ help: this can be removed + | +note: the formatting specifier is referencing the binding already + --> $DIR/issue-105225.rs:7:16 + | +LL | println!("{x}", x); + | ^ + +error: redundant argument + --> $DIR/issue-105225.rs:10:27 + | +LL | println!("{x} {}", x, x); + | ^ help: this can be removed + | +note: the formatting specifier is referencing the binding already + --> $DIR/issue-105225.rs:10:16 + | +LL | println!("{x} {}", x, x); + | ^ + +error: redundant argument + --> $DIR/issue-105225.rs:13:27 + | +LL | println!("{} {x}", x, x); + | ^ help: this can be removed + | +note: the formatting specifier is referencing the binding already + --> $DIR/issue-105225.rs:13:19 + | +LL | println!("{} {x}", x, x); + | ^ + +error: redundant arguments + --> $DIR/issue-105225.rs:16:25 + | +LL | println!("{x} {y}", x, y); + | ^ ^ + | +note: the formatting specifiers are referencing the bindings already + --> $DIR/issue-105225.rs:16:16 + | +LL | println!("{x} {y}", x, y); + | ^ ^ +help: this can be removed + | +LL - println!("{x} {y}", x, y); +LL + println!("{x} {y}", ); + | + +error: redundant arguments + --> $DIR/issue-105225.rs:19:43 + | +LL | println!("{} {} {x} {y} {}", x, x, x, y, y); + | ^ ^ + | +note: the formatting specifiers are referencing the bindings already + --> $DIR/issue-105225.rs:19:26 + | +LL | println!("{} {} {x} {y} {}", x, x, x, y, y); + | ^ +help: this can be removed + | +LL - println!("{} {} {x} {y} {}", x, x, x, y, y); +LL + println!("{} {} {x} {y} {}", x, x, x, ); + | + +error: aborting due to 5 previous errors +