]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/format_args.rs
Re-enable `uninlined_format_args` on multiline `format!`
[rust.git] / clippy_lints / src / format_args.rs
index 97d2973cce8e7054014bc6357c42ce0a2c75424c..fd3ce2f3d6cd6ab503011cfe479a104abcb63a86 100644 (file)
@@ -1,17 +1,22 @@
 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::{is_format_macro, is_panic, FormatArgsExpn, FormatParam, FormatParamUsage};
+use clippy_utils::macros::{
+    is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
+};
+use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::snippet_opt;
-use clippy_utils::ty::implements_trait;
-use clippy_utils::{is_diag_trait_item, meets_msrv, msrvs};
+use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
 use if_chain::if_chain;
 use itertools::Itertools;
-use rustc_errors::Applicability;
+use rustc_errors::{
+    Applicability,
+    SuggestionStyle::{CompletelyHidden, ShowCode},
+};
 use rustc_hir::{Expr, ExprKind, HirId, QPath};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment};
 use rustc_middle::ty::Ty;
-use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::def_id::DefId;
 use rustc_span::edition::Edition::Edition2021;
     "using non-inlined variables in `format!` calls"
 }
 
-impl_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects [formatting parameters] that have no effect on the output of
+    /// `format!()`, `println!()` or similar macros.
+    ///
+    /// ### Why is this bad?
+    /// Shorter format specifiers are easier to read, it may also indicate that
+    /// an expected formatting operation such as adding padding isn't happening.
+    ///
+    /// ### Example
+    /// ```rust
+    /// println!("{:.}", 1.0);
+    ///
+    /// println!("not padded: {:5}", format_args!("..."));
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// println!("{}", 1.0);
+    ///
+    /// println!("not padded: {}", format_args!("..."));
+    /// // OR
+    /// println!("padded: {:5}", format!("..."));
+    /// ```
+    ///
+    /// [formatting parameters]: https://doc.rust-lang.org/std/fmt/index.html#formatting-parameters
+    #[clippy::version = "1.66.0"]
+    pub UNUSED_FORMAT_SPECS,
+    complexity,
+    "use of a format specifier that has no effect"
+}
+
+impl_lint_pass!(FormatArgs => [
+    FORMAT_IN_FORMAT_ARGS,
+    TO_STRING_IN_FORMAT_ARGS,
+    UNINLINED_FORMAT_ARGS,
+    UNUSED_FORMAT_SPECS,
+]);
 
 pub struct FormatArgs {
-    msrv: Option<RustcVersion>,
+    msrv: Msrv,
 }
 
 impl FormatArgs {
     #[must_use]
-    pub fn new(msrv: Option<RustcVersion>) -> Self {
+    pub fn new(msrv: Msrv) -> Self {
         Self { msrv }
     }
 }
 
 impl<'tcx> LateLintPass<'tcx> for FormatArgs {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
-        if_chain! {
-            if let Some(format_args) = FormatArgsExpn::parse(cx, expr);
-            let expr_expn_data = expr.span.ctxt().outer_expn_data();
-            let outermost_expn_data = outermost_expn_data(expr_expn_data);
-            if let Some(macro_def_id) = outermost_expn_data.macro_def_id;
-            if is_format_macro(cx, macro_def_id);
-            if let ExpnKind::Macro(_, name) = outermost_expn_data.kind;
-            then {
-                for arg in &format_args.args {
-                    if !arg.format.is_default() {
-                        continue;
-                    }
-                    if is_aliased(&format_args, arg.param.value.hir_id) {
-                        continue;
-                    }
-                    check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
-                    check_to_string_in_format_args(cx, name, arg.param.value);
+        if let Some(format_args) = FormatArgsExpn::parse(cx, expr)
+            && let expr_expn_data = expr.span.ctxt().outer_expn_data()
+            && let outermost_expn_data = outermost_expn_data(expr_expn_data)
+            && let Some(macro_def_id) = outermost_expn_data.macro_def_id
+            && is_format_macro(cx, macro_def_id)
+            && let ExpnKind::Macro(_, name) = outermost_expn_data.kind
+        {
+            for arg in &format_args.args {
+                check_unused_format_specifier(cx, arg);
+                if !arg.format.is_default() {
+                    continue;
                 }
-                if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
-                    check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
+                if is_aliased(&format_args, arg.param.value.hir_id) {
+                    continue;
                 }
+                check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
+                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);
             }
         }
     }
@@ -160,6 +200,76 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
     extract_msrv_attr!(LateContext);
 }
 
+fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
+    let param_ty = cx.typeck_results().expr_ty(arg.param.value).peel_refs();
+
+    if let Count::Implied(Some(mut span)) = arg.format.precision
+        && !span.is_empty()
+    {
+        span_lint_and_then(
+            cx,
+            UNUSED_FORMAT_SPECS,
+            span,
+            "empty precision specifier has no effect",
+            |diag| {
+                if param_ty.is_floating_point() {
+                    diag.note("a precision specifier is not required to format floats");
+                }
+
+                if arg.format.is_default() {
+                    // If there's no other specifiers remove the `:` too
+                    span = arg.format_span();
+                }
+
+                diag.span_suggestion_verbose(span, "remove the `.`", "", Applicability::MachineApplicable);
+            },
+        );
+    }
+
+    if is_type_diagnostic_item(cx, param_ty, sym::Arguments) && !arg.format.is_default_for_trait() {
+        span_lint_and_then(
+            cx,
+            UNUSED_FORMAT_SPECS,
+            arg.span,
+            "format specifiers have no effect on `format_args!()`",
+            |diag| {
+                let mut suggest_format = |spec, span| {
+                    let message = format!("for the {spec} to apply consider using `format!()`");
+
+                    if let Some(mac_call) = root_macro_call(arg.param.value.span)
+                        && cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id)
+                        && arg.span.eq_ctxt(mac_call.span)
+                    {
+                        diag.span_suggestion(
+                            cx.sess().source_map().span_until_char(mac_call.span, '!'),
+                            message,
+                            "format",
+                            Applicability::MaybeIncorrect,
+                        );
+                    } else if let Some(span) = span {
+                        diag.span_help(span, message);
+                    }
+                };
+
+                if !arg.format.width.is_implied() {
+                    suggest_format("width", arg.format.width.span());
+                }
+
+                if !arg.format.precision.is_implied() {
+                    suggest_format("precision", arg.format.precision.span());
+                }
+
+                diag.span_suggestion_verbose(
+                    arg.format_span(),
+                    "if the current behavior is intentional, remove the format specifiers",
+                    "",
+                    Applicability::MaybeIncorrect,
+                );
+            },
+        );
+    }
+}
+
 fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
     if args.format_string.span.from_expansion() {
         return;
@@ -179,10 +289,9 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
         return;
     }
 
-    // Temporarily ignore multiline spans: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
-    if fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)) {
-        return;
-    }
+    // multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
+    // in those cases, make the code suggestion hidden
+    let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span));
 
     span_lint_and_then(
         cx,
@@ -190,7 +299,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
         call_site,
         "variables can be used directly in the `format!` string",
         |diag| {
-            diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable);
+            diag.multipart_suggestion_with_style(
+                "change this to",
+                fixes,
+                Applicability::MachineApplicable,
+                if multiline_fix { CompletelyHidden } else { ShowCode },
+            );
         },
     );
 }
@@ -249,7 +363,7 @@ fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symb
 fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Expr<'_>) {
     if_chain! {
         if !value.span.from_expansion();
-        if let ExprKind::MethodCall(_, receiver, [], _) = value.kind;
+        if let ExprKind::MethodCall(_, receiver, [], to_string_span) = value.kind;
         if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id);
         if is_diag_trait_item(cx, method_def_id, sym::ToString);
         let receiver_ty = cx.typeck_results().expr_ty(receiver);
@@ -265,7 +379,7 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
                 span_lint_and_sugg(
                     cx,
                     TO_STRING_IN_FORMAT_ARGS,
-                    value.span.with_lo(receiver.span.hi()),
+                    to_string_span.with_lo(receiver.span.hi()),
                     &format!(
                         "`to_string` applied to a type that implements `Display` in `{name}!` args"
                     ),