Auto merge of #9349 - Alexendoo:format-args-expn, r=flip1995

Refactor `FormatArgsExpn`

It now for each format argument `{..}` has:
- The `Expr` it points to, and how it does so (named/named inline/numbered/implicit)
- The parsed `FormatSpec` (format trait/fill/align/etc., the precision/width and any value they point to)
- Many spans

The caller no longer needs to pair up arguments to their value, or separately interpret the `specs` `Expr`s when it isn't `None`

The gist is that it combines the result of [`rustc_parse_format::Parser`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_parse_format/struct.Parser.html) with the macro expansion itself

This unfortunately makes the code a bit longer, however we need to use both as neither have all the information we're after. `rustc_parse_format` doesn't have the information to resolve named arguments to their values. The macro expansion doesn't contain whether the positions are implicit/numbered/named, or the spans for format arguments

Wanted by #9233 and #8518 to be able to port the changes from #9040

Also fixes #8643, previously the format args seem to have been paired up with the wrong values somehow

changelog: [`format_in_format_args`]: Fix false positive due to misattributed arguments

r? `@flip1995`
cc `@nyurik`

Author: bors

clippy_lints/src/format.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn};
-use clippy_utils::source::{snippet_opt, snippet_with_applicability};
+use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::sugg::Sugg;
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -56,29 +56,27 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
         };
 
         let mut applicability = Applicability::MachineApplicable;
-        if format_args.value_args.is_empty() {
-            match *format_args.format_string_parts {
+        if format_args.args.is_empty() {
+            match *format_args.format_string.parts {
                 [] => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability),
                 [_] => {
-                    if let Some(s_src) = snippet_opt(cx, format_args.format_string_span) {
-                        // Simulate macro expansion, converting {{ and }} to { and }.
-                        let s_expand = s_src.replace("{{", "{").replace("}}", "}");
-                        let sugg = format!("{}.to_string()", s_expand);
-                        span_useless_format(cx, call_site, sugg, applicability);
-                    }
+                    // Simulate macro expansion, converting {{ and }} to { and }.
+                    let s_expand = format_args.format_string.snippet.replace("{{", "{").replace("}}", "}");
+                    let sugg = format!("{}.to_string()", s_expand);
+                    span_useless_format(cx, call_site, sugg, applicability);
                 },
                 [..] => {},
             }
-        } else if let [value] = *format_args.value_args {
+        } else if let [arg] = &*format_args.args {
+            let value = arg.param.value;
             if_chain! {
-                if format_args.format_string_parts == [kw::Empty];
+                if format_args.format_string.parts == [kw::Empty];
                 if match cx.typeck_results().expr_ty(value).peel_refs().kind() {
                     ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::String, adt.did()),
                     ty::Str => true,
                     _ => false,
                 };
-                if let Some(args) = format_args.args();
-                if args.iter().all(|arg| arg.format_trait == sym::Display && !arg.has_string_formatting());
+                if !arg.format.has_string_formatting();
                 then {
                     let is_new_string = match value.kind {
                         ExprKind::Binary(..) => true,

clippy_lints/src/format_args.rs

@@ -1,11 +1,12 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::is_diag_trait_item;
-use clippy_utils::macros::{is_format_macro, FormatArgsArg, FormatArgsExpn};
+use clippy_utils::macros::{is_format_macro, FormatArgsExpn};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::implements_trait;
 use if_chain::if_chain;
+use itertools::Itertools;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{Expr, ExprKind, HirId};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment};
 use rustc_middle::ty::Ty;
@@ -74,20 +75,16 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
             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;
-            if let Some(args) = format_args.args();
             then {
-                for (i, arg) in args.iter().enumerate() {
-                    if arg.format_trait != sym::Display {
+                for arg in &format_args.args {
+                    if arg.format.has_string_formatting() {
                         continue;
                     }
-                    if arg.has_string_formatting() {
+                    if is_aliased(&format_args, arg.param.value.hir_id) {
                         continue;
                     }
-                    if is_aliased(&args, i) {
-                        continue;
-                    }
-                    check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.value);
-                    check_to_string_in_format_args(cx, name, arg.value);
+                    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);
                 }
             }
         }
@@ -167,12 +164,12 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
     }
 }
 
-// Returns true if `args[i]` "refers to" or "is referred to by" another argument.
-fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool {
-    let value = args[i].value;
-    args.iter()
-        .enumerate()
-        .any(|(j, arg)| i != j && std::ptr::eq(value, arg.value))
+// Returns true if `hir_id` is referred to by multiple format params
+fn is_aliased(args: &FormatArgsExpn<'_>, hir_id: HirId) -> bool {
+    args.params()
+        .filter(|param| param.value.hir_id == hir_id)
+        .at_most_one()
+        .is_err()
 }
 
 fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>)

clippy_lints/src/format_impl.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
-use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArgsArg, FormatArgsExpn};
+use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn};
 use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -168,10 +168,9 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
         if let macro_def_id = outer_macro.def_id;
         if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
         if is_format_macro(cx, macro_def_id);
-        if let Some(args) = format_args.args();
         then {
-            for arg in args {
-                if arg.format_trait != impl_trait.name {
+            for arg in format_args.args {
+                if arg.format.r#trait != impl_trait.name {
                     continue;
                 }
                 check_format_arg_self(cx, expr, &arg, impl_trait);
@@ -180,11 +179,11 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
     }
 }
 
-fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArgsArg<'_>, impl_trait: FormatTrait) {
+fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArg<'_>, impl_trait: FormatTrait) {
     // Handle multiple dereferencing of references e.g. &&self
     // Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl)
     // Since the argument to fmt is itself a reference: &self
-    let reference = peel_ref_operators(cx, arg.value);
+    let reference = peel_ref_operators(cx, arg.param.value);
     let map = cx.tcx.hir();
     // Is the reference self?
     if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) {

clippy_lints/src/methods/uninit_assumed_init.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::{is_expr_diagnostic_item, ty::is_uninit_value_valid_for_ty};
+use clippy_utils::{is_path_diagnostic_item, ty::is_uninit_value_valid_for_ty};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
@@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
     if_chain! {
         if let hir::ExprKind::Call(callee, args) = recv.kind;
         if args.is_empty();
-        if is_expr_diagnostic_item(cx, callee, sym::maybe_uninit_uninit);
+        if is_path_diagnostic_item(cx, callee, sym::maybe_uninit_uninit);
         if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr));
         then {
             span_lint(

clippy_lints/src/transmute/transmuting_null.rs

@@ -1,6 +1,6 @@
 use clippy_utils::consts::{constant_context, Constant};
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::is_expr_diagnostic_item;
+use clippy_utils::is_path_diagnostic_item;
 use if_chain::if_chain;
 use rustc_ast::LitKind;
 use rustc_hir::{Expr, ExprKind};
@@ -45,7 +45,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
     // `std::mem::transmute(std::ptr::null::<i32>())`
     if_chain! {
         if let ExprKind::Call(func1, []) = arg.kind;
-        if is_expr_diagnostic_item(cx, func1, sym::ptr_null);
+        if is_path_diagnostic_item(cx, func1, sym::ptr_null);
         then {
             span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG);
             return true;

clippy_utils/Cargo.toml

@@ -7,6 +7,7 @@ publish = false
 [dependencies]
 arrayvec = { version = "0.7", default-features = false }
 if_chain = "1.0"
+itertools = "0.10.1"
 rustc-semver = "1.1"
 
 [features]

clippy_utils/src/lib.rs

@@ -27,6 +27,7 @@ extern crate rustc_infer;
 extern crate rustc_lexer;
 extern crate rustc_lint;
 extern crate rustc_middle;
+extern crate rustc_parse_format;
 extern crate rustc_session;
 extern crate rustc_span;
 extern crate rustc_target;
@@ -371,15 +372,19 @@ pub fn match_qpath(path: &QPath<'_>, segments: &[&str]) -> bool {
 
 /// If the expression is a path, resolves it to a `DefId` and checks if it matches the given path.
 ///
-/// Please use `is_expr_diagnostic_item` if the target is a diagnostic item.
+/// Please use `is_path_diagnostic_item` if the target is a diagnostic item.
 pub fn is_expr_path_def_path(cx: &LateContext<'_>, expr: &Expr<'_>, segments: &[&str]) -> bool {
     path_def_id(cx, expr).map_or(false, |id| match_def_path(cx, id, segments))
 }
 
-/// If the expression is a path, resolves it to a `DefId` and checks if it matches the given
-/// diagnostic item.
-pub fn is_expr_diagnostic_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool {
-    path_def_id(cx, expr).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
+/// If `maybe_path` is a path node which resolves to an item, resolves it to a `DefId` and checks if
+/// it matches the given diagnostic item.
+pub fn is_path_diagnostic_item<'tcx>(
+    cx: &LateContext<'_>,
+    maybe_path: &impl MaybePath<'tcx>,
+    diag_item: Symbol,
+) -> bool {
+    path_def_id(cx, maybe_path).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
 }
 
 /// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the