Auto merge of #6913 - camsteffen:method-chain, r=flip1995

Destructure args in `methods`

changelog: none

This changes the main pattern in `methods` to match and destructure the method call args at the same time as the method name, and pass individual arg `Expr`s to the lint impls.

```rust
// before
["expect", ..] => expect::check(cx, expr, arg_lists[0]);
// after
("expect", [arg]) => expect::check(cx, expr, recv, arg);
```

This makes the code safer since there is no risk of out of bounds `args[n]` everywhere. There will be no more collecting `method_names`, `arg_lists`, `method_spans` as a separate step - everything comes out of the `match`es. Chained methods are parsed in a nested `match`. This makes the code more verbose in some ways, but IMO it is much easier to follow.

~Definitely should wait for #6896. Just putting out the idea.~

Author: bors

clippy_lints/src/methods/bind_instead_of_map.rs

@@ -80,7 +80,7 @@ pub(crate) trait BindInsteadOfMap {
     fn lint_closure_autofixable(
         cx: &LateContext<'_>,
         expr: &hir::Expr<'_>,
-        args: &[hir::Expr<'_>],
+        recv: &hir::Expr<'_>,
         closure_expr: &hir::Expr<'_>,
         closure_args_span: Span,
     ) -> bool {
@@ -103,7 +103,7 @@ pub(crate) trait BindInsteadOfMap {
                 };
 
                 let closure_args_snip = snippet(cx, closure_args_span, "..");
-                let option_snip = snippet(cx, args[0].span, "..");
+                let option_snip = snippet(cx, recv.span, "..");
                 let note = format!("{}.{}({} {})", option_snip, Self::GOOD_METHOD_NAME, closure_args_snip, some_inner_snip);
                 span_lint_and_sugg(
                     cx,
@@ -158,17 +158,17 @@ pub(crate) trait BindInsteadOfMap {
     }
 
     /// Lint use of `_.and_then(|x| Some(y))` for `Option`s
-    fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
-        if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) {
+    fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
+        if !match_type(cx, cx.typeck_results().expr_ty(recv), Self::TYPE_QPATH) {
             return false;
         }
 
-        match args[1].kind {
+        match arg.kind {
             hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => {
                 let closure_body = cx.tcx.hir().body(body_id);
                 let closure_expr = remove_blocks(&closure_body.value);
 
-                if Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) {
+                if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, closure_args_span) {
                     true
                 } else {
                     Self::lint_closure(cx, expr, closure_expr)
@@ -182,7 +182,7 @@ pub(crate) trait BindInsteadOfMap {
                     expr.span,
                     Self::no_op_msg().as_ref(),
                     "use the expression directly",
-                    snippet(cx, args[0].span, "..").into(),
+                    snippet(cx, recv.span, "..").into(),
                     Applicability::MachineApplicable,
                 );
                 true

clippy_lints/src/methods/bytes_nth.rs

@@ -3,16 +3,15 @@ use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::is_type_diagnostic_item;
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::Expr;
 use rustc_lint::LateContext;
 use rustc_span::sym;
 
 use super::BYTES_NTH;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx Expr<'tcx>, n_arg: &'tcx Expr<'tcx>) {
     if_chain! {
-        if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
-        let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs();
+        let ty = cx.typeck_results().expr_ty(recv).peel_refs();
         let caller_type = if is_type_diagnostic_item(cx, ty, sym::string_type) {
             Some("String")
         } else if ty.is_str() {
@@ -31,8 +30,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'
                 "try",
                 format!(
                     "{}.as_bytes().get({})",
-                    snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability),
-                    snippet_with_applicability(cx, args[1].span, "..", &mut applicability)
+                    snippet_with_applicability(cx, recv.span, "..", &mut applicability),
+                    snippet_with_applicability(cx, n_arg.span, "..", &mut applicability)
                 ),
                 applicability,
             );

clippy_lints/src/methods/expect_used.rs

@@ -7,8 +7,8 @@ use rustc_span::sym;
 use super::EXPECT_USED;
 
 /// lint use of `expect()` for `Option`s and `Result`s
-pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
-    let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();
+pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
+    let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();
 
     let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
         Some((EXPECT_USED, "an Option", "None"))

clippy_lints/src/methods/filetype_is_file.rs

@@ -8,8 +8,8 @@ use rustc_span::source_map::Span;
 
 use super::FILETYPE_IS_FILE;
 
-pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
-    let ty = cx.typeck_results().expr_ty(&args[0]);
+pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
+    let ty = cx.typeck_results().expr_ty(recv);
 
     if !match_type(cx, ty, &paths::FILE_TYPE) {
         return;

clippy_lints/src/methods/filter_map.rs

@@ -46,21 +46,17 @@ fn is_method<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, method_name: Sy
     }
 }
 
-fn is_option_filter_map<'tcx>(
-    cx: &LateContext<'tcx>,
-    filter_arg: &'tcx hir::Expr<'_>,
-    map_arg: &'tcx hir::Expr<'_>,
-) -> bool {
+fn is_option_filter_map<'tcx>(cx: &LateContext<'tcx>, filter_arg: &hir::Expr<'_>, map_arg: &hir::Expr<'_>) -> bool {
     is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_some))
 }
 
 /// lint use of `filter().map()` for `Iterators`
-fn lint_filter_some_map_unwrap<'tcx>(
-    cx: &LateContext<'tcx>,
-    expr: &'tcx hir::Expr<'_>,
-    filter_recv: &'tcx hir::Expr<'_>,
-    filter_arg: &'tcx hir::Expr<'_>,
-    map_arg: &'tcx hir::Expr<'_>,
+fn lint_filter_some_map_unwrap(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    filter_recv: &hir::Expr<'_>,
+    filter_arg: &hir::Expr<'_>,
+    map_arg: &hir::Expr<'_>,
     target_span: Span,
     methods_span: Span,
 ) {
@@ -86,14 +82,28 @@ fn lint_filter_some_map_unwrap<'tcx>(
 }
 
 /// lint use of `filter().map()` or `find().map()` for `Iterators`
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool, target_span: Span) {
+#[allow(clippy::too_many_arguments)]
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &hir::Expr<'_>,
+    filter_recv: &hir::Expr<'_>,
+    filter_arg: &hir::Expr<'_>,
+    filter_span: Span,
+    map_recv: &hir::Expr<'_>,
+    map_arg: &hir::Expr<'_>,
+    map_span: Span,
+    is_find: bool,
+) {
+    lint_filter_some_map_unwrap(
+        cx,
+        expr,
+        filter_recv,
+        filter_arg,
+        map_arg,
+        map_span,
+        filter_span.with_hi(expr.span.hi()),
+    );
     if_chain! {
-            if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
-            if let ExprKind::MethodCall(_, _, [filter_recv, filter_arg], filter_span) = map_recv.kind;
-            then {
-              lint_filter_some_map_unwrap(cx, expr, filter_recv, filter_arg,
-                map_arg, target_span, filter_span.to(map_span));
-              if_chain! {
             if is_trait_method(cx, map_recv, sym::Iterator);
 
             // filter(|x| ...is_some())...
@@ -148,7 +158,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_
             };
             if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
             then {
-                let span = filter_span.to(map_span);
+                let span = filter_span.with_hi(expr.span.hi());
                 let (filter_name, lint) = if is_find {
                     ("find", MANUAL_FIND_MAP)
                 } else {
@@ -160,7 +170,5 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_
                     snippet(cx, map_arg.span, ".."), to_opt);
                 span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
             }
-        }
-        }
     }
 }

clippy_lints/src/methods/filter_map_identity.rs

@@ -8,15 +8,8 @@ use rustc_span::{source_map::Span, sym};
 
 use super::FILTER_MAP_IDENTITY;
 
-pub(super) fn check(
-    cx: &LateContext<'_>,
-    expr: &hir::Expr<'_>,
-    filter_map_args: &[hir::Expr<'_>],
-    filter_map_span: Span,
-) {
+pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) {
     if is_trait_method(cx, expr, sym::Iterator) {
-        let arg_node = &filter_map_args[1].kind;
-
         let apply_lint = |message: &str| {
             span_lint_and_sugg(
                 cx,
@@ -30,8 +23,8 @@ pub(super) fn check(
         };
 
         if_chain! {
-            if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node;
-            let body = cx.tcx.hir().body(*body_id);
+            if let hir::ExprKind::Closure(_, _, body_id, _, _) = filter_map_arg.kind;
+            let body = cx.tcx.hir().body(body_id);
 
             if let hir::PatKind::Binding(_, binding_id, ..) = body.params[0].pat.kind;
             if path_to_local_id(&body.value, binding_id);
@@ -41,7 +34,7 @@ pub(super) fn check(
         }
 
         if_chain! {
-            if let hir::ExprKind::Path(ref qpath) = arg_node;
+            if let hir::ExprKind::Path(ref qpath) = filter_map_arg.kind;
 
             if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY);
 

clippy_lints/src/methods/filter_map_next.rs

@@ -14,7 +14,8 @@ const FILTER_MAP_NEXT_MSRV: RustcVersion = RustcVersion::new(1, 30, 0);
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'_>,
-    filter_args: &'tcx [hir::Expr<'_>],
+    recv: &'tcx hir::Expr<'_>,
+    arg: &'tcx hir::Expr<'_>,
     msrv: Option<&RustcVersion>,
 ) {
     if is_trait_method(cx, expr, sym::Iterator) {
@@ -24,9 +25,9 @@ pub(super) fn check<'tcx>(
 
         let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
                    `.find_map(..)` instead";
-        let filter_snippet = snippet(cx, filter_args[1].span, "..");
+        let filter_snippet = snippet(cx, arg.span, "..");
         if filter_snippet.lines().count() <= 1 {
-            let iter_snippet = snippet(cx, filter_args[0].span, "..");
+            let iter_snippet = snippet(cx, recv.span, "..");
             span_lint_and_sugg(
                 cx,
                 FILTER_MAP_NEXT,

clippy_lints/src/methods/filter_next.rs

@@ -9,14 +9,19 @@ use rustc_span::sym;
 use super::FILTER_NEXT;
 
 /// lint use of `filter().next()` for `Iterators`
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'_>,
+    filter_arg: &'tcx hir::Expr<'_>,
+) {
     // lint if caller of `.filter().next()` is an Iterator
     if is_trait_method(cx, expr, sym::Iterator) {
         let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
                    `.find(..)` instead";
-        let filter_snippet = snippet(cx, filter_args[1].span, "..");
+        let filter_snippet = snippet(cx, filter_arg.span, "..");
         if filter_snippet.lines().count() <= 1 {
-            let iter_snippet = snippet(cx, filter_args[0].span, "..");
+            let iter_snippet = snippet(cx, recv.span, "..");
             // add note if not multi-line
             span_lint_and_sugg(
                 cx,

clippy_lints/src/methods/flat_map_identity.rs

@@ -12,11 +12,11 @@ use super::FLAT_MAP_IDENTITY;
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'_>,
-    flat_map_args: &'tcx [hir::Expr<'_>],
+    flat_map_arg: &'tcx hir::Expr<'_>,
     flat_map_span: Span,
 ) {
     if is_trait_method(cx, expr, sym::Iterator) {
-        let arg_node = &flat_map_args[1].kind;
+        let arg_node = &flat_map_arg.kind;
 
         let apply_lint = |message: &str| {
             span_lint_and_sugg(

clippy_lints/src/methods/get_unwrap.rs

@@ -11,18 +11,20 @@ use rustc_span::sym;
 
 use super::GET_UNWRAP;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args: &'tcx [hir::Expr<'_>], is_mut: bool) {
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'tcx>,
+    get_arg: &'tcx hir::Expr<'_>,
+    is_mut: bool,
+) {
     // Note: we don't want to lint `get_mut().unwrap` for `HashMap` or `BTreeMap`,
     // because they do not implement `IndexMut`
     let mut applicability = Applicability::MachineApplicable;
-    let expr_ty = cx.typeck_results().expr_ty(&get_args[0]);
-    let get_args_str = if get_args.len() > 1 {
-        snippet_with_applicability(cx, get_args[1].span, "..", &mut applicability)
-    } else {
-        return; // not linting on a .get().unwrap() chain or variant
-    };
+    let expr_ty = cx.typeck_results().expr_ty(recv);
+    let get_args_str = snippet_with_applicability(cx, get_arg.span, "..", &mut applicability);
     let mut needs_ref;
-    let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
+    let caller_type = if derefs_to_slice(cx, recv, expr_ty).is_some() {
         needs_ref = get_args_str.parse::<usize>().is_ok();
         "slice"
     } else if is_type_diagnostic_item(cx, expr_ty, sym::vec_type) {
@@ -77,7 +79,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args
         format!(
             "{}{}[{}]",
             borrow_str,
-            snippet_with_applicability(cx, get_args[0].span, "..", &mut applicability),
+            snippet_with_applicability(cx, recv.span, "..", &mut applicability),
             get_args_str
         ),
         applicability,

clippy_lints/src/methods/iter_cloned_collect.rs

@@ -9,10 +9,10 @@ use rustc_span::sym;
 
 use super::ITER_CLONED_COLLECT;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, recv: &'tcx hir::Expr<'_>) {
     if_chain! {
         if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::vec_type);
-        if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0]));
+        if let Some(slice) = derefs_to_slice(cx, recv, cx.typeck_results().expr_ty(recv));
         if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite());
 
         then {

clippy_lints/src/methods/iter_count.rs

@@ -10,9 +10,9 @@ use rustc_span::sym;
 
 use super::ITER_COUNT;
 
-pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>], iter_method: &str) {
-    let ty = cx.typeck_results().expr_ty(&iter_args[0]);
-    let caller_type = if derefs_to_slice(cx, &iter_args[0], ty).is_some() {
+pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx Expr<'tcx>, iter_method: &str) {
+    let ty = cx.typeck_results().expr_ty(recv);
+    let caller_type = if derefs_to_slice(cx, recv, ty).is_some() {
         "slice"
     } else if is_type_diagnostic_item(cx, ty, sym::vec_type) {
         "Vec"
@@ -42,7 +42,7 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'
         "try",
         format!(
             "{}.len()",
-            snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability),
+            snippet_with_applicability(cx, recv.span, "..", &mut applicability),
         ),
         applicability,
     );

clippy_lints/src/methods/iter_next_slice.rs

@@ -13,9 +13,7 @@ use rustc_span::symbol::sym;
 
 use super::ITER_NEXT_SLICE;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
-    let caller_expr = &iter_args[0];
-
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, caller_expr: &'tcx hir::Expr<'_>) {
     // Skip lint if the `iter().next()` expression is a for loop argument,
     // since it is already covered by `&loops::ITER_NEXT_LOOP`
     let mut parent_expr_opt = get_parent_expr(cx, expr);

clippy_lints/src/methods/iter_nth.rs

@@ -11,20 +11,20 @@ use super::ITER_NTH;
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &hir::Expr<'_>,
-    nth_and_iter_args: &[&'tcx [hir::Expr<'tcx>]],
+    iter_recv: &'tcx hir::Expr<'tcx>,
+    nth_recv: &hir::Expr<'_>,
+    nth_arg: &hir::Expr<'_>,
     is_mut: bool,
 ) {
-    let iter_args = nth_and_iter_args[1];
     let mut_str = if is_mut { "_mut" } else { "" };
-    let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0])).is_some() {
+    let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() {
         "slice"
-    } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vec_type) {
+    } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::vec_type) {
         "Vec"
-    } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vecdeque_type) {
+    } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::vecdeque_type) {
         "VecDeque"
     } else {
-        let nth_args = nth_and_iter_args[0];
-        iter_nth_zero::check(cx, expr, &nth_args);
+        iter_nth_zero::check(cx, expr, nth_recv, nth_arg);
         return; // caller is not a type that we want to lint
     };