From bd6c2df066ce2bd3296faf537b9fbb9a1b967d73 Mon Sep 17 00:00:00 2001 From: rail <12975677+rail-rain@users.noreply.github.com> Date: Sat, 23 Mar 2019 15:30:17 +0900 Subject: [PATCH 1/3] Change explicit_counter_loop's message to reflect original variable name --- clippy_lints/src/loops.rs | 6 ++++-- tests/ui/explicit_counter_loop.stderr | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 0016625631c7..d88f3e6782f5 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -777,7 +777,7 @@ fn check_for_loop<'a, 'tcx>( check_for_loop_range(cx, pat, arg, body, expr); check_for_loop_reverse_range(cx, arg, expr); check_for_loop_arg(cx, pat, arg, expr); - check_for_loop_explicit_counter(cx, arg, body, expr); + check_for_loop_explicit_counter(cx, pat, arg, body, expr); check_for_loop_over_map_kv(cx, pat, arg, body, expr); check_for_mut_range_bound(cx, arg, body); detect_manual_memcpy(cx, pat, arg, body, expr); @@ -1453,6 +1453,7 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr) { fn check_for_loop_explicit_counter<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, + pat: &'tcx Pat, arg: &'tcx Expr, body: &'tcx Expr, expr: &'tcx Expr, @@ -1495,8 +1496,9 @@ fn check_for_loop_explicit_counter<'a, 'tcx>( expr.span, &format!( "the variable `{0}` is used as a loop counter. Consider using `for ({0}, \ - item) in {1}.enumerate()` or similar iterators", + {1}) in {2}.enumerate()` or similar iterators", name, + snippet(cx, pat.span, "_"), snippet(cx, arg.span, "_") ), ); diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index b1cfb31432f2..84ca0db8388b 100644 --- a/tests/ui/explicit_counter_loop.stderr +++ b/tests/ui/explicit_counter_loop.stderr @@ -1,4 +1,4 @@ -error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in &vec.enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:6:15 | LL | for _v in &vec { @@ -6,19 +6,19 @@ LL | for _v in &vec { | = note: `-D clippy::explicit-counter-loop` implied by `-D warnings` -error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in &vec.enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:12:15 | LL | for _v in &vec { | ^^^^ -error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators +error: the variable `count` is used as a loop counter. Consider using `for (count, ch) in text.chars().enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:51:19 | LL | for ch in text.chars() { | ^^^^^^^^^^^^ -error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators +error: the variable `count` is used as a loop counter. Consider using `for (count, ch) in text.chars().enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:62:19 | LL | for ch in text.chars() { From 9698e41994fad31474a20e2dc5f8664364404e1b Mon Sep 17 00:00:00 2001 From: rail <12975677+rail-rain@users.noreply.github.com> Date: Sat, 23 Mar 2019 15:36:48 +0900 Subject: [PATCH 2/3] Change explicit_counter_loop's message to add parentheses if necessary --- clippy_lints/src/loops.rs | 6 +++++- tests/ui/explicit_counter_loop.rs | 9 +++++++++ tests/ui/explicit_counter_loop.stderr | 12 +++++++++--- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index d88f3e6782f5..8f9323056bce 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1499,7 +1499,11 @@ fn check_for_loop_explicit_counter<'a, 'tcx>( {1}) in {2}.enumerate()` or similar iterators", name, snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") + if higher::range(cx, arg).is_some() { + format!("({})", snippet(cx, arg.span, "_")) + } else { + format!("{}", sugg::Sugg::hir(cx, arg, "_").maybe_par()) + } ), ); } diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs index 5efac85cf220..71ef1e8674ac 100644 --- a/tests/ui/explicit_counter_loop.rs +++ b/tests/ui/explicit_counter_loop.rs @@ -113,3 +113,12 @@ mod issue_3308 { } } } + +mod issue_1670 { + pub fn test() { + let mut count = 0; + for _i in 3..10 { + count += 1; + } + } +} diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index 84ca0db8388b..8982c28eb72e 100644 --- a/tests/ui/explicit_counter_loop.stderr +++ b/tests/ui/explicit_counter_loop.stderr @@ -1,4 +1,4 @@ -error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in &vec.enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in (&vec).enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:6:15 | LL | for _v in &vec { @@ -6,7 +6,7 @@ LL | for _v in &vec { | = note: `-D clippy::explicit-counter-loop` implied by `-D warnings` -error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in &vec.enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in (&vec).enumerate()` or similar iterators --> $DIR/explicit_counter_loop.rs:12:15 | LL | for _v in &vec { @@ -24,5 +24,11 @@ error: the variable `count` is used as a loop counter. Consider using `for (coun LL | for ch in text.chars() { | ^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: the variable `count` is used as a loop counter. Consider using `for (count, _i) in (3..10).enumerate()` or similar iterators + --> $DIR/explicit_counter_loop.rs:120:19 + | +LL | for _i in 3..10 { + | ^^^^^ + +error: aborting due to 5 previous errors From 2b82c71b559d9886e3ab3a7ad0194a5fe4aab58b Mon Sep 17 00:00:00 2001 From: rail <12975677+rail-rain@users.noreply.github.com> Date: Thu, 28 Mar 2019 08:51:57 +0900 Subject: [PATCH 3/3] use `span_lint_and_sugg` in `explicit_counter_loop` --- clippy_lints/src/loops.rs | 24 +++++++++++++++++------- tests/ui/explicit_counter_loop.stderr | 20 ++++++++++---------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 8f9323056bce..c2b44ee7018b 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1490,21 +1490,31 @@ fn check_for_loop_explicit_counter<'a, 'tcx>( if visitor2.state == VarState::Warn { if let Some(name) = visitor2.name { - span_lint( + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( cx, EXPLICIT_COUNTER_LOOP, expr.span, - &format!( - "the variable `{0}` is used as a loop counter. Consider using `for ({0}, \ - {1}) in {2}.enumerate()` or similar iterators", + &format!("the variable `{}` is used as a loop counter.", name), + "consider using", + format!( + "for ({}, {}) in {}.enumerate()", name, - snippet(cx, pat.span, "_"), + snippet_with_applicability(cx, pat.span, "item", &mut applicability), if higher::range(cx, arg).is_some() { - format!("({})", snippet(cx, arg.span, "_")) + format!( + "({})", + snippet_with_applicability(cx, arg.span, "_", &mut applicability) + ) } else { - format!("{}", sugg::Sugg::hir(cx, arg, "_").maybe_par()) + format!( + "{}", + sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability) + .maybe_par() + ) } ), + applicability, ); } } diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index 8982c28eb72e..5efd51abf181 100644 --- a/tests/ui/explicit_counter_loop.stderr +++ b/tests/ui/explicit_counter_loop.stderr @@ -1,34 +1,34 @@ -error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in (&vec).enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. --> $DIR/explicit_counter_loop.rs:6:15 | LL | for _v in &vec { - | ^^^^ + | ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()` | = note: `-D clippy::explicit-counter-loop` implied by `-D warnings` -error: the variable `_index` is used as a loop counter. Consider using `for (_index, _v) in (&vec).enumerate()` or similar iterators +error: the variable `_index` is used as a loop counter. --> $DIR/explicit_counter_loop.rs:12:15 | LL | for _v in &vec { - | ^^^^ + | ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()` -error: the variable `count` is used as a loop counter. Consider using `for (count, ch) in text.chars().enumerate()` or similar iterators +error: the variable `count` is used as a loop counter. --> $DIR/explicit_counter_loop.rs:51:19 | LL | for ch in text.chars() { - | ^^^^^^^^^^^^ + | ^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()` -error: the variable `count` is used as a loop counter. Consider using `for (count, ch) in text.chars().enumerate()` or similar iterators +error: the variable `count` is used as a loop counter. --> $DIR/explicit_counter_loop.rs:62:19 | LL | for ch in text.chars() { - | ^^^^^^^^^^^^ + | ^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()` -error: the variable `count` is used as a loop counter. Consider using `for (count, _i) in (3..10).enumerate()` or similar iterators +error: the variable `count` is used as a loop counter. --> $DIR/explicit_counter_loop.rs:120:19 | LL | for _i in 3..10 { - | ^^^^^ + | ^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()` error: aborting due to 5 previous errors