diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index fd3ce2f3d6c..f0995a81329 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -1,6 +1,6 @@ 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::FormatParamKind::{Implicit, Named, NamedInline, Numbered, Starred}; use clippy_utils::macros::{ is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage, }; @@ -106,19 +106,25 @@ declare_clippy_lint! { /// format!("{var:.prec$}"); /// ``` /// - /// ### Known Problems - /// - /// There may be a false positive if the format string is expanded from certain proc macros: - /// - /// ```ignore - /// println!(indoc!("{}"), var); + /// If allow-mixed-uninlined-format-args is set to false in clippy.toml, + /// the following code will also trigger the lint: + /// ```rust + /// # let var = 42; + /// format!("{} {}", var, 1+2); /// ``` + /// Use instead: + /// ```rust + /// # let var = 42; + /// format!("{var} {}", 1+2); + /// ``` + /// + /// ### Known Problems /// /// If a format string contains a numbered argument that cannot be inlined /// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`. #[clippy::version = "1.65.0"] pub UNINLINED_FORMAT_ARGS, - pedantic, + style, "using non-inlined variables in `format!` calls" } @@ -162,12 +168,16 @@ impl_lint_pass!(FormatArgs => [ pub struct FormatArgs { msrv: Msrv, + ignore_mixed: bool, } impl FormatArgs { #[must_use] - pub fn new(msrv: Msrv) -> Self { - Self { msrv } + pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self { + Self { + msrv, + ignore_mixed: allow_mixed_uninlined_format_args, + } } } @@ -192,7 +202,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { check_to_string_in_format_args(cx, name, arg.param.value); } if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) { - check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id); + check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id, self.ignore_mixed); } } } @@ -270,7 +280,13 @@ fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) { } } -fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) { +fn check_uninlined_args( + cx: &LateContext<'_>, + args: &FormatArgsExpn<'_>, + call_site: Span, + def_id: DefId, + ignore_mixed: bool, +) { if args.format_string.span.from_expansion() { return; } @@ -285,7 +301,7 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si // we cannot remove any other arguments in the format string, // because the index numbers might be wrong after inlining. // Example of an un-inlinable format: print!("{}{1}", foo, 2) - if !args.params().all(|p| check_one_arg(args, &p, &mut fixes)) || fixes.is_empty() { + if !args.params().all(|p| check_one_arg(args, &p, &mut fixes, ignore_mixed)) || fixes.is_empty() { return; } @@ -309,7 +325,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si ); } -fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool { +fn check_one_arg( + args: &FormatArgsExpn<'_>, + param: &FormatParam<'_>, + fixes: &mut Vec<(Span, String)>, + ignore_mixed: bool, +) -> bool { if matches!(param.kind, Implicit | Starred | Named(_) | Numbered) && let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind && let [segment] = path.segments @@ -324,8 +345,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut fixes.push((arg_span, String::new())); true // successful inlining, continue checking } else { - // if we can't inline a numbered argument, we can't continue - param.kind != Numbered + // Do not continue inlining (return false) in case + // * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)` + // * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already + param.kind != Numbered && (!ignore_mixed || matches!(param.kind, NamedInline(_))) } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 17dbd983b6c..3fe39488ab8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -828,7 +828,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: )) }); store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks)); - store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv()))); + let allow_mixed_uninlined = conf.allow_mixed_uninlined_format_args; + store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined))); 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/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index b37d4239477..b6dc8cd7ab1 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -402,6 +402,10 @@ define_Conf! { /// A list of paths to types that should be treated like `Arc`, i.e. ignored but /// for the generic parameters for determining interior mutability (ignore_interior_mutability: Vec = Vec::from(["bytes::Bytes".into()])), + /// Lint: UNINLINED_FORMAT_ARGS. + /// + /// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)` + (allow_mixed_uninlined_format_args: bool = true), } /// Search for the configuration file. diff --git a/tests/ui-toml/allow_mixed_uninlined_format_args/clippy.toml b/tests/ui-toml/allow_mixed_uninlined_format_args/clippy.toml new file mode 100644 index 00000000000..b95e806aae2 --- /dev/null +++ b/tests/ui-toml/allow_mixed_uninlined_format_args/clippy.toml @@ -0,0 +1 @@ +allow-mixed-uninlined-format-args = false diff --git a/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.fixed b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.fixed new file mode 100644 index 00000000000..aa8b45b5fe7 --- /dev/null +++ b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.fixed @@ -0,0 +1,14 @@ +// run-rustfix +#![warn(clippy::uninlined_format_args)] + +fn main() { + let local_i32 = 1; + let local_f64 = 2.0; + let local_opt: Option = Some(3); + + println!("val='{local_i32}'"); + println!("Hello x is {local_f64:.local_i32$}"); + println!("Hello {local_i32} is {local_f64:.*}", 5); + println!("Hello {local_i32} is {local_f64:.*}", 5); + println!("{local_i32}, {}", local_opt.unwrap()); +} diff --git a/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs new file mode 100644 index 00000000000..ad2e4863ee8 --- /dev/null +++ b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs @@ -0,0 +1,14 @@ +// run-rustfix +#![warn(clippy::uninlined_format_args)] + +fn main() { + let local_i32 = 1; + let local_f64 = 2.0; + let local_opt: Option = Some(3); + + println!("val='{}'", local_i32); + println!("Hello {} is {:.*}", "x", local_i32, local_f64); + println!("Hello {} is {:.*}", local_i32, 5, local_f64); + println!("Hello {} is {2:.*}", local_i32, 5, local_f64); + println!("{}, {}", local_i32, local_opt.unwrap()); +} diff --git a/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.stderr b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.stderr new file mode 100644 index 00000000000..ee941762196 --- /dev/null +++ b/tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.stderr @@ -0,0 +1,76 @@ +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:9:5 + | +LL | println!("val='{}'", local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::uninlined-format-args` implied by `-D warnings` +help: change this to + | +LL - println!("val='{}'", local_i32); +LL + println!("val='{local_i32}'"); + | + +error: literal with an empty format string + --> $DIR/uninlined_format_args.rs:10:35 + | +LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64); + | ^^^ + | + = note: `-D clippy::print-literal` implied by `-D warnings` +help: try this + | +LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64); +LL + println!("Hello x is {:.*}", local_i32, local_f64); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:10:5 + | +LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64); +LL + println!("Hello {} is {local_f64:.local_i32$}", "x"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:11:5 + | +LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64); +LL + println!("Hello {local_i32} is {local_f64:.*}", 5); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:12:5 + | +LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64); +LL + println!("Hello {local_i32} is {local_f64:.*}", 5); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:13:5 + | +LL | println!("{}, {}", local_i32, local_opt.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("{}, {}", local_i32, local_opt.unwrap()); +LL + println!("{local_i32}, {}", local_opt.unwrap()); + | + +error: aborting due to 6 previous errors + 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 f91d285c2e0..01a5e962c94 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,6 +1,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of allow-dbg-in-tests allow-expect-in-tests + allow-mixed-uninlined-format-args allow-print-in-tests allow-unwrap-in-tests allowed-scripts diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed index 46ec7771114..9d08e80cf9a 100644 --- a/tests/ui/uninlined_format_args.fixed +++ b/tests/ui/uninlined_format_args.fixed @@ -54,11 +54,11 @@ fn tester(fn_arg: i32) { println!("{local_i32:<3}"); println!("{local_i32:#010x}"); println!("{local_f64:.1}"); - println!("Hello {} is {local_f64:.local_i32$}", "x"); - println!("Hello {local_i32} is {local_f64:.*}", 5); - println!("Hello {local_i32} is {local_f64:.*}", 5); + println!("Hello {} is {:.*}", "x", local_i32, local_f64); + println!("Hello {} is {:.*}", local_i32, 5, local_f64); + println!("Hello {} is {2:.*}", local_i32, 5, local_f64); println!("{local_i32} {local_f64}"); - println!("{local_i32}, {}", local_opt.unwrap()); + println!("{}, {}", local_i32, local_opt.unwrap()); println!("{val}"); println!("{val}"); println!("{} {1}", local_i32, 42); diff --git a/tests/ui/uninlined_format_args.stderr b/tests/ui/uninlined_format_args.stderr index 05ed5b6616c..a12abf8bef8 100644 --- a/tests/ui/uninlined_format_args.stderr +++ b/tests/ui/uninlined_format_args.stderr @@ -177,42 +177,6 @@ LL - println!("{:.1}", local_f64); LL + println!("{local_f64:.1}"); | -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:59:5 - | -LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64); -LL + println!("Hello {} is {local_f64:.local_i32$}", "x"); - | - -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:60:5 - | -LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64); -LL + println!("Hello {local_i32} is {local_f64:.*}", 5); - | - -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:61:5 - | -LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64); -LL + println!("Hello {local_i32} is {local_f64:.*}", 5); - | - error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:62:5 | @@ -225,18 +189,6 @@ LL - println!("{} {}", local_i32, local_f64); LL + println!("{local_i32} {local_f64}"); | -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:63:5 - | -LL | println!("{}, {}", local_i32, local_opt.unwrap()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("{}, {}", local_i32, local_opt.unwrap()); -LL + println!("{local_i32}, {}", local_opt.unwrap()); - | - error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:64:5 | @@ -904,5 +856,5 @@ LL - println!("expand='{}'", local_i32); LL + println!("expand='{local_i32}'"); | -error: aborting due to 76 previous errors +error: aborting due to 72 previous errors