Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 07:00:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #4691 - HMPerson1:suggest_iter, r=phansch

Browse source 
Fix suggestion of `explicit_counter_loop`

changelog: In the suggestion of `explicit_counter_loop`, if the `for` loop argument doesn't implement `Iterator`, then we suggest `x.into_iter().enumerate()` (or `x.iter{_mut}()` as appropriate). Also, the span of the suggestion has been corrected.

Fixes #4678
This commit is contained in:
bors 2019-10-23 20:12:14 +00:00
commit 087e5eaea5
4 changed files with 92 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

63
clippy_lints/src/loops.rs
View file

@ -27,9 +27,10 @@ use syntax_pos::{BytePos, Symbol};
use crate::utils::paths;
use crate::utils::{
get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_const, is_refutable, last_path_segment,
match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability,
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq,
get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg,
span_lint_and_then, SpanlessEq,
};
declare_clippy_lint! {
@ -1460,27 +1461,26 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(
if visitor2.state == VarState::Warn {
if let Some(name) = visitor2.name {
let mut applicability = Applicability::MachineApplicable;
// for some reason this is the only way to get the `Span`
// of the entire `for` loop
let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind {
arms[0].body.span
} else {
unreachable!()
};
span_lint_and_sugg(
cx,
EXPLICIT_COUNTER_LOOP,
expr.span,
for_span.with_hi(arg.span.hi()),
&format!("the variable `{}` is used as a loop counter.", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
if higher::range(cx, arg).is_some() {
format!(
"({})",
snippet_with_applicability(cx, arg.span, "_", &mut applicability)
)
} else {
format!(
"{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability).maybe_par()
)
}
make_iterator_snippet(cx, arg, &mut applicability),
),
applicability,
);
@ -1490,6 +1490,39 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(
}
}
/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
/// actual `Iterator` that the loop uses.
fn make_iterator_snippet(cx: &LateContext<'_, '_>, arg: &Expr, applic_ref: &mut Applicability) -> String {
let impls_iterator = get_trait_def_id(cx, &paths::ITERATOR)
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(arg), id, &[]));
if impls_iterator {
format!(
"{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
)
} else {
// (&x).into_iter() ==> x.iter()
// (&mut x).into_iter() ==> x.iter_mut()
match &arg.kind {
ExprKind::AddrOf(mutability, arg_inner) if has_iter_method(cx, cx.tables.expr_ty(&arg_inner)).is_some() => {
let meth_name = match mutability {
MutMutable => "iter_mut",
MutImmutable => "iter",
};
format!(
"{}.{}()",
sugg::Sugg::hir_with_applicability(cx, &arg_inner, "_", applic_ref).maybe_par(),
meth_name,
)
},
_ => format!(
"{}.into_iter()",
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
),
}
}
}
/// Checks for the `FOR_KV_MAP` lint.
fn check_for_loop_over_map_kv<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,

14
clippy_lints/src/utils/sugg.rs
View file

@ -46,7 +46,7 @@ impl<'a> Sugg<'a> {
pub fn hir_opt(cx: &LateContext<'_, '_>, expr: &hir::Expr) -> Option<Self> {
snippet_opt(cx, expr.span).map(|snippet| {
let snippet = Cow::Owned(snippet);
Self::hir_from_snippet(expr, snippet)
Self::hir_from_snippet(cx, expr, snippet)
})
}
@ -84,12 +84,20 @@ impl<'a> Sugg<'a> {
pub fn hir_with_macro_callsite(cx: &LateContext<'_, '_>, expr: &hir::Expr, default: &'a str) -> Self {
let snippet = snippet_with_macro_callsite(cx, expr.span, default);
Self::hir_from_snippet(expr, snippet)
Self::hir_from_snippet(cx, expr, snippet)
}
/// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*`
/// function variants of `Sugg`, since these use different snippet functions.
fn hir_from_snippet(expr: &hir::Expr, snippet: Cow<'a, str>) -> Self {
fn hir_from_snippet(cx: &LateContext<'_, '_>, expr: &hir::Expr, snippet: Cow<'a, str>) -> Self {
if let Some(range) = higher::range(cx, expr) {
let op = match range.limits {
ast::RangeLimits::HalfOpen => AssocOp::DotDot,
ast::RangeLimits::Closed => AssocOp::DotDotEq,
};
return Sugg::BinOp(op, snippet);
}
match expr.kind {
hir::ExprKind::AddrOf(..)
| hir::ExprKind::Box(..)

10
tests/ui/explicit_counter_loop.rs
View file

@ -12,6 +12,16 @@ fn main() {
for _v in &vec {
_index += 1
}
let mut _index = 0;
for _v in &mut vec {
_index += 1;
}
let mut _index = 0;
for _v in vec {
_index += 1;
}
}
mod issue_1219 {

34
tests/ui/explicit_counter_loop.stderr
View file

@ -1,34 +1,46 @@
error: the variable `_index` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:6:15
--> $DIR/explicit_counter_loop.rs:6:5
|
LL | for _v in &vec {
| ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()`
| ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
|
= note: `-D clippy::explicit-counter-loop` implied by `-D warnings`
error: the variable `_index` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:12:15
--> $DIR/explicit_counter_loop.rs:12:5
|
LL | for _v in &vec {
| ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()`
| ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
error: the variable `_index` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:17:5
|
LL | for _v in &mut vec {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter_mut().enumerate()`
error: the variable `_index` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:22:5
|
LL | for _v in vec {
| ^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.into_iter().enumerate()`
error: the variable `count` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:51:19
--> $DIR/explicit_counter_loop.rs:61:9
|
LL | for ch in text.chars() {
| ^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
error: the variable `count` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:62:19
--> $DIR/explicit_counter_loop.rs:72:9
|
LL | for ch in text.chars() {
| ^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
error: the variable `count` is used as a loop counter.
--> $DIR/explicit_counter_loop.rs:120:19
--> $DIR/explicit_counter_loop.rs:130:9
|
LL | for _i in 3..10 {
| ^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
| ^^^^^^^^^^^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
error: aborting due to 5 previous errors
error: aborting due to 7 previous errors