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!`
This commit is contained in:
Yuri Astrakhan 2023-03-28 17:04:30 -04:00
parent 627363e811
commit 81dceed8ba
13 changed files with 406 additions and 9 deletions

View file

@ -7,6 +7,7 @@
- [Configuration](configuration.md)
- [Lint Configuration](lint_configuration.md)
- [Clippy's Lints](lints.md)
- [Attributes for Crate Authors](attribs.md)
- [Continuous Integration](continuous_integration/README.md)
- [GitHub Actions](continuous_integration/github_actions.md)
- [GitLab CI](continuous_integration/gitlab.md)

53
book/src/attribs.md Normal file
View file

@ -0,0 +1,53 @@
# Attributes for Crate Authors
In some cases it is possible to extend Clippy coverage to 3rd party libraries.
To do this, Clippy provides attributes that can be applied to items in the 3rd party crate.
## `#[clippy::format_args]`
_Available since Clippy v1.84_
This attribute can be added to a macro that supports `format!`, `println!`, or similar syntax.
It tells Clippy that the macro is a formatting macro, and that the arguments to the macro
should be linted as if they were arguments to `format!`. Any lint that would apply to a
`format!` call will also apply to the macro call. The macro may have additional arguments
before the format string, and these will be ignored.
### Example
```rust
/// A macro that prints a message if a condition is true.
#[macro_export]
#[clippy::format_args]
macro_rules! print_if {
($condition:expr, $($args:tt)+) => {{
if $condition {
println!($($args)+)
}
}};
}
```
## `#[clippy::has_significant_drop]`
_Available since Clippy v1.60_
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.
### Example
```rust
#[clippy::has_significant_drop]
struct CounterWrapper<'a> {
counter: &'a Counter,
}
impl<'a> Drop for CounterWrapper<'a> {
fn drop(&mut self) {
self.counter.i.fetch_sub(1, Ordering::Relaxed);
}
}
```

View file

@ -27,7 +27,10 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, DeprecationStatus)] = &[
("cyclomatic_complexity", DeprecationStatus::Replaced("cognitive_complexity")),
("dump", DeprecationStatus::None),
("msrv", DeprecationStatus::None),
// The following attributes are for the 3rd party crate authors.
// See book/src/attribs.md
("has_significant_drop", DeprecationStatus::None),
("format_args", DeprecationStatus::None),
];
pub struct LimitStack {

View file

@ -1,5 +1,6 @@
#![allow(clippy::similar_names)] // `expr` and `expn`
use crate::get_unique_attr;
use crate::visitors::{Descend, for_each_expr_without_closures};
use arrayvec::ArrayVec;
@ -7,7 +8,7 @@ use rustc_ast::{FormatArgs, FormatArgument, FormatPlaceholder};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::{Lrc, OnceLock};
use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath};
use rustc_lint::LateContext;
use rustc_lint::{LateContext, LintContext};
use rustc_span::def_id::DefId;
use rustc_span::hygiene::{self, MacroKind, SyntaxContext};
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 {
if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
FORMAT_MACRO_DIAG_ITEMS.contains(&name)
} else {
false
// Allow users to tag any macro as being format!-like
// TODO: consider deleting FORMAT_MACRO_DIAG_ITEMS and using just this method
get_unique_attr(cx.sess(), cx.tcx.get_attrs_unchecked(macro_def_id), "format_args").is_some()
}
}

View file

@ -119,3 +119,32 @@ fn test2() {
format!("something failed at {}", Location::caller())
);
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn user_format() {
let error = Error::new(ErrorKind::Other, "bad thing");
let x = 'x';
usr_println!(true, "error: {}", format!("boom at {}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, "{}: {}", error, format!("boom at {}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, "{:?}: {}", error, format!("boom at {}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, "{{}}: {}", format!("boom at {}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, r#"error: "{}""#, format!("boom at {}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, "error: {}", format!(r#"boom at "{}""#, Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
usr_println!(true, "error: {}", format!("boom at {} {0}", Location::caller()));
//~^ ERROR: `format!` in `usr_println!` args
}

View file

@ -174,5 +174,68 @@ LL | panic!("error: {}", format!("something failed at {}", Location::caller(
= help: combine the `format!(..)` arguments with the outer `panic!(..)` call
= help: or consider changing `format!` to `format_args!`
error: aborting due to 18 previous errors
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:136:5
|
LL | usr_println!(true, "error: {}", format!("boom at {}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:138:5
|
LL | usr_println!(true, "{}: {}", error, format!("boom at {}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:140:5
|
LL | usr_println!(true, "{:?}: {}", error, format!("boom at {}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:142:5
|
LL | usr_println!(true, "{{}}: {}", format!("boom at {}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:144:5
|
LL | usr_println!(true, r#"error: "{}""#, format!("boom at {}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:146:5
|
LL | usr_println!(true, "error: {}", format!(r#"boom at "{}""#, Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: `format!` in `usr_println!` args
--> tests/ui/format_args_unfixable.rs:148:5
|
LL | usr_println!(true, "error: {}", format!("boom at {} {0}", Location::caller()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: combine the `format!(..)` arguments with the outer `usr_println!(..)` call
= help: or consider changing `format!` to `format_args!`
error: aborting due to 25 previous errors

View file

@ -257,8 +257,6 @@ fn tester2() {
my_concat!("{}", local_i32);
my_good_macro!("{}", local_i32);
my_good_macro!("{}", local_i32,);
// FIXME: Broken false positives, currently unhandled
my_bad_macro!("{}", local_i32);
my_bad_macro2!("{}", local_i32);
used_twice! {
@ -267,3 +265,22 @@ fn tester2() {
local_i32,
};
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn user_format() {
let local_i32 = 1;
let local_f64 = 2.0;
usr_println!(true, "val='{local_i32}'");
usr_println!(true, "{local_i32}");
usr_println!(true, "{local_i32:#010x}");
usr_println!(true, "{local_f64:.1}");
}

View file

@ -262,8 +262,6 @@ fn tester2() {
my_concat!("{}", local_i32);
my_good_macro!("{}", local_i32);
my_good_macro!("{}", local_i32,);
// FIXME: Broken false positives, currently unhandled
my_bad_macro!("{}", local_i32);
my_bad_macro2!("{}", local_i32);
used_twice! {
@ -272,3 +270,22 @@ fn tester2() {
local_i32,
};
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn user_format() {
let local_i32 = 1;
let local_f64 = 2.0;
usr_println!(true, "val='{}'", local_i32);
usr_println!(true, "{}", local_i32);
usr_println!(true, "{:#010x}", local_i32);
usr_println!(true, "{:.1}", local_f64);
}

View file

@ -845,5 +845,53 @@ LL - println!("expand='{}'", local_i32);
LL + println!("expand='{local_i32}'");
|
error: aborting due to 71 previous errors
error: variables can be used directly in the `format!` string
--> tests/ui/uninlined_format_args.rs:287:5
|
LL | usr_println!(true, "val='{}'", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - usr_println!(true, "val='{}'", local_i32);
LL + usr_println!(true, "val='{local_i32}'");
|
error: variables can be used directly in the `format!` string
--> tests/ui/uninlined_format_args.rs:288:5
|
LL | usr_println!(true, "{}", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - usr_println!(true, "{}", local_i32);
LL + usr_println!(true, "{local_i32}");
|
error: variables can be used directly in the `format!` string
--> tests/ui/uninlined_format_args.rs:289:5
|
LL | usr_println!(true, "{:#010x}", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - usr_println!(true, "{:#010x}", local_i32);
LL + usr_println!(true, "{local_i32:#010x}");
|
error: variables can be used directly in the `format!` string
--> tests/ui/uninlined_format_args.rs:290:5
|
LL | usr_println!(true, "{:.1}", local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - usr_println!(true, "{:.1}", local_f64);
LL + usr_println!(true, "{local_f64:.1}");
|
error: aborting due to 75 previous errors

View file

@ -33,3 +33,38 @@ fn should_not_lint() {
let args = format_args!("");
println!("{args}");
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn should_lint_user() {
// prints `.`, not ` .`
usr_println!(true, "{:5}.", format!(""));
//~^ ERROR: format specifiers have no effect on `format_args!()`
//prints `abcde`, not `abc`
usr_println!(true, "{:.3}", format!("abcde"));
//~^ ERROR: format specifiers have no effect on `format_args!()`
usr_println!(true, "{}.", format_args_from_macro!());
//~^ ERROR: format specifiers have no effect on `format_args!()`
let args = format_args!("");
usr_println!(true, "{args}");
//~^ ERROR: format specifiers have no effect on `format_args!()`
}
fn should_not_lint_user() {
usr_println!(true, "{}", format_args!(""));
// Technically the same as `{}`, but the `format_args` docs specifically mention that you can use
// debug formatting so allow it
usr_println!(true, "{:?}", format_args!(""));
let args = format_args!("");
usr_println!(true, "{args}");
}

View file

@ -33,3 +33,38 @@ fn should_not_lint() {
let args = format_args!("");
println!("{args}");
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn should_lint_user() {
// prints `.`, not ` .`
usr_println!(true, "{}.", format_args!(""));
//~^ ERROR: format specifiers have no effect on `format_args!()`
//prints `abcde`, not `abc`
usr_println!(true, "{}", format_args!("abcde"));
//~^ ERROR: format specifiers have no effect on `format_args!()`
usr_println!(true, "{}.", format_args_from_macro!());
//~^ ERROR: format specifiers have no effect on `format_args!()`
let args = format_args!("");
usr_println!(true, "{args}");
//~^ ERROR: format specifiers have no effect on `format_args!()`
}
fn should_not_lint_user() {
usr_println!(true, "{}", format_args!(""));
// Technically the same as `{}`, but the `format_args` docs specifically mention that you can use
// debug formatting so allow it
usr_println!(true, "{:?}", format_args!(""));
let args = format_args!("");
usr_println!(true, "{args}");
}

View file

@ -33,3 +33,38 @@ fn should_not_lint() {
let args = format_args!("");
println!("{args}");
}
#[clippy::format_args]
macro_rules! usr_println {
($target:expr, $($args:tt)*) => {{
if $target {
println!($($args)*)
}
}};
}
fn should_lint_user() {
// prints `.`, not ` .`
usr_println!(true, "{:5}.", format_args!(""));
//~^ ERROR: format specifiers have no effect on `format_args!()`
//prints `abcde`, not `abc`
usr_println!(true, "{:.3}", format_args!("abcde"));
//~^ ERROR: format specifiers have no effect on `format_args!()`
usr_println!(true, "{:5}.", format_args_from_macro!());
//~^ ERROR: format specifiers have no effect on `format_args!()`
let args = format_args!("");
usr_println!(true, "{args:5}");
//~^ ERROR: format specifiers have no effect on `format_args!()`
}
fn should_not_lint_user() {
usr_println!(true, "{}", format_args!(""));
// Technically the same as `{}`, but the `format_args` docs specifically mention that you can use
// debug formatting so allow it
usr_println!(true, "{:?}", format_args!(""));
let args = format_args!("");
usr_println!(true, "{args}");
}

View file

@ -58,5 +58,63 @@ LL - println!("{args:5}");
LL + println!("{args}");
|
error: aborting due to 4 previous errors
error: format specifiers have no effect on `format_args!()`
--> tests/ui/unused_format_specs.rs:48:25
|
LL | usr_println!(true, "{:5}.", format_args!(""));
| ^^^^
|
help: for the width to apply consider using `format!()`
|
LL | usr_println!(true, "{:5}.", format!(""));
| ~~~~~~
help: if the current behavior is intentional, remove the format specifiers
|
LL - usr_println!(true, "{:5}.", format_args!(""));
LL + usr_println!(true, "{}.", format_args!(""));
|
error: format specifiers have no effect on `format_args!()`
--> tests/ui/unused_format_specs.rs:51:25
|
LL | usr_println!(true, "{:.3}", format_args!("abcde"));
| ^^^^^
|
help: for the precision to apply consider using `format!()`
|
LL | usr_println!(true, "{:.3}", format!("abcde"));
| ~~~~~~
help: if the current behavior is intentional, remove the format specifiers
|
LL - usr_println!(true, "{:.3}", format_args!("abcde"));
LL + usr_println!(true, "{}", format_args!("abcde"));
|
error: format specifiers have no effect on `format_args!()`
--> tests/ui/unused_format_specs.rs:54:25
|
LL | usr_println!(true, "{:5}.", format_args_from_macro!());
| ^^^^
|
= help: for the width to apply consider using `format!()`
help: if the current behavior is intentional, remove the format specifiers
|
LL - usr_println!(true, "{:5}.", format_args_from_macro!());
LL + usr_println!(true, "{}.", format_args_from_macro!());
|
error: format specifiers have no effect on `format_args!()`
--> tests/ui/unused_format_specs.rs:58:25
|
LL | usr_println!(true, "{args:5}");
| ^^^^^^^^
|
= help: for the width to apply consider using `format!()`
help: if the current behavior is intentional, remove the format specifiers
|
LL - usr_println!(true, "{args:5}");
LL + usr_println!(true, "{args}");
|
error: aborting due to 8 previous errors