Skip to content

Commit 0314d67

Browse files
GuillaumeGomezGuillaumeGomez
committed
Limit literal_string_with_formatting_args to known variables
1 parent 544f30e commit 0314d67

File tree

6 files changed

+107
-51
lines changed

6 files changed

+107
-51
lines changed

‎clippy_lints/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
962962
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
963963
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
964964
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
965-
store.register_early_pass(|| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg));
965+
store.register_late_pass(|_| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg));
966966
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
967967
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
968968
store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));

‎clippy_lints/src/literal_string_with_formatting_args.rs‎

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
use rustc_ast::ast::{Expr, ExprKind};
2-
use rustc_ast::token::LitKind;
3-
use rustc_lint::{EarlyContext, EarlyLintPass};
1+
use rustc_ast::{LitKind, StrStyle};
2+
use rustc_hir::{Expr, ExprKind};
3+
use rustc_lexer::is_ident;
4+
use rustc_lint::{LateContext, LateLintPass};
45
use rustc_parse_format::{ParseMode, Parser, Piece};
56
use rustc_session::declare_lint_pass;
67
use rustc_span::BytePos;
78

89
use clippy_utils::diagnostics::span_lint;
10+
use clippy_utils::mir::enclosing_mir;
911

1012
declare_clippy_lint! {
1113
/// ### What it does
@@ -35,18 +37,23 @@ declare_clippy_lint! {
3537

3638
declare_lint_pass!(LiteralStringWithFormattingArg => [LITERAL_STRING_WITH_FORMATTING_ARGS]);
3739

38-
impl EarlyLintPass for LiteralStringWithFormattingArg {
39-
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
40+
impl LateLintPass<'_> for LiteralStringWithFormattingArg {
41+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
4042
if expr.span.from_expansion() {
4143
return;
4244
}
4345
if let ExprKind::Lit(lit) = expr.kind {
44-
let add = match lit.kind {
45-
LitKind::Str => 1,
46-
LitKind::StrRaw(nb) => nb as usize + 2,
46+
let (add, symbol) = match lit.node {
47+
LitKind::Str(symbol, style) => {
48+
let add = match style {
49+
StrStyle::Cooked => 1,
50+
StrStyle::Raw(nb) => nb as usize + 2,
51+
};
52+
(add, symbol)
53+
},
4754
_ => return,
4855
};
49-
let fmt_str = lit.symbol.as_str();
56+
let fmt_str = symbol.as_str();
5057
let lo = expr.span.lo();
5158
let mut current = fmt_str;
5259
let mut diff_len = 0;
@@ -82,31 +89,66 @@ impl EarlyLintPass for LiteralStringWithFormattingArg {
8289
.find('}')
8390
.map_or(pos.end, |found| start + 1 + found)
8491
+ 1;
85-
spans.push(
92+
let ident_start = start + 1;
93+
let colon_pos = fmt_str[ident_start..end].find(':');
94+
let ident_end = colon_pos.unwrap_or(end - 1);
95+
let mut name = None;
96+
if ident_start < ident_end
97+
&& let arg = &fmt_str[ident_start..ident_end]
98+
&& !arg.is_empty()
99+
{
100+
if is_ident(arg) {
101+
name = Some(arg.to_string());
102+
} else if colon_pos.is_none() && !arg.chars().all(|c| c.is_ascii_digit()) {
103+
// Not a `{0}` or a `{0:?}`.
104+
continue;
105+
}
106+
} else if colon_pos.is_none() {
107+
// Not a `{:?}`.
108+
continue;
109+
}
110+
spans.push((
86111
expr.span
87112
.with_hi(lo + BytePos((start + add).try_into().unwrap()))
88113
.with_lo(lo + BytePos((end + add).try_into().unwrap())),
89-
);
114+
name,
115+
));
90116
}
91117
}
92-
match spans.len() {
93-
0 => {},
94-
1 => {
95-
span_lint(
96-
cx,
97-
LITERAL_STRING_WITH_FORMATTING_ARGS,
98-
spans,
99-
"this looks like a formatting argument but it is not part of a formatting macro",
100-
);
101-
},
102-
_ => {
103-
span_lint(
104-
cx,
105-
LITERAL_STRING_WITH_FORMATTING_ARGS,
106-
spans,
107-
"these look like formatting arguments but are not part of a formatting macro",
108-
);
109-
},
118+
if !spans.is_empty()
119+
&& let Some(mir) = enclosing_mir(cx.tcx, expr.hir_id)
120+
{
121+
let spans = spans
122+
.iter()
123+
.filter_map(|(span, name)| {
124+
if let Some(name) = name {
125+
// We need to check that the name is a local.
126+
if !mir.var_debug_info.iter().any(|local| local.name.as_str() == name) {
127+
return None;
128+
}
129+
}
130+
Some(*span)
131+
})
132+
.collect::<Vec<_>>();
133+
match spans.len() {
134+
0 => {},
135+
1 => {
136+
span_lint(
137+
cx,
138+
LITERAL_STRING_WITH_FORMATTING_ARGS,
139+
spans,
140+
"this looks like a formatting argument but it is not part of a formatting macro",
141+
);
142+
},
143+
_ => {
144+
span_lint(
145+
cx,
146+
LITERAL_STRING_WITH_FORMATTING_ARGS,
147+
spans,
148+
"these look like formatting arguments but are not part of a formatting macro",
149+
);
150+
},
151+
}
110152
}
111153
}
112154
}

‎lintcheck/src/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
clippy::collapsible_else_if,
1919
clippy::needless_borrows_for_generic_args,
2020
clippy::module_name_repetitions,
21-
clippy::literal_string_with_formatting_arg
21+
clippy::literal_string_with_formatting_args
2222
)]
2323

2424
mod config;

‎tests/ui/literal_string_with_formatting_arg.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ fn main() {
66
let y = "hello";
77
x.expect("{y} {}"); //~ literal_string_with_formatting_args
88
x.expect(" {y} bla"); //~ literal_string_with_formatting_args
9+
x.expect(" {0}"); //~ literal_string_with_formatting_args
910
x.expect("{:?}"); //~ literal_string_with_formatting_args
1011
x.expect("{y:?}"); //~ literal_string_with_formatting_args
1112
x.expect(" {y:?} {y:?} "); //~ literal_string_with_formatting_args
@@ -23,6 +24,7 @@ fn main() {
2324
x.expect(" { } "); // For now we ignore `{}` to limit false positives.
2425
x.expect("{{y} {x");
2526
x.expect("{{y:?}");
27+
x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint
2628
x.expect("{y:...}");
2729
let _ = "fn main {\n\
2830
}";

‎tests/ui/literal_string_with_formatting_arg.stderr‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,77 @@
11
error: this looks like a formatting argument but it is not part of a formatting macro
2-
--> tests/ui/literal_string_with_formatting_args.rs:7:15
2+
--> tests/ui/literal_string_with_formatting_arg.rs:7:15
33
|
44
LL | x.expect("{y} {}");
55
| ^^^
66
|
7-
= note: `-D clippy::literal-string-with-formatting-arg` implied by `-D warnings`
7+
= note: `-D clippy::literal-string-with-formatting-args` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::literal_string_with_formatting_args)]`
99

1010
error: this looks like a formatting argument but it is not part of a formatting macro
11-
--> tests/ui/literal_string_with_formatting_args.rs:8:16
11+
--> tests/ui/literal_string_with_formatting_arg.rs:8:16
1212
|
1313
LL | x.expect(" {y} bla");
1414
| ^^^
1515

1616
error: this looks like a formatting argument but it is not part of a formatting macro
17-
--> tests/ui/literal_string_with_formatting_args.rs:9:15
17+
--> tests/ui/literal_string_with_formatting_arg.rs:9:16
18+
|
19+
LL | x.expect(" {0}");
20+
| ^^^
21+
22+
error: this looks like a formatting argument but it is not part of a formatting macro
23+
--> tests/ui/literal_string_with_formatting_arg.rs:10:15
1824
|
1925
LL | x.expect("{:?}");
2026
| ^^^^
2127

2228
error: this looks like a formatting argument but it is not part of a formatting macro
23-
--> tests/ui/literal_string_with_formatting_args.rs:10:15
29+
--> tests/ui/literal_string_with_formatting_arg.rs:11:15
2430
|
2531
LL | x.expect("{y:?}");
2632
| ^^^^^
2733

2834
error: these look like formatting arguments but are not part of a formatting macro
29-
--> tests/ui/literal_string_with_formatting_args.rs:11:16
35+
--> tests/ui/literal_string_with_formatting_arg.rs:12:16
3036
|
3137
LL | x.expect(" {y:?} {y:?} ");
3238
| ^^^^^ ^^^^^
3339

3440
error: this looks like a formatting argument but it is not part of a formatting macro
35-
--> tests/ui/literal_string_with_formatting_args.rs:12:23
41+
--> tests/ui/literal_string_with_formatting_arg.rs:13:23
3642
|
3743
LL | x.expect(" {y:..} {y:?} ");
3844
| ^^^^^
3945

4046
error: these look like formatting arguments but are not part of a formatting macro
41-
--> tests/ui/literal_string_with_formatting_args.rs:13:16
47+
--> tests/ui/literal_string_with_formatting_arg.rs:14:16
4248
|
4349
LL | x.expect(r"{y:?} {y:?} ");
4450
| ^^^^^ ^^^^^
4551

4652
error: this looks like a formatting argument but it is not part of a formatting macro
47-
--> tests/ui/literal_string_with_formatting_args.rs:14:16
53+
--> tests/ui/literal_string_with_formatting_arg.rs:15:16
4854
|
4955
LL | x.expect(r"{y:?} y:?}");
5056
| ^^^^^
5157

5258
error: these look like formatting arguments but are not part of a formatting macro
53-
--> tests/ui/literal_string_with_formatting_args.rs:15:19
59+
--> tests/ui/literal_string_with_formatting_arg.rs:16:19
5460
|
5561
LL | x.expect(r##" {y:?} {y:?} "##);
5662
| ^^^^^ ^^^^^
5763

5864
error: this looks like a formatting argument but it is not part of a formatting macro
59-
--> tests/ui/literal_string_with_formatting_args.rs:17:18
65+
--> tests/ui/literal_string_with_formatting_arg.rs:18:18
6066
|
6167
LL | x.expect("———{:?}");
6268
| ^^^^
6369

64-
error: aborting due to 10 previous errors
70+
error: this looks like a formatting argument but it is not part of a formatting macro
71+
--> tests/ui/literal_string_with_formatting_arg.rs:27:19
72+
|
73+
LL | x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint
74+
| ^^^^^
75+
76+
error: aborting due to 12 previous errors
6577

‎tests/ui/regex.stderr‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,51 +196,51 @@ LL | let binary_trivial_empty = BRegex::new("^$");
196196
= help: consider using `str::is_empty`
197197

198198
error: compiling a regex in a loop
199-
--> tests/ui/regex.rs:125:21
199+
--> tests/ui/regex.rs:126:21
200200
|
201201
LL | let regex = Regex::new("a.b");
202202
| ^^^^^^^^^^
203203
|
204204
help: move the regex construction outside this loop
205-
--> tests/ui/regex.rs:122:5
205+
--> tests/ui/regex.rs:123:5
206206
|
207207
LL | loop {
208208
| ^^^^
209209
= note: `-D clippy::regex-creation-in-loops` implied by `-D warnings`
210210
= help: to override `-D warnings` add `#[allow(clippy::regex_creation_in_loops)]`
211211

212212
error: compiling a regex in a loop
213-
--> tests/ui/regex.rs:127:21
213+
--> tests/ui/regex.rs:128:21
214214
|
215215
LL | let regex = BRegex::new("a.b");
216216
| ^^^^^^^^^^^
217217
|
218218
help: move the regex construction outside this loop
219-
--> tests/ui/regex.rs:122:5
219+
--> tests/ui/regex.rs:123:5
220220
|
221221
LL | loop {
222222
| ^^^^
223223

224224
error: compiling a regex in a loop
225-
--> tests/ui/regex.rs:133:25
225+
--> tests/ui/regex.rs:134:25
226226
|
227227
LL | let regex = Regex::new("a.b");
228228
| ^^^^^^^^^^
229229
|
230230
help: move the regex construction outside this loop
231-
--> tests/ui/regex.rs:122:5
231+
--> tests/ui/regex.rs:123:5
232232
|
233233
LL | loop {
234234
| ^^^^
235235

236236
error: compiling a regex in a loop
237-
--> tests/ui/regex.rs:138:32
237+
--> tests/ui/regex.rs:139:32
238238
|
239239
LL | let nested_regex = Regex::new("a.b");
240240
| ^^^^^^^^^^
241241
|
242242
help: move the regex construction outside this loop
243-
--> tests/ui/regex.rs:137:9
243+
--> tests/ui/regex.rs:138:9
244244
|
245245
LL | for _ in 0..10 {
246246
| ^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)