From 1ee2e4ffe8a4ca40676fe6b372570d4e51c2f375 Mon Sep 17 00:00:00 2001 From: llogiq Date: Mon, 1 Jun 2015 15:09:17 +0200 Subject: [PATCH 1/3] Fixed block check, also added macro test to collapsible_if and inline_always --- src/attrs.rs | 17 +++++--- src/collapsible_if.rs | 60 ++++++++++++++++------------ src/mut_mut.rs | 2 +- tests/compile-fail/collapsible_if.rs | 6 +++ tests/compile-test.rs | 2 +- 5 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/attrs.rs b/src/attrs.rs index 3ad3889f9db..f056ac6ee8c 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 mut_mut::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 31ac1e62be6..b9ebeed01de 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 mut_mut::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/mut_mut.rs b/src/mut_mut.rs index a4c2d3932a3..3ba9c47e7fe 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -51,7 +51,7 @@ fn check_expr_expd(cx: &Context, expr: &Expr, info: Option<&ExpnInfo>) { }) } -fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { +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| diff --git a/tests/compile-fail/collapsible_if.rs b/tests/compile-fail/collapsible_if.rs index 3aa86c893c6..7b7ff13f24b 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 6fcf71d38ad..04f3fc16b1b 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)); From 30de91d3e9e724c8ebc5d3df342aa4e66a88dd47 Mon Sep 17 00:00:00 2001 From: llogiq Date: Mon, 1 Jun 2015 22:30:34 +0200 Subject: [PATCH 2/3] moved in_macro to (new) utils.rs --- src/attrs.rs | 2 +- src/collapsible_if.rs | 2 +- src/mut_mut.rs | 11 +---------- src/utils.rs | 12 ++++++++++++ 4 files changed, 15 insertions(+), 12 deletions(-) create mode 100644 src/utils.rs diff --git a/src/attrs.rs b/src/attrs.rs index f056ac6ee8c..5ee8745c33e 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -6,7 +6,7 @@ use syntax::ast::*; use syntax::ptr::P; use syntax::codemap::{Span, ExpnInfo}; use syntax::parse::token::InternedString; -use mut_mut::in_macro; +use utils::in_macro; declare_lint! { pub INLINE_ALWAYS, Warn, "#[inline(always)] is usually a bad idea."} diff --git a/src/collapsible_if.rs b/src/collapsible_if.rs index b9ebeed01de..85c1b25a673 100644 --- a/src/collapsible_if.rs +++ b/src/collapsible_if.rs @@ -19,7 +19,7 @@ use syntax::ast::*; use syntax::ptr::P; use syntax::codemap::{Span, Spanned, ExpnInfo}; use syntax::print::pprust::expr_to_string; -use mut_mut::in_macro; +use utils::in_macro; declare_lint! { pub COLLAPSIBLE_IF, diff --git a/src/mut_mut.rs b/src/mut_mut.rs index 3ba9c47e7fe..202b5600dbe 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>) { }) } -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") - ) - }) - }) -} - 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 00000000000..e6e0c0e4868 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,12 @@ +use rustc::lint::Context; +use syntax::codemap::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") + ) + }) + }) +} From e8ca19da244e2dce17c599fd35565f6e47ff19e9 Mon Sep 17 00:00:00 2001 From: llogiq Date: Mon, 1 Jun 2015 22:36:56 +0200 Subject: [PATCH 3/3] fixed modules/visibility --- src/lib.rs | 1 + src/utils.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 37cdf6a9c58..06922ef5674 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/utils.rs b/src/utils.rs index e6e0c0e4868..c67ded3d93d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,7 +1,7 @@ use rustc::lint::Context; use syntax::codemap::ExpnInfo; -fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { +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|