From 0e1cbc5cd1d2e2a52b934dda1701d46fdb4e09b8 Mon Sep 17 00:00:00 2001 From: tamaron Date: Tue, 1 Feb 2022 13:43:39 +0900 Subject: [PATCH 1/3] fix code --- clippy_lints/src/loops/utils.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs index eac0f03b1..9082ab943 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,17 +332,20 @@ 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 meth_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(), + sugg::Sugg::hir_with_applicability(cx, caller, "_", applic_ref).maybe_par(), meth_name, ) }, From b13704a9cd866b9c1ed73c26f697158c16b75873 Mon Sep 17 00:00:00 2001 From: tamaron Date: Tue, 1 Feb 2022 13:44:24 +0900 Subject: [PATCH 2/3] update test suites --- tests/ui/explicit_counter_loop.stderr | 4 ++-- tests/ui/manual_flatten.rs | 2 -- tests/ui/manual_flatten.stderr | 26 +++++++++++++------------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index 9edddea65..f9f8407d5 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 7db6b7309..6c5232ec5 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 be5f8a1d8..392e1a393 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); From f5fd9ded000d7968b6641d2b17f4eca53a4a8f67 Mon Sep 17 00:00:00 2001 From: tamaron Date: Wed, 2 Feb 2022 11:25:15 +0900 Subject: [PATCH 3/3] chore --- clippy_lints/src/loops/utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs index 9082ab943..b6c746d3e 100644 --- a/clippy_lints/src/loops/utils.rs +++ b/clippy_lints/src/loops/utils.rs @@ -335,7 +335,7 @@ pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic 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 meth_name = match mutbl { + let method_name = match mutbl { Mutability::Mut => "iter_mut", Mutability::Not => "iter", }; @@ -346,7 +346,7 @@ pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic format!( "{}.{}()", sugg::Sugg::hir_with_applicability(cx, caller, "_", applic_ref).maybe_par(), - meth_name, + method_name, ) }, _ => format!(