Merge pull request #914 from oli-obk/non_expressive_names

similar_names fixes

Author: llogiq

src/non_expressive_names.rs

@@ -3,7 +3,7 @@ use syntax::codemap::Span;
 use syntax::parse::token::InternedString;
 use syntax::ast::*;
 use syntax::attr;
-use syntax::visit;
+use syntax::visit::{Visitor, walk_block, walk_pat, walk_expr};
 use utils::{span_lint_and_then, in_macro, span_lint};
 
 /// **What it does:** This lint warns about names that are very similar and thus confusing
@@ -68,12 +68,17 @@ const WHITELIST: &'static [&'static [&'static str]] = &[
 
 struct SimilarNamesNameVisitor<'a, 'b: 'a, 'c: 'b>(&'a mut SimilarNamesLocalVisitor<'b, 'c>);
 
-impl<'v, 'a, 'b, 'c> visit::Visitor<'v> for SimilarNamesNameVisitor<'a, 'b, 'c> {
+impl<'v, 'a, 'b, 'c> Visitor<'v> for SimilarNamesNameVisitor<'a, 'b, 'c> {
     fn visit_pat(&mut self, pat: &'v Pat) {
-        if let PatKind::Ident(_, id, _) = pat.node {
-            self.check_name(id.span, id.node.name);
+        match pat.node {
+            PatKind::Ident(_, id, _) => self.check_name(id.span, id.node.name),
+            PatKind::Struct(_, ref fields, _) => for field in fields {
+                if !field.node.is_shorthand {
+                    self.visit_pat(&field.node.pat);
+                }
+            },
+            _ => walk_pat(self, pat),
         }
-        visit::walk_pat(self, pat);
     }
 }
 
@@ -219,22 +224,22 @@ impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> {
     }
 }
 
-impl<'v, 'a, 'b> visit::Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> {
+impl<'v, 'a, 'b> Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> {
     fn visit_local(&mut self, local: &'v Local) {
         if let Some(ref init) = local.init {
-            self.apply(|this| visit::walk_expr(this, &**init));
+            self.apply(|this| walk_expr(this, &**init));
         }
         // add the pattern after the expression because the bindings aren't available yet in the init expression
         SimilarNamesNameVisitor(self).visit_pat(&*local.pat);
     }
     fn visit_block(&mut self, blk: &'v Block) {
-        self.apply(|this| visit::walk_block(this, blk));
+        self.apply(|this| walk_block(this, blk));
     }
     fn visit_arm(&mut self, arm: &'v Arm) {
         self.apply(|this| {
             // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier
             SimilarNamesNameVisitor(this).visit_pat(&arm.pats[0]);
-            this.apply(|this| visit::walk_expr(this, &arm.body));
+            this.apply(|this| walk_expr(this, &arm.body));
         });
     }
     fn visit_item(&mut self, _: &'v Item) {
@@ -254,10 +259,10 @@ impl EarlyLintPass for NonExpressiveNames {
                 };
                 // initialize with function arguments
                 for arg in &decl.inputs {
-                    visit::walk_pat(&mut SimilarNamesNameVisitor(&mut visitor), &arg.pat);
+                    SimilarNamesNameVisitor(&mut visitor).visit_pat(&arg.pat);
                 }
                 // walk all other bindings
-                visit::walk_block(&mut visitor, blk);
+                walk_block(&mut visitor, blk);
             }
         }
     }

src/shadow.rs

@@ -195,7 +195,7 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bind
     }
 }
 
-fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &Option<T>, prev_span: Span)
+fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, pattern_span: Span, init: &Option<T>, prev_span: Span)
     where T: Deref<Target = Expr>
 {
     fn note_orig(cx: &LateContext, mut db: DiagnosticWrapper, lint: &'static Lint, span: Span) {
@@ -209,25 +209,25 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &
                                SHADOW_SAME,
                                span,
                                &format!("{} is shadowed by itself in {}",
-                                        snippet(cx, lspan, "_"),
+                                        snippet(cx, pattern_span, "_"),
                                         snippet(cx, expr.span, "..")));
             note_orig(cx, db, SHADOW_SAME, prev_span);
         } else if contains_self(name, expr) {
             let db = span_note_and_lint(cx,
                                         SHADOW_REUSE,
-                                        lspan,
+                                        pattern_span,
                                         &format!("{} is shadowed by {} which reuses the original value",
-                                                 snippet(cx, lspan, "_"),
+                                                 snippet(cx, pattern_span, "_"),
                                                  snippet(cx, expr.span, "..")),
                                         expr.span,
                                         "initialization happens here");
             note_orig(cx, db, SHADOW_REUSE, prev_span);
         } else {
             let db = span_note_and_lint(cx,
                                         SHADOW_UNRELATED,
-                                        lspan,
+                                        pattern_span,
                                         &format!("{} is shadowed by {}",
-                                                 snippet(cx, lspan, "_"),
+                                                 snippet(cx, pattern_span, "_"),
                                                  snippet(cx, expr.span, "..")),
                                         expr.span,
                                         "initialization happens here");
@@ -238,7 +238,7 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &
         let db = span_lint(cx,
                            SHADOW_UNRELATED,
                            span,
-                           &format!("{} shadows a previous declaration", snippet(cx, lspan, "_")));
+                           &format!("{} shadows a previous declaration", snippet(cx, pattern_span, "_")));
         note_orig(cx, db, SHADOW_UNRELATED, prev_span);
     }
 }

tests/compile-fail/non_expressive_names.rs

@@ -11,8 +11,15 @@
 //~| NOTE: lint level defined here
 //~| NOTE: lint level defined here
 //~| NOTE: lint level defined here
+//~| NOTE: lint level defined here
 #![allow(unused)]
 
+
+struct Foo {
+    apple: i32,
+    bpple: i32,
+}
+
 fn main() {
     let specter: i32;
     let spectre: i32;
@@ -90,6 +97,13 @@ fn main() {
     let rx_cake: i32;
 }
 
+fn foo() {
+    let Foo { apple, bpple } = unimplemented!();
+    let Foo { apple: spring, //~NOTE existing binding defined here
+        bpple: sprang } = unimplemented!(); //~ ERROR: name is too similar
+    //~^HELP for further information
+}
+
 #[derive(Clone, Debug)]
 enum MaybeInst {
     Split,