Skip to content

Commit bef92b8

Browse files
borsbors
committed
Auto merge of rust-lang#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 rust-lang#8155 --- changelog: Fix that [`explicit_counter_loop`] suggests `into_iter()` despite that triggering [`into_iter_on_ref`] in some cases
2 parents cf53710 + f5fd9de commit bef92b8

File tree

4 files changed

+26
-25
lines changed

4 files changed

+26
-25
lines changed

‎clippy_lints/src/loops/utils.rs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_hir::intravisit::{walk_expr, walk_local, walk_pat, walk_stmt, Visitor}
77
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, Local, Mutability, Pat, PatKind, Stmt};
88
use rustc_lint::LateContext;
99
use rustc_middle::hir::nested_filter;
10-
use rustc_middle::ty::Ty;
10+
use rustc_middle::ty::{self, Ty};
1111
use rustc_span::source_map::Spanned;
1212
use rustc_span::symbol::{sym, Symbol};
1313
use rustc_typeck::hir_ty_to_ty;
@@ -332,18 +332,21 @@ pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic
332332
} else {
333333
// (&x).into_iter() ==> x.iter()
334334
// (&mut x).into_iter() ==> x.iter_mut()
335-
match &arg.kind {
336-
ExprKind::AddrOf(BorrowKind::Ref, mutability, arg_inner)
337-
if has_iter_method(cx, cx.typeck_results().expr_ty(arg_inner)).is_some() =>
338-
{
339-
let meth_name = match mutability {
335+
let arg_ty = cx.typeck_results().expr_ty_adjusted(arg);
336+
match &arg_ty.kind() {
337+
ty::Ref(_, inner_ty, mutbl) if has_iter_method(cx, inner_ty).is_some() => {
338+
let method_name = match mutbl {
340339
Mutability::Mut => "iter_mut",
341340
Mutability::Not => "iter",
342341
};
342+
let caller = match &arg.kind {
343+
ExprKind::AddrOf(BorrowKind::Ref, _, arg_inner) => arg_inner,
344+
_ => arg,
345+
};
343346
format!(
344347
"{}.{}()",
345-
sugg::Sugg::hir_with_applicability(cx, arg_inner, "_", applic_ref).maybe_par(),
346-
meth_name,
348+
sugg::Sugg::hir_with_applicability(cx, caller, "_", applic_ref).maybe_par(),
349+
method_name,
347350
)
348351
},
349352
_ => format!(

‎tests/ui/explicit_counter_loop.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ error: the variable `idx_usize` is used as a loop counter
4646
--> $DIR/explicit_counter_loop.rs:170:9
4747
|
4848
LL | for _item in slice {
49-
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.into_iter().enumerate()`
49+
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.iter().enumerate()`
5050

5151
error: the variable `idx_u32` is used as a loop counter
5252
--> $DIR/explicit_counter_loop.rs:182:9
5353
|
5454
LL | for _item in slice {
55-
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.into_iter())`
55+
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.iter())`
5656
|
5757
= note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
5858

‎tests/ui/manual_flatten.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ fn main() {
2626
}
2727

2828
// Test for loop over an implicit reference
29-
// Note: if `clippy::manual_flatten` is made autofixable, this case will
30-
// lead to a follow-up lint `clippy::into_iter_on_ref`
3129
let z = &y;
3230
for n in z {
3331
if let Ok(n) = n {

‎tests/ui/manual_flatten.stderr‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ LL | | }
6363
| |_________^
6464

6565
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
66-
--> $DIR/manual_flatten.rs:32:5
66+
--> $DIR/manual_flatten.rs:30:5
6767
|
6868
LL | for n in z {
69-
| ^ - help: try: `z.into_iter().flatten()`
69+
| ^ - help: try: `z.iter().flatten()`
7070
| _____|
7171
| |
7272
LL | | if let Ok(n) = n {
@@ -76,15 +76,15 @@ LL | | }
7676
| |_____^
7777
|
7878
help: ...and remove the `if let` statement in the for loop
79-
--> $DIR/manual_flatten.rs:33:9
79+
--> $DIR/manual_flatten.rs:31:9
8080
|
8181
LL | / if let Ok(n) = n {
8282
LL | | println!("{}", n);
8383
LL | | }
8484
| |_________^
8585

8686
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
87-
--> $DIR/manual_flatten.rs:41:5
87+
--> $DIR/manual_flatten.rs:39:5
8888
|
8989
LL | for n in z {
9090
| ^ - help: try: `z.flatten()`
@@ -97,15 +97,15 @@ LL | | }
9797
| |_____^
9898
|
9999
help: ...and remove the `if let` statement in the for loop
100-
--> $DIR/manual_flatten.rs:42:9
100+
--> $DIR/manual_flatten.rs:40:9
101101
|
102102
LL | / if let Some(m) = n {
103103
LL | | println!("{}", m);
104104
LL | | }
105105
| |_________^
106106

107107
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
108-
--> $DIR/manual_flatten.rs:74:5
108+
--> $DIR/manual_flatten.rs:72:5
109109
|
110110
LL | for n in &vec_of_ref {
111111
| ^ ----------- help: try: `vec_of_ref.iter().copied().flatten()`
@@ -118,18 +118,18 @@ LL | | }
118118
| |_____^
119119
|
120120
help: ...and remove the `if let` statement in the for loop
121-
--> $DIR/manual_flatten.rs:75:9
121+
--> $DIR/manual_flatten.rs:73:9
122122
|
123123
LL | / if let Some(n) = n {
124124
LL | | println!("{:?}", n);
125125
LL | | }
126126
| |_________^
127127

128128
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
129-
--> $DIR/manual_flatten.rs:81:5
129+
--> $DIR/manual_flatten.rs:79:5
130130
|
131131
LL | for n in vec_of_ref {
132-
| ^ ---------- help: try: `vec_of_ref.into_iter().copied().flatten()`
132+
| ^ ---------- help: try: `vec_of_ref.iter().copied().flatten()`
133133
| _____|
134134
| |
135135
LL | | if let Some(n) = n {
@@ -139,18 +139,18 @@ LL | | }
139139
| |_____^
140140
|
141141
help: ...and remove the `if let` statement in the for loop
142-
--> $DIR/manual_flatten.rs:82:9
142+
--> $DIR/manual_flatten.rs:80:9
143143
|
144144
LL | / if let Some(n) = n {
145145
LL | | println!("{:?}", n);
146146
LL | | }
147147
| |_________^
148148

149149
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
150-
--> $DIR/manual_flatten.rs:88:5
150+
--> $DIR/manual_flatten.rs:86:5
151151
|
152152
LL | for n in slice_of_ref {
153-
| ^ ------------ help: try: `slice_of_ref.into_iter().copied().flatten()`
153+
| ^ ------------ help: try: `slice_of_ref.iter().copied().flatten()`
154154
| _____|
155155
| |
156156
LL | | if let Some(n) = n {
@@ -160,7 +160,7 @@ LL | | }
160160
| |_____^
161161
|
162162
help: ...and remove the `if let` statement in the for loop
163-
--> $DIR/manual_flatten.rs:89:9
163+
--> $DIR/manual_flatten.rs:87:9
164164
|
165165
LL | / if let Some(n) = n {
166166
LL | | println!("{:?}", n);

0 commit comments

Comments
 (0)