diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 61f550ce0beb..8d06a5a6dd37 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -1,3 +1,4 @@ +use crate::write::extract_str_literal; use arrayvec::ArrayVec; use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; @@ -9,6 +10,7 @@ use clippy_utils::macros::{ use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_type_lang_item}; use itertools::Itertools; +use rustc_ast::token::LitKind; use rustc_ast::{ FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions, FormatPlaceholder, FormatTrait, @@ -324,8 +326,8 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { // 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) - for (pos, usage) in self.format_arg_positions() { - if !self.check_one_arg(pos, usage, &mut fixes) { + for (pos, usage, pl) in self.format_arg_positions() { + if !self.check_one_arg(pos, usage, pl, &mut fixes) { return; } } @@ -360,9 +362,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { ); } - fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool { + fn check_one_arg( + &self, + pos: &FormatArgPosition, + usage: FormatParamUsage, + placeholder: Option<&FormatPlaceholder>, + fixes: &mut Vec<(Span, String)>, + ) -> bool { let index = pos.index.unwrap(); - let arg = &self.format_args.arguments.all_args()[index]; + // If the argument is not found by its index, something is weird, ignore and stop processing this + // case. + let Some(arg) = &self.format_args.arguments.by_index(index) else { + return false; + }; if !matches!(arg.kind, FormatArgumentKind::Captured(_)) && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind @@ -379,6 +391,21 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { fixes.push((pos_span, replacement)); fixes.push((arg_span, String::new())); true // successful inlining, continue checking + } else if !matches!(arg.kind, FormatArgumentKind::Captured(_)) + && let Some(FormatPlaceholder{span: Some(placeholder_span), ..}) = placeholder + && let rustc_ast::ExprKind::Lit(lit) = &arg.expr.kind + && lit.kind == LitKind::Str // FIXME: lit.kind must match the format string kind + && !arg.expr.span.from_expansion() + && let Some(value_string) = snippet_opt(self.cx, arg.expr.span) + && let Some((value_string, false)) = extract_str_literal(&value_string) // FIXME: handle raw strings + // && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind + // && let [segment] = path.segments.as_slice() + // && segment.args.is_none() + && let Some(arg_span) = format_arg_removal_span(self.format_args, index) + { + fixes.push((*placeholder_span, value_string)); + fixes.push((arg_span, String::new())); + true // successful inlining, continue checking } else { // Do not continue inlining (return false) in case // * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)` @@ -454,17 +481,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { } } - fn format_arg_positions(&self) -> impl Iterator { + fn format_arg_positions( + &self, + ) -> impl Iterator)> { self.format_args.template.iter().flat_map(|piece| match piece { FormatArgsPiece::Placeholder(placeholder) => { let mut positions = ArrayVec::<_, 3>::new(); - positions.push((&placeholder.argument, FormatParamUsage::Argument)); + positions.push((&placeholder.argument, FormatParamUsage::Argument, Some(placeholder))); if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width { - positions.push((position, FormatParamUsage::Width)); + positions.push((position, FormatParamUsage::Width, None)); } if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision { - positions.push((position, FormatParamUsage::Precision)); + positions.push((position, FormatParamUsage::Precision, None)); } positions @@ -476,7 +505,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> { /// Returns true if the format argument at `index` is referred to by multiple format params fn is_aliased(&self, index: usize) -> bool { self.format_arg_positions() - .filter(|(position, _)| position.index == Ok(index)) + .filter(|(position, ..)| position.index == Ok(index)) .at_most_one() .is_err() } diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index be16d2e5cc30..4957f94ca904 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -594,7 +594,7 @@ fn positional_arg_piece_span(piece: &FormatArgsPiece) -> Option<(Span, usize)> { /// `r#"a"#` -> (`a`, true) /// /// `"b"` -> (`b`, false) -fn extract_str_literal(literal: &str) -> Option<(String, bool)> { +pub fn extract_str_literal(literal: &str) -> Option<(String, bool)> { let (literal, raw) = match literal.strip_prefix('r') { Some(stripped) => (stripped.trim_matches('#'), true), None => (literal, false), 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 index b754f67edfb6..b746dcfa7fd5 100644 --- 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 @@ -21,7 +21,7 @@ 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"); +LL + println!("Hello x is {local_f64:.local_i32$}"); | error: literal with an empty format string diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed index 3f5b0e52ece0..8336ac95d0ca 100644 --- a/tests/ui/uninlined_format_args.fixed +++ b/tests/ui/uninlined_format_args.fixed @@ -59,7 +59,7 @@ fn tester(fn_arg: i32) { println!("{local_i32:<3}"); println!("{local_i32:#010x}"); println!("{local_f64:.1}"); - println!("Hello {} is {:.*}", "x", local_i32, local_f64); + println!("Hello x is {local_f64:.local_i32$}"); println!("Hello {} is {:.*}", local_i32, 5, local_f64); println!("Hello {} is {2:.*}", local_i32, 5, local_f64); println!("{local_i32} {local_f64}"); @@ -83,12 +83,12 @@ fn tester(fn_arg: i32) { println!("{local_i32} {local_f64}"); println!("{local_f64} {local_i32}"); println!("{local_f64} {local_i32} {local_f64} {local_i32}"); - println!("{1} {0}", "str", local_i32); + println!("{local_i32} str"); println!("{local_i32}"); - println!("{local_i32:width$}"); - println!("{local_i32:width$}"); - println!("{local_i32:.prec$}"); - println!("{local_i32:.prec$}"); + println!("{local_i32:0$}", width); + println!("{local_i32:w$}", w = width); + println!("{local_i32:.0$}", prec); + println!("{local_i32:.p$}", p = prec); println!("{val:val$}"); println!("{val:val$}"); println!("{val:val$.val$}"); @@ -267,3 +267,17 @@ fn tester2() { local_i32, }; } + +fn literals() { + let var = 5; + println!("foo"); + println!("foo"); + println!("{:var$}", "foo"); + println!("foo"); + println!("{0:1$}", "foo", 5); + println!("var {var} lit foo"); + println!("var {var} lit foo"); + println!("var foo lit foo"); + println!("var foo lit foo {var},"); + println!("var {0} lit {} {},", "foo", 5); +} diff --git a/tests/ui/uninlined_format_args.rs b/tests/ui/uninlined_format_args.rs index b311aa4912cd..b6aa8f5774be 100644 --- a/tests/ui/uninlined_format_args.rs +++ b/tests/ui/uninlined_format_args.rs @@ -272,3 +272,17 @@ fn tester2() { local_i32, }; } + +fn literals() { + let var = 5; + println!("{}", "foo"); + println!("{:5}", "foo"); + println!("{:var$}", "foo"); + println!("{:-5}", "foo"); + println!("{0:1$}", "foo", 5); + println!("var {} lit {}", var, "foo"); + println!("var {1} lit {0}", "foo", var); + println!("var {} lit {0}", "foo"); + println!("var {0} lit {} {},", "foo", var); + println!("var {0} lit {} {},", "foo", 5); +} diff --git a/tests/ui/uninlined_format_args.stderr b/tests/ui/uninlined_format_args.stderr index 829d646b8669..d9fc7039b6b6 100644 --- a/tests/ui/uninlined_format_args.stderr +++ b/tests/ui/uninlined_format_args.stderr @@ -178,6 +178,18 @@ 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:64: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 x is {local_f64:.local_i32$}"); + | + error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:67:5 | @@ -407,63 +419,27 @@ LL + println!("{local_f64} {local_i32} {local_f64} {local_i32}"); | error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:89:5 - | -LL | println!("{v}", v = local_i32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("{v}", v = local_i32); -LL + println!("{local_i32}"); - | - -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:90:5 - | -LL | println!("{local_i32:0$}", width); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("{local_i32:0$}", width); -LL + println!("{local_i32:width$}"); - | - -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:91:5 + --> $DIR/uninlined_format_args.rs:88:5 | -LL | println!("{local_i32:w$}", w = width); +LL | println!("{1} {0}", "str", local_i32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: change this to | -LL - println!("{local_i32:w$}", w = width); -LL + println!("{local_i32:width$}"); +LL - println!("{1} {0}", "str", local_i32); +LL + println!("{local_i32} str"); | error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:92:5 - | -LL | println!("{local_i32:.0$}", prec); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: change this to - | -LL - println!("{local_i32:.0$}", prec); -LL + println!("{local_i32:.prec$}"); - | - -error: variables can be used directly in the `format!` string - --> $DIR/uninlined_format_args.rs:93:5 + --> $DIR/uninlined_format_args.rs:89:5 | -LL | println!("{local_i32:.p$}", p = prec); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | println!("{v}", v = local_i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: change this to | -LL - println!("{local_i32:.p$}", p = prec); -LL + println!("{local_i32:.prec$}"); +LL - println!("{v}", v = local_i32); +LL + println!("{local_i32}"); | error: variables can be used directly in the `format!` string @@ -845,5 +821,89 @@ 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 + --> $DIR/uninlined_format_args.rs:278:5 + | +LL | println!("{}", "foo"); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("{}", "foo"); +LL + println!("foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:279:5 + | +LL | println!("{:5}", "foo"); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("{:5}", "foo"); +LL + println!("foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:281:5 + | +LL | println!("{:-5}", "foo"); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("{:-5}", "foo"); +LL + println!("foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:283:5 + | +LL | println!("var {} lit {}", var, "foo"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("var {} lit {}", var, "foo"); +LL + println!("var {var} lit foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:284:5 + | +LL | println!("var {1} lit {0}", "foo", var); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("var {1} lit {0}", "foo", var); +LL + println!("var {var} lit foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:285:5 + | +LL | println!("var {} lit {0}", "foo"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("var {} lit {0}", "foo"); +LL + println!("var foo lit foo"); + | + +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:286:5 + | +LL | println!("var {0} lit {} {},", "foo", var); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: change this to + | +LL - println!("var {0} lit {} {},", "foo", var); +LL + println!("var foo lit foo {var},"); + | + +error: aborting due to 76 previous errors