From 3447bfccd98d13ff78fe04b4dbbc21fd0de20756 Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 14 Jul 2016 17:42:40 +0200 Subject: [PATCH 1/3] Fix `MANY_SINGLE_CHAR_NAMES`'s docs --- clippy_lints/src/non_expressive_names.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 17f12afcaec..6d3f44036c0 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/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, From c1eb5828fafae326919c6999bfda075c87dcc296 Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 14 Jul 2016 18:32:09 +0200 Subject: [PATCH 2/3] Fix suggestion spans for `NEEDLESS_RETURN` --- clippy_lints/src/returns.rs | 20 ++++++++++---------- tests/compile-fail/needless_return.rs | 11 +++++------ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index fda151cd6d7..deea50f0303 100644 --- a/clippy_lints/src/returns.rs +++ b/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) { 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)) { - if in_external_macro(cx, spans.1) { + fn emit_return_lint(&mut self, cx: &EarlyContext, ret_span: Span, inner_span: Span) { + if in_external_macro(cx, inner_span) { return; } - span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, "unneeded return statement", |db| { - if let Some(snippet) = snippet_opt(cx, spans.1) { - db.span_suggestion(spans.0, "remove `return` as shown:", snippet); + span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { + if let Some(snippet) = snippet_opt(cx, inner_span) { + db.span_suggestion(ret_span, "remove `return` as shown:", snippet); } }); } diff --git a/tests/compile-fail/needless_return.rs b/tests/compile-fail/needless_return.rs index 80fed2818ef..5a391a358b1 100644 --- a/tests/compile-fail/needless_return.rs +++ b/tests/compile-fail/needless_return.rs @@ -37,12 +37,11 @@ fn test_if_block() -> bool { fn test_match(x: bool) -> bool { match x { - true => { - return false; - //~^ ERROR unneeded return statement - //~| HELP remove `return` as shown - //~| SUGGESTION false - } + true => return false, + //~^ ERROR unneeded return statement + //~| HELP remove `return` as shown + //~| SUGGESTION false + false => { return true; //~^ ERROR unneeded return statement From ea665c38f1ad049935a775d19082adedede9e00e Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 14 Jul 2016 19:31:17 +0200 Subject: [PATCH 3/3] Fix FP with `USELESS_VEC` and non-copy types --- clippy_lints/src/methods.rs | 15 ++---- clippy_lints/src/utils/mod.rs | 7 ++- clippy_lints/src/vec.rs | 87 ++++++++++++++++++++--------------- tests/compile-fail/vec.rs | 9 +++- 4 files changed, 68 insertions(+), 50 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index f70ec4eac9f..755c325fcbc 100644 --- a/clippy_lints/src/methods.rs +++ b/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, - match_type, method_chain_args, return_ty, same_tys, snippet, span_lint, - span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth}; +use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, match_path, + match_trait_method, match_type, method_chain_args, return_ty, same_tys, snippet, + 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) -} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 189801c8c0b..bcc4be745c0 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/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) +} diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 114817989d8..cc9d5a5f224 100644 --- a/clippy_lints/src/vec.rs +++ b/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 TypeVariants::TySlice(..) = ty.ty.sty, + let ty::TypeVariants::TyRef(_, ref ty) = cx.tcx.expr_ty_adjusted(expr).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 `:` 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) { - if let Some(vec_args) = higher::vec_macro(cx, vec) { - 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); - }); +/// Return the item type of the vector (ie. the `T` in `Vec`). +fn vec_type(ty: ty::Ty) -> ty::Ty { + if let ty::TyStruct(_, substs) = ty.sty { + substs.types.get(ty::subst::ParamSpace::TypeSpace, 0) + } else { + panic!("The type of `vec!` is a not a struct?"); } } - diff --git a/tests/compile-fail/vec.rs b/tests/compile-fail/vec.rs index 92c99c20e36..7a790e62116 100644 --- a/tests/compile-fail/vec.rs +++ b/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) {} @@ -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); } }