Enhance LocalUsedVisitor to check closure bodies

6 files changed, 45 insertions, 26 deletions

diff --git a/clippy_lints/src/collapsible_match.rs b/clippy_lints/src/collapsible_match.rs
index 834f294283e..75a973fe37e 100644
--- a/clippy_lints/src/collapsible_match.rs
+++ b/clippy_lints/src/collapsible_match.rs
@@ -60,7 +60,7 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
     }
 }
 
-fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) {
+fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) {
     if_chain! {
         let expr = strip_singleton_blocks(arm.body);
         if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
@@ -84,14 +84,13 @@ fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) {
         // the "wild-like" branches must be equal
         if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
         // the binding must not be used in the if guard
+        let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
         if match arm.guard {
             None => true,
-            Some(Guard::If(expr) | Guard::IfLet(_, expr)) => {
-                !LocalUsedVisitor::new(binding_id).check_expr(expr)
-            }
+            Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr),
         };
         // ...or anywhere in the inner match
-        if !arms_inner.iter().any(|arm| LocalUsedVisitor::new(binding_id).check_arm(arm));
+        if !arms_inner.iter().any(|arm| used_visitor.check_arm(arm));
         then {
             span_lint_and_then(
                 cx,
diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs
index 6beaa51729a..5863eef8a26 100644
--- a/clippy_lints/src/let_if_seq.rs
+++ b/clippy_lints/src/let_if_seq.rs
@@ -63,10 +63,11 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
                 if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind;
                 if let hir::StmtKind::Expr(ref if_) = expr.kind;
                 if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.kind;
-                if !LocalUsedVisitor::new(canonical_id).check_expr(cond);
+                let mut used_visitor = LocalUsedVisitor::new(cx, canonical_id);
+                if !used_visitor.check_expr(cond);
                 if let hir::ExprKind::Block(ref then, _) = then.kind;
-                if let Some(value) = check_assign(canonical_id, &*then);
-                if !LocalUsedVisitor::new(canonical_id).check_expr(value);
+                if let Some(value) = check_assign(cx, canonical_id, &*then);
+                if !used_visitor.check_expr(value);
                 then {
                     let span = stmt.span.to(if_.span);
 
@@ -78,7 +79,7 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
 
                     let (default_multi_stmts, default) = if let Some(ref else_) = *else_ {
                         if let hir::ExprKind::Block(ref else_, _) = else_.kind {
-                            if let Some(default) = check_assign(canonical_id, else_) {
+                            if let Some(default) = check_assign(cx, canonical_id, else_) {
                                 (else_.stmts.len() > 1, default)
                             } else if let Some(ref default) = local.init {
                                 (true, &**default)
@@ -133,7 +134,11 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
     }
 }
 
-fn check_assign<'tcx>(decl: hir::HirId, block: &'tcx hir::Block<'_>) -> Option<&'tcx hir::Expr<'tcx>> {
+fn check_assign<'tcx>(
+    cx: &LateContext<'tcx>,
+    decl: hir::HirId,
+    block: &'tcx hir::Block<'_>,
+) -> Option<&'tcx hir::Expr<'tcx>> {
     if_chain! {
         if block.expr.is_none();
         if let Some(expr) = block.stmts.iter().last();
@@ -141,7 +146,7 @@ fn check_assign<'tcx>(decl: hir::HirId, block: &'tcx hir::Block<'_>) -> Option<&
         if let hir::ExprKind::Assign(ref var, ref value, _) = expr.kind;
         if path_to_local_id(var, decl);
         then {
-            let mut v = LocalUsedVisitor::new(decl);
+            let mut v = LocalUsedVisitor::new(cx, decl);
 
             if block.stmts.iter().take(block.stmts.len()-1).any(|stmt| v.check_stmt(stmt)) {
                 return None;
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index b5a9632ee19..eb185377e20 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1893,8 +1893,8 @@ fn check_for_loop_over_map_kv<'tcx>(
             let arg_span = arg.span;
             let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() {
                 ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
-                    (key, _) if pat_is_wild(key, body) => (pat[1].span, "value", ty, mutbl),
-                    (_, value) if pat_is_wild(value, body) => (pat[0].span, "key", ty, Mutability::Not),
+                    (key, _) if pat_is_wild(cx, key, body) => (pat[1].span, "value", ty, mutbl),
+                    (_, value) if pat_is_wild(cx, value, body) => (pat[0].span, "key", ty, Mutability::Not),
                     _ => return,
                 },
                 _ => return,
@@ -2145,11 +2145,11 @@ fn check_for_mutation<'tcx>(
 }
 
 /// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
-fn pat_is_wild<'tcx>(pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
+fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
     match *pat {
         PatKind::Wild => true,
         PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
-            !LocalUsedVisitor::new(id).check_expr(body)
+            !LocalUsedVisitor::new(cx, id).check_expr(body)
         },
         _ => false,
     }
@@ -2188,7 +2188,7 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
             then {
                 let index_used_directly = path_to_local_id(idx, self.var);
                 let indexed_indirectly = {
-                    let mut used_visitor = LocalUsedVisitor::new(self.var);
+                    let mut used_visitor = LocalUsedVisitor::new(self.cx, self.var);
                     walk_expr(&mut used_visitor, idx);
                     used_visitor.used
                 };
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index c4aa2b30e7b..e33001b16bc 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -911,7 +911,7 @@ fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms
     }
 }
 
-fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
+fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) {
     let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs();
     if is_type_diagnostic_item(cx, ex_ty, sym::result_type) {
         for arm in arms {
@@ -924,7 +924,9 @@ fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
                         // Looking for unused bindings (i.e.: `_e`)
                         inner.iter().for_each(|pat| {
                             if let PatKind::Binding(_, id, ident, None) = pat.kind {
-                                if ident.as_str().starts_with('_') && !LocalUsedVisitor::new(id).check_expr(arm.body) {
+                                if ident.as_str().starts_with('_')
+                                    && !LocalUsedVisitor::new(cx, id).check_expr(arm.body)
+                                {
                                     ident_bind_name = (&ident.name.as_str()).to_string();
                                     matching_wild = true;
                                 }
diff --git a/clippy_lints/src/utils/visitors.rs b/clippy_lints/src/utils/visitors.rs
index a4064c3e705..24409346ca6 100644
--- a/clippy_lints/src/utils/visitors.rs
+++ b/clippy_lints/src/utils/visitors.rs
@@ -133,14 +133,16 @@ where
     }
 }
 
-pub struct LocalUsedVisitor {
+pub struct LocalUsedVisitor<'hir> {
+    hir: Map<'hir>,
     pub local_hir_id: HirId,
     pub used: bool,
 }
 
-impl LocalUsedVisitor {
-    pub fn new(local_hir_id: HirId) -> Self {
+impl<'hir> LocalUsedVisitor<'hir> {
+    pub fn new(cx: &LateContext<'hir>, local_hir_id: HirId) -> Self {
         Self {
+            hir: cx.tcx.hir(),
             local_hir_id,
             used: false,
         }
@@ -151,23 +153,26 @@ impl LocalUsedVisitor {
         std::mem::replace(&mut self.used, false)
     }
 
-    pub fn check_arm(&mut self, arm: &Arm<'_>) -> bool {
+    pub fn check_arm(&mut self, arm: &'hir Arm<'_>) -> bool {
         self.check(arm, Self::visit_arm)
     }
 
-    pub fn check_expr(&mut self, expr: &Expr<'_>) -> bool {
+    pub fn check_expr(&mut self, expr: &'hir Expr<'_>) -> bool {
         self.check(expr, Self::visit_expr)
     }
 
-    pub fn check_stmt(&mut self, stmt: &Stmt<'_>) -> bool {
+    pub fn check_stmt(&mut self, stmt: &'hir Stmt<'_>) -> bool {
         self.check(stmt, Self::visit_stmt)
     }
 }
 
-impl<'v> Visitor<'v> for LocalUsedVisitor {
+impl<'v> Visitor<'v> for LocalUsedVisitor<'v> {
     type Map = Map<'v>;
 
     fn visit_expr(&mut self, expr: &'v Expr<'v>) {
+        if self.used {
+            return;
+        }
         if path_to_local_id(expr, self.local_hir_id) {
             self.used = true;
         } else {
@@ -176,6 +181,6 @@ impl<'v> Visitor<'v> for LocalUsedVisitor {
     }
 
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
+        NestedVisitorMap::OnlyBodies(self.hir)
     }
 }
diff --git a/tests/ui/collapsible_match.rs b/tests/ui/collapsible_match.rs
index a83e6c77b12..3294da7e814 100644
--- a/tests/ui/collapsible_match.rs
+++ b/tests/ui/collapsible_match.rs
@@ -224,6 +224,14 @@ fn negative_cases(res_opt: Result<Option<u32>, String>, res_res: Result<Result<u
         },
         _ => return,
     }
+    if let Ok(val) = res_opt {
+        if let Some(n) = val {
+            let _ = || {
+                // usage in closure
+                println!("{:?}", val);
+            };
+        }
+    }
 }
 
 fn make<T>() -> T {