Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit d18d01a

Browse files
borsbors
committed
Auto merge of rust-lang#11576 - koka831:fix/10128, r=llogiq
write_literal: Fix index of the remaining positional arguments - fixes rust-lang/rust-clippy#10128 - `clippy --fix` replaces multiple warnings at once e.g.) ```rust writeln!(v, "{0} {1}", "hello", "world"); // before: `writeln!(v, "hello {1}", "world");` // now: `writeln!(v, "hello world");` ``` changelog: [`print_literal`], [`write_literal`]: Now handles positional argument properly
2 parents 29ed6fa + b413bf6 commit d18d01a

File tree

9 files changed

+147
-188
lines changed

9 files changed

+147
-188
lines changed

‎clippy_lints/src/write.rs‎

Lines changed: 66 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ use clippy_utils::macros::{find_format_args, format_arg_removal_span, root_macro
33
use clippy_utils::source::{expand_past_previous_comma, snippet_opt};
44
use clippy_utils::{is_in_cfg_test, is_in_test_function};
55
use rustc_ast::token::LitKind;
6-
use rustc_ast::{FormatArgPosition, FormatArgs, FormatArgsPiece, FormatOptions, FormatPlaceholder, FormatTrait};
6+
use rustc_ast::{
7+
FormatArgPosition, FormatArgPositionKind, FormatArgs, FormatArgsPiece, FormatOptions, FormatPlaceholder,
8+
FormatTrait,
9+
};
710
use rustc_errors::Applicability;
811
use rustc_hir::{Expr, Impl, Item, ItemKind};
912
use rustc_lint::{LateContext, LateLintPass, LintContext};
1013
use rustc_session::{declare_tool_lint, impl_lint_pass};
11-
use rustc_span::{sym, BytePos};
14+
use rustc_span::{sym, BytePos, Span};
1215

1316
declare_clippy_lint! {
1417
/// ### What it does
@@ -450,13 +453,25 @@ fn check_empty_string(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call
450453
fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
451454
let arg_index = |argument: &FormatArgPosition| argument.index.unwrap_or_else(|pos| pos);
452455

456+
let lint_name = if name.starts_with("write") {
457+
WRITE_LITERAL
458+
} else {
459+
PRINT_LITERAL
460+
};
461+
453462
let mut counts = vec![0u32; format_args.arguments.all_args().len()];
454463
for piece in &format_args.template {
455464
if let FormatArgsPiece::Placeholder(placeholder) = piece {
456465
counts[arg_index(&placeholder.argument)] += 1;
457466
}
458467
}
459468

469+
let mut suggestion: Vec<(Span, String)> = vec![];
470+
// holds index of replaced positional arguments; used to decrement the index of the remaining
471+
// positional arguments.
472+
let mut replaced_position: Vec<usize> = vec![];
473+
let mut sug_span: Option<Span> = None;
474+
460475
for piece in &format_args.template {
461476
if let FormatArgsPiece::Placeholder(FormatPlaceholder {
462477
argument,
@@ -493,12 +508,6 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
493508
_ => continue,
494509
};
495510

496-
let lint = if name.starts_with("write") {
497-
WRITE_LITERAL
498-
} else {
499-
PRINT_LITERAL
500-
};
501-
502511
let Some(format_string_snippet) = snippet_opt(cx, format_args.span) else { continue };
503512
let format_string_is_raw = format_string_snippet.starts_with('r');
504513

@@ -519,29 +528,58 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
519528
},
520529
};
521530

522-
span_lint_and_then(
523-
cx,
524-
lint,
525-
arg.expr.span,
526-
"literal with an empty format string",
527-
|diag| {
528-
if let Some(replacement) = replacement
529-
// `format!("{}", "a")`, `format!("{named}", named = "b")
530-
// ~~~~~ ~~~~~~~~~~~~~
531-
&& let Some(removal_span) = format_arg_removal_span(format_args, index)
532-
{
533-
let replacement = replacement.replace('{', "{{").replace('}', "}}");
534-
diag.multipart_suggestion(
535-
"try",
536-
vec![(*placeholder_span, replacement), (removal_span, String::new())],
537-
Applicability::MachineApplicable,
538-
);
539-
}
540-
},
541-
);
531+
sug_span = Some(sug_span.unwrap_or(arg.expr.span).to(arg.expr.span));
542532

533+
if let Some((_, index)) = positional_arg_piece_span(piece) {
534+
replaced_position.push(index);
535+
}
536+
537+
if let Some(replacement) = replacement
538+
// `format!("{}", "a")`, `format!("{named}", named = "b")
539+
// ~~~~~ ~~~~~~~~~~~~~
540+
&& let Some(removal_span) = format_arg_removal_span(format_args, index) {
541+
let replacement = replacement.replace('{', "{{").replace('}', "}}");
542+
suggestion.push((*placeholder_span, replacement));
543+
suggestion.push((removal_span, String::new()));
544+
}
543545
}
544546
}
547+
548+
// Decrement the index of the remaining by the number of replaced positional arguments
549+
if !suggestion.is_empty() {
550+
for piece in &format_args.template {
551+
if let Some((span, index)) = positional_arg_piece_span(piece)
552+
&& suggestion.iter().all(|(s, _)| *s != span) {
553+
let decrement = replaced_position.iter().filter(|i| **i < index).count();
554+
suggestion.push((span, format!("{{{}}}", index.saturating_sub(decrement))));
555+
}
556+
}
557+
}
558+
559+
if let Some(span) = sug_span {
560+
span_lint_and_then(cx, lint_name, span, "literal with an empty format string", |diag| {
561+
if !suggestion.is_empty() {
562+
diag.multipart_suggestion("try", suggestion, Applicability::MachineApplicable);
563+
}
564+
});
565+
}
566+
}
567+
568+
/// Extract Span and its index from the given `piece`, iff it's positional argument.
569+
fn positional_arg_piece_span(piece: &FormatArgsPiece) -> Option<(Span, usize)> {
570+
match piece {
571+
FormatArgsPiece::Placeholder(FormatPlaceholder {
572+
argument:
573+
FormatArgPosition {
574+
index: Ok(index),
575+
kind: FormatArgPositionKind::Number,
576+
..
577+
},
578+
span: Some(span),
579+
..
580+
}) => Some((*span, *index)),
581+
_ => None,
582+
}
545583
}
546584

547585
/// Removes the raw marker, `#`s and quotes from a str, and returns if the literal is raw

‎tests/ui/print_literal.fixed‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,14 @@ fn main() {
3939
// throw a warning
4040
println!("hello world");
4141
//~^ ERROR: literal with an empty format string
42-
//~| ERROR: literal with an empty format string
4342
println!("world hello");
4443
//~^ ERROR: literal with an empty format string
45-
//~| ERROR: literal with an empty format string
4644

4745
// named args shouldn't change anything either
4846
println!("hello world");
4947
//~^ ERROR: literal with an empty format string
50-
//~| ERROR: literal with an empty format string
5148
println!("world hello");
5249
//~^ ERROR: literal with an empty format string
53-
//~| ERROR: literal with an empty format string
5450

5551
// The string literal from `file!()` has a callsite span that isn't marked as coming from an
5652
// expansion

‎tests/ui/print_literal.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,14 @@ fn main() {
3939
// throw a warning
4040
println!("{0} {1}", "hello", "world");
4141
//~^ ERROR: literal with an empty format string
42-
//~| ERROR: literal with an empty format string
4342
println!("{1} {0}", "hello", "world");
4443
//~^ ERROR: literal with an empty format string
45-
//~| ERROR: literal with an empty format string
4644

4745
// named args shouldn't change anything either
4846
println!("{foo} {bar}", foo = "hello", bar = "world");
4947
//~^ ERROR: literal with an empty format string
50-
//~| ERROR: literal with an empty format string
5148
println!("{bar} {foo}", foo = "hello", bar = "world");
5249
//~^ ERROR: literal with an empty format string
53-
//~| ERROR: literal with an empty format string
5450

5551
// The string literal from `file!()` has a callsite span that isn't marked as coming from an
5652
// expansion

‎tests/ui/print_literal.stderr‎

Lines changed: 12 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -52,97 +52,49 @@ error: literal with an empty format string
5252
--> $DIR/print_literal.rs:40:25
5353
|
5454
LL | println!("{0} {1}", "hello", "world");
55-
| ^^^^^^^
55+
| ^^^^^^^^^^^^^^^^
5656
|
5757
help: try
5858
|
5959
LL - println!("{0} {1}", "hello", "world");
60-
LL + println!("hello {1}", "world");
60+
LL + println!("hello world");
6161
|
6262

6363
error: literal with an empty format string
64-
--> $DIR/print_literal.rs:40:34
65-
|
66-
LL | println!("{0} {1}", "hello", "world");
67-
| ^^^^^^^
68-
|
69-
help: try
70-
|
71-
LL - println!("{0} {1}", "hello", "world");
72-
LL + println!("{0} world", "hello");
73-
|
74-
75-
error: literal with an empty format string
76-
--> $DIR/print_literal.rs:43:34
64+
--> $DIR/print_literal.rs:42:25
7765
|
7866
LL | println!("{1} {0}", "hello", "world");
79-
| ^^^^^^^
67+
| ^^^^^^^^^^^^^^^^
8068
|
8169
help: try
8270
|
8371
LL - println!("{1} {0}", "hello", "world");
84-
LL + println!("world {0}", "hello");
85-
|
86-
87-
error: literal with an empty format string
88-
--> $DIR/print_literal.rs:43:25
89-
|
90-
LL | println!("{1} {0}", "hello", "world");
91-
| ^^^^^^^
92-
|
93-
help: try
94-
|
95-
LL - println!("{1} {0}", "hello", "world");
96-
LL + println!("{1} hello", "world");
97-
|
98-
99-
error: literal with an empty format string
100-
--> $DIR/print_literal.rs:48:35
101-
|
102-
LL | println!("{foo} {bar}", foo = "hello", bar = "world");
103-
| ^^^^^^^
104-
|
105-
help: try
106-
|
107-
LL - println!("{foo} {bar}", foo = "hello", bar = "world");
108-
LL + println!("hello {bar}", bar = "world");
72+
LL + println!("world hello");
10973
|
11074

11175
error: literal with an empty format string
112-
--> $DIR/print_literal.rs:48:50
76+
--> $DIR/print_literal.rs:46:35
11377
|
11478
LL | println!("{foo} {bar}", foo = "hello", bar = "world");
115-
| ^^^^^^^
79+
| ^^^^^^^^^^^^^^^^^^^^^^
11680
|
11781
help: try
11882
|
11983
LL - println!("{foo} {bar}", foo = "hello", bar = "world");
120-
LL + println!("{foo} world", foo = "hello");
84+
LL + println!("hello world");
12185
|
12286

12387
error: literal with an empty format string
124-
--> $DIR/print_literal.rs:51:50
125-
|
126-
LL | println!("{bar} {foo}", foo = "hello", bar = "world");
127-
| ^^^^^^^
128-
|
129-
help: try
130-
|
131-
LL - println!("{bar} {foo}", foo = "hello", bar = "world");
132-
LL + println!("world {foo}", foo = "hello");
133-
|
134-
135-
error: literal with an empty format string
136-
--> $DIR/print_literal.rs:51:35
88+
--> $DIR/print_literal.rs:48:35
13789
|
13890
LL | println!("{bar} {foo}", foo = "hello", bar = "world");
139-
| ^^^^^^^
91+
| ^^^^^^^^^^^^^^^^^^^^^^
14092
|
14193
help: try
14294
|
14395
LL - println!("{bar} {foo}", foo = "hello", bar = "world");
144-
LL + println!("{bar} hello", bar = "world");
96+
LL + println!("world hello");
14597
|
14698

147-
error: aborting due to 12 previous errors
99+
error: aborting due to 8 previous errors
148100

‎tests/ui/write_literal.fixed‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,22 @@ fn main() {
4343
// throw a warning
4444
writeln!(v, "hello world");
4545
//~^ ERROR: literal with an empty format string
46-
//~| ERROR: literal with an empty format string
4746
writeln!(v, "world hello");
4847
//~^ ERROR: literal with an empty format string
49-
//~| ERROR: literal with an empty format string
5048

5149
// named args shouldn't change anything either
5250
writeln!(v, "hello world");
5351
//~^ ERROR: literal with an empty format string
54-
//~| ERROR: literal with an empty format string
5552
writeln!(v, "world hello");
5653
//~^ ERROR: literal with an empty format string
57-
//~| ERROR: literal with an empty format string
54+
55+
// #10128
56+
writeln!(v, "hello {0} world", 2);
57+
//~^ ERROR: literal with an empty format string
58+
writeln!(v, "world {0} hello", 2);
59+
//~^ ERROR: literal with an empty format string
60+
writeln!(v, "hello {0} {1}, {bar}", 2, 3, bar = 4);
61+
//~^ ERROR: literal with an empty format string
62+
writeln!(v, "hello {0} {1}, world {2}", 2, 3, 4);
63+
//~^ ERROR: literal with an empty format string
5864
}

‎tests/ui/write_literal.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,22 @@ fn main() {
4343
// throw a warning
4444
writeln!(v, "{0} {1}", "hello", "world");
4545
//~^ ERROR: literal with an empty format string
46-
//~| ERROR: literal with an empty format string
4746
writeln!(v, "{1} {0}", "hello", "world");
4847
//~^ ERROR: literal with an empty format string
49-
//~| ERROR: literal with an empty format string
5048

5149
// named args shouldn't change anything either
5250
writeln!(v, "{foo} {bar}", foo = "hello", bar = "world");
5351
//~^ ERROR: literal with an empty format string
54-
//~| ERROR: literal with an empty format string
5552
writeln!(v, "{bar} {foo}", foo = "hello", bar = "world");
5653
//~^ ERROR: literal with an empty format string
57-
//~| ERROR: literal with an empty format string
54+
55+
// #10128
56+
writeln!(v, "{0} {1} {2}", "hello", 2, "world");
57+
//~^ ERROR: literal with an empty format string
58+
writeln!(v, "{2} {1} {0}", "hello", 2, "world");
59+
//~^ ERROR: literal with an empty format string
60+
writeln!(v, "{0} {1} {2}, {bar}", "hello", 2, 3, bar = 4);
61+
//~^ ERROR: literal with an empty format string
62+
writeln!(v, "{0} {1} {2}, {3} {4}", "hello", 2, 3, "world", 4);
63+
//~^ ERROR: literal with an empty format string
5864
}

0 commit comments

Comments
 (0)