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!`
rust-lang · Sep 20, 2024 · 58b6bd6 · 58b6bd6
1 parent 2e5b680
commit 58b6bd6
Show file tree

Hide file tree

Showing 7 changed files with 155 additions and 5 deletions.
diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs
@@ -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 {

diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs
@@ -1,13 +1,14 @@
 #![allow(clippy::similar_names)] // `expr` and `expn`
 
+use crate::get_unique_attr;
 use crate::visitors::{for_each_expr_without_closures, Descend};
 
 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::{sym, BytePos, ExpnData, ExpnId, ExpnKind, Span, SpanData, Symbol};
@@ -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()
     }
 }
 

diff --git a/tests/ui/format_args_unfixable_user.rs b/tests/ui/format_args_unfixable_user.rs
@@ -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::{stdout, Error, ErrorKind, Write};
+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())
+    );
+}
diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed
@@ -190,6 +190,7 @@ macro_rules! _internal {
     };
 }
 
+#[clippy::format_args]
 macro_rules! my_println2 {
    ($target:expr, $($args:tt)+) => {{
        if $target {
@@ -198,6 +199,7 @@ macro_rules! my_println2 {
     }};
 }
 
+#[clippy::format_args]
 macro_rules! my_println2_args {
     ($target:expr, $($args:tt)+) => {{
        if $target {
@@ -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)*)
@@ -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);

diff --git a/tests/ui/uninlined_format_args.rs b/tests/ui/uninlined_format_args.rs
@@ -195,6 +195,7 @@ macro_rules! _internal {
     };
 }
 
+#[clippy::format_args]
 macro_rules! my_println2 {
    ($target:expr, $($args:tt)+) => {{
        if $target {
@@ -203,6 +204,7 @@ macro_rules! my_println2 {
     }};
 }
 
+#[clippy::format_args]
 macro_rules! my_println2_args {
     ($target:expr, $($args:tt)+) => {{
        if $target {
@@ -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)*)

diff --git a/tests/ui/uninlined_format_args.stderr b/tests/ui/uninlined_format_args.stderr
@@ -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
 
diff --git a/tests/ui/unused_format_specs.rs b/tests/ui/unused_format_specs.rs
@@ -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);
+}