From b413bf6c4ee671df153b930dc2ba5b159c652c1d Mon Sep 17 00:00:00 2001 From: koka Date: Thu, 28 Sep 2023 17:12:00 +0900 Subject: [PATCH] Fix index of the remaining positional arguments --- clippy_lints/src/write.rs | 94 +++++++++++++++++++++++---------- tests/ui/print_literal.fixed | 4 -- tests/ui/print_literal.rs | 4 -- tests/ui/print_literal.stderr | 72 +++++-------------------- tests/ui/write_literal.fixed | 14 +++-- tests/ui/write_literal.rs | 14 +++-- tests/ui/write_literal.stderr | 74 +++++++++++++------------- tests/ui/write_literal_2.rs | 6 +-- tests/ui/write_literal_2.stderr | 53 ++++--------------- 9 files changed, 147 insertions(+), 188 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index da083fb14aae..21b675ff6798 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -3,12 +3,15 @@ use clippy_utils::macros::{find_format_args, format_arg_removal_span, root_macro use clippy_utils::source::{expand_past_previous_comma, snippet_opt}; use clippy_utils::{is_in_cfg_test, is_in_test_function}; use rustc_ast::token::LitKind; -use rustc_ast::{FormatArgPosition, FormatArgs, FormatArgsPiece, FormatOptions, FormatPlaceholder, FormatTrait}; +use rustc_ast::{ + FormatArgPosition, FormatArgPositionKind, FormatArgs, FormatArgsPiece, FormatOptions, FormatPlaceholder, + FormatTrait, +}; use rustc_errors::Applicability; use rustc_hir::{Expr, Impl, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{sym, BytePos}; +use rustc_span::{sym, BytePos, Span}; declare_clippy_lint! { /// ### What it does @@ -450,6 +453,12 @@ fn check_empty_string(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) { let arg_index = |argument: &FormatArgPosition| argument.index.unwrap_or_else(|pos| pos); + let lint_name = if name.starts_with("write") { + WRITE_LITERAL + } else { + PRINT_LITERAL + }; + let mut counts = vec![0u32; format_args.arguments.all_args().len()]; for piece in &format_args.template { if let FormatArgsPiece::Placeholder(placeholder) = piece { @@ -457,6 +466,12 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) { } } + let mut suggestion: Vec<(Span, String)> = vec![]; + // holds index of replaced positional arguments; used to decrement the index of the remaining + // positional arguments. + let mut replaced_position: Vec = vec![]; + let mut sug_span: Option = None; + for piece in &format_args.template { if let FormatArgsPiece::Placeholder(FormatPlaceholder { argument, @@ -493,12 +508,6 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) { _ => continue, }; - let lint = if name.starts_with("write") { - WRITE_LITERAL - } else { - PRINT_LITERAL - }; - let Some(format_string_snippet) = snippet_opt(cx, format_args.span) else { continue }; let format_string_is_raw = format_string_snippet.starts_with('r'); @@ -519,29 +528,58 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) { }, }; - span_lint_and_then( - cx, - lint, - arg.expr.span, - "literal with an empty format string", - |diag| { - if let Some(replacement) = replacement - // `format!("{}", "a")`, `format!("{named}", named = "b") - // ~~~~~ ~~~~~~~~~~~~~ - && let Some(removal_span) = format_arg_removal_span(format_args, index) - { - let replacement = replacement.replace('{', "{{").replace('}', "}}"); - diag.multipart_suggestion( - "try", - vec![(*placeholder_span, replacement), (removal_span, String::new())], - Applicability::MachineApplicable, - ); - } - }, - ); + sug_span = Some(sug_span.unwrap_or(arg.expr.span).to(arg.expr.span)); + if let Some((_, index)) = positional_arg_piece_span(piece) { + replaced_position.push(index); + } + + if let Some(replacement) = replacement + // `format!("{}", "a")`, `format!("{named}", named = "b") + // ~~~~~ ~~~~~~~~~~~~~ + && let Some(removal_span) = format_arg_removal_span(format_args, index) { + let replacement = replacement.replace('{', "{{").replace('}', "}}"); + suggestion.push((*placeholder_span, replacement)); + suggestion.push((removal_span, String::new())); + } } } + + // Decrement the index of the remaining by the number of replaced positional arguments + if !suggestion.is_empty() { + for piece in &format_args.template { + if let Some((span, index)) = positional_arg_piece_span(piece) + && suggestion.iter().all(|(s, _)| *s != span) { + let decrement = replaced_position.iter().filter(|i| **i < index).count(); + suggestion.push((span, format!("{{{}}}", index.saturating_sub(decrement)))); + } + } + } + + if let Some(span) = sug_span { + span_lint_and_then(cx, lint_name, span, "literal with an empty format string", |diag| { + if !suggestion.is_empty() { + diag.multipart_suggestion("try", suggestion, Applicability::MachineApplicable); + } + }); + } +} + +/// Extract Span and its index from the given `piece`, iff it's positional argument. +fn positional_arg_piece_span(piece: &FormatArgsPiece) -> Option<(Span, usize)> { + match piece { + FormatArgsPiece::Placeholder(FormatPlaceholder { + argument: + FormatArgPosition { + index: Ok(index), + kind: FormatArgPositionKind::Number, + .. + }, + span: Some(span), + .. + }) => Some((*span, *index)), + _ => None, + } } /// Removes the raw marker, `#`s and quotes from a str, and returns if the literal is raw diff --git a/tests/ui/print_literal.fixed b/tests/ui/print_literal.fixed index 88cd3a54b410..04a484258a4a 100644 --- a/tests/ui/print_literal.fixed +++ b/tests/ui/print_literal.fixed @@ -39,18 +39,14 @@ fn main() { // throw a warning println!("hello world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string println!("world hello"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // named args shouldn't change anything either println!("hello world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string println!("world hello"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // The string literal from `file!()` has a callsite span that isn't marked as coming from an // expansion diff --git a/tests/ui/print_literal.rs b/tests/ui/print_literal.rs index bd7444c9606a..38c036f56c5d 100644 --- a/tests/ui/print_literal.rs +++ b/tests/ui/print_literal.rs @@ -39,18 +39,14 @@ fn main() { // throw a warning println!("{0} {1}", "hello", "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string println!("{1} {0}", "hello", "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // named args shouldn't change anything either println!("{foo} {bar}", foo = "hello", bar = "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string println!("{bar} {foo}", foo = "hello", bar = "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // The string literal from `file!()` has a callsite span that isn't marked as coming from an // expansion diff --git a/tests/ui/print_literal.stderr b/tests/ui/print_literal.stderr index 1d9751b92e9c..b2f85be9d94a 100644 --- a/tests/ui/print_literal.stderr +++ b/tests/ui/print_literal.stderr @@ -52,97 +52,49 @@ error: literal with an empty format string --> $DIR/print_literal.rs:40:25 | LL | println!("{0} {1}", "hello", "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^ | help: try | LL - println!("{0} {1}", "hello", "world"); -LL + println!("hello {1}", "world"); +LL + println!("hello world"); | error: literal with an empty format string - --> $DIR/print_literal.rs:40:34 - | -LL | println!("{0} {1}", "hello", "world"); - | ^^^^^^^ - | -help: try - | -LL - println!("{0} {1}", "hello", "world"); -LL + println!("{0} world", "hello"); - | - -error: literal with an empty format string - --> $DIR/print_literal.rs:43:34 + --> $DIR/print_literal.rs:42:25 | LL | println!("{1} {0}", "hello", "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^ | help: try | LL - println!("{1} {0}", "hello", "world"); -LL + println!("world {0}", "hello"); - | - -error: literal with an empty format string - --> $DIR/print_literal.rs:43:25 - | -LL | println!("{1} {0}", "hello", "world"); - | ^^^^^^^ - | -help: try - | -LL - println!("{1} {0}", "hello", "world"); -LL + println!("{1} hello", "world"); - | - -error: literal with an empty format string - --> $DIR/print_literal.rs:48:35 - | -LL | println!("{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ - | -help: try - | -LL - println!("{foo} {bar}", foo = "hello", bar = "world"); -LL + println!("hello {bar}", bar = "world"); +LL + println!("world hello"); | error: literal with an empty format string - --> $DIR/print_literal.rs:48:50 + --> $DIR/print_literal.rs:46:35 | LL | println!("{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^ | help: try | LL - println!("{foo} {bar}", foo = "hello", bar = "world"); -LL + println!("{foo} world", foo = "hello"); +LL + println!("hello world"); | error: literal with an empty format string - --> $DIR/print_literal.rs:51:50 - | -LL | println!("{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ - | -help: try - | -LL - println!("{bar} {foo}", foo = "hello", bar = "world"); -LL + println!("world {foo}", foo = "hello"); - | - -error: literal with an empty format string - --> $DIR/print_literal.rs:51:35 + --> $DIR/print_literal.rs:48:35 | LL | println!("{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^ | help: try | LL - println!("{bar} {foo}", foo = "hello", bar = "world"); -LL + println!("{bar} hello", bar = "world"); +LL + println!("world hello"); | -error: aborting due to 12 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/write_literal.fixed b/tests/ui/write_literal.fixed index ee577574d289..3d216b76cbf3 100644 --- a/tests/ui/write_literal.fixed +++ b/tests/ui/write_literal.fixed @@ -43,16 +43,22 @@ fn main() { // throw a warning writeln!(v, "hello world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string writeln!(v, "world hello"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // named args shouldn't change anything either writeln!(v, "hello world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string writeln!(v, "world hello"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string + + // #10128 + writeln!(v, "hello {0} world", 2); + //~^ ERROR: literal with an empty format string + writeln!(v, "world {0} hello", 2); + //~^ ERROR: literal with an empty format string + writeln!(v, "hello {0} {1}, {bar}", 2, 3, bar = 4); + //~^ ERROR: literal with an empty format string + writeln!(v, "hello {0} {1}, world {2}", 2, 3, 4); + //~^ ERROR: literal with an empty format string } diff --git a/tests/ui/write_literal.rs b/tests/ui/write_literal.rs index 588e8fd413a4..79d6daa2e3b5 100644 --- a/tests/ui/write_literal.rs +++ b/tests/ui/write_literal.rs @@ -43,16 +43,22 @@ fn main() { // throw a warning writeln!(v, "{0} {1}", "hello", "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string writeln!(v, "{1} {0}", "hello", "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // named args shouldn't change anything either writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string + + // #10128 + writeln!(v, "{0} {1} {2}", "hello", 2, "world"); + //~^ ERROR: literal with an empty format string + writeln!(v, "{2} {1} {0}", "hello", 2, "world"); + //~^ ERROR: literal with an empty format string + writeln!(v, "{0} {1} {2}, {bar}", "hello", 2, 3, bar = 4); + //~^ ERROR: literal with an empty format string + writeln!(v, "{0} {1} {2}, {3} {4}", "hello", 2, 3, "world", 4); + //~^ ERROR: literal with an empty format string } diff --git a/tests/ui/write_literal.stderr b/tests/ui/write_literal.stderr index 372a54cf769f..ee0d536e954f 100644 --- a/tests/ui/write_literal.stderr +++ b/tests/ui/write_literal.stderr @@ -52,96 +52,96 @@ error: literal with an empty format string --> $DIR/write_literal.rs:44:28 | LL | writeln!(v, "{0} {1}", "hello", "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^ | help: try | LL - writeln!(v, "{0} {1}", "hello", "world"); -LL + writeln!(v, "hello {1}", "world"); +LL + writeln!(v, "hello world"); | error: literal with an empty format string - --> $DIR/write_literal.rs:44:37 + --> $DIR/write_literal.rs:46:28 | -LL | writeln!(v, "{0} {1}", "hello", "world"); - | ^^^^^^^ +LL | writeln!(v, "{1} {0}", "hello", "world"); + | ^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{0} {1}", "hello", "world"); -LL + writeln!(v, "{0} world", "hello"); +LL - writeln!(v, "{1} {0}", "hello", "world"); +LL + writeln!(v, "world hello"); | error: literal with an empty format string - --> $DIR/write_literal.rs:47:37 + --> $DIR/write_literal.rs:50:38 | -LL | writeln!(v, "{1} {0}", "hello", "world"); - | ^^^^^^^ +LL | writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); + | ^^^^^^^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{1} {0}", "hello", "world"); -LL + writeln!(v, "world {0}", "hello"); +LL - writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); +LL + writeln!(v, "hello world"); | error: literal with an empty format string - --> $DIR/write_literal.rs:47:28 + --> $DIR/write_literal.rs:52:38 | -LL | writeln!(v, "{1} {0}", "hello", "world"); - | ^^^^^^^ +LL | writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); + | ^^^^^^^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{1} {0}", "hello", "world"); -LL + writeln!(v, "{1} hello", "world"); +LL - writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); +LL + writeln!(v, "world hello"); | error: literal with an empty format string - --> $DIR/write_literal.rs:52:38 + --> $DIR/write_literal.rs:56:32 | -LL | writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ +LL | writeln!(v, "{0} {1} {2}", "hello", 2, "world"); + | ^^^^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); -LL + writeln!(v, "hello {bar}", bar = "world"); +LL - writeln!(v, "{0} {1} {2}", "hello", 2, "world"); +LL + writeln!(v, "hello {0} world", 2); | error: literal with an empty format string - --> $DIR/write_literal.rs:52:53 + --> $DIR/write_literal.rs:58:32 | -LL | writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ +LL | writeln!(v, "{2} {1} {0}", "hello", 2, "world"); + | ^^^^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{foo} {bar}", foo = "hello", bar = "world"); -LL + writeln!(v, "{foo} world", foo = "hello"); +LL - writeln!(v, "{2} {1} {0}", "hello", 2, "world"); +LL + writeln!(v, "world {0} hello", 2); | error: literal with an empty format string - --> $DIR/write_literal.rs:55:53 + --> $DIR/write_literal.rs:60:39 | -LL | writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ +LL | writeln!(v, "{0} {1} {2}, {bar}", "hello", 2, 3, bar = 4); + | ^^^^^^^ | help: try | -LL - writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); -LL + writeln!(v, "world {foo}", foo = "hello"); +LL - writeln!(v, "{0} {1} {2}, {bar}", "hello", 2, 3, bar = 4); +LL + writeln!(v, "hello {0} {1}, {bar}", 2, 3, bar = 4); | error: literal with an empty format string - --> $DIR/write_literal.rs:55:38 + --> $DIR/write_literal.rs:62:41 | -LL | writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ +LL | writeln!(v, "{0} {1} {2}, {3} {4}", "hello", 2, 3, "world", 4); + | ^^^^^^^^^^^^^^^^^^^^^^ | help: try | -LL - writeln!(v, "{bar} {foo}", foo = "hello", bar = "world"); -LL + writeln!(v, "{bar} hello", bar = "world"); +LL - writeln!(v, "{0} {1} {2}, {3} {4}", "hello", 2, 3, "world", 4); +LL + writeln!(v, "hello {0} {1}, world {2}", 2, 3, 4); | error: aborting due to 12 previous errors diff --git a/tests/ui/write_literal_2.rs b/tests/ui/write_literal_2.rs index 197e01149e0a..b2ed552d46bc 100644 --- a/tests/ui/write_literal_2.rs +++ b/tests/ui/write_literal_2.rs @@ -31,10 +31,7 @@ fn main() { v, "some {}\ {} \\ {}", - "1", - "2", - "3", - //~^ ERROR: literal with an empty format string + "1", "2", "3", ); writeln!(v, "{}", "\\"); //~^ ERROR: literal with an empty format string @@ -49,7 +46,6 @@ fn main() { // hard mode writeln!(v, r#"{}{}"#, '#', '"'); //~^ ERROR: literal with an empty format string - //~| ERROR: literal with an empty format string // should not lint writeln!(v, r"{}", "\r"); } diff --git a/tests/ui/write_literal_2.stderr b/tests/ui/write_literal_2.stderr index 9a16d2f240dd..81ef49de082e 100644 --- a/tests/ui/write_literal_2.stderr +++ b/tests/ui/write_literal_2.stderr @@ -82,42 +82,17 @@ LL ~ world!", error: literal with an empty format string --> $DIR/write_literal_2.rs:34:9 | -LL | "1", - | ^^^ +LL | "1", "2", "3", + | ^^^^^^^^^^^^^ | help: try | LL ~ "some 1\ -LL ~ {} \\ {}", +LL ~ 2 \\ 3", | error: literal with an empty format string - --> $DIR/write_literal_2.rs:35:9 - | -LL | "2", - | ^^^ - | -help: try - | -LL ~ 2 \\ {}", -LL ~ "1", - | - -error: literal with an empty format string - --> $DIR/write_literal_2.rs:36:9 - | -LL | "3", - | ^^^ - | -help: try - | -LL ~ {} \\ 3", -LL | "1", -LL ~ "2", - | - -error: literal with an empty format string - --> $DIR/write_literal_2.rs:39:23 + --> $DIR/write_literal_2.rs:36:23 | LL | writeln!(v, "{}", "\\"); | ^^^^ @@ -129,7 +104,7 @@ LL + writeln!(v, "\\"); | error: literal with an empty format string - --> $DIR/write_literal_2.rs:41:24 + --> $DIR/write_literal_2.rs:38:24 | LL | writeln!(v, r"{}", "\\"); | ^^^^ @@ -141,7 +116,7 @@ LL + writeln!(v, r"\"); | error: literal with an empty format string - --> $DIR/write_literal_2.rs:43:26 + --> $DIR/write_literal_2.rs:40:26 | LL | writeln!(v, r#"{}"#, "\\"); | ^^^^ @@ -153,7 +128,7 @@ LL + writeln!(v, r#"\"#); | error: literal with an empty format string - --> $DIR/write_literal_2.rs:45:23 + --> $DIR/write_literal_2.rs:42:23 | LL | writeln!(v, "{}", r"\"); | ^^^^ @@ -165,7 +140,7 @@ LL + writeln!(v, "\\"); | error: literal with an empty format string - --> $DIR/write_literal_2.rs:47:23 + --> $DIR/write_literal_2.rs:44:23 | LL | writeln!(v, "{}", "\r"); | ^^^^ @@ -177,16 +152,10 @@ LL + writeln!(v, "\r"); | error: literal with an empty format string - --> $DIR/write_literal_2.rs:50:28 - | -LL | writeln!(v, r#"{}{}"#, '#', '"'); - | ^^^ - -error: literal with an empty format string - --> $DIR/write_literal_2.rs:50:33 + --> $DIR/write_literal_2.rs:47:28 | LL | writeln!(v, r#"{}{}"#, '#', '"'); - | ^^^ + | ^^^^^^^^ -error: aborting due to 17 previous errors +error: aborting due to 14 previous errors