Skip to content

Commit c31d970

Browse files
shulaodashulaoda
committed
fix(useless_format): fire on wrapped in a block-producing macro
1 parent d72868d commit c31d970

7 files changed

Lines changed: 129 additions & 22 deletions

File tree

‎clippy_lints/src/format.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::macros::{FormatArgsStorage, find_format_arg_expr, root_macro_call_first_node};
2+
use clippy_utils::macros::{FormatArgsStorage, find_format_arg_expr, first_node_in_macro, matching_root_macro_call};
33
use clippy_utils::source::{SpanRangeExt, snippet_with_context};
44
use clippy_utils::sugg::Sugg;
55
use rustc_ast::{FormatArgsPiece, FormatOptions, FormatTrait};
@@ -53,8 +53,13 @@ impl UselessFormat {
5353

5454
impl<'tcx> LateLintPass<'tcx> for UselessFormat {
5555
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
56-
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
57-
&& cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id)
56+
// Loosened from `root_macro_call_first_node` so the lint also fires when `format!` is
57+
// the tail of a block emitted by another macro. The `!= macro_call.expn` check filters
58+
// HIR nodes inside `format!`'s own expansion (its outer block's tail, its nested
59+
// `format_args!`), which would otherwise also pass `first_node_in_macro` and cause the
60+
// lint to fire multiple times per call.
61+
if let Some(macro_call) = matching_root_macro_call(cx, expr.span, sym::format_macro)
62+
&& first_node_in_macro(cx, expr).is_some_and(|p_expn| p_expn != macro_call.expn)
5863
&& let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn)
5964
{
6065
let mut applicability = Applicability::MachineApplicable;

‎tests/ui/format.fixed‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![feature(decl_macro)]
12
#![warn(clippy::useless_format)]
23
#![allow(
34
clippy::print_literal,
@@ -103,3 +104,41 @@ fn main() {
103104
let _ = xx.to_string();
104105
//~^ useless_format
105106
}
107+
108+
// `format!` as the tail expression of a block emitted by another macro
109+
// (e.g. rustc's `with_forced_trimmed_paths!`). The lint must still fire.
110+
mod block_wrap {
111+
macro_rules! plain_mr {
112+
($e:expr) => {
113+
$e
114+
};
115+
}
116+
macro_rules! block_mr {
117+
($e:expr) => {{
118+
let _g: i32 = 0;
119+
$e
120+
}};
121+
}
122+
pub macro plain_dm($e:expr) {
123+
$e
124+
}
125+
pub macro block_dm($e:expr) {{
126+
let _g: i32 = 0;
127+
$e
128+
}}
129+
130+
fn s() -> String {
131+
String::from("x")
132+
}
133+
134+
pub fn check() {
135+
let _ = plain_mr!(s().to_string());
136+
//~^ useless_format
137+
let _ = block_mr!(s().to_string());
138+
//~^ useless_format
139+
let _ = plain_dm!(s().to_string());
140+
//~^ useless_format
141+
let _ = block_dm!(s().to_string());
142+
//~^ useless_format
143+
}
144+
}

‎tests/ui/format.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![feature(decl_macro)]
12
#![warn(clippy::useless_format)]
23
#![allow(
34
clippy::print_literal,
@@ -106,3 +107,41 @@ fn main() {
106107
let _ = format!("{xx}");
107108
//~^ useless_format
108109
}
110+
111+
// `format!` as the tail expression of a block emitted by another macro
112+
// (e.g. rustc's `with_forced_trimmed_paths!`). The lint must still fire.
113+
mod block_wrap {
114+
macro_rules! plain_mr {
115+
($e:expr) => {
116+
$e
117+
};
118+
}
119+
macro_rules! block_mr {
120+
($e:expr) => {{
121+
let _g: i32 = 0;
122+
$e
123+
}};
124+
}
125+
pub macro plain_dm($e:expr) {
126+
$e
127+
}
128+
pub macro block_dm($e:expr) {{
129+
let _g: i32 = 0;
130+
$e
131+
}}
132+
133+
fn s() -> String {
134+
String::from("x")
135+
}
136+
137+
pub fn check() {
138+
let _ = plain_mr!(format!("{}", s()));
139+
//~^ useless_format
140+
let _ = block_mr!(format!("{}", s()));
141+
//~^ useless_format
142+
let _ = plain_dm!(format!("{}", s()));
143+
//~^ useless_format
144+
let _ = block_dm!(format!("{}", s()));
145+
//~^ useless_format
146+
}
147+
}

‎tests/ui/format.stderr‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: useless use of `format!`
2-
--> tests/ui/format.rs:20:5
2+
--> tests/ui/format.rs:21:5
33
|
44
LL | format!("foo");
55
| ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()`
@@ -8,19 +8,19 @@ LL | format!("foo");
88
= help: to override `-D warnings` add `#[allow(clippy::useless_format)]`
99

1010
error: useless use of `format!`
11-
--> tests/ui/format.rs:22:5
11+
--> tests/ui/format.rs:23:5
1212
|
1313
LL | format!("{{}}");
1414
| ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{}".to_string()`
1515

1616
error: useless use of `format!`
17-
--> tests/ui/format.rs:24:5
17+
--> tests/ui/format.rs:25:5
1818
|
1919
LL | format!("{{}} abc {{}}");
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{} abc {}".to_string()`
2121

2222
error: useless use of `format!`
23-
--> tests/ui/format.rs:26:5
23+
--> tests/ui/format.rs:27:5
2424
|
2525
LL | / format!(
2626
LL | |
@@ -36,70 +36,94 @@ LL ~ " bar"##.to_string();
3636
|
3737

3838
error: useless use of `format!`
39-
--> tests/ui/format.rs:32:13
39+
--> tests/ui/format.rs:33:13
4040
|
4141
LL | let _ = format!("");
4242
| ^^^^^^^^^^^ help: consider using `String::new()`: `String::new()`
4343

4444
error: useless use of `format!`
45-
--> tests/ui/format.rs:35:5
45+
--> tests/ui/format.rs:36:5
4646
|
4747
LL | format!("{}", "foo");
4848
| ^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()`
4949

5050
error: useless use of `format!`
51-
--> tests/ui/format.rs:44:5
51+
--> tests/ui/format.rs:45:5
5252
|
5353
LL | format!("{}", arg);
5454
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `arg.to_string()`
5555

5656
error: useless use of `format!`
57-
--> tests/ui/format.rs:75:5
57+
--> tests/ui/format.rs:76:5
5858
|
5959
LL | format!("{}", 42.to_string());
6060
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `42.to_string()`
6161

6262
error: useless use of `format!`
63-
--> tests/ui/format.rs:78:5
63+
--> tests/ui/format.rs:79:5
6464
|
6565
LL | format!("{}", x.display().to_string());
6666
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.display().to_string()`
6767

6868
error: useless use of `format!`
69-
--> tests/ui/format.rs:83:18
69+
--> tests/ui/format.rs:84:18
7070
|
7171
LL | let _ = Some(format!("{}", a + "bar"));
7272
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `a + "bar"`
7373

7474
error: useless use of `format!`
75-
--> tests/ui/format.rs:88:22
75+
--> tests/ui/format.rs:89:22
7676
|
7777
LL | let _s: String = format!("{}", &*v.join("\n"));
7878
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `(&*v.join("\n")).to_string()`
7979

8080
error: useless use of `format!`
81-
--> tests/ui/format.rs:95:13
81+
--> tests/ui/format.rs:96:13
8282
|
8383
LL | let _ = format!("{x}");
8484
| ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()`
8585

8686
error: useless use of `format!`
87-
--> tests/ui/format.rs:98:13
87+
--> tests/ui/format.rs:99:13
8888
|
8989
LL | let _ = format!("{y}", y = x);
9090
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()`
9191

9292
error: useless use of `format!`
93-
--> tests/ui/format.rs:103:13
93+
--> tests/ui/format.rs:104:13
9494
|
9595
LL | let _ = format!("{abc}");
9696
| ^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `abc.to_string()`
9797

9898
error: useless use of `format!`
99-
--> tests/ui/format.rs:106:13
99+
--> tests/ui/format.rs:107:13
100100
|
101101
LL | let _ = format!("{xx}");
102102
| ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `xx.to_string()`
103103

104-
error: aborting due to 15 previous errors
104+
error: useless use of `format!`
105+
--> tests/ui/format.rs:138:27
106+
|
107+
LL | let _ = plain_mr!(format!("{}", s()));
108+
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()`
109+
110+
error: useless use of `format!`
111+
--> tests/ui/format.rs:140:27
112+
|
113+
LL | let _ = block_mr!(format!("{}", s()));
114+
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()`
115+
116+
error: useless use of `format!`
117+
--> tests/ui/format.rs:142:27
118+
|
119+
LL | let _ = plain_dm!(format!("{}", s()));
120+
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()`
121+
122+
error: useless use of `format!`
123+
--> tests/ui/format.rs:144:27
124+
|
125+
LL | let _ = block_dm!(format!("{}", s()));
126+
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()`
127+
128+
error: aborting due to 19 previous errors
105129

‎tests/ui/unused_format_specs.1.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::unused_format_specs)]
2-
#![allow(unused)]
2+
#![allow(clippy::useless_format)]
33

44
macro_rules! format_args_from_macro {
55
() => {

‎tests/ui/unused_format_specs.2.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::unused_format_specs)]
2-
#![allow(unused)]
2+
#![allow(clippy::useless_format)]
33

44
macro_rules! format_args_from_macro {
55
() => {

‎tests/ui/unused_format_specs.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::unused_format_specs)]
2-
#![allow(unused)]
2+
#![allow(clippy::useless_format)]
33

44
macro_rules! format_args_from_macro {
55
() => {

0 commit comments

Comments
 (0)