Auto merge of #8901 - Jarcho:sharing_code, r=dswij

[rust.git] / clippy_lints / src / shadow.rs

diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs
index 2ca7c18800ee2e362e299393058239cf35324ce3..bf318c055dad1d48ae38e37b2ceb70fad1a75b74 100644 (file)
--- a/clippy_lints/src/shadow.rs
+++ b/clippy_lints/src/shadow.rs
@@ -5,7 +5,7 @@
 use rustc_hir::def::Res;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::hir_id::ItemLocalId;
-use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Node, Pat, PatKind, QPath, UnOp};
+use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{Span, Symbol};
@@ -23,12 +23,15 @@
     /// ### Example
     /// ```rust
     /// # let x = 1;
-    /// // Bad
     /// let x = &x;
+    /// ```
     ///
-    /// // Good
+    /// Use instead:
+    /// ```rust
+    /// # let x = 1;
     /// let y = &x; // use different variable name
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub SHADOW_SAME,
     restriction,
     "rebinding a name to itself, e.g., `let mut x = &mut x`"
@@ -55,6 +58,7 @@
     /// let x = 2;
     /// let y = x + 1;
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub SHADOW_REUSE,
     restriction,
     "rebinding a name to an expression that re-uses the original value, e.g., `let x = x + 1`"
@@ -77,13 +81,17 @@
     /// # let y = 1;
     /// # let z = 2;
     /// let x = y;
-    ///
-    /// // Bad
     /// let x = z; // shadows the earlier binding
+    /// ```
     ///
-    /// // Good
+    /// Use instead:
+    /// ```rust
+    /// # let y = 1;
+    /// # let z = 2;
+    /// let x = y;
     /// let w = z; // use different variable name
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub SHADOW_UNRELATED,
     restriction,
     "rebinding a name without even using the original value"
@@ -102,11 +110,16 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
             PatKind::Binding(_, hir_id, ident, _) => (hir_id, ident),
             _ => return,
         };
+
+        if pat.span.desugaring_kind().is_some() {
+            return;
+        }
+
         if ident.span.from_expansion() || ident.span.is_dummy() {
             return;
         }
-        let HirId { owner, local_id } = id;
 
+        let HirId { owner, local_id } = id;
         // get (or insert) the list of items for this owner and symbol
         let data = self.bindings.last_mut().unwrap();
         let items_with_name = data.entry(ident.name).or_default();
@@ -131,14 +144,20 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
 
     fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
         let hir = cx.tcx.hir();
-        if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
+        if !matches!(
+            hir.body_owner_kind(hir.body_owner_def_id(body.id())),
+            BodyOwnerKind::Closure
+        ) {
             self.bindings.push(FxHashMap::default());
         }
     }
 
     fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
         let hir = cx.tcx.hir();
-        if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
+        if !matches!(
+            hir.body_owner_kind(hir.body_owner_def_id(body.id())),
+            BodyOwnerKind::Closure
+        ) {
             self.bindings.pop();
         }
     }
@@ -146,9 +165,13 @@ fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
 
 fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
     let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
-    let first_scope = scope_tree.var_scope(first);
-    let second_scope = scope_tree.var_scope(second);
-    scope_tree.is_subscope_of(second_scope, first_scope)
+    if let Some(first_scope) = scope_tree.var_scope(first) {
+        if let Some(second_scope) = scope_tree.var_scope(second) {
+            return scope_tree.is_subscope_of(second_scope, first_scope);
+        }
+    }
+
+    false
 }
 
 fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) {
@@ -162,11 +185,7 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span)
             (SHADOW_SAME, msg)
         },
         Some(expr) if is_local_used(cx, expr, shadowed) => {
-            let msg = format!(
-                "`{}` is shadowed by `{}` which reuses the original value",
-                snippet(cx, pat.span, "_"),
-                snippet(cx, expr.span, "..")
-            );
+            let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_"));
             (SHADOW_REUSE, msg)
         },
         _ => {
@@ -216,14 +235,14 @@ fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_
     }
 }
 
-/// Finds the "init" expression for a pattern: `let <pat> = <init>;` or
+/// Finds the "init" expression for a pattern: `let <pat> = <init>;` (or `if let`) or
 /// `match <init> { .., <pat> => .., .. }`
 fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> {
     for (_, node) in cx.tcx.hir().parent_iter(hir_id) {
         let init = match node {
             Node::Arm(_) | Node::Pat(_) => continue,
             Node::Expr(expr) => match expr.kind {
-                ExprKind::Match(e, _, _) => Some(e),
+                ExprKind::Match(e, _, _) | ExprKind::Let(&Let { init: e, .. }) => Some(e),
                 _ => None,
             },
             Node::Local(local) => local.init,