From 5af8f1521af89e1f477b32c8eb86909895169df1 Mon Sep 17 00:00:00 2001 From: tanveerraza789 Date: Tue, 13 Dec 2022 03:04:42 +0530 Subject: [PATCH 1/7] Add suggestion for captured and passed arg case --- compiler/rustc_builtin_macros/src/format.rs | 16 +++++++++++++++- tests/ui/macros/issue-105225.rs | 5 +++++ tests/ui/macros/issue-105225.stderr | 10 ++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/ui/macros/issue-105225.rs create mode 100644 tests/ui/macros/issue-105225.stderr diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index db2ef7fba4b8e..4a9d63ba4f383 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -478,7 +478,21 @@ fn make_format_args( let msg = if let FormatArgumentKind::Named(_) = args.explicit_args()[i].kind { "named argument never used" } else { - "argument never used" + let mut err = "argument never used"; + match args.explicit_args()[i].expr.to_ty() { + Some(expr) => match expr.kind.is_simple_path() { + Some(symbol) => { + let current_arg = symbol.as_str(); + let current_arg_ph = "{".to_owned() + current_arg + "}"; + if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { + err = "argument never used, consider removing it" + } + } + None => {} + }, + None => {} + } + err }; (args.explicit_args()[i].expr.span, msg) }) diff --git a/tests/ui/macros/issue-105225.rs b/tests/ui/macros/issue-105225.rs new file mode 100644 index 0000000000000..ac687accf11f0 --- /dev/null +++ b/tests/ui/macros/issue-105225.rs @@ -0,0 +1,5 @@ +fn main() { + let x = 10; + println!("{x}", x); + //~^ ERROR argument never used, consider removing it +} diff --git a/tests/ui/macros/issue-105225.stderr b/tests/ui/macros/issue-105225.stderr new file mode 100644 index 0000000000000..c3dfc4cba78d0 --- /dev/null +++ b/tests/ui/macros/issue-105225.stderr @@ -0,0 +1,10 @@ +error: argument never used, consider removing it + --> $DIR/issue-105225.rs:3:21 + | +LL | println!("{x}", x); + | ----- ^ argument never used, consider removing it + | | + | formatting specifier missing + +error: aborting due to previous error + From 98407cb2796a1dd23eaa899ac408b30e26114bbb Mon Sep 17 00:00:00 2001 From: tanveerraza789 Date: Tue, 13 Dec 2022 22:14:25 +0530 Subject: [PATCH 2/7] Fixed suggestion --- compiler/rustc_builtin_macros/src/format.rs | 14 +++++--------- tests/ui/macros/issue-105225.rs | 2 +- tests/ui/macros/issue-105225.stderr | 4 ++-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 4a9d63ba4f383..b984bb8052877 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -479,19 +479,15 @@ fn make_format_args( "named argument never used" } else { let mut err = "argument never used"; - match args.explicit_args()[i].expr.to_ty() { - Some(expr) => match expr.kind.is_simple_path() { - Some(symbol) => { + if let Some(expr) = args.explicit_args()[i].expr.to_ty() { + if let Some(symbol) = expr.kind.is_simple_path() { let current_arg = symbol.as_str(); - let current_arg_ph = "{".to_owned() + current_arg + "}"; + let current_arg_ph = format!("{{{current_arg}}}"); if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { - err = "argument never used, consider removing it" + err = "argument is a duplicate of an inline argument" } } - None => {} - }, - None => {} - } + } err }; (args.explicit_args()[i].expr.span, msg) diff --git a/tests/ui/macros/issue-105225.rs b/tests/ui/macros/issue-105225.rs index ac687accf11f0..78dbfe02ef9dc 100644 --- a/tests/ui/macros/issue-105225.rs +++ b/tests/ui/macros/issue-105225.rs @@ -1,5 +1,5 @@ fn main() { let x = 10; println!("{x}", x); - //~^ ERROR argument never used, consider removing it + //~^ ERROR argument is a duplicate of an inline argument } diff --git a/tests/ui/macros/issue-105225.stderr b/tests/ui/macros/issue-105225.stderr index c3dfc4cba78d0..0596d6a3b89d2 100644 --- a/tests/ui/macros/issue-105225.stderr +++ b/tests/ui/macros/issue-105225.stderr @@ -1,8 +1,8 @@ -error: argument never used, consider removing it +error: argument is a duplicate of an inline argument --> $DIR/issue-105225.rs:3:21 | LL | println!("{x}", x); - | ----- ^ argument never used, consider removing it + | ----- ^ argument is a duplicate of an inline argument | | | formatting specifier missing From 075e891834713f26935d3f36329f8d43cfbd1abe Mon Sep 17 00:00:00 2001 From: tanveerraza789 Date: Thu, 22 Dec 2022 22:25:31 +0530 Subject: [PATCH 3/7] Update suggestion msgs --- compiler/rustc_builtin_macros/src/format.rs | 48 +++++++++++++++++---- tests/ui/macros/issue-105225.rs | 5 ++- tests/ui/macros/issue-105225.stderr | 29 +++++++++++-- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index b984bb8052877..15472f1e8ec73 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -470,6 +470,8 @@ fn make_format_args( report_invalid_references(ecx, &invalid_refs, &template, fmt_span, &args, parser); } + let mut duplicate_explicit_arg = Vec::new(); + let unused = used .iter() .enumerate() @@ -478,17 +480,17 @@ fn make_format_args( let msg = if let FormatArgumentKind::Named(_) = args.explicit_args()[i].kind { "named argument never used" } else { - let mut err = "argument never used"; if let Some(expr) = args.explicit_args()[i].expr.to_ty() { if let Some(symbol) = expr.kind.is_simple_path() { - let current_arg = symbol.as_str(); - let current_arg_ph = format!("{{{current_arg}}}"); - if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { - err = "argument is a duplicate of an inline argument" - } + let current_arg = symbol.as_str().to_owned(); + let current_arg_ph = format!("{{{current_arg}}}"); + if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { + duplicate_explicit_arg + .push((current_arg, args.explicit_args()[i].expr.span)); } } - err + } + "argument never used" }; (args.explicit_args()[i].expr.span, msg) }) @@ -498,7 +500,15 @@ 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, + duplicate_explicit_arg, + 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 @@ -581,14 +591,21 @@ fn invalid_placeholder_type_error( fn report_missing_placeholders( ecx: &mut ExtCtxt<'_>, unused: Vec<(Span, &str)>, + duplicate_explicit_arg: Vec<(String, Span)>, detect_foreign_fmt: bool, str_style: Option, fmt_str: &str, fmt_span: Span, ) { + let mut dup_exist = false; let mut diag = if let &[(span, msg)] = &unused[..] { let mut diag = ecx.struct_span_err(span, msg); diag.span_label(span, msg); + for (_var, sp) in &duplicate_explicit_arg { + if !dup_exist && span == *sp { + dup_exist = true; + } + } diag } else { let mut diag = ecx.struct_span_err( @@ -598,6 +615,11 @@ fn report_missing_placeholders( diag.span_label(fmt_span, "multiple missing formatting specifiers"); for &(span, msg) in &unused { diag.span_label(span, msg); + for (_var, sp) in &duplicate_explicit_arg { + if !dup_exist && span == *sp { + dup_exist = true; + } + } } diag }; @@ -685,7 +707,15 @@ fn report_missing_placeholders( if !found_foreign && unused.len() == 1 { diag.span_label(fmt_span, "formatting specifier missing"); } - + if dup_exist { + diag.note("the formatting string captures that binding directly, it doesn't need to be included in the argument list"); + let (_names, spans): (Vec<_>, Vec<_>) = duplicate_explicit_arg.into_iter().unzip(); + let multi = if spans.len() == 1 { false } else { true }; + diag.span_help( + spans, + format!("Consider removing {}", if multi { "these" } else { "this" }), + ); + } diag.emit(); } diff --git a/tests/ui/macros/issue-105225.rs b/tests/ui/macros/issue-105225.rs index 78dbfe02ef9dc..c2530be272336 100644 --- a/tests/ui/macros/issue-105225.rs +++ b/tests/ui/macros/issue-105225.rs @@ -1,5 +1,8 @@ fn main() { let x = 10; println!("{x}", x); - //~^ ERROR argument is a duplicate of an inline argument + //~^ ERROR argument never used + let y = 20; + println!("{x} {y}", x, y); + //~^ ERROR multiple unused formatting arguments } diff --git a/tests/ui/macros/issue-105225.stderr b/tests/ui/macros/issue-105225.stderr index 0596d6a3b89d2..0392d30db220f 100644 --- a/tests/ui/macros/issue-105225.stderr +++ b/tests/ui/macros/issue-105225.stderr @@ -1,10 +1,33 @@ -error: argument is a duplicate of an inline argument +error: argument never used --> $DIR/issue-105225.rs:3:21 | LL | println!("{x}", x); - | ----- ^ argument is a duplicate of an inline argument + | ----- ^ argument never used | | | formatting specifier missing + | + = note: the formatting string captures that binding directly, it doesn't need to be included in the argument list +help: Consider removing this + --> $DIR/issue-105225.rs:3:21 + | +LL | println!("{x}", x); + | ^ + +error: multiple unused formatting arguments + --> $DIR/issue-105225.rs:6:25 + | +LL | println!("{x} {y}", x, y); + | --------- ^ ^ argument never used + | | | + | | argument never used + | multiple missing formatting specifiers + | + = note: the formatting string captures that binding directly, it doesn't need to be included in the argument list +help: Consider removing these + --> $DIR/issue-105225.rs:6:25 + | +LL | println!("{x} {y}", x, y); + | ^ ^ -error: aborting due to previous error +error: aborting due to 2 previous errors From 01a492c9910972046b8f5d08581e4eeda088811d Mon Sep 17 00:00:00 2001 From: tanveerraza789 Date: Fri, 23 Dec 2022 10:33:53 +0530 Subject: [PATCH 4/7] Removed unused `name` from duplicate_explicit_arg --- compiler/rustc_builtin_macros/src/format.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 15472f1e8ec73..cb8f0da3414d1 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -485,8 +485,7 @@ fn make_format_args( let current_arg = symbol.as_str().to_owned(); let current_arg_ph = format!("{{{current_arg}}}"); if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { - duplicate_explicit_arg - .push((current_arg, args.explicit_args()[i].expr.span)); + duplicate_explicit_arg.push(args.explicit_args()[i].expr.span); } } } @@ -591,7 +590,7 @@ fn invalid_placeholder_type_error( fn report_missing_placeholders( ecx: &mut ExtCtxt<'_>, unused: Vec<(Span, &str)>, - duplicate_explicit_arg: Vec<(String, Span)>, + duplicate_explicit_arg: Vec, detect_foreign_fmt: bool, str_style: Option, fmt_str: &str, @@ -601,7 +600,7 @@ fn report_missing_placeholders( let mut diag = if let &[(span, msg)] = &unused[..] { let mut diag = ecx.struct_span_err(span, msg); diag.span_label(span, msg); - for (_var, sp) in &duplicate_explicit_arg { + for sp in &duplicate_explicit_arg { if !dup_exist && span == *sp { dup_exist = true; } @@ -615,7 +614,7 @@ fn report_missing_placeholders( diag.span_label(fmt_span, "multiple missing formatting specifiers"); for &(span, msg) in &unused { diag.span_label(span, msg); - for (_var, sp) in &duplicate_explicit_arg { + for sp in &duplicate_explicit_arg { if !dup_exist && span == *sp { dup_exist = true; } @@ -709,12 +708,8 @@ fn report_missing_placeholders( } if dup_exist { diag.note("the formatting string captures that binding directly, it doesn't need to be included in the argument list"); - let (_names, spans): (Vec<_>, Vec<_>) = duplicate_explicit_arg.into_iter().unzip(); - let multi = if spans.len() == 1 { false } else { true }; - diag.span_help( - spans, - format!("Consider removing {}", if multi { "these" } else { "this" }), - ); + let this_these = if duplicate_explicit_arg.len() == 1 { "this" } else { "these" }; + diag.span_help(duplicate_explicit_arg, format!("Consider removing {}", this_these)); } diag.emit(); } From 77a96735accbd9e0e020adcc7a0682cf39b420c9 Mon Sep 17 00:00:00 2001 From: tanveerraza789 Date: Sat, 14 Jan 2023 14:05:08 +0530 Subject: [PATCH 5/7] Add better suggestion and more tests --- compiler/rustc_builtin_macros/src/format.rs | 77 ++++++++++++++------- tests/ui/macros/issue-105225.rs | 6 ++ tests/ui/macros/issue-105225.stderr | 73 ++++++++++++++++--- 3 files changed, 123 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index cb8f0da3414d1..102532392661c 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1,3 +1,5 @@ +use parse::Piece::NextArgument; +use parse::Position::ArgumentNamed; use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; @@ -34,6 +36,14 @@ enum PositionUsedAs { Precision, Width, } + +#[derive(Clone, Copy, Debug)] +struct RedundantArgDiagBuilder<'a> { + arg_span: Span, + fmt_span: InnerSpan, + fmt_str: &'a str, +} + use PositionUsedAs::*; struct MacroInput { @@ -335,8 +345,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); } @@ -470,7 +480,7 @@ fn make_format_args( report_invalid_references(ecx, &invalid_refs, &template, fmt_span, &args, parser); } - let mut duplicate_explicit_arg = Vec::new(); + let mut redundant_expl_args = Vec::new(); let unused = used .iter() @@ -482,10 +492,23 @@ fn make_format_args( } else { if let Some(expr) = args.explicit_args()[i].expr.to_ty() { if let Some(symbol) = expr.kind.is_simple_path() { - let current_arg = symbol.as_str().to_owned(); - let current_arg_ph = format!("{{{current_arg}}}"); - if current_arg.len() > 0 && fmt_str.contains(current_arg_ph.as_str()) { - duplicate_explicit_arg.push(args.explicit_args()[i].expr.span); + for piece in &pieces { + if let NextArgument(arg) = piece { + if let ArgumentNamed(specifier) = arg.position { + if specifier == symbol.to_string() { + redundant_expl_args.push({ + RedundantArgDiagBuilder { + arg_span: expr.span, + fmt_span: InnerSpan { + start: arg.position_span.start, + end: arg.position_span.end, + }, + fmt_str: specifier, + } + }); + } + } + } } } } @@ -502,7 +525,7 @@ fn make_format_args( report_missing_placeholders( ecx, unused, - duplicate_explicit_arg, + redundant_expl_args, detect_foreign_fmt, str_style, fmt_str, @@ -590,21 +613,15 @@ fn invalid_placeholder_type_error( fn report_missing_placeholders( ecx: &mut ExtCtxt<'_>, unused: Vec<(Span, &str)>, - duplicate_explicit_arg: Vec, + redundant_expl_args: Vec>, detect_foreign_fmt: bool, str_style: Option, fmt_str: &str, fmt_span: Span, ) { - let mut dup_exist = false; let mut diag = if let &[(span, msg)] = &unused[..] { let mut diag = ecx.struct_span_err(span, msg); diag.span_label(span, msg); - for sp in &duplicate_explicit_arg { - if !dup_exist && span == *sp { - dup_exist = true; - } - } diag } else { let mut diag = ecx.struct_span_err( @@ -614,11 +631,6 @@ fn report_missing_placeholders( diag.span_label(fmt_span, "multiple missing formatting specifiers"); for &(span, msg) in &unused { diag.span_label(span, msg); - for sp in &duplicate_explicit_arg { - if !dup_exist && span == *sp { - dup_exist = true; - } - } } diag }; @@ -706,11 +718,28 @@ fn report_missing_placeholders( if !found_foreign && unused.len() == 1 { diag.span_label(fmt_span, "formatting specifier missing"); } - if dup_exist { - diag.note("the formatting string captures that binding directly, it doesn't need to be included in the argument list"); - let this_these = if duplicate_explicit_arg.len() == 1 { "this" } else { "these" }; - diag.span_help(duplicate_explicit_arg, format!("Consider removing {}", this_these)); + if !redundant_expl_args.is_empty() { + let mut fmt_spans = Vec::new(); + let mut arg_spans = Vec::new(); + let mut arg = Vec::new(); + for rd in &redundant_expl_args { + fmt_spans.push(Span::from_inner(fmt_span, rd.fmt_span)); + arg_spans.push(rd.arg_span); + arg.push(rd.fmt_str); + } + let mut m_span: MultiSpan = fmt_spans.clone().into(); + for i in 0..fmt_spans.len() { + m_span.push_span_label( + fmt_spans[i], + format!("this formatting argument captures `{}` directly", arg[i]), + ); + } + for arg_span in arg_spans { + m_span.push_span_label(arg_span, "this can be removed".to_string()); + } + diag.span_help(m_span, "the formatting string captures that binding directly, it doesn't need to be included in the argument list"); } + diag.emit(); } diff --git a/tests/ui/macros/issue-105225.rs b/tests/ui/macros/issue-105225.rs index c2530be272336..ffb8d52e2fdc8 100644 --- a/tests/ui/macros/issue-105225.rs +++ b/tests/ui/macros/issue-105225.rs @@ -5,4 +5,10 @@ fn main() { let y = 20; println!("{x} {y}", x, y); //~^ ERROR multiple unused formatting arguments + println!("{x} {y}", y, x); + //~^ ERROR multiple unused formatting arguments + println!("{} {y}", x, y); + //~^ ERROR argument never used + println!("{} {} {y} {} {}", y, y, y, y, y); + //~^ ERROR argument never used } diff --git a/tests/ui/macros/issue-105225.stderr b/tests/ui/macros/issue-105225.stderr index 0392d30db220f..2195a4545c02d 100644 --- a/tests/ui/macros/issue-105225.stderr +++ b/tests/ui/macros/issue-105225.stderr @@ -6,12 +6,13 @@ LL | println!("{x}", x); | | | formatting specifier missing | - = note: the formatting string captures that binding directly, it doesn't need to be included in the argument list -help: Consider removing this - --> $DIR/issue-105225.rs:3:21 +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> $DIR/issue-105225.rs:3:16 | LL | println!("{x}", x); - | ^ + | ^ - this can be removed + | | + | this formatting argument captures `x` directly error: multiple unused formatting arguments --> $DIR/issue-105225.rs:6:25 @@ -22,12 +23,66 @@ LL | println!("{x} {y}", x, y); | | argument never used | multiple missing formatting specifiers | - = note: the formatting string captures that binding directly, it doesn't need to be included in the argument list -help: Consider removing these - --> $DIR/issue-105225.rs:6:25 +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> $DIR/issue-105225.rs:6:16 | LL | println!("{x} {y}", x, y); - | ^ ^ + | ^ ^ - - this can be removed + | | | | + | | | this can be removed + | | this formatting argument captures `y` directly + | this formatting argument captures `x` directly + +error: multiple unused formatting arguments + --> $DIR/issue-105225.rs:8:25 + | +LL | println!("{x} {y}", y, x); + | --------- ^ ^ argument never used + | | | + | | argument never used + | multiple missing formatting specifiers + | +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> $DIR/issue-105225.rs:8:16 + | +LL | println!("{x} {y}", y, x); + | ^ ^ - - this can be removed + | | | | + | | | this can be removed + | | this formatting argument captures `y` directly + | this formatting argument captures `x` directly + +error: argument never used + --> $DIR/issue-105225.rs:10:27 + | +LL | println!("{} {y}", x, y); + | -------- ^ argument never used + | | + | formatting specifier missing + | +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> $DIR/issue-105225.rs:10:19 + | +LL | println!("{} {y}", x, y); + | ^ - this can be removed + | | + | this formatting argument captures `y` directly + +error: argument never used + --> $DIR/issue-105225.rs:12:45 + | +LL | println!("{} {} {y} {} {}", y, y, y, y, y); + | ----------------- ^ argument never used + | | + | formatting specifier missing + | +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> $DIR/issue-105225.rs:12:22 + | +LL | println!("{} {} {y} {} {}", y, y, y, y, y); + | ^ - this can be removed + | | + | this formatting argument captures `y` directly -error: aborting due to 2 previous errors +error: aborting due to 5 previous errors From abe777b9d6ac5ae5571f59ac30bb84002767f819 Mon Sep 17 00:00:00 2001 From: Tanveer Raza Date: Mon, 13 Mar 2023 18:56:20 +0530 Subject: [PATCH 6/7] Required changes and logic skeleton --- compiler/rustc_builtin_macros/src/format.rs | 55 ++++++++++++++------- tests/ui/macros/issue-105225.rs | 2 + tests/ui/macros/issue-105225.stderr | 21 +++++++- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 102532392661c..accbca7e76535 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -480,8 +480,20 @@ fn make_format_args( report_invalid_references(ecx, &invalid_refs, &template, fmt_span, &args, parser); } + // Vec that contains all redundant arg, will be printed for suggestion let mut redundant_expl_args = Vec::new(); + // `placeholder` contains pieces that may be linked to one of redundant explicit argument + let _placeholders: Vec<&parse::Piece<'_>> = pieces + .iter() + .filter(|p| { + if let NextArgument(arg) = *p && let ArgumentNamed(_) = arg.position { + return true; + } + false + }) + .collect(); + let unused = used .iter() .enumerate() @@ -490,24 +502,21 @@ fn make_format_args( let msg = if let FormatArgumentKind::Named(_) = args.explicit_args()[i].kind { "named argument never used" } else { - if let Some(expr) = args.explicit_args()[i].expr.to_ty() { - if let Some(symbol) = expr.kind.is_simple_path() { - for piece in &pieces { - if let NextArgument(arg) = piece { - if let ArgumentNamed(specifier) = arg.position { - if specifier == symbol.to_string() { - redundant_expl_args.push({ - RedundantArgDiagBuilder { - arg_span: expr.span, - fmt_span: InnerSpan { - start: arg.position_span.start, - end: arg.position_span.end, - }, - fmt_str: specifier, - } - }); - } - } + if let Some(expr) = args.explicit_args()[i].expr.to_ty() + && let Some(symbol) = expr.kind.is_simple_path() { + for piece in &pieces { + if let NextArgument(arg) = piece + && let ArgumentNamed(specifier) = arg.position { + if specifier == symbol.to_string() { + redundant_expl_args.push( + RedundantArgDiagBuilder { + arg_span: expr.span, + fmt_span: InnerSpan { + start: arg.position_span.start, + end: arg.position_span.end, + }, + fmt_str: specifier, + }); } } } @@ -518,6 +527,14 @@ fn make_format_args( }) .collect::>(); + // Get symbol names + // let mut unused_symbol = Vec::with_capacity(unused.len()); + // unused.iter().for_each(|(unused_span, _)| { + // // Logic to extract symbol(string) from `Span` type + // }); + + // Some logic to check if the symbol has redundant argument associated with it. + if !unused.is_empty() { // If there's a lot of unused arguments, // let's check if this format arguments looks like another syntax (printf / shell). @@ -718,6 +735,8 @@ fn report_missing_placeholders( if !found_foreign && unused.len() == 1 { diag.span_label(fmt_span, "formatting specifier missing"); } + + // Diag message buileder if !redundant_expl_args.is_empty() { let mut fmt_spans = Vec::new(); let mut arg_spans = Vec::new(); diff --git a/tests/ui/macros/issue-105225.rs b/tests/ui/macros/issue-105225.rs index ffb8d52e2fdc8..c5493a543709d 100644 --- a/tests/ui/macros/issue-105225.rs +++ b/tests/ui/macros/issue-105225.rs @@ -11,4 +11,6 @@ fn main() { //~^ ERROR argument never used println!("{} {} {y} {} {}", y, y, y, y, y); //~^ ERROR argument never used + println!("{x} {x} {y} {x} {x}", x, y, x, y, y); + //~^ ERROR argument never used } diff --git a/tests/ui/macros/issue-105225.stderr b/tests/ui/macros/issue-105225.stderr index 2195a4545c02d..73c7cb92563db 100644 --- a/tests/ui/macros/issue-105225.stderr +++ b/tests/ui/macros/issue-105225.stderr @@ -84,5 +84,24 @@ LL | println!("{} {} {y} {} {}", y, y, y, y, y); | | | this formatting argument captures `y` directly -error: aborting due to 5 previous errors +error: multiple unused formatting arguments + --> new.rs:14:37 + | +14 | println!("{x} {x} {y} {x} {x}", x, y, x, y, y); + | --------------------- ^ ^ ^ ^ ^ argument never used + | | | | | | + | | | | | argument never used + | | | | argument never used + | | | argument never used + | | argument never used + | multiple missing formatting specifiers + | +help: the formatting string captures that binding directly, it doesn't need to be included in the argument list + --> new.rs:14:16 + | +LL | println!("{x} {x} {y} {x} {x}", x, y, x, y, y); + | ^ - this can be removed + | | + | this formatting argument captures `y` directly +error: aborting due to 6 previous errors From cd4f48c308b13393eceeb43f8350939c20599b69 Mon Sep 17 00:00:00 2001 From: Tanveer Raza Date: Mon, 10 Apr 2023 23:23:09 +0530 Subject: [PATCH 7/7] Remove unwanted comments --- compiler/rustc_builtin_macros/src/format.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index accbca7e76535..849fe6324cf3c 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -527,14 +527,6 @@ fn make_format_args( }) .collect::>(); - // Get symbol names - // let mut unused_symbol = Vec::with_capacity(unused.len()); - // unused.iter().for_each(|(unused_span, _)| { - // // Logic to extract symbol(string) from `Span` type - // }); - - // Some logic to check if the symbol has redundant argument associated with it. - if !unused.is_empty() { // If there's a lot of unused arguments, // let's check if this format arguments looks like another syntax (printf / shell).