From 64bfc7af9e58cb6f2b5c10227fa49053b2eeda36 Mon Sep 17 00:00:00 2001 From: francorbacho Date: Mon, 25 Sep 2023 16:50:17 +0200 Subject: [PATCH] Separate report_redundant_placeholders() into its own function --- compiler/rustc_builtin_macros/src/format.rs | 113 +++++++++++--------- 1 file changed, 62 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 6e00d0f12d4c1..111069236cbf9 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -8,11 +8,11 @@ use rustc_ast::{ FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait, }; use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; -use rustc_errors::{Applicability, MultiSpan, PResult, SingleLabelManySpans}; +use rustc_errors::{Applicability, DiagnosticBuilder, MultiSpan, PResult, SingleLabelManySpans}; use rustc_expand::base::{self, *}; use rustc_parse_format as parse; use rustc_span::symbol::{Ident, Symbol}; -use rustc_span::{BytePos, InnerSpan, Span}; +use rustc_span::{BytePos, ErrorGuaranteed, InnerSpan, Span}; use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY; use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics, LintId}; @@ -606,55 +606,8 @@ fn report_missing_placeholders( }) .collect::>(); - let mut args_spans = vec![]; - let mut fmt_spans = FxIndexSet::default(); - - 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) - .collect::>(); - - if !matching_placeholders.is_empty() { - args_spans.push(unnamed_arg.expr.span); - for placeholder in &matching_placeholders { - fmt_spans.insert(*placeholder); - } - } - } - - if !args_spans.is_empty() { - let mut multispan = MultiSpan::from(args_spans.clone()); - - let msg = if fmt_spans.len() > 1 { - "the formatting strings already captures the bindings \ - directly, they don't need to be included in the argument list" - } else { - "the formatting string already captures the binding \ - directly, it doesn't need to be included in the argument list" - }; - - for (span, binding) in fmt_spans { - multispan.push_span_label( - *span, - format!("this formatting specifier is referencing the `{binding}` binding"), - ); - } - - for span in &args_spans { - multispan.push_span_label(*span, "this can be removed"); - } - - diag.span_help(multispan, msg); - diag.emit(); + if !placeholders.is_empty() { + report_redundant_placeholders(diag, &args, used, placeholders); return; } @@ -745,6 +698,64 @@ fn report_missing_placeholders( diag.emit(); } +fn report_redundant_placeholders( + mut diag: DiagnosticBuilder<'_, ErrorGuaranteed>, + args: &FormatArguments, + used: &[bool], + placeholders: Vec<(Span, &str)>, +) { + let mut args_spans = vec![]; + let mut fmt_spans = FxIndexSet::default(); + + 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) + .collect::>(); + + if !matching_placeholders.is_empty() { + args_spans.push(unnamed_arg.expr.span); + for placeholder in &matching_placeholders { + fmt_spans.insert(*placeholder); + } + } + } + + if !args_spans.is_empty() { + let mut multispan = MultiSpan::from(args_spans.clone()); + + let msg = if fmt_spans.len() > 1 { + "the formatting strings already captures the bindings \ + directly, they don't need to be included in the argument list" + } else { + "the formatting string already captures the binding \ + directly, it doesn't need to be included in the argument list" + }; + + for (span, binding) in fmt_spans { + multispan.push_span_label( + *span, + format!("this formatting specifier is referencing the `{binding}` binding"), + ); + } + + for span in &args_spans { + multispan.push_span_label(*span, "this can be removed"); + } + + diag.span_help(multispan, msg); + diag.emit(); + } +} + /// 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