Enhance uninlined_format_args to inline str and char literals

Author: profetia

clippy_lints/src/format_args.rs

@@ -407,8 +407,8 @@ impl<'tcx> FormatArgsExpr<'_, '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 (placeholder, pos, usage) in self.format_arg_positions() {
+            if !self.check_one_arg(placeholder, pos, usage, &mut fixes) {
                 return;
             }
         }
@@ -443,31 +443,64 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> {
         );
     }
 
-    fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool {
+    fn check_one_arg(
+        &self,
+        placeholder: &FormatPlaceholder,
+        pos: &FormatArgPosition,
+        usage: FormatParamUsage,
+        fixes: &mut Vec<(Span, String)>,
+    ) -> bool {
         let index = pos.index.unwrap();
         let arg = &self.format_args.arguments.all_args()[index];
 
-        if !matches!(arg.kind, FormatArgumentKind::Captured(_))
-            && 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)
-            && let Some(pos_span) = pos.span
-        {
-            let replacement = match usage {
-                FormatParamUsage::Argument => segment.ident.name.to_string(),
-                FormatParamUsage::Width => format!("{}$", segment.ident.name),
-                FormatParamUsage::Precision => format!(".{}$", segment.ident.name),
-            };
-            fixes.push((pos_span, replacement));
-            fixes.push((arg_span, String::new()));
-            true // successful inlining, continue checking
-        } else {
+        if matches!(arg.kind, FormatArgumentKind::Captured(_)) {
             // 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
             pos.kind != FormatArgPositionKind::Number
                 && (!self.ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured(_)))
+        } else {
+            match &arg.expr.kind {
+                rustc_ast::ExprKind::Path(None, path)
+                    if let [segment] = path.segments.as_slice()
+                        && segment.args.is_none()
+                        && let Some(arg_span) = format_arg_removal_span(self.format_args, index)
+                        && let Some(pos_span) = pos.span =>
+                {
+                    let replacement = match usage {
+                        FormatParamUsage::Argument => segment.ident.name.to_string(),
+                        FormatParamUsage::Width => format!("{}$", segment.ident.name),
+                        FormatParamUsage::Precision => format!(".{}$", segment.ident.name),
+                    };
+                    fixes.push((pos_span, replacement));
+                    fixes.push((arg_span, String::new()));
+                    true // successful inlining, continue checking
+                },
+                rustc_ast::ExprKind::Lit(lit)
+                    if matches!(
+                        lit.kind,
+                        rustc_ast::token::LitKind::Str | rustc_ast::token::LitKind::Char
+                    ) && matches!(usage, FormatParamUsage::Argument)
+                        // Only inline string and char literals when there are no format options
+                        && placeholder.format_options == FormatOptions::default()
+                        && let Some(pos_span) = pos.span
+                        && let Some(arg_span) = format_arg_removal_span(self.format_args, index)
+                        && arg.expr.span.eq_ctxt(arg_span) =>
+                {
+                    // Skip the surrounding `{` and `}` when replacing
+                    let pos_span = pos_span
+                        .with_lo(pos_span.lo() - rustc_span::BytePos(1))
+                        .with_hi(pos_span.hi() + rustc_span::BytePos(1));
+                    let replacement = lit.symbol.to_string();
+                    fixes.push((pos_span, replacement));
+                    fixes.push((arg_span, String::new()));
+                    true
+                },
+                _ => {
+                    pos.kind != FormatArgPositionKind::Number
+                        && (!self.ignore_mixed || matches!(arg.kind, FormatArgumentKind::Captured(_)))
+                },
+            }
         }
     }
 
@@ -578,17 +611,17 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> {
         }
     }
 
-    fn format_arg_positions(&self) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
+    fn format_arg_positions(&self) -> impl Iterator<Item = (&FormatPlaceholder, &FormatArgPosition, FormatParamUsage)> {
         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, &placeholder.argument, FormatParamUsage::Argument));
                 if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width {
-                    positions.push((position, FormatParamUsage::Width));
+                    positions.push((placeholder, position, FormatParamUsage::Width));
                 }
                 if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision {
-                    positions.push((position, FormatParamUsage::Precision));
+                    positions.push((placeholder, position, FormatParamUsage::Precision));
                 }
 
                 positions
@@ -600,7 +633,7 @@ impl<'tcx> FormatArgsExpr<'_, '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()
     }

tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.fixed

@@ -18,3 +18,11 @@ fn main() {
     println!("{local_i32}, {}", local_opt.unwrap());
     //~^ uninlined_format_args
 }
+
+fn issue16310() {
+    let x = 1;
+    format!("The answer is: 42 and x is {x}");
+    //~^ uninlined_format_args
+    format!("The answer is: 4 and x is {x}");
+    //~^ uninlined_format_args
+}

tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs

@@ -18,3 +18,11 @@ fn main() {
     println!("{}, {}", local_i32, local_opt.unwrap());
     //~^ uninlined_format_args
 }
+
+fn issue16310() {
+    let x = 1;
+    format!("The answer is: {} and x is {}", "42", x);
+    //~^ uninlined_format_args
+    format!("The answer is: {} and x is {}", '4', x);
+    //~^ uninlined_format_args
+}

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
@@ -74,5 +74,29 @@ LL -     println!("{}, {}", local_i32, local_opt.unwrap());
 LL +     println!("{local_i32}, {}", local_opt.unwrap());
    |
 
-error: aborting due to 6 previous errors
+error: variables can be used directly in the `format!` string
+  --> tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs:24:5
+   |
+LL |     format!("The answer is: {} and x is {}", "42", x);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: change this to
+   |
+LL -     format!("The answer is: {} and x is {}", "42", x);
+LL +     format!("The answer is: 42 and x is {x}");
+   |
+
+error: variables can be used directly in the `format!` string
+  --> tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.rs:26:5
+   |
+LL |     format!("The answer is: {} and x is {}", '4', x);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: change this to
+   |
+LL -     format!("The answer is: {} and x is {}", '4', x);
+LL +     format!("The answer is: 4 and x is {x}");
+   |
+
+error: aborting due to 8 previous errors
 

tests/ui/uninlined_format_args.fixed

@@ -6,7 +6,8 @@
     clippy::eq_op,
     clippy::format_in_format_args,
     clippy::print_literal,
-    clippy::unnecessary_literal_unwrap
+    clippy::unnecessary_literal_unwrap,
+    clippy::useless_format
 )]
 
 extern crate proc_macros;
@@ -78,7 +79,8 @@ fn tester(fn_arg: i32) {
     //~^ uninlined_format_args
     println!("{local_f64:.1}");
     //~^ uninlined_format_args
-    println!("Hello {} is {:.*}", "x", local_i32, local_f64);
+    println!("Hello x is {local_f64:.local_i32$}");
+    //~^ uninlined_format_args
     println!("Hello {} is {:.*}", local_i32, 5, local_f64);
     println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
     println!("{local_i32} {local_f64}");
@@ -121,7 +123,8 @@ fn tester(fn_arg: i32) {
     //~^ uninlined_format_args
     println!("{local_f64} {local_i32} {local_f64} {local_i32}");
     //~^ uninlined_format_args
-    println!("{1} {0}", "str", local_i32);
+    println!("{local_i32} str");
+    //~^ uninlined_format_args
     println!("{local_i32}");
     //~^ uninlined_format_args
     println!("{local_i32:width$}");
@@ -363,3 +366,16 @@ fn user_format() {
     usr_println!(true, "{local_f64:.1}");
     //~^ uninlined_format_args
 }
+
+fn issue16310() {
+    format!("The answer is: 42");
+    //~^ uninlined_format_args
+    format!("The answer is: 4");
+    //~^ uninlined_format_args
+
+    let n_refs = 10;
+    let receiver_snippet = "receiver_snippet";
+    format!("{:&>n_refs$}{receiver_snippet}", "");
+
+    format!("env: {}", env!("USER"));
+}

tests/ui/uninlined_format_args.rs

@@ -6,7 +6,8 @@
     clippy::eq_op,
     clippy::format_in_format_args,
     clippy::print_literal,
-    clippy::unnecessary_literal_unwrap
+    clippy::unnecessary_literal_unwrap,
+    clippy::useless_format
 )]
 
 extern crate proc_macros;
@@ -81,6 +82,7 @@ fn tester(fn_arg: i32) {
     println!("{:.1}", local_f64);
     //~^ uninlined_format_args
     println!("Hello {} is {:.*}", "x", local_i32, local_f64);
+    //~^ uninlined_format_args
     println!("Hello {} is {:.*}", local_i32, 5, local_f64);
     println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
     println!("{} {}", local_i32, local_f64);
@@ -124,6 +126,7 @@ fn tester(fn_arg: i32) {
     println!("{1} {0} {1} {0}", local_i32, local_f64);
     //~^ uninlined_format_args
     println!("{1} {0}", "str", local_i32);
+    //~^ uninlined_format_args
     println!("{v}", v = local_i32);
     //~^ uninlined_format_args
     println!("{local_i32:0$}", width);
@@ -368,3 +371,16 @@ fn user_format() {
     usr_println!(true, "{:.1}", local_f64);
     //~^ uninlined_format_args
 }
+
+fn issue16310() {
+    format!("The answer is: {}", "42");
+    //~^ uninlined_format_args
+    format!("The answer is: {}", '4');
+    //~^ uninlined_format_args
+
+    let n_refs = 10;
+    let receiver_snippet = "receiver_snippet";
+    format!("{:&>n_refs$}{receiver_snippet}", "");
+
+    format!("env: {}", env!("USER"));
+}