review feedback

Author: nyurik

clippy_lints/src/format_args.rs

@@ -105,9 +105,7 @@ declare_clippy_lint! {
     /// Format string uses an indexed argument that cannot be inlined.
     /// Supporting this case requires re-indexing of the format string.
     ///
-    /// Until implemnted, `print!("{0}={1}", var, 1+2)` should be changed to `print!("{var}={0}", 1+2)` by hand.
-    ///
-    /// changelog: [`needless-format-args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
+    /// Until implemented, `print!("{0}={1}", var, 1+2)` should be changed to `print!("{var}={0}", 1+2)` by hand.
     #[clippy::version = "1.64.0"]
     pub NEEDLESS_FORMAT_ARGS,
     nursery,
@@ -117,14 +115,14 @@ declare_clippy_lint! {
 impl_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, NEEDLESS_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
 
 pub struct FormatArgs {
-    support_arg_capture: bool,
+    format_args_capture: bool,
 }
 
 impl FormatArgs {
     #[must_use]
     pub fn new(msrv: Option<RustcVersion>) -> Self {
         Self {
-            support_arg_capture: meets_msrv(msrv, msrvs::FORMAT_ARGS_CAPTURE),
+            format_args_capture: meets_msrv(msrv, msrvs::FORMAT_ARGS_CAPTURE),
         }
     }
 }
@@ -150,14 +148,15 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
                     check_to_string_in_format_args(cx, name, arg.param.value);
                 }
 
-                // if at least some of the arguments/format/precision are referenced by an index,
-                // e.g.  format!("{} {1}", foo, bar)  or  format!("{:1$}", foo, 2)
-                // we cannot remove an argument from a list until we support renumbering.
-                // We are OK if we inline all numbered arguments.
-                if !self.support_arg_capture {
+                if !self.format_args_capture {
                     return;
                 }
                 let mut inline_spans = Vec::new();
+                // If any of the arguments are referenced by an index number,
+                // and that argument is not a simple variable and cannot be inlined,
+                // 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)
                 let do_inlining = format_args.params().all(|p| check_inline(cx, &p, &mut inline_spans));
                 if do_inlining && !inline_spans.is_empty()
                 {

clippy_utils/src/macros.rs

@@ -545,6 +545,7 @@ fn span_from_inner(base: SpanData, inner: rpf::InnerSpan) -> Span {
     )
 }
 
+/// How a format parameter is used in the format string
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum FormatParamKind {
     /// An implicit parameter , such as `{}` or `{:?}`.
@@ -559,10 +560,14 @@ pub enum FormatParamKind {
     NamedInline(Symbol),
 }
 
+/// Where a format parameter is being used in the format string
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum FormatParamUsage {
+    /// Appears as an argument, e.g. `format!("{}", foo)`
     Argument,
+    /// Appears as a width, e.g. `format!("{:width$}", foo, width = 1)`
     Width,
+    /// Appears as a precision, e.g. `format!("{:.precision$}", foo, precision = 1)`
     Precision,
 }
 
@@ -577,13 +582,12 @@ pub enum FormatParamUsage {
 /// and a [`FormatParamKind::NamedInline("precision")`] `.kind` with a `.value` of `2`
 #[derive(Debug, Copy, Clone)]
 pub struct FormatParam<'tcx> {
-    /// How this format param is being used - argument/width/precision
-    pub usage: FormatParamUsage,
-
     /// The expression this parameter refers to.
     pub value: &'tcx Expr<'tcx>,
     /// How this parameter refers to its `value`.
     pub kind: FormatParamKind,
+    /// Where this format param is being used - argument/width/precision
+    pub usage: FormatParamUsage,
     /// Span of the parameter, may be zero width. Includes the whitespace of implicit parameters.
     ///
     /// ```text

src/docs/needless_format_args.txt

@@ -30,6 +30,4 @@ format!("{var:.prec$}");  // precision and asterisk support
 Format string uses an indexed argument that cannot be inlined.
 Supporting this case requires re-indexing of the format string.
 
-Until implemnted, `print!("{0}={1}", var, 1+2)` should be changed to `print!("{var}={0}", 1+2)` by hand.
-
-changelog: [`needless-format-args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
+Until implemented, `print!("{0}={1}", var, 1+2)` should be changed to `print!("{var}={0}", 1+2)` by hand.

tests/ui/needless_format_args.fixed

@@ -6,6 +6,7 @@
 #![allow(named_arguments_used_positionally)]
 #![allow(unused_variables)]
 #![warn(clippy::needless_format_args)]
+#![feature(custom_inner_attributes)]
 
 fn tester(fn_arg: i32) {
     let local_i32 = 1;
@@ -120,3 +121,15 @@ fn tester(fn_arg: i32) {
 fn main() {
     tester(42);
 }
+
+fn _under_msrv() {
+    #![clippy::msrv = "1.57"]
+    let local_i32 = 1;
+    println!("don't expand='{local_i32}'");
+}
+
+fn _meets_msrv() {
+    #![clippy::msrv = "1.58"]
+    let local_i32 = 1;
+    println!("expand='{local_i32}'");
+}

tests/ui/needless_format_args.rs

@@ -6,6 +6,7 @@
 #![allow(named_arguments_used_positionally)]
 #![allow(unused_variables)]
 #![warn(clippy::needless_format_args)]
+#![feature(custom_inner_attributes)]
 
 fn tester(fn_arg: i32) {
     let local_i32 = 1;
@@ -123,3 +124,15 @@ fn tester(fn_arg: i32) {
 fn main() {
     tester(42);
 }
+
+fn _under_msrv() {
+    #![clippy::msrv = "1.57"]
+    let local_i32 = 1;
+    println!("don't expand='{}'", local_i32);
+}
+
+fn _meets_msrv() {
+    #![clippy::msrv = "1.58"]
+    let local_i32 = 1;
+    println!("expand='{}'", local_i32);
+}