From 2a51f8d2becadffee4eeb96937d14060889178cc Mon Sep 17 00:00:00 2001 From: Oliver 'ker' Schneider Date: Sun, 24 Jan 2016 10:16:56 +0100 Subject: [PATCH] lint on items following statements --- README.md | 3 +- src/items_after_statements.rs | 62 ++++++++++++++++++++++ src/lib.rs | 3 ++ src/mut_mut.rs | 8 +-- tests/compile-fail/cmp_owned.rs | 5 +- tests/compile-fail/item_after_statement.rs | 9 ++++ 6 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 src/items_after_statements.rs create mode 100644 tests/compile-fail/item_after_statement.rs diff --git a/README.md b/README.md index c45acf4af77..5848759c2b8 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 96 lints included in this crate: +There are 97 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -37,6 +37,7 @@ name [identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` [inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases +[items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | warn | finds blocks where an item comes after a statement [iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended [len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()` [len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead diff --git a/src/items_after_statements.rs b/src/items_after_statements.rs new file mode 100644 index 00000000000..5f109dac058 --- /dev/null +++ b/src/items_after_statements.rs @@ -0,0 +1,62 @@ +//! lint when items are used after statements + +use rustc::lint::*; +use syntax::attr::*; +use syntax::ast::*; +use utils::in_macro; + +/// **What it does:** It `Warn`s on blocks where there are items that are declared in the middle of or after the statements +/// +/// **Why is this bad?** Items live for the entire scope they are declared in. But statements are processed in order. This might cause confusion as it's hard to figure out which item is meant in a statement. +/// +/// **Known problems:** None +/// +/// **Example:** +/// ```rust +/// fn foo() { +/// println!("cake"); +/// } +/// fn main() { +/// foo(); // prints "foo" +/// fn foo() { +/// println!("foo"); +/// } +/// foo(); // prints "foo" +/// } +declare_lint! { pub ITEMS_AFTER_STATEMENTS, Warn, "finds blocks where an item comes after a statement" } + +pub struct ItemsAfterStatemets; + +impl LintPass for ItemsAfterStatemets { + fn get_lints(&self) -> LintArray { + lint_array!(ITEMS_AFTER_STATEMENTS) + } +} + +impl EarlyLintPass for ItemsAfterStatemets { + fn check_block(&mut self, cx: &EarlyContext, item: &Block) { + if in_macro(cx, item.span) { + return; + } + let mut stmts = item.stmts.iter().map(|stmt| &stmt.node); + // skip initial items + while let Some(&StmtDecl(ref decl, _)) = stmts.next() { + if let DeclLocal(_) = decl.node { + break; + } + } + // lint on all further items + for stmt in stmts { + if let StmtDecl(ref decl, _) = *stmt { + if let DeclItem(ref it) = decl.node { + if in_macro(cx, it.span) { + return; + } + cx.struct_span_lint(ITEMS_AFTER_STATEMENTS, it.span, + "adding items after statements is confusing, since items exist from the start of the scope") + .emit(); + } + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 4a832cc7b89..cd69ac23c19 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,6 +43,7 @@ pub mod needless_bool; pub mod approx_const; pub mod eta_reduction; pub mod identity_op; +pub mod items_after_statements; pub mod minmax; pub mod mut_mut; pub mod mut_reference; @@ -97,6 +98,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_early_lint_pass(box precedence::Precedence); reg.register_late_lint_pass(box eta_reduction::EtaPass); reg.register_late_lint_pass(box identity_op::IdentityOp); + reg.register_early_lint_pass(box items_after_statements::ItemsAfterStatemets); reg.register_late_lint_pass(box mut_mut::MutMut); reg.register_late_lint_pass(box mut_reference::UnnecessaryMutPassed); reg.register_late_lint_pass(box len_zero::LenZero); @@ -176,6 +178,7 @@ pub fn plugin_registrar(reg: &mut Registry) { escape::BOXED_LOCAL, eta_reduction::REDUNDANT_CLOSURE, identity_op::IDENTITY_OP, + items_after_statements::ITEMS_AFTER_STATEMENTS, len_zero::LEN_WITHOUT_IS_EMPTY, len_zero::LEN_ZERO, lifetimes::NEEDLESS_LIFETIMES, diff --git a/src/mut_mut.rs b/src/mut_mut.rs index 1bdb4e9a3d6..4623ca38533 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -37,10 +37,6 @@ impl LateLintPass for MutMut { } fn check_expr_mut(cx: &LateContext, expr: &Expr) { - if in_external_macro(cx, expr.span) { - return; - } - fn unwrap_addr(expr: &Expr) -> Option<&Expr> { match expr.node { ExprAddrOf(MutMutable, ref e) => Some(e), @@ -48,6 +44,10 @@ fn check_expr_mut(cx: &LateContext, expr: &Expr) { } } + if in_external_macro(cx, expr.span) { + return; + } + unwrap_addr(expr).map_or((), |e| { unwrap_addr(e).map_or_else(|| { if let TyRef(_, TypeAndMut{mutbl: MutMutable, ..}) = cx.tcx.expr_ty(e).sty { diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs index c4c9ee60fab..c06949eb01a 100644 --- a/tests/compile-fail/cmp_owned.rs +++ b/tests/compile-fail/cmp_owned.rs @@ -3,8 +3,6 @@ #[deny(cmp_owned)] fn main() { - let x = "oh"; - #[allow(str_to_string)] fn with_to_string(x : &str) { x != "foo".to_string(); @@ -13,6 +11,9 @@ fn main() { "foo".to_string() != x; //~^ ERROR this creates an owned instance just for comparison. Consider using `"foo" != x` to compare without allocation } + + let x = "oh"; + with_to_string(x); x != "foo".to_owned(); //~ERROR this creates an owned instance diff --git a/tests/compile-fail/item_after_statement.rs b/tests/compile-fail/item_after_statement.rs new file mode 100644 index 00000000000..f104081faa9 --- /dev/null +++ b/tests/compile-fail/item_after_statement.rs @@ -0,0 +1,9 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(items_after_statements)] + +fn main() { + foo(); + fn foo() { println!("foo"); } //~ ERROR adding items after statements is confusing + foo(); +}