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 Sep 22, 2024
1 parent 43e3384 commit b802d47
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 5 deletions.
1 change: 1 addition & 0 deletions clippy_utils/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, DeprecationStatus)] = &[
("dump", DeprecationStatus::None),
("msrv", DeprecationStatus::None),
("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
63 changes: 63 additions & 0 deletions tests/ui/format_args_unfixable_user.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#![warn(clippy::format_in_format_args, clippy::to_string_in_format_args)]
#![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::uninlined_format_args)]

use std::io::{Error, ErrorKind, Write, stdout};
use std::ops::Deref;
use std::panic::Location;

macro_rules! _internal {
($($args:tt)*) => {
println!("{}", format_args!($($args)*))
};
}

macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!($($args)+)
}
}};
}

macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!("foo: {}", format_args!($($args)+))
}
}};
}

fn main() {
let error = Error::new(ErrorKind::Other, "bad thing");

my_println2!(true, "error: {}", format!("something failed at {}", Location::caller()));
my_println2!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller())
);

my_println2!(
true,
"error: {}",
format!("something failed at {}", Location::caller().to_string())
);
my_println2!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller().to_string())
);

my_println2!(true, "error: {}", Location::caller().to_string());
my_println2!(true, "{}: {}", error, Location::caller().to_string());

my_println2_args!(true, "error: {}", format!("something failed at {}", Location::caller()));
my_println2_args!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller())
);
}
8 changes: 6 additions & 2 deletions tests/ui/uninlined_format_args.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ macro_rules! _internal {
};
}

#[clippy::format_args]
macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
Expand All @@ -198,6 +199,7 @@ macro_rules! my_println2 {
}};
}

#[clippy::format_args]
macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
Expand All @@ -206,12 +208,14 @@ macro_rules! my_println2_args {
}};
}

#[clippy::format_args]
macro_rules! my_concat {
($fmt:literal $(, $e:expr)*) => {
println!(concat!("ERROR: ", $fmt), $($e,)*)
}
}

#[clippy::format_args]
macro_rules! my_good_macro {
($fmt:literal $(, $e:expr)* $(,)?) => {
println!($fmt $(, $e)*)
Expand Down Expand Up @@ -255,8 +259,8 @@ fn tester2() {
my_println2_args!(true, "{}", local_i32);
my_println2!(true, "{}", local_i32);
my_concat!("{}", local_i32);
my_good_macro!("{}", local_i32);
my_good_macro!("{}", local_i32,);
my_good_macro!("{local_i32}");
my_good_macro!("{local_i32}",);

// FIXME: Broken false positives, currently unhandled
my_bad_macro!("{}", local_i32);
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/uninlined_format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ macro_rules! _internal {
};
}

#[clippy::format_args]
macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
Expand All @@ -203,6 +204,7 @@ macro_rules! my_println2 {
}};
}

#[clippy::format_args]
macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
Expand All @@ -211,12 +213,14 @@ macro_rules! my_println2_args {
}};
}

#[clippy::format_args]
macro_rules! my_concat {
($fmt:literal $(, $e:expr)*) => {
println!(concat!("ERROR: ", $fmt), $($e,)*)
}
}

#[clippy::format_args]
macro_rules! my_good_macro {
($fmt:literal $(, $e:expr)* $(,)?) => {
println!($fmt $(, $e)*)
Expand Down
26 changes: 25 additions & 1 deletion tests/ui/uninlined_format_args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -845,5 +845,29 @@ 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:267:5
|
LL | my_good_macro!("{}", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - my_good_macro!("{}", local_i32);
LL + my_good_macro!("{local_i32}");
|

error: variables can be used directly in the `format!` string
--> tests/ui/uninlined_format_args.rs:268:5
|
LL | my_good_macro!("{}", local_i32,);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - my_good_macro!("{}", local_i32,);
LL + my_good_macro!("{local_i32}",);
|

error: aborting due to 73 previous errors

51 changes: 51 additions & 0 deletions tests/ui/unused_format_specs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#![warn(clippy::unused_format_specs)]
#![allow(unused)]

macro_rules! _internal {
($($args:tt)*) => {
println!("{}", format_args!($($args)*))
};
}

macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!($($args)+)
}
}};
}

macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!("foo: {}", format_args!($($args)+))
}
}};
}

fn main() {
let f = 1.0f64;
println!("{:.}", 1.0);
println!("{f:.} {f:.?}");
println!("{:.}", 1);

my_println2!(true, "{:.}", 1.0);
my_println2!(true, "{f:.} {f:.?}");
my_println2!(true, "{:.}", 1);

my_println2_args!(true, "{:.}", 1.0);
my_println2_args!(true, "{f:.} {f:.?}");
my_println2_args!(true, "{:.}", 1);
}

fn should_not_lint() {
let f = 1.0f64;
println!("{:.1}", 1.0);
println!("{f:.w$} {f:.*?}", 3, w = 2);

my_println2!(true, "{:.1}", 1.0);
my_println2!(true, "{f:.w$} {f:.*?}", 3, w = 2);

my_println2_args!(true, "{:.1}", 1.0);
my_println2_args!(true, "{f:.w$} {f:.*?}", 3, w = 2);
}

0 comments on commit b802d47

Please sign in to comment.