diff --git a/src/lib.rs b/src/lib.rs index 2f71d8cc9df..6503c5e1006 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,7 +75,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box misc::ModuloOne); reg.register_late_lint_pass(box unicode::Unicode); reg.register_late_lint_pass(box strings::StringAdd); - reg.register_late_lint_pass(box returns::ReturnPass); + reg.register_early_lint_pass(box returns::ReturnPass); reg.register_late_lint_pass(box methods::MethodsPass); reg.register_late_lint_pass(box shadow::ShadowPass); reg.register_late_lint_pass(box types::LetPass); diff --git a/src/returns.rs b/src/returns.rs index d04307ffd5d..3df4efd0889 100644 --- a/src/returns.rs +++ b/src/returns.rs @@ -1,10 +1,10 @@ use rustc::lint::*; -use rustc_front::hir::*; -use reexport::*; +use syntax::ast::*; +//use reexport::*; use syntax::codemap::{Span, Spanned}; -use rustc_front::visit::FnKind; +use syntax::visit::FnKind; -use utils::{span_lint, snippet, match_path, in_external_macro}; +use utils::{span_lint, snippet, match_path_ast, in_external_macro}; declare_lint!(pub NEEDLESS_RETURN, Warn, "using a return statement like `return expr;` where an expression would suffice"); @@ -17,7 +17,7 @@ pub struct ReturnPass; impl ReturnPass { // Check the final stmt or expr in a block for unnecessary return. - fn check_block_return(&mut self, cx: &LateContext, block: &Block) { + fn check_block_return(&mut self, cx: &EarlyContext, block: &Block) { if let Some(ref expr) = block.expr { self.check_final_expr(cx, expr); } else if let Some(stmt) = block.stmts.last() { @@ -30,7 +30,7 @@ impl ReturnPass { } // Check a the final expression in a block if it's a return. - fn check_final_expr(&mut self, cx: &LateContext, expr: &Expr) { + fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr) { match expr.node { // simple return is always "bad" ExprRet(Some(ref inner)) => { @@ -48,7 +48,7 @@ impl ReturnPass { self.check_final_expr(cx, elsexpr); } // a match expr, check all arms - ExprMatch(_, ref arms, _) => { + ExprMatch(_, ref arms) => { for arm in arms { self.check_final_expr(cx, &arm.body); } @@ -57,7 +57,7 @@ impl ReturnPass { } } - fn emit_return_lint(&mut self, cx: &LateContext, spans: (Span, Span)) { + fn emit_return_lint(&mut self, cx: &EarlyContext, spans: (Span, Span)) { if in_external_macro(cx, spans.1) {return;} span_lint(cx, NEEDLESS_RETURN, spans.0, &format!( "unneeded return statement. Consider using `{}` \ @@ -66,7 +66,7 @@ impl ReturnPass { } // Check for "let x = EXPR; x" - fn check_let_return(&mut self, cx: &LateContext, block: &Block) { + fn check_let_return(&mut self, cx: &EarlyContext, block: &Block) { // we need both a let-binding stmt and an expr if_let_chain! { [ @@ -77,14 +77,14 @@ impl ReturnPass { let Some(ref initexpr) = local.init, let PatIdent(_, Spanned { node: id, .. }, _) = local.pat.node, let ExprPath(_, ref path) = retexpr.node, - match_path(path, &[&id.name.as_str()]) + match_path_ast(path, &[&id.name.as_str()]) ], { self.emit_let_lint(cx, retexpr.span, initexpr.span); } } } - fn emit_let_lint(&mut self, cx: &LateContext, lint_span: Span, note_span: Span) { + fn emit_let_lint(&mut self, cx: &EarlyContext, lint_span: Span, note_span: Span) { if in_external_macro(cx, note_span) {return;} span_lint(cx, LET_AND_RETURN, lint_span, "returning the result of a let binding from a block. \ @@ -102,13 +102,13 @@ impl LintPass for ReturnPass { } } -impl LateLintPass for ReturnPass { - fn check_fn(&mut self, cx: &LateContext, _: FnKind, _: &FnDecl, +impl EarlyLintPass for ReturnPass { + fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, _: &FnDecl, block: &Block, _: Span, _: NodeId) { self.check_block_return(cx, block); } - fn check_block(&mut self, cx: &LateContext, block: &Block) { + fn check_block(&mut self, cx: &EarlyContext, block: &Block) { self.check_let_return(cx, block); } } diff --git a/src/shadow.rs b/src/shadow.rs index 8fd8d0d15ac..13df95bf8dd 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -7,7 +7,7 @@ use rustc_front::visit::FnKind; use rustc::lint::*; use rustc::middle::def::Def::{DefVariant, DefStruct}; -use utils::{in_external_macro, snippet, span_lint, span_note_and_lint}; +use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint, span_note_and_lint}; declare_lint!(pub SHADOW_SAME, Allow, "rebinding a name to itself, e.g. `let mut x = &mut x`"); @@ -60,6 +60,7 @@ fn check_block(cx: &LateContext, block: &Block, bindings: &mut Vec<(Name, Span)> fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) { if in_external_macro(cx, decl.span) { return; } + if is_from_for_desugar(decl) { return; } if let DeclLocal(ref local) = decl.node { let Local{ ref pat, ref ty, ref init, id: _, span } = **local; if let &Some(ref t) = ty { check_ty(cx, t, bindings) } diff --git a/src/types.rs b/src/types.rs index e9125085bb5..584f9b34988 100644 --- a/src/types.rs +++ b/src/types.rs @@ -9,7 +9,8 @@ use syntax::ast::IntTy::*; use syntax::ast::UintTy::*; use syntax::ast::FloatTy::*; -use utils::{match_type, snippet, span_lint, span_help_and_lint, in_macro, in_external_macro}; +use utils::{match_type, snippet, span_lint, span_help_and_lint}; +use utils::{is_from_for_desugar, in_macro, in_external_macro}; use utils::{LL_PATH, VEC_PATH}; /// Handles all the linting of funky types @@ -61,9 +62,10 @@ fn check_let_unit(cx: &LateContext, decl: &Decl) { if *bindtype == ty::TyTuple(vec![]) { if in_external_macro(cx, decl.span) || in_macro(cx, local.pat.span) { return; } - span_lint(cx, LET_UNIT_VALUE, decl.span, &format!( - "this let-binding has unit value. Consider omitting `let {} =`", - snippet(cx, local.pat.span, ".."))); + if is_from_for_desugar(decl) { return; } + span_lint(cx, LET_UNIT_VALUE, decl.span, &format!( + "this let-binding has unit value. Consider omitting `let {} =`", + snippet(cx, local.pat.span, ".."))); } } } diff --git a/src/utils.rs b/src/utils.rs index ac155617011..34a6d25af25 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -7,6 +7,7 @@ use rustc::middle::def_id::DefId; use rustc::middle::ty; use std::borrow::Cow; use syntax::ast::Lit_::*; +use syntax::ast; // module DefPaths for certain structs/enums we check for pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; @@ -15,15 +16,56 @@ pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; +/// Produce a nested chain of if-lets and ifs from the patterns: +/// +/// if_let_chain! { +/// [ +/// Some(y) = x, +/// y.len() == 2, +/// Some(z) = y, +/// ], +/// { +/// block +/// } +/// } +/// +/// becomes +/// +/// if let Some(y) = x { +/// if y.len() == 2 { +/// if let Some(z) = y { +/// block +/// } +/// } +/// } +#[macro_export] +macro_rules! if_let_chain { + ([let $pat:pat = $expr:expr, $($tt:tt)+], $block:block) => { + if let $pat = $expr { + if_let_chain!{ [$($tt)+], $block } + } + }; + ([let $pat:pat = $expr:expr], $block:block) => { + if let $pat = $expr { + $block + } + }; + ([$expr:expr, $($tt:tt)+], $block:block) => { + if $expr { + if_let_chain!{ [$($tt)+], $block } + } + }; + ([$expr:expr], $block:block) => { + if $expr { + $block + } + }; +} + /// returns true this expn_info was expanded by any macro pub fn in_macro(cx: &LateContext, span: Span) -> bool { cx.sess().codemap().with_expn_info(span.expn_id, - |info| info.map_or(false, |i| { - match i.callee.format { - ExpnFormat::CompilerExpansion(..) => false, - _ => true, - } - })) + |info| info.is_some()) } /// returns true if the macro that expanded the crate was outside of @@ -34,17 +76,9 @@ pub fn in_external_macro(cx: &T, span: Span) -> bool { fn in_macro_ext(cx: &T, opt_info: Option<&ExpnInfo>) -> bool { // no ExpnInfo = no macro opt_info.map_or(false, |info| { - match info.callee.format { - ExpnFormat::CompilerExpansion(..) => { - if info.callee.name().as_str() == "closure expansion" { - return false; - } - }, - ExpnFormat::MacroAttribute(..) => { - // these are all plugins - return true; - }, - _ => (), + if let ExpnFormat::MacroAttribute(..) = info.callee.format { + // these are all plugins + return true; } // no span for the callee = external macro info.callee.span.map_or(true, |span| { @@ -102,6 +136,13 @@ pub fn match_path(path: &Path, segments: &[&str]) -> bool { |(a, b)| a.identifier.name.as_str() == *b) } +/// match a Path against a slice of segment string literals, e.g. +/// `match_path(path, &["std", "rt", "begin_unwind"])` +pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool { + path.segments.iter().rev().zip(segments.iter().rev()).all( + |(a, b)| a.identifier.name.as_str() == *b) +} + /// get the name of the item the expression is in, if available pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option { let parent_id = cx.tcx.map.get_parent(expr.id); @@ -115,6 +156,24 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option { } } +/// checks if a `let` decl is from a for loop desugaring +pub fn is_from_for_desugar(decl: &Decl) -> bool { + if_let_chain! { + [ + let DeclLocal(ref loc) = decl.node, + let Some(ref expr) = loc.init, + // FIXME: This should check for MatchSource::ForLoop + // but right now there's a bug where the match source isn't + // set during lowering + // https://github.com/rust-lang/rust/pull/28973 + let ExprMatch(_, _, _) = expr.node + ], + { return true; } + }; + false +} + + /// convert a span to a code snippet if available, otherwise use default, e.g. /// `snippet(cx, expr.span, "..")` pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> { @@ -262,49 +321,3 @@ pub fn is_integer_literal(expr: &Expr, value: u64) -> bool } false } - -/// Produce a nested chain of if-lets and ifs from the patterns: -/// -/// if_let_chain! { -/// [ -/// Some(y) = x, -/// y.len() == 2, -/// Some(z) = y, -/// ], -/// { -/// block -/// } -/// } -/// -/// becomes -/// -/// if let Some(y) = x { -/// if y.len() == 2 { -/// if let Some(z) = y { -/// block -/// } -/// } -/// } -#[macro_export] -macro_rules! if_let_chain { - ([let $pat:pat = $expr:expr, $($tt:tt)+], $block:block) => { - if let $pat = $expr { - if_let_chain!{ [$($tt)+], $block } - } - }; - ([let $pat:pat = $expr:expr], $block:block) => { - if let $pat = $expr { - $block - } - }; - ([$expr:expr, $($tt:tt)+], $block:block) => { - if $expr { - if_let_chain!{ [$($tt)+], $block } - } - }; - ([$expr:expr], $block:block) => { - if $expr { - $block - } - }; -} diff --git a/tests/compile-fail/strings.rs b/tests/compile-fail/strings.rs index 7e21294a3d1..1ba8616ed29 100755 --- a/tests/compile-fail/strings.rs +++ b/tests/compile-fail/strings.rs @@ -6,7 +6,7 @@ fn add_only() { // ignores assignment distinction let mut x = "".to_owned(); - for _ in (1..3) { + for _ in 1..3 { x = x + "."; //~ERROR you added something to a string. } @@ -20,7 +20,7 @@ fn add_only() { // ignores assignment distinction fn add_assign_only() { let mut x = "".to_owned(); - for _ in (1..3) { + for _ in 1..3 { x = x + "."; //~ERROR you assigned the result of adding something to this string. } @@ -34,7 +34,7 @@ fn add_assign_only() { fn both() { let mut x = "".to_owned(); - for _ in (1..3) { + for _ in 1..3 { x = x + "."; //~ERROR you assigned the result of adding something to this string. }