Skip to content

Commit

Permalink
Support user format-like macros
Browse files Browse the repository at this point in the history
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!`
  • Loading branch information
nyurik committed Nov 14, 2024
1 parent f58088b commit 9c829be
Show file tree
Hide file tree
Showing 13 changed files with 405 additions and 9 deletions.
1 change: 1 addition & 0 deletions book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
52 changes: 52 additions & 0 deletions book/src/attribs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# 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!`-like 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);
}
}
```
3 changes: 3 additions & 0 deletions clippy_utils/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions clippy_utils/src/macros.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
#![allow(clippy::similar_names)] // `expr` and `expn`

use crate::get_unique_attr;
use crate::visitors::{Descend, for_each_expr_without_closures};

use arrayvec::ArrayVec;
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};
Expand Down Expand Up @@ -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()
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/ui/format_args_unfixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
65 changes: 64 additions & 1 deletion tests/ui/format_args_unfixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

21 changes: 19 additions & 2 deletions tests/ui/uninlined_format_args.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand All @@ -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}");
}
21 changes: 19 additions & 2 deletions tests/ui/uninlined_format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand All @@ -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);
}
50 changes: 49 additions & 1 deletion tests/ui/uninlined_format_args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

35 changes: 35 additions & 0 deletions tests/ui/unused_format_specs.1.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
35 changes: 35 additions & 0 deletions tests/ui/unused_format_specs.2.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Loading

0 comments on commit 9c829be

Please sign in to comment.