mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 23:24:24 +00:00
Differentiate between mutable iteration and immutable iteration in needless_range_loop
This commit is contained in:
Oliver Schneider
parent
088555c4ea
commit
652df0fb79
3 changed files with 100 additions and 5 deletions
|
|
@ -952,10 +952,12 @@ fn check_for_loop_range<'a, 'tcx>(
|
|||
let mut visitor = VarVisitor {
|
||||
cx: cx,
|
||||
var: canonical_id,
|
||||
indexed_mut: HashSet::new(),
|
||||
indexed: HashMap::new(),
|
||||
indexed_directly: HashMap::new(),
|
||||
referenced: HashSet::new(),
|
||||
nonindex: false,
|
||||
prefer_mutable: false,
|
||||
};
|
||||
walk_expr(&mut visitor, body);
|
||||
|
||||
|
|
@ -1009,6 +1011,12 @@ fn check_for_loop_range<'a, 'tcx>(
|
|||
"".to_owned()
|
||||
};
|
||||
|
||||
let (ref_mut, method) = if visitor.indexed_mut.contains(&indexed) {
|
||||
("mut ", "iter_mut")
|
||||
} else {
|
||||
("", "iter")
|
||||
};
|
||||
|
||||
if visitor.nonindex {
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
|
|
@ -1021,16 +1029,16 @@ fn check_for_loop_range<'a, 'tcx>(
|
|||
"consider using an iterator".to_string(),
|
||||
vec![
|
||||
(pat.span, format!("({}, <item>)", ident.node)),
|
||||
(arg.span, format!("{}.iter().enumerate(){}{}", indexed, take, skip)),
|
||||
(arg.span, format!("{}.{}().enumerate(){}{}", indexed, method, take, skip)),
|
||||
],
|
||||
);
|
||||
},
|
||||
);
|
||||
} else {
|
||||
let repl = if starts_at_zero && take.is_empty() {
|
||||
format!("&{}", indexed)
|
||||
format!("&{}{}", ref_mut, indexed)
|
||||
} else {
|
||||
format!("{}.iter(){}{}", indexed, take, skip)
|
||||
format!("{}.{}(){}{}", indexed, method, take, skip)
|
||||
};
|
||||
|
||||
span_lint_and_then(
|
||||
|
|
@ -1537,6 +1545,8 @@ struct VarVisitor<'a, 'tcx: 'a> {
|
|||
cx: &'a LateContext<'a, 'tcx>,
|
||||
/// var name to look for as index
|
||||
var: ast::NodeId,
|
||||
/// indexed variables that are used mutably
|
||||
indexed_mut: HashSet<Name>,
|
||||
/// indexed variables, the extend is `None` for global
|
||||
indexed: HashMap<Name, Option<region::Scope>>,
|
||||
/// subset of `indexed` of vars that are indexed directly: `v[i]`
|
||||
|
|
@ -1548,6 +1558,9 @@ struct VarVisitor<'a, 'tcx: 'a> {
|
|||
/// has the loop variable been used in expressions other than the index of
|
||||
/// an index op?
|
||||
nonindex: bool,
|
||||
/// Whether we are inside the `$` in `&mut $` or `$ = foo` or `$.bar`, where bar
|
||||
/// takes `&mut self`
|
||||
prefer_mutable: bool,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
|
||||
|
|
@ -1572,6 +1585,9 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
|
|||
};
|
||||
|
||||
if index_used {
|
||||
if self.prefer_mutable {
|
||||
self.indexed_mut.insert(seqvar.segments[0].name);
|
||||
}
|
||||
let def = self.cx.tables.qpath_def(seqpath, seqexpr.hir_id);
|
||||
match def {
|
||||
Def::Local(node_id) | Def::Upvar(node_id, ..) => {
|
||||
|
|
@ -1615,8 +1631,47 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
walk_expr(self, expr);
|
||||
let old = self.prefer_mutable;
|
||||
match expr.node {
|
||||
ExprAssignOp(_, ref lhs, ref rhs) |
|
||||
ExprAssign(ref lhs, ref rhs) => {
|
||||
self.prefer_mutable = true;
|
||||
self.visit_expr(lhs);
|
||||
self.prefer_mutable = false;
|
||||
self.visit_expr(rhs);
|
||||
},
|
||||
ExprAddrOf(mutbl, ref expr) => {
|
||||
if mutbl == MutMutable {
|
||||
self.prefer_mutable = true;
|
||||
}
|
||||
self.visit_expr(expr);
|
||||
},
|
||||
ExprCall(ref f, ref args) => {
|
||||
for (ty, expr) in self.cx.tables.expr_ty(f).fn_sig(self.cx.tcx).inputs().skip_binder().iter().zip(args) {
|
||||
self.prefer_mutable = false;
|
||||
if let ty::TyRef(_, mutbl) = ty.sty {
|
||||
if mutbl.mutbl == MutMutable {
|
||||
self.prefer_mutable = true;
|
||||
}
|
||||
}
|
||||
self.visit_expr(expr);
|
||||
}
|
||||
},
|
||||
ExprMethodCall(_, _, ref args) => {
|
||||
let def_id = self.cx.tables.type_dependent_defs()[expr.hir_id].def_id();
|
||||
for (ty, expr) in self.cx.tcx.fn_sig(def_id).inputs().skip_binder().iter().zip(args) {
|
||||
self.prefer_mutable = false;
|
||||
if let ty::TyRef(_, mutbl) = ty.sty {
|
||||
if mutbl.mutbl == MutMutable {
|
||||
self.prefer_mutable = true;
|
||||
}
|
||||
}
|
||||
self.visit_expr(expr);
|
||||
}
|
||||
},
|
||||
_ => walk_expr(self, expr),
|
||||
}
|
||||
self.prefer_mutable = old;
|
||||
}
|
||||
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
|
||||
NestedVisitorMap::None
|
||||
|
|
|
|||
|
|
@ -24,4 +24,17 @@ fn main() {
|
|||
for i in 3..10 {
|
||||
println!("{}", ns[calc_idx(i) % 4]);
|
||||
}
|
||||
|
||||
let mut ms = vec![1, 2, 3, 4, 5, 6];
|
||||
for i in 0..ms.len() {
|
||||
ms[i] *= 2;
|
||||
}
|
||||
assert_eq!(ms, vec![2, 4, 6, 8, 10, 12]);
|
||||
|
||||
let mut ms = vec![1, 2, 3, 4, 5, 6];
|
||||
for i in 0..ms.len() {
|
||||
let x = &mut ms[i];
|
||||
*x *= 2;
|
||||
}
|
||||
assert_eq!(ms, vec![2, 4, 6, 8, 10, 12]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,3 +12,30 @@ help: consider using an iterator
|
|||
8 | for <item> in ns.iter().take(10).skip(3) {
|
||||
| ^^^^^^
|
||||
|
||||
error: the loop variable `i` is only used to index `ms`.
|
||||
--> $DIR/needless_range_loop.rs:29:5
|
||||
|
|
||||
29 | / for i in 0..ms.len() {
|
||||
30 | | ms[i] *= 2;
|
||||
31 | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using an iterator
|
||||
|
|
||||
29 | for <item> in &mut ms {
|
||||
| ^^^^^^
|
||||
|
||||
error: the loop variable `i` is only used to index `ms`.
|
||||
--> $DIR/needless_range_loop.rs:35:5
|
||||
|
|
||||
35 | / for i in 0..ms.len() {
|
||||
36 | | let x = &mut ms[i];
|
||||
37 | | *x *= 2;
|
||||
38 | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using an iterator
|
||||
|
|
||||
35 | for <item> in &mut ms {
|
||||
| ^^^^^^
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue