diff --git a/src/loops.rs b/src/loops.rs index 830fac94e46..22dfef55ec1 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -289,7 +289,7 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E 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_over_map_kv(cx, pat, arg, expr); + check_for_loop_over_map_kv(cx, pat, arg, body, expr); } /// Check for looping over a range and then indexing a sequence with it. @@ -520,12 +520,13 @@ fn check_for_loop_explicit_counter(cx: &LateContext, arg: &Expr, body: &Expr, ex } // Check for the FOR_KV_MAP lint. -fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) { +fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) { if let PatTup(ref pat) = pat.node { if pat.len() == 2 { + let (pat_span, kind) = match (&pat[0].node, &pat[1].node) { - (key, _) if pat_is_wild(key) => (&pat[1].span, "values"), - (_, value) if pat_is_wild(value) => (&pat[0].span, "keys"), + (key, _) if pat_is_wild(key, body) => (&pat[1].span, "values"), + (_, value) if pat_is_wild(value, body) => (&pat[0].span, "keys"), _ => return }; @@ -558,14 +559,39 @@ fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Ex } // Return true if the pattern is a `PatWild` or an ident prefixed with '_'. -fn pat_is_wild(pat: &Pat_) -> bool { +fn pat_is_wild(pat: &Pat_, body: &Expr) -> bool { match *pat { PatWild => true, - PatIdent(_, ident, None) if ident.node.name.as_str().starts_with('_') => true, + PatIdent(_, ident, None) if ident.node.name.as_str().starts_with('_') => { + let mut visitor = UsedVisitor { + var: ident.node, + used: false, + }; + walk_expr(&mut visitor, body); + !visitor.used + }, _ => false, } } +struct UsedVisitor { + var: Ident, // var to look for + used: bool, // has the var been used otherwise? +} + +impl<'a> Visitor<'a> for UsedVisitor { + fn visit_expr(&mut self, expr: &Expr) { + if let ExprPath(None, ref path) = expr.node { + if path.segments.len() == 1 && path.segments[0].identifier == self.var { + self.used = true; + return + } + } + + walk_expr(self, expr); + } +} + /// Recover the essential nodes of a desugared for loop: /// `for pat in arg { body }` becomes `(pat, arg, body)`. fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> { diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 7049a21b15e..e361ebe777f 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -290,10 +290,23 @@ fn main() { } let rm = &m; - for (k, _values) in rm { + for (k, _value) in rm { //~^ you seem to want to iterate on a map's keys //~| HELP use the corresponding method //~| SUGGESTION for k in rm.keys() let _k = k; } + + test_for_kv_map(); +} + +#[allow(used_underscore_binding)] +fn test_for_kv_map() { + let m : HashMap = HashMap::new(); + + // No error, _value is actually used + for (k, _value) in &m { + let _ = _value; + let _k = k; + } }