diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index ec7f1dd0d846..f7abc24a25b9 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -368,9 +368,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R }} todo!(); }} - "#, - context_import = context_import, - name_upper = name_upper, + "# ); } else { let _ = writedoc!( @@ -384,9 +382,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R pub(super) fn check(cx: &{context_import}) {{ todo!(); }} - "#, - context_import = context_import, - name_upper = name_upper, + "# ); } diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 837618c9294b..ea42e726f506 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -537,17 +537,13 @@ fn declare_deprecated(name: &str, path: &Path, reason: &str) -> io::Result<()> { /// Nothing. This lint has been deprecated. /// /// ### Deprecation reason - /// {} - #[clippy::version = \"{}\"] - pub {}, - \"{}\" + /// {deprecation_reason} + #[clippy::version = \"{version}\"] + pub {name}, + \"{reason}\" }} - ", - deprecation_reason, - version, - name, - reason, + " ) } diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index fd3ce2f3d6cd..110253ca4bc9 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::is_diag_trait_item; use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred}; use clippy_utils::macros::{ - is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage, + is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage, }; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::snippet_opt; @@ -177,7 +177,6 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { && let expr_expn_data = expr.span.ctxt().outer_expn_data() && let outermost_expn_data = outermost_expn_data(expr_expn_data) && let Some(macro_def_id) = outermost_expn_data.macro_def_id - && is_format_macro(cx, macro_def_id) && let ExpnKind::Macro(_, name) = outermost_expn_data.kind { for arg in &format_args.args { diff --git a/clippy_lints/src/format_impl.rs b/clippy_lints/src/format_impl.rs index ed1342a54654..6b43fc839875 100644 --- a/clippy_lints/src/format_impl.rs +++ b/clippy_lints/src/format_impl.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; -use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn}; +use clippy_utils::macros::{root_macro_call_first_node, FormatArg, FormatArgsExpn}; use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -165,9 +165,7 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, // Check each arg in format calls - do we ever use Display on self (directly or via deref)? if_chain! { if let Some(outer_macro) = root_macro_call_first_node(cx, expr); - if let macro_def_id = outer_macro.def_id; if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn); - if is_format_macro(cx, macro_def_id); then { for arg in format_args.args { if arg.format.r#trait != impl_trait.name { diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index d13b34a66cca..62dc34e2e439 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -19,33 +19,6 @@ use rustc_span::{sym, BytePos, ExpnData, ExpnId, ExpnKind, Pos, Span, SpanData, use std::iter::{once, zip}; use std::ops::ControlFlow; -const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[ - sym::assert_eq_macro, - sym::assert_macro, - sym::assert_ne_macro, - sym::debug_assert_eq_macro, - sym::debug_assert_macro, - sym::debug_assert_ne_macro, - sym::eprint_macro, - sym::eprintln_macro, - sym::format_args_macro, - sym::format_macro, - sym::print_macro, - sym::println_macro, - sym::std_panic_macro, - sym::write_macro, - sym::writeln_macro, -]; - -/// Returns true if a given Macro `DefId` is a format macro (e.g. `println!`) -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 - } -} - /// A macro call, like `vec![1, 2, 3]`. /// /// Use `tcx.item_name(macro_call.def_id)` to get the macro name. diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed index 46ec7771114f..5f34bc870e4e 100644 --- a/tests/ui/uninlined_format_args.fixed +++ b/tests/ui/uninlined_format_args.fixed @@ -120,8 +120,8 @@ fn tester(fn_arg: i32) { println!("{local_i32:width$.prec$}"); println!("{width:width$.prec$}"); println!("{}", format!("{local_i32}")); - my_println!("{}", local_i32); - my_println_args!("{}", local_i32); + my_println!("{local_i32}"); + my_println_args!("{local_i32}"); // these should NOT be modified by the lint println!(concat!("nope ", "{}"), local_i32); @@ -159,10 +159,6 @@ fn tester(fn_arg: i32) { } } -fn main() { - tester(42); -} - #[clippy::msrv = "1.57"] fn _under_msrv() { let local_i32 = 1; @@ -174,3 +170,94 @@ fn _meets_msrv() { let local_i32 = 1; println!("expand='{local_i32}'"); } + +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)+)) + } + }}; +} + +macro_rules! my_concat { + ($fmt:literal $(, $e:expr)*) => { + println!(concat!("ERROR: ", $fmt), $($e,)*) + } +} + +macro_rules! my_good_macro { + ($fmt:literal $(, $e:expr)* $(,)?) => { + println!($fmt $(, $e)*) + } +} + +macro_rules! my_bad_macro { + ($fmt:literal, $($e:expr),*) => { + println!($fmt, $($e,)*) + } +} + +macro_rules! my_bad_macro2 { + ($fmt:literal) => { + let s = $fmt.clone(); + println!("{}", s); + }; + ($fmt:literal, $($e:expr)+) => { + println!($fmt, $($e,)*) + }; +} + +// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul... +// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962 +macro_rules! used_twice { + ( + large = $large:literal, + small = $small:literal, + $val:expr, + ) => { + if $val < 5 { + println!($small, $val); + } else { + println!($large, $val); + } + }; +} + +fn tester2() { + let local_i32 = 1; + 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}",); + + // FIXME: Broken false positives, currently unhandled + // my_bad_macro!("{}", local_i32); + // my_bad_macro2!("{}", local_i32); + // used_twice! { + // large = "large value: {}", + // small = "small value: {}", + // local_i32, + // }; +} + +// Add new tests right above this line to keep existing test line numbers intact. +// The main function is the only one that be kept at the end. +fn main() { + tester(42); + tester2(); +} diff --git a/tests/ui/uninlined_format_args.rs b/tests/ui/uninlined_format_args.rs index 35b3677a8968..2dd92e77da9f 100644 --- a/tests/ui/uninlined_format_args.rs +++ b/tests/ui/uninlined_format_args.rs @@ -164,10 +164,6 @@ fn tester(fn_arg: i32) { } } -fn main() { - tester(42); -} - #[clippy::msrv = "1.57"] fn _under_msrv() { let local_i32 = 1; @@ -179,3 +175,94 @@ fn _meets_msrv() { let local_i32 = 1; println!("expand='{}'", local_i32); } + +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)+)) + } + }}; +} + +macro_rules! my_concat { + ($fmt:literal $(, $e:expr)*) => { + println!(concat!("ERROR: ", $fmt), $($e,)*) + } +} + +macro_rules! my_good_macro { + ($fmt:literal $(, $e:expr)* $(,)?) => { + println!($fmt $(, $e)*) + } +} + +macro_rules! my_bad_macro { + ($fmt:literal, $($e:expr),*) => { + println!($fmt, $($e,)*) + } +} + +macro_rules! my_bad_macro2 { + ($fmt:literal) => { + let s = $fmt.clone(); + println!("{}", s); + }; + ($fmt:literal, $($e:expr)+) => { + println!($fmt, $($e,)*) + }; +} + +// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul... +// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962 +macro_rules! used_twice { + ( + large = $large:literal, + small = $small:literal, + $val:expr, + ) => { + if $val < 5 { + println!($small, $val); + } else { + println!($large, $val); + } + }; +} + +fn tester2() { + let local_i32 = 1; + 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,); + + // FIXME: Broken false positives, currently unhandled + // my_bad_macro!("{}", local_i32); + // my_bad_macro2!("{}", local_i32); + // used_twice! { + // large = "large value: {}", + // small = "small value: {}", + // local_i32, + // }; +} + +// Add new tests right above this line to keep existing test line numbers intact. +// The main function is the only one that be kept at the end. +fn main() { + tester(42); + tester2(); +} diff --git a/tests/ui/uninlined_format_args.stderr b/tests/ui/uninlined_format_args.stderr index 05ed5b6616c0..915c64b09928 100644 --- a/tests/ui/uninlined_format_args.stderr +++ b/tests/ui/uninlined_format_args.stderr @@ -834,6 +834,30 @@ LL - println!("{}", format!("{}", local_i32)); LL + println!("{}", format!("{local_i32}")); | +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:126:5 + | +LL | my_println!("{}", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - my_println!("{}", local_i32); +LL + my_println!("{local_i32}"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:127:5 + | +LL | my_println_args!("{}", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - my_println_args!("{}", local_i32); +LL + my_println_args!("{local_i32}"); + | + error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:143:5 | @@ -893,7 +917,7 @@ LL + panic!("p3 {local_i32}"); | error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:180:5 + --> $DIR/uninlined_format_args.rs:176:5 | LL | println!("expand='{}'", local_i32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -904,5 +928,53 @@ LL - println!("expand='{}'", local_i32); LL + println!("expand='{local_i32}'"); | -error: aborting due to 76 previous errors +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:247:5 + | +LL | my_println2_args!(true, "{}", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - my_println2_args!(true, "{}", local_i32); +LL + my_println2_args!(true, "{local_i32}"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:248:5 + | +LL | my_println2!(true, "{}", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - my_println2!(true, "{}", local_i32); +LL + my_println2!(true, "{local_i32}"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:250: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 + --> $DIR/uninlined_format_args.rs:251: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 82 previous errors