about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-02-02 12:37:18 +0000
committerbors <bors@rust-lang.org>2022-02-02 12:37:18 +0000
commitbef92b864d3591cffbee23ea88461b26af0b4d75 (patch)
tree7f4929c6846b310c056d250c107aa721cb6dcc0e
parentcf53710744e2a4868f4c6015ea7be15cd8087a45 (diff)
parentf5fd9ded000d7968b6641d2b17f4eca53a4a8f67 (diff)
downloadrust-bef92b864d3591cffbee23ea88461b26af0b4d75.tar.gz
rust-bef92b864d3591cffbee23ea88461b26af0b4d75.zip
Auto merge of #8382 - tamaroning:suggest_iter_instead_of_into_iter, r=giraffate
[explicit_counter_loop] suggests `.into_iter()`, despite that triggering [into_iter_on_ref] in some cases

I have modified `fn make_iterator_snippet` in clippy_lints/src/loops/utils.rs ,so this change has some little influence on another lint [manual_flatten] .

fixes #8155

---
changelog: Fix that [`explicit_counter_loop`] suggests `into_iter()` despite that triggering [`into_iter_on_ref`] in some cases
-rw-r--r--clippy_lints/src/loops/utils.rs19
-rw-r--r--tests/ui/explicit_counter_loop.stderr4
-rw-r--r--tests/ui/manual_flatten.rs2
-rw-r--r--tests/ui/manual_flatten.stderr26
4 files changed, 26 insertions, 25 deletions
diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs
index eac0f03b142..b6c746d3e39 100644
--- a/clippy_lints/src/loops/utils.rs
+++ b/clippy_lints/src/loops/utils.rs
@@ -7,7 +7,7 @@ use rustc_hir::intravisit::{walk_expr, walk_local, walk_pat, walk_stmt, Visitor}
 use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, Local, Mutability, Pat, PatKind, Stmt};
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
-use rustc_middle::ty::Ty;
+use rustc_middle::ty::{self, Ty};
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{sym, Symbol};
 use rustc_typeck::hir_ty_to_ty;
@@ -332,18 +332,21 @@ pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic
     } else {
         // (&x).into_iter() ==> x.iter()
         // (&mut x).into_iter() ==> x.iter_mut()
-        match &arg.kind {
-            ExprKind::AddrOf(BorrowKind::Ref, mutability, arg_inner)
-                if has_iter_method(cx, cx.typeck_results().expr_ty(arg_inner)).is_some() =>
-            {
-                let meth_name = match mutability {
+        let arg_ty = cx.typeck_results().expr_ty_adjusted(arg);
+        match &arg_ty.kind() {
+            ty::Ref(_, inner_ty, mutbl) if has_iter_method(cx, inner_ty).is_some() => {
+                let method_name = match mutbl {
                     Mutability::Mut => "iter_mut",
                     Mutability::Not => "iter",
                 };
+                let caller = match &arg.kind {
+                    ExprKind::AddrOf(BorrowKind::Ref, _, arg_inner) => arg_inner,
+                    _ => arg,
+                };
                 format!(
                     "{}.{}()",
-                    sugg::Sugg::hir_with_applicability(cx, arg_inner, "_", applic_ref).maybe_par(),
-                    meth_name,
+                    sugg::Sugg::hir_with_applicability(cx, caller, "_", applic_ref).maybe_par(),
+                    method_name,
                 )
             },
             _ => format!(
diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr
index 9edddea651c..f9f8407d577 100644
--- a/tests/ui/explicit_counter_loop.stderr
+++ b/tests/ui/explicit_counter_loop.stderr
@@ -46,13 +46,13 @@ error: the variable `idx_usize` is used as a loop counter
   --> $DIR/explicit_counter_loop.rs:170:9
    |
 LL |         for _item in slice {
-   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.into_iter().enumerate()`
+   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.iter().enumerate()`
 
 error: the variable `idx_u32` is used as a loop counter
   --> $DIR/explicit_counter_loop.rs:182:9
    |
 LL |         for _item in slice {
-   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.into_iter())`
+   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.iter())`
    |
    = note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
 
diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs
index 7db6b730963..6c5232ec5f5 100644
--- a/tests/ui/manual_flatten.rs
+++ b/tests/ui/manual_flatten.rs
@@ -26,8 +26,6 @@ fn main() {
     }
 
     // Test for loop over an implicit reference
-    // Note: if `clippy::manual_flatten` is made autofixable, this case will
-    // lead to a follow-up lint `clippy::into_iter_on_ref`
     let z = &y;
     for n in z {
         if let Ok(n) = n {
diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr
index be5f8a1d818..392e1a39393 100644
--- a/tests/ui/manual_flatten.stderr
+++ b/tests/ui/manual_flatten.stderr
@@ -63,10 +63,10 @@ LL | |         }
    | |_________^
 
 error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:32:5
+  --> $DIR/manual_flatten.rs:30:5
    |
 LL |       for n in z {
-   |       ^        - help: try: `z.into_iter().flatten()`
+   |       ^        - help: try: `z.iter().flatten()`
    |  _____|
    | |
 LL | |         if let Ok(n) = n {
@@ -76,7 +76,7 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:33:9
+  --> $DIR/manual_flatten.rs:31:9
    |
 LL | /         if let Ok(n) = n {
 LL | |             println!("{}", n);
@@ -84,7 +84,7 @@ LL | |         }
    | |_________^
 
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:41:5
+  --> $DIR/manual_flatten.rs:39:5
    |
 LL |       for n in z {
    |       ^        - help: try: `z.flatten()`
@@ -97,7 +97,7 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:42:9
+  --> $DIR/manual_flatten.rs:40:9
    |
 LL | /         if let Some(m) = n {
 LL | |             println!("{}", m);
@@ -105,7 +105,7 @@ LL | |         }
    | |_________^
 
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:74:5
+  --> $DIR/manual_flatten.rs:72:5
    |
 LL |       for n in &vec_of_ref {
    |       ^        ----------- help: try: `vec_of_ref.iter().copied().flatten()`
@@ -118,7 +118,7 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:75:9
+  --> $DIR/manual_flatten.rs:73:9
    |
 LL | /         if let Some(n) = n {
 LL | |             println!("{:?}", n);
@@ -126,10 +126,10 @@ LL | |         }
    | |_________^
 
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:81:5
+  --> $DIR/manual_flatten.rs:79:5
    |
 LL |       for n in vec_of_ref {
-   |       ^        ---------- help: try: `vec_of_ref.into_iter().copied().flatten()`
+   |       ^        ---------- help: try: `vec_of_ref.iter().copied().flatten()`
    |  _____|
    | |
 LL | |         if let Some(n) = n {
@@ -139,7 +139,7 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:82:9
+  --> $DIR/manual_flatten.rs:80:9
    |
 LL | /         if let Some(n) = n {
 LL | |             println!("{:?}", n);
@@ -147,10 +147,10 @@ LL | |         }
    | |_________^
 
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:88:5
+  --> $DIR/manual_flatten.rs:86:5
    |
 LL |       for n in slice_of_ref {
-   |       ^        ------------ help: try: `slice_of_ref.into_iter().copied().flatten()`
+   |       ^        ------------ help: try: `slice_of_ref.iter().copied().flatten()`
    |  _____|
    | |
 LL | |         if let Some(n) = n {
@@ -160,7 +160,7 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:89:9
+  --> $DIR/manual_flatten.rs:87:9
    |
 LL | /         if let Some(n) = n {
 LL | |             println!("{:?}", n);