diff --git a/src/attrs.rs b/src/attrs.rs index 3ad3889f9..5ee8745c3 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -4,8 +4,9 @@ use rustc::plugin::Registry; use rustc::lint::*; use syntax::ast::*; use syntax::ptr::P; -use syntax::codemap::Span; +use syntax::codemap::{Span, ExpnInfo}; use syntax::parse::token::InternedString; +use utils::in_macro; declare_lint! { pub INLINE_ALWAYS, Warn, "#[inline(always)] is usually a bad idea."} @@ -20,19 +21,25 @@ impl LintPass for AttrPass { } fn check_item(&mut self, cx: &Context, item: &Item) { - check_attrs(cx, &item.ident, &item.attrs) + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) } fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) { - check_attrs(cx, &item.ident, &item.attrs) + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) } fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) { - check_attrs(cx, &item.ident, &item.attrs) + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) } } -fn check_attrs(cx: &Context, ident: &Ident, attrs: &[Attribute]) { +fn check_attrs(cx: &Context, info: Option<&ExpnInfo>, ident: &Ident, + attrs: &[Attribute]) { + if in_macro(cx, info) { return; } + for attr in attrs { if let MetaList(ref inline, ref values) = attr.node.value.node { if values.len() != 1 || inline != &"inline" { continue; } diff --git a/src/collapsible_if.rs b/src/collapsible_if.rs index 31ac1e62b..85c1b25a6 100644 --- a/src/collapsible_if.rs +++ b/src/collapsible_if.rs @@ -17,8 +17,9 @@ use rustc::lint::*; use rustc::middle::def::*; use syntax::ast::*; use syntax::ptr::P; -use syntax::codemap::{Span, Spanned}; +use syntax::codemap::{Span, Spanned, ExpnInfo}; use syntax::print::pprust::expr_to_string; +use utils::in_macro; declare_lint! { pub COLLAPSIBLE_IF, @@ -34,20 +35,23 @@ impl LintPass for CollapsibleIf { lint_array!(COLLAPSIBLE_IF) } - fn check_expr(&mut self, cx: &Context, e: &Expr) { - if let ExprIf(ref check, ref then_block, None) = e.node { - let expr = check_block(then_block); - let expr = match expr { - Some(e) => e, - None => return - }; - if let ExprIf(ref check_inner, _, None) = expr.node { - let (check, check_inner) = (check_to_string(check), check_to_string(check_inner)); - cx.span_lint(COLLAPSIBLE_IF, e.span, - &format!("This if statement can be collapsed. Try: if {} && {}", check, check_inner)); - } - } - } + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + cx.sess().codemap().with_expn_info(expr.span.expn_id, + |info| check_expr_expd(cx, expr, info)) + } +} + +fn check_expr_expd(cx: &Context, e: &Expr, info: Option<&ExpnInfo>) { + if in_macro(cx, info) { return; } + + if let ExprIf(ref check, ref then, None) = e.node { + if let Some(&Expr{ node: ExprIf(ref check_inner, _, None), ..}) = + single_stmt_of_block(then) { + cx.span_lint(COLLAPSIBLE_IF, e.span, &format!( + "This if statement can be collapsed. Try: if {} && {}\n{:?}", + check_to_string(check), check_to_string(check_inner), e)); + } + } } fn requires_brackets(e: &Expr) -> bool { @@ -65,16 +69,20 @@ fn check_to_string(e: &Expr) -> String { } } -fn check_block(b: &Block) -> Option<&P> { - if b.stmts.len() == 1 && b.expr.is_none() { - let stmt = &b.stmts[0]; - return match stmt.node { - StmtExpr(ref e, _) => Some(e), - _ => None - }; +fn single_stmt_of_block(block: &Block) -> Option<&Expr> { + if block.stmts.len() == 1 && block.expr.is_none() { + if let StmtExpr(ref expr, _) = block.stmts[0].node { + single_stmt_of_expr(expr) + } else { None } + } else { + if block.stmts.is_empty() { + if let Some(ref p) = block.expr { Some(&*p) } else { None } + } else { None } } - if let Some(ref e) = b.expr { - return Some(e); - } - None +} + +fn single_stmt_of_expr(expr: &Expr) -> Option<&Expr> { + if let ExprBlock(ref block) = expr.node { + single_stmt_of_block(block) + } else { Some(expr) } } diff --git a/src/lib.rs b/src/lib.rs index 37cdf6a9c..06922ef56 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,7 @@ pub mod mut_mut; pub mod len_zero; pub mod attrs; pub mod collapsible_if; +pub mod utils; #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { diff --git a/src/mut_mut.rs b/src/mut_mut.rs index a4c2d3932..202b5600d 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -3,6 +3,7 @@ use syntax::ast::*; use rustc::lint::{Context, LintPass, LintArray, Lint}; use rustc::middle::ty::{expr_ty, sty, ty_ptr, ty_rptr, mt}; use syntax::codemap::{BytePos, ExpnInfo, MacroFormat, Span}; +use utils::in_macro; declare_lint!(pub MUT_MUT, Warn, "Warn on usage of double-mut refs, e.g. '&mut &mut ...'"); @@ -51,16 +52,6 @@ fn check_expr_expd(cx: &Context, expr: &Expr, info: Option<&ExpnInfo>) { }) } -fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { - opt_info.map_or(false, |info| { - info.callee.span.map_or(true, |span| { - cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| - !code.starts_with("macro_rules") - ) - }) - }) -} - fn unwrap_mut(ty : &Ty) -> Option<&Ty> { match ty.node { TyPtr(MutTy{ ty: ref pty, mutbl: MutMutable }) => Option::Some(pty), diff --git a/src/utils.rs b/src/utils.rs new file mode 100644 index 000000000..c67ded3d9 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,12 @@ +use rustc::lint::Context; +use syntax::codemap::ExpnInfo; + +pub fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { + opt_info.map_or(false, |info| { + info.callee.span.map_or(true, |span| { + cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| + !code.starts_with("macro_rules") + ) + }) + }) +} diff --git a/tests/compile-fail/collapsible_if.rs b/tests/compile-fail/collapsible_if.rs index 3aa86c893..7b7ff13f2 100644 --- a/tests/compile-fail/collapsible_if.rs +++ b/tests/compile-fail/collapsible_if.rs @@ -34,4 +34,10 @@ fn main() { } } + if x == "hello" { + print!("Hello "); + if y == "world" { + println!("world!") + } + } } diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 6fcf71d38..04f3fc16b 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; fn run_mode(mode: &'static str) { let mut config = compiletest::default_config(); let cfg_mode = mode.parse().ok().expect("Invalid mode"); - config.target_rustcflags = Some("-l regex_macros -L target/debug/".to_string()); + config.target_rustcflags = Some("-L target/debug/".to_string()); config.mode = cfg_mode; config.src_base = PathBuf::from(format!("tests/{}", mode));