From 783879e6fe48b5bd776bf2b1c9314fda3e30447d Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 28 Mar 2023 20:52:55 -0400 Subject: [PATCH] Partial no-op refactoring of #9948 This contains preparatory work for #9948 to keep that change to the minimum, and make it easier to review it. --- clippy_dev/src/new_lint.rs | 8 +-- clippy_dev/src/update_lints.rs | 14 ++--- tests/ui/format_args_unfixable.rs | 44 ++++++++++++++ tests/ui/format_args_unfixable.stderr | 36 +++++------ tests/ui/uninlined_format_args.fixed | 86 ++++++++++++++++++++++++++- tests/ui/uninlined_format_args.rs | 86 ++++++++++++++++++++++++++- 6 files changed, 239 insertions(+), 35 deletions(-) diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 420214d92..13a277034 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -369,9 +369,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 _: fmt::Result = writedoc!( @@ -385,9 +383,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 779e4d0e1..95222a9ac 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/tests/ui/format_args_unfixable.rs b/tests/ui/format_args_unfixable.rs index eb0ac15bf..423bfaf97 100644 --- a/tests/ui/format_args_unfixable.rs +++ b/tests/ui/format_args_unfixable.rs @@ -1,4 +1,5 @@ #![warn(clippy::format_in_format_args, clippy::to_string_in_format_args)] +#![allow(unused)] #![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::uninlined_format_args)] use std::io::{stdout, Error, ErrorKind, Write}; @@ -57,3 +58,46 @@ fn main() { my_macro!(); println!("error: {}", my_other_macro!()); } + +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 test2() { + let error = Error::new(ErrorKind::Other, "bad thing"); + + // None of these should be linted without the config change + my_println2!(true, "error: {}", format!("something failed at {}", Location::caller())); + my_println2!( + true, + "{}: {}", + error, + format!("something failed at {}", Location::caller()) + ); + + 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/format_args_unfixable.stderr b/tests/ui/format_args_unfixable.stderr index b291d475a..c1be48c3b 100644 --- a/tests/ui/format_args_unfixable.stderr +++ b/tests/ui/format_args_unfixable.stderr @@ -1,5 +1,5 @@ error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:25:5 + --> $DIR/format_args_unfixable.rs:26:5 | LL | println!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | println!("error: {}", format!("something failed at {}", Location::calle = note: `-D clippy::format-in-format-args` implied by `-D warnings` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:26:5 + --> $DIR/format_args_unfixable.rs:27:5 | LL | println!("{}: {}", error, format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -18,7 +18,7 @@ LL | println!("{}: {}", error, format!("something failed at {}", Location::c = help: or consider changing `format!` to `format_args!` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:27:5 + --> $DIR/format_args_unfixable.rs:28:5 | LL | println!("{:?}: {}", error, format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -27,7 +27,7 @@ LL | println!("{:?}: {}", error, format!("something failed at {}", Location: = help: or consider changing `format!` to `format_args!` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:28:5 + --> $DIR/format_args_unfixable.rs:29:5 | LL | println!("{{}}: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +36,7 @@ LL | println!("{{}}: {}", format!("something failed at {}", Location::caller = help: or consider changing `format!` to `format_args!` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:29:5 + --> $DIR/format_args_unfixable.rs:30:5 | LL | println!(r#"error: "{}""#, format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -45,7 +45,7 @@ LL | println!(r#"error: "{}""#, format!("something failed at {}", Location:: = help: or consider changing `format!` to `format_args!` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:30:5 + --> $DIR/format_args_unfixable.rs:31:5 | LL | println!("error: {}", format!(r#"something failed at "{}""#, Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -54,7 +54,7 @@ LL | println!("error: {}", format!(r#"something failed at "{}""#, Location:: = help: or consider changing `format!` to `format_args!` error: `format!` in `println!` args - --> $DIR/format_args_unfixable.rs:31:5 + --> $DIR/format_args_unfixable.rs:32:5 | LL | println!("error: {}", format!("something failed at {} {0}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -63,7 +63,7 @@ LL | println!("error: {}", format!("something failed at {} {0}", Location::c = help: or consider changing `format!` to `format_args!` error: `format!` in `format!` args - --> $DIR/format_args_unfixable.rs:32:13 + --> $DIR/format_args_unfixable.rs:33:13 | LL | let _ = format!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | let _ = format!("error: {}", format!("something failed at {}", Location = help: or consider changing `format!` to `format_args!` error: `format!` in `write!` args - --> $DIR/format_args_unfixable.rs:33:13 + --> $DIR/format_args_unfixable.rs:34:13 | LL | let _ = write!( | _____________^ @@ -86,7 +86,7 @@ LL | | ); = help: or consider changing `format!` to `format_args!` error: `format!` in `writeln!` args - --> $DIR/format_args_unfixable.rs:38:13 + --> $DIR/format_args_unfixable.rs:39:13 | LL | let _ = writeln!( | _____________^ @@ -100,7 +100,7 @@ LL | | ); = help: or consider changing `format!` to `format_args!` error: `format!` in `print!` args - --> $DIR/format_args_unfixable.rs:43:5 + --> $DIR/format_args_unfixable.rs:44:5 | LL | print!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -109,7 +109,7 @@ LL | print!("error: {}", format!("something failed at {}", Location::caller( = help: or consider changing `format!` to `format_args!` error: `format!` in `eprint!` args - --> $DIR/format_args_unfixable.rs:44:5 + --> $DIR/format_args_unfixable.rs:45:5 | LL | eprint!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -118,7 +118,7 @@ LL | eprint!("error: {}", format!("something failed at {}", Location::caller = help: or consider changing `format!` to `format_args!` error: `format!` in `eprintln!` args - --> $DIR/format_args_unfixable.rs:45:5 + --> $DIR/format_args_unfixable.rs:46:5 | LL | eprintln!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -127,7 +127,7 @@ LL | eprintln!("error: {}", format!("something failed at {}", Location::call = help: or consider changing `format!` to `format_args!` error: `format!` in `format_args!` args - --> $DIR/format_args_unfixable.rs:46:13 + --> $DIR/format_args_unfixable.rs:47:13 | LL | let _ = format_args!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -136,7 +136,7 @@ LL | let _ = format_args!("error: {}", format!("something failed at {}", Loc = help: or consider changing `format!` to `format_args!` error: `format!` in `assert!` args - --> $DIR/format_args_unfixable.rs:47:5 + --> $DIR/format_args_unfixable.rs:48:5 | LL | assert!(true, "error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -145,7 +145,7 @@ LL | assert!(true, "error: {}", format!("something failed at {}", Location:: = help: or consider changing `format!` to `format_args!` error: `format!` in `assert_eq!` args - --> $DIR/format_args_unfixable.rs:48:5 + --> $DIR/format_args_unfixable.rs:49:5 | LL | assert_eq!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -154,7 +154,7 @@ LL | assert_eq!(0, 0, "error: {}", format!("something failed at {}", Locatio = help: or consider changing `format!` to `format_args!` error: `format!` in `assert_ne!` args - --> $DIR/format_args_unfixable.rs:49:5 + --> $DIR/format_args_unfixable.rs:50:5 | LL | assert_ne!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -163,7 +163,7 @@ LL | assert_ne!(0, 0, "error: {}", format!("something failed at {}", Locatio = help: or consider changing `format!` to `format_args!` error: `format!` in `panic!` args - --> $DIR/format_args_unfixable.rs:50:5 + --> $DIR/format_args_unfixable.rs:51:5 | LL | panic!("error: {}", format!("something failed at {}", Location::caller())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed index 5ecb9a5bf..3122081a4 100644 --- a/tests/ui/uninlined_format_args.fixed +++ b/tests/ui/uninlined_format_args.fixed @@ -1,7 +1,7 @@ // aux-build:proc_macros.rs // run-rustfix #![warn(clippy::uninlined_format_args)] -#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)] +#![allow(named_arguments_used_positionally, unused)] #![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)] extern crate proc_macros; @@ -178,3 +178,87 @@ fn _meets_msrv() { fn _do_not_fire() { println!("{:?}", None::<()>); } + +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, + }; +} diff --git a/tests/ui/uninlined_format_args.rs b/tests/ui/uninlined_format_args.rs index 835afac39..b153ef256 100644 --- a/tests/ui/uninlined_format_args.rs +++ b/tests/ui/uninlined_format_args.rs @@ -1,7 +1,7 @@ // aux-build:proc_macros.rs // run-rustfix #![warn(clippy::uninlined_format_args)] -#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)] +#![allow(named_arguments_used_positionally, unused)] #![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)] extern crate proc_macros; @@ -183,3 +183,87 @@ fn _meets_msrv() { fn _do_not_fire() { println!("{:?}", None::<()>); } + +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, + }; +}