Skip to content

Commit 8e0ebf1

Browse files
nyuriknyurik
committed
Support user format-like macros
Add support for `#[clippy::format_args]` attribute that can be attached to any macro to indicate that it functions the same as the built-in format macros like `format!`, `println!` and `write!`
1 parent f58088b commit 8e0ebf1

13 files changed

+400
-9
lines changed

‎book/src/SUMMARY.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- [Configuration](configuration.md)
88
- [Lint Configuration](lint_configuration.md)
99
- [Clippy's Lints](lints.md)
10+
- [Attributes for Crate Authors](attribs.md)
1011
- [Continuous Integration](continuous_integration/README.md)
1112
- [GitHub Actions](continuous_integration/github_actions.md)
1213
- [GitLab CI](continuous_integration/gitlab.md)

‎book/src/attribs.md

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Attributes for Crate Authors
2+
3+
In some cases it is possible extend Clippy coverage to include 3rd party libraries.
4+
5+
## `#[clippy::format_args]`
6+
7+
_Available since Clippy v1.84_
8+
9+
This attribute can be added to a macro that supports `format!`-like syntax.
10+
It tells Clippy that the macro is a formatting macro, and that the arguments to the macro
11+
should be linted as if they were arguments to `format!`. Any lint that would apply to a
12+
`format!` call will also apply to the macro call. The macro may have additional arguments
13+
before the format string, and these will be ignored.
14+
15+
### Example
16+
17+
```rust
18+
/// A macro that prints a message if a condition is true.
19+
#[macro_export]
20+
#[clippy::format_args]
21+
macro_rules! print_if {
22+
($condition:expr, $($args:tt)+) => {{
23+
if $condition {
24+
println!($($args)+)
25+
}
26+
}};
27+
}
28+
```
29+
30+
## `#[clippy::has_significant_drop]`
31+
32+
_Available since Clippy v1.60_
33+
34+
The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have an important side effect, such as unlocking a mutex, making it important for users to be able to accurately understand their lifetimes. When a temporary is returned in a function call in a match scrutinee, its lifetime lasts until the end of the match block, which may be surprising.
35+
36+
### Example
37+
38+
```rust
39+
#[clippy::has_significant_drop]
40+
struct CounterWrapper<'a> {
41+
counter: &'a Counter,
42+
}
43+
44+
impl<'a> Drop for CounterWrapper<'a> {
45+
fn drop(&mut self) {
46+
self.counter.i.fetch_sub(1, Ordering::Relaxed);
47+
}
48+
}
49+
```

‎clippy_utils/src/attrs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, DeprecationStatus)] = &[
2828
("dump", DeprecationStatus::None),
2929
("msrv", DeprecationStatus::None),
3030
("has_significant_drop", DeprecationStatus::None),
31+
("format_args", DeprecationStatus::None),
3132
];
3233

3334
pub struct LimitStack {

‎clippy_utils/src/macros.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
#![allow(clippy::similar_names)] // `expr` and `expn`
22

3+
use crate::get_unique_attr;
34
use crate::visitors::{Descend, for_each_expr_without_closures};
45

56
use arrayvec::ArrayVec;
67
use rustc_ast::{FormatArgs, FormatArgument, FormatPlaceholder};
78
use rustc_data_structures::fx::FxHashMap;
89
use rustc_data_structures::sync::{Lrc, OnceLock};
910
use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath};
10-
use rustc_lint::LateContext;
11+
use rustc_lint::{LateContext, LintContext};
1112
use rustc_span::def_id::DefId;
1213
use rustc_span::hygiene::{self, MacroKind, SyntaxContext};
1314
use rustc_span::{BytePos, ExpnData, ExpnId, ExpnKind, Span, SpanData, Symbol, sym};
@@ -36,7 +37,9 @@ pub fn is_format_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
3637
if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
3738
FORMAT_MACRO_DIAG_ITEMS.contains(&name)
3839
} else {
39-
false
40+
// Allow users to tag any macro as being format!-like
41+
// TODO: consider deleting FORMAT_MACRO_DIAG_ITEMS and using just this method
42+
get_unique_attr(cx.sess(), cx.tcx.get_attrs_unchecked(macro_def_id), "format_args").is_some()
4043
}
4144
}
4245

‎tests/ui/format_args_unfixable.rs

+29
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,32 @@ fn test2() {
119119
format!("something failed at {}", Location::caller())
120120
);
121121
}
122+
123+
#[clippy::format_args]
124+
macro_rules! usr_println {
125+
($target:expr, $($args:tt)*) => {{
126+
if $target {
127+
println!($($args)*)
128+
}
129+
}};
130+
}
131+
132+
fn user_format() {
133+
let error = Error::new(ErrorKind::Other, "bad thing");
134+
let x = 'x';
135+
136+
usr_println!(true, "error: {}", format!("boom at {}", Location::caller()));
137+
//~^ ERROR: `format!` in `usr_println!` args
138+
usr_println!(true, "{}: {}", error, format!("boom at {}", Location::caller()));
139+
//~^ ERROR: `format!` in `usr_println!` args
140+
usr_println!(true, "{:?}: {}", error, format!("boom at {}", Location::caller()));
141+
//~^ ERROR: `format!` in `usr_println!` args
142+
usr_println!(true, "{{}}: {}", format!("boom at {}", Location::caller()));
143+
//~^ ERROR: `format!` in `usr_println!` args
144+
usr_println!(true, r#"error: "{}""#, format!("boom at {}", Location::caller()));
145+
//~^ ERROR: `format!` in `usr_println!` args
146+
usr_println!(true, "error: {}", format!(r#"boom at "{}""#, Location::caller()));
147+
//~^ ERROR: `format!` in `usr_println!` args
148+
usr_println!(true, "error: {}", format!("boom at {} {0}", Location::caller()));
149+
//~^ ERROR: `format!` in `usr_println!` args
150+
}

‎tests/ui/format_args_unfixable.stderr

+64-1
Original file line numberDiff line numberDiff line change
@@ -174,5 +174,68 @@ LL | panic!("error: {}", format!("something failed at {}", Location::caller(
174174
= help: combine the `format!(..)` arguments with the outer `panic!(..)` call
175175
= help: or consider changing `format!` to `format_args!`
176176

177-
error: aborting due to 18 previous errors
177+
error: `format!` in `usr_println!` args
178+
--> tests/ui/format_args_unfixable.rs:136:5
179+
|
180+
LL | usr_println!(true, "error: {}", format!("boom at {}", Location::caller()));
181+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
182+
|
183+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
184+
= help: or consider changing `format!` to `format_args!`
185+
186+
error: `format!` in `usr_println!` args
187+
--> tests/ui/format_args_unfixable.rs:138:5
188+
|
189+
LL | usr_println!(true, "{}: {}", error, format!("boom at {}", Location::caller()));
190+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
191+
|
192+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
193+
= help: or consider changing `format!` to `format_args!`
194+
195+
error: `format!` in `usr_println!` args
196+
--> tests/ui/format_args_unfixable.rs:140:5
197+
|
198+
LL | usr_println!(true, "{:?}: {}", error, format!("boom at {}", Location::caller()));
199+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
200+
|
201+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
202+
= help: or consider changing `format!` to `format_args!`
203+
204+
error: `format!` in `usr_println!` args
205+
--> tests/ui/format_args_unfixable.rs:142:5
206+
|
207+
LL | usr_println!(true, "{{}}: {}", format!("boom at {}", Location::caller()));
208+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
209+
|
210+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
211+
= help: or consider changing `format!` to `format_args!`
212+
213+
error: `format!` in `usr_println!` args
214+
--> tests/ui/format_args_unfixable.rs:144:5
215+
|
216+
LL | usr_println!(true, r#"error: "{}""#, format!("boom at {}", Location::caller()));
217+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
218+
|
219+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
220+
= help: or consider changing `format!` to `format_args!`
221+
222+
error: `format!` in `usr_println!` args
223+
--> tests/ui/format_args_unfixable.rs:146:5
224+
|
225+
LL | usr_println!(true, "error: {}", format!(r#"boom at "{}""#, Location::caller()));
226+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
227+
|
228+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
229+
= help: or consider changing `format!` to `format_args!`
230+
231+
error: `format!` in `usr_println!` args
232+
--> tests/ui/format_args_unfixable.rs:148:5
233+
|
234+
LL | usr_println!(true, "error: {}", format!("boom at {} {0}", Location::caller()));
235+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
236+
|
237+
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
238+
= help: or consider changing `format!` to `format_args!`
239+
240+
error: aborting due to 25 previous errors
178241

‎tests/ui/uninlined_format_args.fixed

+19-2
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,6 @@ fn tester2() {
257257
my_concat!("{}", local_i32);
258258
my_good_macro!("{}", local_i32);
259259
my_good_macro!("{}", local_i32,);
260-
261-
// FIXME: Broken false positives, currently unhandled
262260
my_bad_macro!("{}", local_i32);
263261
my_bad_macro2!("{}", local_i32);
264262
used_twice! {
@@ -267,3 +265,22 @@ fn tester2() {
267265
local_i32,
268266
};
269267
}
268+
269+
#[clippy::format_args]
270+
macro_rules! usr_println {
271+
($target:expr, $($args:tt)*) => {{
272+
if $target {
273+
println!($($args)*)
274+
}
275+
}};
276+
}
277+
278+
fn user_format() {
279+
let local_i32 = 1;
280+
let local_f64 = 2.0;
281+
282+
usr_println!(true, "val='{local_i32}'");
283+
usr_println!(true, "{local_i32}");
284+
usr_println!(true, "{local_i32:#010x}");
285+
usr_println!(true, "{local_f64:.1}");
286+
}

‎tests/ui/uninlined_format_args.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ fn tester2() {
262262
my_concat!("{}", local_i32);
263263
my_good_macro!("{}", local_i32);
264264
my_good_macro!("{}", local_i32,);
265-
266-
// FIXME: Broken false positives, currently unhandled
267265
my_bad_macro!("{}", local_i32);
268266
my_bad_macro2!("{}", local_i32);
269267
used_twice! {
@@ -272,3 +270,22 @@ fn tester2() {
272270
local_i32,
273271
};
274272
}
273+
274+
#[clippy::format_args]
275+
macro_rules! usr_println {
276+
($target:expr, $($args:tt)*) => {{
277+
if $target {
278+
println!($($args)*)
279+
}
280+
}};
281+
}
282+
283+
fn user_format() {
284+
let local_i32 = 1;
285+
let local_f64 = 2.0;
286+
287+
usr_println!(true, "val='{}'", local_i32);
288+
usr_println!(true, "{}", local_i32);
289+
usr_println!(true, "{:#010x}", local_i32);
290+
usr_println!(true, "{:.1}", local_f64);
291+
}

‎tests/ui/uninlined_format_args.stderr

+49-1
Original file line numberDiff line numberDiff line change
@@ -845,5 +845,53 @@ LL - println!("expand='{}'", local_i32);
845845
LL + println!("expand='{local_i32}'");
846846
|
847847

848-
error: aborting due to 71 previous errors
848+
error: variables can be used directly in the `format!` string
849+
--> tests/ui/uninlined_format_args.rs:287:5
850+
|
851+
LL | usr_println!(true, "val='{}'", local_i32);
852+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
853+
|
854+
help: change this to
855+
|
856+
LL - usr_println!(true, "val='{}'", local_i32);
857+
LL + usr_println!(true, "val='{local_i32}'");
858+
|
859+
860+
error: variables can be used directly in the `format!` string
861+
--> tests/ui/uninlined_format_args.rs:288:5
862+
|
863+
LL | usr_println!(true, "{}", local_i32);
864+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
865+
|
866+
help: change this to
867+
|
868+
LL - usr_println!(true, "{}", local_i32);
869+
LL + usr_println!(true, "{local_i32}");
870+
|
871+
872+
error: variables can be used directly in the `format!` string
873+
--> tests/ui/uninlined_format_args.rs:289:5
874+
|
875+
LL | usr_println!(true, "{:#010x}", local_i32);
876+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
877+
|
878+
help: change this to
879+
|
880+
LL - usr_println!(true, "{:#010x}", local_i32);
881+
LL + usr_println!(true, "{local_i32:#010x}");
882+
|
883+
884+
error: variables can be used directly in the `format!` string
885+
--> tests/ui/uninlined_format_args.rs:290:5
886+
|
887+
LL | usr_println!(true, "{:.1}", local_f64);
888+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
889+
|
890+
help: change this to
891+
|
892+
LL - usr_println!(true, "{:.1}", local_f64);
893+
LL + usr_println!(true, "{local_f64:.1}");
894+
|
895+
896+
error: aborting due to 75 previous errors
849897

‎tests/ui/unused_format_specs.1.fixed

+35
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,38 @@ fn should_not_lint() {
3333
let args = format_args!("");
3434
println!("{args}");
3535
}
36+
37+
#[clippy::format_args]
38+
macro_rules! usr_println {
39+
($target:expr, $($args:tt)*) => {{
40+
if $target {
41+
println!($($args)*)
42+
}
43+
}};
44+
}
45+
46+
fn should_lint_user() {
47+
// prints `.`, not ` .`
48+
usr_println!(true, "{:5}.", format!(""));
49+
//~^ ERROR: format specifiers have no effect on `format_args!()`
50+
//prints `abcde`, not `abc`
51+
usr_println!(true, "{:.3}", format!("abcde"));
52+
//~^ ERROR: format specifiers have no effect on `format_args!()`
53+
54+
usr_println!(true, "{}.", format_args_from_macro!());
55+
//~^ ERROR: format specifiers have no effect on `format_args!()`
56+
57+
let args = format_args!("");
58+
usr_println!(true, "{args}");
59+
//~^ ERROR: format specifiers have no effect on `format_args!()`
60+
}
61+
62+
fn should_not_lint_user() {
63+
usr_println!(true, "{}", format_args!(""));
64+
// Technically the same as `{}`, but the `format_args` docs specifically mention that you can use
65+
// debug formatting so allow it
66+
usr_println!(true, "{:?}", format_args!(""));
67+
68+
let args = format_args!("");
69+
usr_println!(true, "{args}");
70+
}

‎tests/ui/unused_format_specs.2.fixed

+35
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,38 @@ fn should_not_lint() {
3333
let args = format_args!("");
3434
println!("{args}");
3535
}
36+
37+
#[clippy::format_args]
38+
macro_rules! usr_println {
39+
($target:expr, $($args:tt)*) => {{
40+
if $target {
41+
println!($($args)*)
42+
}
43+
}};
44+
}
45+
46+
fn should_lint_user() {
47+
// prints `.`, not ` .`
48+
usr_println!(true, "{}.", format_args!(""));
49+
//~^ ERROR: format specifiers have no effect on `format_args!()`
50+
//prints `abcde`, not `abc`
51+
usr_println!(true, "{}", format_args!("abcde"));
52+
//~^ ERROR: format specifiers have no effect on `format_args!()`
53+
54+
usr_println!(true, "{}.", format_args_from_macro!());
55+
//~^ ERROR: format specifiers have no effect on `format_args!()`
56+
57+
let args = format_args!("");
58+
usr_println!(true, "{args}");
59+
//~^ ERROR: format specifiers have no effect on `format_args!()`
60+
}
61+
62+
fn should_not_lint_user() {
63+
usr_println!(true, "{}", format_args!(""));
64+
// Technically the same as `{}`, but the `format_args` docs specifically mention that you can use
65+
// debug formatting so allow it
66+
usr_println!(true, "{:?}", format_args!(""));
67+
68+
let args = format_args!("");
69+
usr_println!(true, "{args}");
70+
}

0 commit comments

Comments
 (0)