Auto merge of #6698 - camsteffen:path-to-local, r=phansch

More path-to-local fixes

changelog: Fix some detections of variable usage in closures

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,

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,15 +134,19 @@ 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();
         if let hir::StmtKind::Semi(ref expr) = expr.kind;
         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;

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
                 };

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;
                                 }

clippy_lints/src/methods/filter_map_identity.rs

@@ -1,4 +1,4 @@
-use crate::utils::{match_qpath, match_trait_method, paths, span_lint_and_sugg};
+use crate::utils::{match_qpath, match_trait_method, path_to_local_id, paths, span_lint_and_sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
@@ -32,12 +32,8 @@ pub(super) fn check(
             if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node;
             let body = cx.tcx.hir().body(*body_id);
 
-            if let hir::PatKind::Binding(_, _, binding_ident, _) = body.params[0].pat.kind;
-            if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.kind;
-
-            if path.segments.len() == 1;
-            if path.segments[0].ident.name == binding_ident.name;
-
+            if let hir::PatKind::Binding(_, binding_id, ..) = body.params[0].pat.kind;
+            if path_to_local_id(&body.value, binding_id);
             then {
                 apply_lint("called `filter_map(|x| x)` on an `Iterator`");
             }

clippy_lints/src/unused_self.rs

@@ -1,12 +1,10 @@
 use if_chain::if_chain;
-use rustc_hir::def::Res;
-use rustc_hir::intravisit::{walk_path, NestedVisitorMap, Visitor};
-use rustc_hir::{HirId, Impl, ImplItem, ImplItemKind, ItemKind, Path};
+use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 use crate::utils::span_lint_and_help;
+use crate::utils::visitors::LocalUsedVisitor;
 
 declare_clippy_lint! {
     /// **What it does:** Checks methods that contain a `self` argument but don't use it
@@ -57,13 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
             then {
                 let self_param = &body.params[0];
                 let self_hir_id = self_param.pat.hir_id;
-                let mut visitor = UnusedSelfVisitor {
-                    cx,
-                    uses_self: false,
-                    self_hir_id: &self_hir_id,
-                };
-                visitor.visit_body(body);
-                if !visitor.uses_self {
+                if !LocalUsedVisitor::new(cx, self_hir_id).check_body(body) {
                     span_lint_and_help(
                         cx,
                         UNUSED_SELF,
@@ -78,28 +70,3 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
         }
     }
 }
-
-struct UnusedSelfVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,
-    uses_self: bool,
-    self_hir_id: &'a HirId,
-}
-
-impl<'a, 'tcx> Visitor<'tcx> for UnusedSelfVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
-
-    fn visit_path(&mut self, path: &'tcx Path<'_>, _id: HirId) {
-        if self.uses_self {
-            // This function already uses `self`
-            return;
-        }
-        if let Res::Local(hir_id) = &path.res {
-            self.uses_self = self.self_hir_id == hir_id
-        }
-        walk_path(self, path);
-    }
-
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
-    }
-}

clippy_lints/src/utils/visitors.rs

@@ -1,7 +1,7 @@
 use crate::utils::path_to_local_id;
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor};
-use rustc_hir::{Arm, Expr, HirId, Stmt};
+use rustc_hir::{Arm, Body, Expr, HirId, Stmt};
 use rustc_lint::LateContext;
 use rustc_middle::hir::map::Map;
 
@@ -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,30 @@ 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_body(&mut self, body: &'hir Body<'_>) -> bool {
+        self.check(body, Self::visit_body)
+    }
+
+    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 +185,6 @@ impl<'v> Visitor<'v> for LocalUsedVisitor {
     }
 
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
+        NestedVisitorMap::OnlyBodies(self.hir)
     }
 }

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 {