Skip to content

Commit 9638f9c

Browse files
sergiogirosergiogiro
committed
Avoid by_ref in suggestions to remove while-let loops
1 parent 03ab7b8 commit 9638f9c

File tree

4 files changed

+116
-39
lines changed

4 files changed

+116
-39
lines changed

‎clippy_lints/src/loops/while_let_on_iterator.rs‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ use super::WHILE_LET_ON_ITERATOR;
44
use clippy_utils::diagnostics::span_lint_and_sugg;
55
use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
66
use clippy_utils::source::snippet_with_applicability;
7+
use clippy_utils::sugg::Sugg;
78
use clippy_utils::visitors::is_res_used;
89
use clippy_utils::{as_some_pattern, get_enclosing_loop_or_multi_call_closure, higher, is_refutable};
910
use rustc_errors::Applicability;
1011
use rustc_hir::def::Res;
1112
use rustc_hir::intravisit::{Visitor, walk_expr};
12-
use rustc_hir::{Closure, Expr, ExprKind, HirId, LetStmt, Mutability, UnOp};
13+
use rustc_hir::{Closure, Expr, ExprKind, HirId, LetStmt, UnOp};
1314
use rustc_lint::LateContext;
1415
use rustc_middle::hir::nested_filter::OnlyBodies;
1516
use rustc_middle::ty::adjustment::Adjust;
@@ -45,24 +46,34 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
4546
// If the iterator is a field or the iterator is accessed after the loop is complete it needs to be
4647
// borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used
4748
// afterwards a mutable borrow of a field isn't necessary.
48-
let by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut)
49-
|| !iter_expr_struct.can_move
50-
|| !iter_expr_struct.fields.is_empty()
51-
|| needs_mutable_borrow(cx, &iter_expr_struct, expr)
52-
{
53-
".by_ref()"
54-
} else {
55-
""
49+
let iterator = {
50+
let base = Sugg::hir_with_applicability(cx, iter_expr, "_", &mut applicability);
51+
if !iter_expr_struct.can_move
52+
|| !iter_expr_struct.fields.is_empty()
53+
|| needs_mutable_borrow(cx, &iter_expr_struct, expr)
54+
{
55+
let needs_deref = cx
56+
.typeck_results()
57+
.expr_adjustments(iter_expr)
58+
.iter()
59+
.any(|a| matches!(a.kind, Adjust::Deref(_)));
60+
if needs_deref {
61+
base.mut_addr_deref()
62+
} else {
63+
base.mut_addr()
64+
}
65+
} else {
66+
base
67+
}
5668
};
5769

58-
let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability);
5970
span_lint_and_sugg(
6071
cx,
6172
WHILE_LET_ON_ITERATOR,
6273
expr.span.with_hi(let_expr.span.hi()),
6374
"this loop could be written as a `for` loop",
6475
"try",
65-
format!("{loop_label}for {loop_var} in {iterator}{by_ref}"),
76+
format!("{loop_label}for {loop_var} in {iterator}"),
6677
applicability,
6778
);
6879
}

‎tests/ui/while_let_on_iterator.fixed‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ fn issue6491() {
202202
// Used in outer loop, needs &mut
203203
let mut it = 1..40;
204204
while let Some(n) = it.next() {
205-
for m in it.by_ref() {
205+
for m in &mut it {
206206
//~^ while_let_on_iterator
207207
if m % 10 == 0 {
208208
break;
@@ -237,7 +237,7 @@ fn issue6491() {
237237

238238
// Used after the loop, needs &mut.
239239
let mut it = 1..40;
240-
for m in it.by_ref() {
240+
for m in &mut it {
241241
//~^ while_let_on_iterator
242242
if m % 10 == 0 {
243243
break;
@@ -255,7 +255,7 @@ fn issue6231() {
255255
let mut it = 1..40;
256256
let mut opt = Some(0);
257257
while let Some(n) = opt.take().or_else(|| it.next()) {
258-
for m in it.by_ref() {
258+
for m in &mut it {
259259
//~^ while_let_on_iterator
260260
if n % 10 == 0 {
261261
break;
@@ -271,7 +271,7 @@ fn issue1924() {
271271
impl<T: Iterator<Item = u32>> S<T> {
272272
fn f(&mut self) -> Option<u32> {
273273
// Used as a field.
274-
for i in self.0.by_ref() {
274+
for i in &mut self.0 {
275275
//~^ while_let_on_iterator
276276
if !(3..8).contains(&i) {
277277
return Some(i);
@@ -304,7 +304,7 @@ fn issue1924() {
304304
}
305305
}
306306
// This one is fine, a different field is borrowed
307-
for i in self.0.0.0.by_ref() {
307+
for i in &mut self.0.0.0 {
308308
//~^ while_let_on_iterator
309309
if i == 1 {
310310
return self.0.1.take();
@@ -334,7 +334,7 @@ fn issue1924() {
334334

335335
// Needs &mut, field of the iterator is accessed after the loop
336336
let mut it = S2(1..40, 0);
337-
for n in it.by_ref() {
337+
for n in &mut it {
338338
//~^ while_let_on_iterator
339339
if n == 0 {
340340
break;
@@ -347,7 +347,7 @@ fn issue7249() {
347347
let mut it = 0..10;
348348
let mut x = || {
349349
// Needs &mut, the closure can be called multiple times
350-
for x in it.by_ref() {
350+
for x in &mut it {
351351
//~^ while_let_on_iterator
352352
if x % 2 == 0 {
353353
break;
@@ -362,7 +362,7 @@ fn issue7510() {
362362
let mut it = 0..10;
363363
let it = &mut it;
364364
// Needs to reborrow `it` as the binding isn't mutable
365-
for x in it.by_ref() {
365+
for x in &mut *it {
366366
//~^ while_let_on_iterator
367367
if x % 2 == 0 {
368368
break;
@@ -374,7 +374,7 @@ fn issue7510() {
374374
let mut it = 0..10;
375375
let it = S(&mut it);
376376
// Needs to reborrow `it.0` as the binding isn't mutable
377-
for x in it.0.by_ref() {
377+
for x in &mut *it.0 {
378378
//~^ while_let_on_iterator
379379
if x % 2 == 0 {
380380
break;
@@ -410,15 +410,15 @@ fn custom_deref() {
410410
}
411411

412412
let mut s = S2(S1 { x: 0..10 });
413-
for x in s.x.by_ref() {
413+
for x in &mut s.x {
414414
//~^ while_let_on_iterator
415415
println!("{}", x);
416416
}
417417
}
418418

419419
fn issue_8113() {
420420
let mut x = [0..10];
421-
for x in x[0].by_ref() {
421+
for x in &mut x[0] {
422422
//~^ while_let_on_iterator
423423
println!("{}", x);
424424
}
@@ -427,7 +427,7 @@ fn issue_8113() {
427427
fn fn_once_closure() {
428428
let mut it = 0..10;
429429
(|| {
430-
for x in it.by_ref() {
430+
for x in &mut it {
431431
//~^ while_let_on_iterator
432432
if x % 2 == 0 {
433433
break;
@@ -449,7 +449,7 @@ fn fn_once_closure() {
449449
fn f2(_: impl FnMut()) {}
450450
let mut it = 0..10;
451451
f2(|| {
452-
for x in it.by_ref() {
452+
for x in &mut it {
453453
//~^ while_let_on_iterator
454454
if x % 2 == 0 {
455455
break;
@@ -492,6 +492,33 @@ fn issue13123() {
492492
}
493493
}
494494

495+
fn issue16089() {
496+
trait CertainTrait: Iterator<Item = u8> {
497+
fn iter_over_self(&mut self) {
498+
let mut a = 0;
499+
for r in &mut *self {
500+
//~^ while_let_on_iterator
501+
a = r;
502+
}
503+
self.use_after_iter()
504+
}
505+
506+
fn use_after_iter(&mut self) {}
507+
}
508+
}
509+
510+
fn issue16089_no_use_after_iter() {
511+
trait CertainTrait: Iterator<Item = u8> {
512+
fn iter_over_self(&mut self) {
513+
let mut a = 0;
514+
for r in self {
515+
//~^ while_let_on_iterator
516+
a = r;
517+
}
518+
}
519+
}
520+
}
521+
495522
fn main() {
496523
let mut it = 0..20;
497524
for _ in it {

‎tests/ui/while_let_on_iterator.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,33 @@ fn issue13123() {
492492
}
493493
}
494494

495+
fn issue16089() {
496+
trait CertainTrait: Iterator<Item = u8> {
497+
fn iter_over_self(&mut self) {
498+
let mut a = 0;
499+
while let Some(r) = self.next() {
500+
//~^ while_let_on_iterator
501+
a = r;
502+
}
503+
self.use_after_iter()
504+
}
505+
506+
fn use_after_iter(&mut self) {}
507+
}
508+
}
509+
510+
fn issue16089_no_use_after_iter() {
511+
trait CertainTrait: Iterator<Item = u8> {
512+
fn iter_over_self(&mut self) {
513+
let mut a = 0;
514+
while let Some(r) = self.next() {
515+
//~^ while_let_on_iterator
516+
a = r;
517+
}
518+
}
519+
}
520+
}
521+
495522
fn main() {
496523
let mut it = 0..20;
497524
while let Some(..) = it.next() {

‎tests/ui/while_let_on_iterator.stderr‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ error: this loop could be written as a `for` loop
4747
--> tests/ui/while_let_on_iterator.rs:205:9
4848
|
4949
LL | while let Some(m) = it.next() {
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()`
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it`
5151

5252
error: this loop could be written as a `for` loop
5353
--> tests/ui/while_let_on_iterator.rs:217:5
@@ -71,67 +71,67 @@ error: this loop could be written as a `for` loop
7171
--> tests/ui/while_let_on_iterator.rs:240:9
7272
|
7373
LL | while let Some(m) = it.next() {
74-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()`
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it`
7575

7676
error: this loop could be written as a `for` loop
7777
--> tests/ui/while_let_on_iterator.rs:258:9
7878
|
7979
LL | while let Some(m) = it.next() {
80-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()`
80+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it`
8181

8282
error: this loop could be written as a `for` loop
8383
--> tests/ui/while_let_on_iterator.rs:274:13
8484
|
8585
LL | while let Some(i) = self.0.next() {
86-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.by_ref()`
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0`
8787

8888
error: this loop could be written as a `for` loop
8989
--> tests/ui/while_let_on_iterator.rs:307:13
9090
|
9191
LL | while let Some(i) = self.0.0.0.next() {
92-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.0.0.by_ref()`
92+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0.0.0`
9393

9494
error: this loop could be written as a `for` loop
9595
--> tests/ui/while_let_on_iterator.rs:337:5
9696
|
9797
LL | while let Some(n) = it.next() {
98-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it.by_ref()`
98+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it`
9999

100100
error: this loop could be written as a `for` loop
101101
--> tests/ui/while_let_on_iterator.rs:350:9
102102
|
103103
LL | while let Some(x) = it.next() {
104-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()`
104+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it`
105105

106106
error: this loop could be written as a `for` loop
107107
--> tests/ui/while_let_on_iterator.rs:365:5
108108
|
109109
LL | while let Some(x) = it.next() {
110-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()`
110+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it`
111111

112112
error: this loop could be written as a `for` loop
113113
--> tests/ui/while_let_on_iterator.rs:377:5
114114
|
115115
LL | while let Some(x) = it.0.next() {
116-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.0.by_ref()`
116+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it.0`
117117

118118
error: this loop could be written as a `for` loop
119119
--> tests/ui/while_let_on_iterator.rs:413:5
120120
|
121121
LL | while let Some(x) = s.x.next() {
122-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in s.x.by_ref()`
122+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut s.x`
123123

124124
error: this loop could be written as a `for` loop
125125
--> tests/ui/while_let_on_iterator.rs:421:5
126126
|
127127
LL | while let Some(x) = x[0].next() {
128-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in x[0].by_ref()`
128+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut x[0]`
129129

130130
error: this loop could be written as a `for` loop
131131
--> tests/ui/while_let_on_iterator.rs:430:9
132132
|
133133
LL | while let Some(x) = it.next() {
134-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()`
134+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it`
135135

136136
error: this loop could be written as a `for` loop
137137
--> tests/ui/while_let_on_iterator.rs:441:9
@@ -143,7 +143,7 @@ error: this loop could be written as a `for` loop
143143
--> tests/ui/while_let_on_iterator.rs:452:9
144144
|
145145
LL | while let Some(x) = it.next() {
146-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()`
146+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it`
147147

148148
error: this loop could be written as a `for` loop
149149
--> tests/ui/while_let_on_iterator.rs:463:9
@@ -164,10 +164,22 @@ LL | 'label: while let Some(n) = it.next() {
164164
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'label: for n in it`
165165

166166
error: this loop could be written as a `for` loop
167-
--> tests/ui/while_let_on_iterator.rs:497:5
167+
--> tests/ui/while_let_on_iterator.rs:499:13
168+
|
169+
LL | while let Some(r) = self.next() {
170+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in &mut *self`
171+
172+
error: this loop could be written as a `for` loop
173+
--> tests/ui/while_let_on_iterator.rs:514:13
174+
|
175+
LL | while let Some(r) = self.next() {
176+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in self`
177+
178+
error: this loop could be written as a `for` loop
179+
--> tests/ui/while_let_on_iterator.rs:524:5
168180
|
169181
LL | while let Some(..) = it.next() {
170182
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
171183

172-
error: aborting due to 28 previous errors
184+
error: aborting due to 30 previous errors
173185

0 commit comments

Comments
 (0)