diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 6a1d7cb852a2..d2ce1e437a4b 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -587,6 +587,11 @@ define_Conf! { /// 2. Paths with any segment that containing the word 'prelude' /// are already allowed by default. (allowed_wildcard_imports: FxHashSet = FxHashSet::default()), + /// Lint: FORMAT_IN_FORMAT_ARGS, RECURSIVE_FORMAT_IMPL, TO_STRING_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, UNUSED_FORMAT_SPECS. + /// + /// Recognize all macros that internally use `format_args!` and similar macros + /// as format macros, instead of just the ones in the standard library. + (include_custom_format_macros: bool = false), } /// Search for the configuration file. diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 8af321e4d553..7b7f32fe59de 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -171,14 +171,16 @@ impl_lint_pass!(FormatArgs => [ pub struct FormatArgs { msrv: Msrv, ignore_mixed: bool, + include_custom: bool, } impl FormatArgs { #[must_use] - pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self { + pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool, include_custom_format_macros: bool) -> Self { Self { msrv, ignore_mixed: allow_mixed_uninlined_format_args, + include_custom: include_custom_format_macros, } } } @@ -186,7 +188,7 @@ impl FormatArgs { impl<'tcx> LateLintPass<'tcx> for FormatArgs { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if let Some(macro_call) = root_macro_call_first_node(cx, expr) - && is_format_macro(cx, macro_call.def_id) + && (self.include_custom || is_format_macro(cx, macro_call.def_id)) && let Some(format_args) = find_format_args(cx, expr, macro_call.expn) { for piece in &format_args.template { diff --git a/clippy_lints/src/format_impl.rs b/clippy_lints/src/format_impl.rs index 93517076cda0..8c3fd111bfaf 100644 --- a/clippy_lints/src/format_impl.rs +++ b/clippy_lints/src/format_impl.rs @@ -101,12 +101,14 @@ struct FormatTraitNames { pub struct FormatImpl { // Whether we are inside Display or Debug trait impl - None for neither format_trait_impl: Option, + include_custom: bool, } impl FormatImpl { - pub fn new() -> Self { + pub fn new(include_custom: bool) -> Self { Self { format_trait_impl: None, + include_custom, } } } @@ -131,6 +133,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl { cx, expr, format_trait_impl, + include_custom: self.include_custom, }; linter.check_to_string_in_display(); linter.check_self_in_format_args(); @@ -143,6 +146,7 @@ struct FormatImplExpr<'a, 'tcx> { cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>, format_trait_impl: FormatTraitNames, + include_custom: bool, } impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> { @@ -174,7 +178,7 @@ impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> { // Check each arg in format calls - do we ever use Display on self (directly or via deref)? if let Some(outer_macro) = root_macro_call_first_node(self.cx, self.expr) && let macro_def_id = outer_macro.def_id - && is_format_macro(self.cx, macro_def_id) + && (self.include_custom || is_format_macro(self.cx, macro_def_id)) && let Some(format_args) = find_format_args(self.cx, self.expr, outer_macro.expn) { for piece in &format_args.template { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5636f46b22fe..3b24cc2cc936 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -546,6 +546,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { excessive_nesting_threshold, future_size_threshold, ref ignore_interior_mutability, + include_custom_format_macros, large_error_threshold, literal_representation_threshold, matches_for_let_else, @@ -817,7 +818,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(mut_key::MutableKeyType::new(ignore_interior_mutability.clone()))); store.register_early_pass(|| Box::new(reference::DerefAddrOf)); store.register_early_pass(|| Box::new(double_parens::DoubleParens)); - store.register_late_pass(|_| Box::new(format_impl::FormatImpl::new())); + store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(include_custom_format_macros))); store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval)); store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse)); store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne)); @@ -942,8 +943,13 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { accept_comment_above_attributes, )) }); - store - .register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined_format_args))); + store.register_late_pass(move |_| { + Box::new(format_args::FormatArgs::new( + msrv(), + allow_mixed_uninlined_format_args, + include_custom_format_macros, + )) + }); store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray)); store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit)); diff --git a/tests/ui-toml/include_custom_format_macros/clippy.toml b/tests/ui-toml/include_custom_format_macros/clippy.toml new file mode 100644 index 000000000000..5a186ea3a8d4 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/clippy.toml @@ -0,0 +1 @@ +include-custom-format-macros = true diff --git a/tests/ui-toml/include_custom_format_macros/format_args_unfixable.rs b/tests/ui-toml/include_custom_format_macros/format_args_unfixable.rs new file mode 100644 index 000000000000..85e39ddbfe88 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/format_args_unfixable.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-toml/include_custom_format_macros/uninlined_format_args.fixed b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.fixed new file mode 100644 index 000000000000..703b64de83c9 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.fixed @@ -0,0 +1,88 @@ +#![warn(clippy::uninlined_format_args)] +#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)] +#![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)] + +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 main() { + let local_i32 = 1; + println!("val='{local_i32}'"); + 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-toml/include_custom_format_macros/uninlined_format_args.rs b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.rs new file mode 100644 index 000000000000..8fc84c7599f9 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.rs @@ -0,0 +1,88 @@ +#![warn(clippy::uninlined_format_args)] +#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)] +#![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)] + +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 main() { + let local_i32 = 1; + println!("val='{}'", local_i32); + 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-toml/include_custom_format_macros/uninlined_format_args.stderr b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.stderr new file mode 100644 index 000000000000..98f79d3e9029 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/uninlined_format_args.stderr @@ -0,0 +1,40 @@ +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:73:5 + | +LL | println!("val='{}'", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::uninlined-format-args` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::uninlined_format_args)]` +help: change this to + | +LL - println!("val='{}'", local_i32); +LL + println!("val='{local_i32}'"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:77: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:78: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 3 previous errors + diff --git a/tests/ui-toml/include_custom_format_macros/unused_format_specs.rs b/tests/ui-toml/include_custom_format_macros/unused_format_specs.rs new file mode 100644 index 000000000000..4abb01381374 --- /dev/null +++ b/tests/ui-toml/include_custom_format_macros/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); +} diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index f097d2503e16..dc26bb82fdb5 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -40,6 +40,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect excessive-nesting-threshold future-size-threshold ignore-interior-mutability + include-custom-format-macros large-error-threshold literal-representation-threshold matches-for-let-else @@ -119,6 +120,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect excessive-nesting-threshold future-size-threshold ignore-interior-mutability + include-custom-format-macros large-error-threshold literal-representation-threshold matches-for-let-else @@ -198,6 +200,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni excessive-nesting-threshold future-size-threshold ignore-interior-mutability + include-custom-format-macros large-error-threshold literal-representation-threshold matches-for-let-else