Merge pull request #2415 from HMPerson1/fix-2356

Fix `ImplItem`s being ignored

Author: oli-obk

clippy_lints/src/const_static_lifetime.rs

@@ -1,4 +1,4 @@
-use syntax::ast::{Item, ItemKind, Ty, TyKind};
+use syntax::ast::*;
 use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
 use utils::{in_macro, snippet, span_lint_and_then};
 
@@ -86,4 +86,22 @@ impl EarlyLintPass for StaticConst {
             }
         }
     }
+
+    fn check_trait_item(&mut self, cx: &EarlyContext, item: &TraitItem) {
+        if !in_macro(item.span) {
+            // Match only constants...
+            if let TraitItemKind::Const(ref var_type, _) = item.node {
+                self.visit_type(var_type, cx);
+            }
+        }
+    }
+
+    fn check_impl_item(&mut self, cx: &EarlyContext, item: &ImplItem) {
+        if !in_macro(item.span) {
+            // Match only constants...
+            if let ImplItemKind::Const(ref var_type, _) = item.node {
+                self.visit_type(var_type, cx);
+            }
+        }
+    }
 }

clippy_lints/src/non_expressive_names.rs

@@ -313,21 +313,32 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
 impl EarlyLintPass for NonExpressiveNames {
     fn check_item(&mut self, cx: &EarlyContext, item: &Item) {
         if let ItemKind::Fn(ref decl, _, _, _, _, ref blk) = item.node {
-            if !attr::contains_name(&item.attrs, "test") {
-                let mut visitor = SimilarNamesLocalVisitor {
-                    names: Vec::new(),
-                    cx: cx,
-                    lint: self,
-                    single_char_names: Vec::new(),
-                };
-                // initialize with function arguments
-                for arg in &decl.inputs {
-                    SimilarNamesNameVisitor(&mut visitor).visit_pat(&arg.pat);
-                }
-                // walk all other bindings
-                walk_block(&mut visitor, blk);
-            }
+            do_check(self, cx, &item.attrs, decl, blk);
+        }
+    }
+
+    fn check_impl_item(&mut self, cx: &EarlyContext, item: &ImplItem) {
+        if let ImplItemKind::Method(ref sig, ref blk) = item.node {
+            do_check(self, cx, &item.attrs, &sig.decl, blk);
+        }
+    }
+
+}
+
+fn do_check(lint: &mut NonExpressiveNames, cx: &EarlyContext, attrs: &[Attribute], decl: &FnDecl, blk: &Block) {
+    if !attr::contains_name(attrs, "test") {
+        let mut visitor = SimilarNamesLocalVisitor {
+            names: Vec::new(),
+            cx: cx,
+            lint: lint,
+            single_char_names: Vec::new(),
+        };
+        // initialize with function arguments
+        for arg in &decl.inputs {
+            SimilarNamesNameVisitor(&mut visitor).visit_pat(&arg.pat);
         }
+        // walk all other bindings
+        walk_block(&mut visitor, blk);
     }
 }
 

clippy_lints/src/utils/mod.rs

@@ -518,6 +518,9 @@ pub fn get_enclosing_block<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, node: NodeI
             Node::NodeItem(&Item {
                 node: ItemFn(_, _, _, _, _, eid),
                 ..
+            }) | Node::NodeImplItem(&ImplItem {
+                node: ImplItemKind::Method(_, eid),
+                ..
             }) => match cx.tcx.hir.body(eid).value.node {
                 ExprBlock(ref block) => Some(block),
                 _ => None,

tests/ui/const_static_lifetime.rs

@@ -35,3 +35,15 @@ fn main() {
     println!("{:?}", VAR_HEIGHT);
     println!("{}", false_positive);
 }
+
+trait Bar {
+    const TRAIT_VAR: &'static str;
+}
+
+impl Foo {
+    const IMPL_VAR: &'static str = "var";
+}
+
+impl Bar for Foo {
+    const TRAIT_VAR: &'static str = "foo";
+}

tests/ui/const_static_lifetime.stderr

@@ -78,5 +78,23 @@ error: Constants have by default a `'static` lifetime
 24 | const VAR_ARRAY: &'static [u8; 1] = b"T"; // ERROR Consider removing 'static.
    |                  -^^^^^^^-------- help: consider removing `'static`: `&[u8; 1]`
 
-error: aborting due to 13 previous errors
+error: Constants have by default a `'static` lifetime
+  --> $DIR/const_static_lifetime.rs:40:23
+   |
+40 |     const TRAIT_VAR: &'static str;
+   |                      -^^^^^^^---- help: consider removing `'static`: `&str`
+
+error: Constants have by default a `'static` lifetime
+  --> $DIR/const_static_lifetime.rs:44:22
+   |
+44 |     const IMPL_VAR: &'static str = "var";
+   |                     -^^^^^^^---- help: consider removing `'static`: `&str`
+
+error: Constants have by default a `'static` lifetime
+  --> $DIR/const_static_lifetime.rs:48:23
+   |
+48 |     const TRAIT_VAR: &'static str = "foo";
+   |                      -^^^^^^^---- help: consider removing `'static`: `&str`
+
+error: aborting due to 16 previous errors
 

tests/ui/issue_2356.rs

@@ -0,0 +1,24 @@
+#![deny(while_let_on_iterator)]
+
+use std::iter::Iterator;
+
+struct Foo;
+
+impl Foo {
+    fn foo1<I: Iterator<Item = usize>>(mut it: I) {
+        while let Some(_) = it.next() {
+            println!("{:?}", it.size_hint());
+        }
+    }
+
+    fn foo2<I: Iterator<Item = usize>>(mut it: I) {
+        while let Some(e) = it.next() {
+            println!("{:?}", e);
+        }
+    }
+}
+
+fn main() {
+    Foo::foo1(vec![].into_iter());
+    Foo::foo2(vec![].into_iter());
+}

tests/ui/issue_2356.stderr

@@ -0,0 +1,14 @@
+error: this loop could be written as a `for` loop
+  --> $DIR/issue_2356.rs:15:29
+   |
+15 |         while let Some(e) = it.next() {
+   |                             ^^^^^^^^^ help: try: `for e in it { .. }`
+   |
+note: lint level defined here
+  --> $DIR/issue_2356.rs:1:9
+   |
+1  | #![deny(while_let_on_iterator)]
+   |         ^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+

tests/ui/non_expressive_names.rs

@@ -141,3 +141,14 @@ fn underscores_and_numbers() {
     let __1___2 = 12; //~ERROR Consider a more descriptive name
     let _1_ok= 1;
 }
+
+struct Bar;
+
+impl Bar {
+    fn bar() {
+        let _1 = 1;
+        let ____1 = 1;
+        let __1___2 = 12;
+        let _1_ok= 1;
+    }
+}

tests/ui/non_expressive_names.stderr

@@ -149,5 +149,23 @@ error: consider choosing a more descriptive name
 141 |     let __1___2 = 12; //~ERROR Consider a more descriptive name
     |         ^^^^^^^
 
-error: aborting due to 14 previous errors
+error: consider choosing a more descriptive name
+   --> $DIR/non_expressive_names.rs:149:13
+    |
+149 |         let _1 = 1;
+    |             ^^
+
+error: consider choosing a more descriptive name
+   --> $DIR/non_expressive_names.rs:150:13
+    |
+150 |         let ____1 = 1;
+    |             ^^^^^
+
+error: consider choosing a more descriptive name
+   --> $DIR/non_expressive_names.rs:151:13
+    |
+151 |         let __1___2 = 12;
+    |             ^^^^^^^
+
+error: aborting due to 17 previous errors