Merge pull request #1099 from Manishearth/fix

Some small fixes

clippy_lints/src/methods.rs

@@ -2,7 +2,7 @@ use rustc::hir;
 use rustc::lint::*;
 use rustc::middle::const_val::ConstVal;
 use rustc::middle::const_qualif::ConstQualif;
-use rustc::ty::subst::{Subst, TypeSpace};
+use rustc::ty::subst::TypeSpace;
 use rustc::ty;
 use rustc_const_eval::EvalHint::ExprTypeChecked;
 use rustc_const_eval::eval_const_expr_partial;
@@ -10,9 +10,9 @@ use std::borrow::Cow;
 use std::fmt;
 use syntax::codemap::Span;
 use syntax::ptr::P;
-use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path, match_trait_method,
+use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, match_path,
-            match_type, method_chain_args, return_ty, same_tys, snippet, span_lint,
+            match_trait_method, match_type, method_chain_args, return_ty, same_tys, snippet,
-            span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
+            span_lint, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
 use utils::MethodArgs;
 use utils::paths;
 use utils::sugg;
@@ -471,7 +471,7 @@ impl LateLintPass for Pass {
 
                     // check conventions w.r.t. conversion method names and predicates
                     let ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(item.id)).ty;
-                    let is_copy = is_copy(cx, ty, item);
+                    let is_copy = is_copy(cx, ty, item.id);
                     for &(ref conv, self_kinds) in &CONVENTIONS {
                         if_let_chain! {[
                             conv.check(&name.as_str()),
@@ -1163,8 +1163,3 @@ fn is_bool(ty: &hir::Ty) -> bool {
         false
     }
 }
-
-fn is_copy<'a, 'ctx>(cx: &LateContext<'a, 'ctx>, ty: ty::Ty<'ctx>, item: &hir::Item) -> bool {
-    let env = ty::ParameterEnvironment::for_item(cx.tcx, item.id);
-    !ty.subst(cx.tcx, env.free_substs).moves_by_default(cx.tcx.global_tcx(), &env, item.span)
-}

clippy_lints/src/non_expressive_names.rs

@@ -25,7 +25,11 @@ declare_lint! {
 ///
 /// **Known problems:** None?
 ///
-/// **Example:** let (a, b, c, d, e, f, g) = (...);
+/// **Example:**
+///
+/// ```rust
+/// let (a, b, c, d, e, f, g) = (...);
+/// ```
 declare_lint! {
     pub MANY_SINGLE_CHAR_NAMES,
     Warn,

clippy_lints/src/returns.rs

@@ -39,7 +39,7 @@ impl ReturnPass {
         if let Some(stmt) = block.stmts.last() {
             match stmt.node {
                 StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
-                    self.check_final_expr(cx, expr);
+                    self.check_final_expr(cx, expr, Some(stmt.span));
                 }
                 _ => (),
             }
@@ -47,11 +47,11 @@ impl ReturnPass {
     }
 
     // Check a the final expression in a block if it's a return.
-    fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
+    fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr, span: Option<Span>) {
         match expr.node {
             // simple return is always "bad"
             ExprKind::Ret(Some(ref inner)) => {
-                self.emit_return_lint(cx, (expr.span, inner.span));
+                self.emit_return_lint(cx, span.expect("`else return` is not possible"), inner.span);
             }
             // a whole block? check it!
             ExprKind::Block(ref block) => {
@@ -62,25 +62,25 @@ impl ReturnPass {
             // (except for unit type functions) so we don't match it
             ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => {
                 self.check_block_return(cx, ifblock);
-                self.check_final_expr(cx, elsexpr);
+                self.check_final_expr(cx, elsexpr, None);
             }
             // a match expr, check all arms
             ExprKind::Match(_, ref arms) => {
                 for arm in arms {
-                    self.check_final_expr(cx, &arm.body);
+                    self.check_final_expr(cx, &arm.body, Some(arm.body.span));
                 }
             }
             _ => (),
         }
     }
 
-    fn emit_return_lint(&mut self, cx: &EarlyContext, spans: (Span, Span)) {
+    fn emit_return_lint(&mut self, cx: &EarlyContext, ret_span: Span, inner_span: Span) {
-        if in_external_macro(cx, spans.1) {
+        if in_external_macro(cx, inner_span) {
             return;
         }
-        span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, "unneeded return statement", |db| {
+        span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| {
-            if let Some(snippet) = snippet_opt(cx, spans.1) {
+            if let Some(snippet) = snippet_opt(cx, inner_span) {
-                db.span_suggestion(spans.0, "remove `return` as shown:", snippet);
+                db.span_suggestion(ret_span, "remove `return` as shown:", snippet);
             }
         });
     }

clippy_lints/src/utils/mod.rs

@@ -15,7 +15,7 @@ use std::env;
 use std::mem;
 use std::str::FromStr;
 use syntax::ast::{self, LitKind};
-use syntax::codemap::{ExpnFormat, ExpnInfo, MultiSpan, Span};
+use syntax::codemap::{ExpnFormat, ExpnInfo, MultiSpan, Span, DUMMY_SP};
 use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
 
@@ -723,3 +723,8 @@ pub fn type_is_unsafe_function(ty: ty::Ty) -> bool {
         _ => false,
     }
 }
+
+pub fn is_copy<'a, 'ctx>(cx: &LateContext<'a, 'ctx>, ty: ty::Ty<'ctx>, env: NodeId) -> bool {
+    let env = ty::ParameterEnvironment::for_item(cx.tcx, env);
+    !ty.subst(cx.tcx, env.free_substs).moves_by_default(cx.tcx.global_tcx(), &env, DUMMY_SP)
+}

clippy_lints/src/vec.rs

@@ -1,10 +1,10 @@
-use rustc::lint::*;
-use rustc::ty::TypeVariants;
 use rustc::hir::*;
+use rustc::lint::*;
+use rustc::ty;
 use rustc_const_eval::EvalHint::ExprTypeChecked;
 use rustc_const_eval::eval_const_expr_partial;
 use syntax::codemap::Span;
-use utils::{higher, snippet, span_lint_and_then};
+use utils::{higher, is_copy, snippet, span_lint_and_then};
 
 /// **What it does:** This lint warns about using `&vec![..]` when using `&[..]` would be possible.
 ///
@@ -35,50 +35,61 @@ impl LateLintPass for Pass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         // search for `&vec![_]` expressions where the adjusted type is `&[_]`
         if_let_chain!{[
-            let TypeVariants::TyRef(_, ref ty) = cx.tcx.expr_ty_adjusted(expr).sty,
+            let ty::TypeVariants::TyRef(_, ref ty) = cx.tcx.expr_ty_adjusted(expr).sty,
-            let TypeVariants::TySlice(..) = ty.ty.sty,
+            let ty::TypeVariants::TySlice(..) = ty.ty.sty,
             let ExprAddrOf(_, ref addressee) = expr.node,
+            let Some(vec_args) = higher::vec_macro(cx, addressee),
         ], {
-            check_vec_macro(cx, addressee, expr.span);
+            check_vec_macro(cx, &vec_args, expr.span);
         }}
 
         // search for `for _ in vec![…]`
-        if let Some((_, arg, _)) = higher::for_loop(expr) {
+        if_let_chain!{[
+            let Some((_, arg, _)) = higher::for_loop(expr),
+            let Some(vec_args) = higher::vec_macro(cx, arg),
+            is_copy(cx, vec_type(cx.tcx.expr_ty_adjusted(arg)), cx.tcx.map.get_parent(expr.id)),
+        ], {
             // report the error around the `vec!` not inside `<std macros>:`
             let span = cx.sess().codemap().source_callsite(arg.span);
-            check_vec_macro(cx, arg, span);
+            check_vec_macro(cx, &vec_args, span);
+        }}
+    }
+}
+
+fn check_vec_macro(cx: &LateContext, vec_args: &higher::VecArgs, span: Span) {
+    let snippet = match *vec_args {
+        higher::VecArgs::Repeat(elem, len) => {
+            if eval_const_expr_partial(cx.tcx, len, ExprTypeChecked, None).is_ok() {
+                format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
+            } else {
+                return;
+            }
         }
-    }
+        higher::VecArgs::Vec(args) => {
+            if let Some(last) = args.iter().last() {
+                let span = Span {
+                    lo: args[0].span.lo,
+                    hi: last.span.hi,
+                    expn_id: args[0].span.expn_id,
+                };
+
+                format!("&[{}]", snippet(cx, span, "..")).into()
+            } else {
+                "&[]".into()
+            }
+        }
+    };
+
+    span_lint_and_then(cx, USELESS_VEC, span, "useless use of `vec!`", |db| {
+        db.span_suggestion(span, "you can use a slice directly", snippet);
+    });
 }
 
-fn check_vec_macro(cx: &LateContext, vec: &Expr, span: Span) {
+/// Return the item type of the vector (ie. the `T` in `Vec<T>`).
-    if let Some(vec_args) = higher::vec_macro(cx, vec) {
+fn vec_type(ty: ty::Ty) -> ty::Ty {
-        let snippet = match vec_args {
+    if let ty::TyStruct(_, substs) = ty.sty {
-            higher::VecArgs::Repeat(elem, len) => {
+        substs.types.get(ty::subst::ParamSpace::TypeSpace, 0)
-                if eval_const_expr_partial(cx.tcx, len, ExprTypeChecked, None).is_ok() {
+    } else {
-                    format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
+        panic!("The type of `vec!` is a not a struct?");
-                } else {
-                    return;
-                }
-            }
-            higher::VecArgs::Vec(args) => {
-                if let Some(last) = args.iter().last() {
-                    let span = Span {
-                        lo: args[0].span.lo,
-                        hi: last.span.hi,
-                        expn_id: args[0].span.expn_id,
-                    };
-
-                    format!("&[{}]", snippet(cx, span, "..")).into()
-                } else {
-                    "&[]".into()
-                }
-            }
-        };
-
-        span_lint_and_then(cx, USELESS_VEC, span, "useless use of `vec!`", |db| {
-            db.span_suggestion(span, "you can use a slice directly", snippet);
-        });
     }
 }
-

tests/compile-fail/needless_return.rs

@@ -37,12 +37,11 @@ fn test_if_block() -> bool {
 
 fn test_match(x: bool) -> bool {
     match x {
-        true => {
+        true => return false,
-            return false;
+        //~^ ERROR unneeded return statement
-            //~^ ERROR unneeded return statement
+        //~| HELP remove `return` as shown
-            //~| HELP remove `return` as shown
+        //~| SUGGESTION false
-            //~| SUGGESTION false
+
-        }
         false => {
             return true;
             //~^ ERROR unneeded return statement

tests/compile-fail/vec.rs

@@ -3,6 +3,9 @@
 
 #![deny(useless_vec)]
 
+#[derive(Debug)]
+struct NonCopy;
+
 fn on_slice(_: &[u8]) {}
 #[allow(ptr_arg)]
 fn on_vec(_: &Vec<u8>) {}
@@ -62,6 +65,10 @@ fn main() {
         //~^ ERROR useless use of `vec!`
         //~| HELP you can use
         //~| SUGGESTION for a in &[1, 2, 3] {
-        println!("{}", a);
+        println!("{:?}", a);
+    }
+
+    for a in vec![NonCopy, NonCopy] {
+        println!("{:?}", a);
     }
 }