From 3c4af496621cbd5a2a89b2532a2e5f3338cdddf1 Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 28 Jun 2016 15:54:23 +0200 Subject: [PATCH 1/3] Rustup to ea0dc9297283daff6486807f43e190b4eb561412 --- clippy_lints/src/booleans.rs | 3 ++- clippy_lints/src/formatting.rs | 12 ++------- clippy_lints/src/items_after_statements.rs | 31 ++++++++++------------ clippy_lints/src/misc_early.rs | 15 +++++------ clippy_lints/src/non_expressive_names.rs | 14 +++++----- clippy_lints/src/returns.rs | 21 ++++++++------- mini-macro/src/lib.rs | 2 +- src/main.rs | 12 ++++----- tests/compile-fail/formatting.rs | 16 ++++++++++- tests/compile-fail/item_after_statement.rs | 10 +++++++ tests/compile-fail/needless_bool.rs | 18 ++++++------- tests/consts.rs | 3 ++- 12 files changed, 86 insertions(+), 71 deletions(-) diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 8b7952b3746d..672dee77783e 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -3,6 +3,7 @@ use rustc::hir::*; use rustc::hir::intravisit::*; use syntax::ast::{LitKind, DUMMY_NODE_ID}; use syntax::codemap::{DUMMY_SP, dummy_spanned}; +use syntax::util::ThinVec; use utils::{span_lint_and_then, in_macro, snippet_opt, SpanlessEq}; /// **What it does:** This lint checks for boolean expressions that can be written more concisely @@ -99,7 +100,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> { Expr { id: DUMMY_NODE_ID, span: DUMMY_SP, - attrs: None, + attrs: ThinVec::new(), node: ExprBinary(dummy_spanned(op), lhs.clone(), rhs.clone()), } }; diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index aa6dd46cf0b9..930fd7ae9a66 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -59,21 +59,13 @@ impl EarlyLintPass for Formatting { fn check_block(&mut self, cx: &EarlyContext, block: &ast::Block) { for w in block.stmts.windows(2) { match (&w[0].node, &w[1].node) { - (&ast::StmtKind::Expr(ref first, _), &ast::StmtKind::Expr(ref second, _)) | - (&ast::StmtKind::Expr(ref first, _), &ast::StmtKind::Semi(ref second, _)) => { + (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Expr(ref second)) | + (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Semi(ref second)) => { check_consecutive_ifs(cx, first, second); } _ => (), } } - - if let Some(ref expr) = block.expr { - if let Some(ref stmt) = block.stmts.iter().last() { - if let ast::StmtKind::Expr(ref first, _) = stmt.node { - check_consecutive_ifs(cx, first, expr); - } - } - } } fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) { diff --git a/clippy_lints/src/items_after_statements.rs b/clippy_lints/src/items_after_statements.rs index 2e6b33ab3903..0afc2e8f7cef 100644 --- a/clippy_lints/src/items_after_statements.rs +++ b/clippy_lints/src/items_after_statements.rs @@ -2,7 +2,7 @@ use rustc::lint::*; use syntax::ast::*; -use utils::in_macro; +use utils::{in_macro, span_lint}; /// **What it does:** This lints checks for items declared after some statement in a block /// @@ -44,26 +44,23 @@ impl EarlyLintPass for ItemsAfterStatements { if in_macro(cx, item.span) { return; } - let mut stmts = item.stmts.iter().map(|stmt| &stmt.node); + // skip initial items - while let Some(&StmtKind::Decl(ref decl, _)) = stmts.next() { - if let DeclKind::Local(_) = decl.node { - break; - } - } + let stmts = item.stmts.iter() + .map(|stmt| &stmt.node) + .skip_while(|s| matches!(**s, StmtKind::Item(..))); + // lint on all further items for stmt in stmts { - if let StmtKind::Decl(ref decl, _) = *stmt { - if let DeclKind::Item(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(); + if let StmtKind::Item(ref it) = *stmt { + if in_macro(cx, it.span) { + return; } + span_lint(cx, + ITEMS_AFTER_STATEMENTS, + it.span, + "adding items after statements is confusing, since items exist from the \ + start of the scope"); } } } diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index e382b7dc5f69..45964cf3ce70 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -171,15 +171,14 @@ impl EarlyLintPass for MiscEarly { fn check_block(&mut self, cx: &EarlyContext, block: &Block) { for w in block.stmts.windows(2) { if_let_chain! {[ - let StmtKind::Decl(ref first, _) = w[0].node, - let DeclKind::Local(ref local) = first.node, + let StmtKind::Local(ref local) = w[0].node, let Option::Some(ref t) = local.init, - let ExprKind::Closure(_,_,_,_) = t.node, - let PatKind::Ident(_,sp_ident,_) = local.pat.node, - let StmtKind::Semi(ref second,_) = w[1].node, - let ExprKind::Assign(_,ref call) = second.node, - let ExprKind::Call(ref closure,_) = call.node, - let ExprKind::Path(_,ref path) = closure.node + let ExprKind::Closure(_, _, _, _) = t.node, + let PatKind::Ident(_, sp_ident, _) = local.pat.node, + let StmtKind::Semi(ref second) = w[1].node, + let ExprKind::Assign(_, ref call) = second.node, + let ExprKind::Call(ref closure, _) = call.node, + let ExprKind::Path(_, ref path) = closure.node ], { if sp_ident.node == (&path.segments[0]).identifier { span_lint(cx, REDUNDANT_CLOSURE_CALL, second.span, "Closure called just once immediately after it was declared"); diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index aa8608fb7bd0..17f12afcaec9 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -68,8 +68,8 @@ const WHITELIST: &'static [&'static [&'static str]] = &[ struct SimilarNamesNameVisitor<'a, 'b: 'a, 'c: 'b>(&'a mut SimilarNamesLocalVisitor<'b, 'c>); -impl<'v, 'a, 'b, 'c> Visitor<'v> for SimilarNamesNameVisitor<'a, 'b, 'c> { - fn visit_pat(&mut self, pat: &'v Pat) { +impl<'a, 'b, 'c> Visitor for SimilarNamesNameVisitor<'a, 'b, 'c> { + fn visit_pat(&mut self, pat: &Pat) { match pat.node { PatKind::Ident(_, id, _) => self.check_name(id.span, id.node.name), PatKind::Struct(_, ref fields, _) => { @@ -226,25 +226,25 @@ impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> { } } -impl<'v, 'a, 'b> Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> { - fn visit_local(&mut self, local: &'v Local) { +impl<'a, 'b> Visitor for SimilarNamesLocalVisitor<'a, 'b> { + fn visit_local(&mut self, local: &Local) { if let Some(ref init) = local.init { self.apply(|this| walk_expr(this, &**init)); } // add the pattern after the expression because the bindings aren't available yet in the init expression SimilarNamesNameVisitor(self).visit_pat(&*local.pat); } - fn visit_block(&mut self, blk: &'v Block) { + fn visit_block(&mut self, blk: &Block) { self.apply(|this| walk_block(this, blk)); } - fn visit_arm(&mut self, arm: &'v Arm) { + fn visit_arm(&mut self, arm: &Arm) { self.apply(|this| { // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier SimilarNamesNameVisitor(this).visit_pat(&arm.pats[0]); this.apply(|this| walk_expr(this, &arm.body)); }); } - fn visit_item(&mut self, _: &'v Item) { + fn visit_item(&mut self, _: &Item) { // do not recurse into inner items } } diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 6beed822a81a..fda151cd6d7e 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -36,13 +36,12 @@ pub struct ReturnPass; impl ReturnPass { // Check the final stmt or expr in a block for unnecessary return. 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() { - if let StmtKind::Semi(ref expr, _) = stmt.node { - if let ExprKind::Ret(Some(ref inner)) = expr.node { - self.emit_return_lint(cx, (stmt.span, inner.span)); + if let Some(stmt) = block.stmts.last() { + match stmt.node { + StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => { + self.check_final_expr(cx, expr); } + _ => (), } } } @@ -88,12 +87,14 @@ impl ReturnPass { // Check for "let x = EXPR; x" fn check_let_return(&mut self, cx: &EarlyContext, block: &Block) { + let mut it = block.stmts.iter(); + // we need both a let-binding stmt and an expr if_let_chain! {[ - let Some(stmt) = block.stmts.last(), - let Some(ref retexpr) = block.expr, - let StmtKind::Decl(ref decl, _) = stmt.node, - let DeclKind::Local(ref local) = decl.node, + let Some(ref retexpr) = it.next_back(), + let StmtKind::Expr(ref retexpr) = retexpr.node, + let Some(stmt) = it.next_back(), + let StmtKind::Local(ref local) = stmt.node, let Some(ref initexpr) = local.init, let PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node, let ExprKind::Path(_, ref path) = retexpr.node, diff --git a/mini-macro/src/lib.rs b/mini-macro/src/lib.rs index 699d17d4d70c..4b0c5ea5afde 100644 --- a/mini-macro/src/lib.rs +++ b/mini-macro/src/lib.rs @@ -5,7 +5,7 @@ extern crate rustc; extern crate rustc_plugin; use syntax::codemap::Span; -use syntax::ast::TokenTree; +use syntax::tokenstream::TokenTree; use syntax::ext::base::{ExtCtxt, MacResult, MacEager}; use syntax::ext::build::AstBuilder; // trait for expr_usize use rustc_plugin::Registry; diff --git a/src/main.rs b/src/main.rs index 17a219cafa08..c4ad6d66177a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,17 +2,17 @@ #![feature(box_syntax)] #![feature(rustc_private)] -extern crate rustc_driver; +extern crate clippy_lints; extern crate getopts; extern crate rustc; -extern crate syntax; +extern crate rustc_driver; +extern crate rustc_errors; extern crate rustc_plugin; -extern crate clippy_lints; +extern crate syntax; use rustc_driver::{driver, CompilerCalls, RustcDefaultCalls, Compilation}; use rustc::session::{config, Session}; use rustc::session::config::{Input, ErrorOutputType}; -use syntax::diagnostics; use std::path::PathBuf; use std::process::Command; @@ -36,7 +36,7 @@ impl<'a> CompilerCalls<'a> for ClippyCompilerCalls { fn early_callback(&mut self, matches: &getopts::Matches, sopts: &config::Options, - descriptions: &diagnostics::registry::Registry, + descriptions: &rustc_errors::registry::Registry, output: ErrorOutputType) -> Compilation { self.0.early_callback(matches, sopts, descriptions, output) @@ -46,7 +46,7 @@ impl<'a> CompilerCalls<'a> for ClippyCompilerCalls { sopts: &config::Options, odir: &Option, ofile: &Option, - descriptions: &diagnostics::registry::Registry) + descriptions: &rustc_errors::registry::Registry) -> Option<(Input, Option)> { self.0.no_input(matches, sopts, odir, ofile, descriptions) } diff --git a/tests/compile-fail/formatting.rs b/tests/compile-fail/formatting.rs index 2436f64d216f..9b8146dc2293 100644 --- a/tests/compile-fail/formatting.rs +++ b/tests/compile-fail/formatting.rs @@ -16,7 +16,9 @@ fn main() { //~| NOTE add the missing `else` or } - let _ = { + let _ = { // if as the last expression + let _ = 0; + if foo() { } if foo() { //~^ ERROR this looks like an `else if` but the `else` is missing @@ -26,6 +28,18 @@ fn main() { } }; + let _ = { // if in the middle of a block + if foo() { + } if foo() { + //~^ ERROR this looks like an `else if` but the `else` is missing + //~| NOTE add the missing `else` or + } + else { + } + + let _ = 0; + }; + if foo() { } else //~^ ERROR this is an `else if` but the formatting might hide it diff --git a/tests/compile-fail/item_after_statement.rs b/tests/compile-fail/item_after_statement.rs index f104081faa99..4be89176fc7d 100644 --- a/tests/compile-fail/item_after_statement.rs +++ b/tests/compile-fail/item_after_statement.rs @@ -2,6 +2,16 @@ #![plugin(clippy)] #![deny(items_after_statements)] +fn ok() { + fn foo() { println!("foo"); } + foo(); +} + +fn last() { + foo(); + fn foo() { println!("foo"); } //~ ERROR adding items after statements is confusing +} + fn main() { foo(); fn foo() { println!("foo"); } //~ ERROR adding items after statements is confusing diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs index 7f2d7754bda0..480c16f16660 100644 --- a/tests/compile-fail/needless_bool.rs +++ b/tests/compile-fail/needless_bool.rs @@ -1,8 +1,8 @@ #![feature(plugin)] #![plugin(clippy)] +#![deny(needless_bool)] #[allow(if_same_then_else)] -#[deny(needless_bool)] fn main() { let x = true; if x { true } else { true }; //~ERROR this if-then-else expression will always return true @@ -22,19 +22,19 @@ fn main() { bool_ret4(x); } -#[deny(needless_bool)] -#[allow(if_same_then_else)] +#[allow(if_same_then_else, needless_return)] fn bool_ret(x: bool) -> bool { - if x { return true } else { return true }; //~ERROR this if-then-else expression will always return true + if x { return true } else { return true }; + //~^ ERROR this if-then-else expression will always return true } -#[deny(needless_bool)] -#[allow(if_same_then_else)] +#[allow(if_same_then_else, needless_return)] fn bool_ret2(x: bool) -> bool { - if x { return false } else { return false }; //~ERROR this if-then-else expression will always return false + if x { return false } else { return false }; + //~^ ERROR this if-then-else expression will always return false } -#[deny(needless_bool)] +#[allow(needless_return)] fn bool_ret3(x: bool) -> bool { if x { return true } else { return false }; //~^ ERROR this if-then-else expression returns a bool literal @@ -42,7 +42,7 @@ fn bool_ret3(x: bool) -> bool { //~| SUGGESTION `return x` } -#[deny(needless_bool)] +#[allow(needless_return)] fn bool_ret4(x: bool) -> bool { if x { return false } else { return true }; //~^ ERROR this if-then-else expression returns a bool literal diff --git a/tests/consts.rs b/tests/consts.rs index 81500f9d3934..773b889ebff7 100644 --- a/tests/consts.rs +++ b/tests/consts.rs @@ -14,6 +14,7 @@ use syntax::ast::{LitIntType, LitKind, StrStyle}; use syntax::codemap::{Spanned, COMMAND_LINE_SP}; use syntax::parse::token::InternedString; use syntax::ptr::P; +use syntax::util::ThinVec; fn spanned(t: T) -> Spanned { Spanned { @@ -27,7 +28,7 @@ fn expr(n: Expr_) -> Expr { id: 1, node: n, span: COMMAND_LINE_SP, - attrs: None, + attrs: ThinVec::new(), } } From 55b78ae47844f65d8d2703a4edd1b9d5a2bfb93e Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 1 Jul 2016 17:41:57 +0200 Subject: [PATCH 2/3] Rustup to ea0dc9297283daff6486807f43e190b4eb561412 II --- clippy_lints/src/collapsible_if.rs | 34 +++++++++------------------- tests/compile-fail/collapsible_if.rs | 5 ++++ 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 2921bc2769c2..572520372709 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -72,8 +72,8 @@ fn check_if(cx: &EarlyContext, expr: &ast::Expr) { fn check_collapsible_maybe_if_let(cx: &EarlyContext, else_: &ast::Expr) { if_let_chain! {[ let ast::ExprKind::Block(ref block) = else_.node, - block.stmts.is_empty(), - let Some(ref else_) = block.expr, + let Some(ref else_) = expr_block(block), + !in_macro(cx, else_.span), ], { match else_.node { ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { @@ -96,7 +96,7 @@ fn check_collapsible_no_if_let( then: &ast::Block, ) { if_let_chain! {[ - let Some(inner) = single_stmt_of_block(then), + let Some(inner) = expr_block(then), let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node, ], { if expr.span.expn_id != inner.span.expn_id { @@ -128,28 +128,16 @@ fn check_to_string(cx: &EarlyContext, e: &ast::Expr) -> Cow<'static, str> { } } -fn single_stmt_of_block(block: &ast::Block) -> Option<&ast::Expr> { - if block.stmts.len() == 1 && block.expr.is_none() { - if let ast::StmtKind::Expr(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 +/// If the block contains only one expression, returns it. +fn expr_block(block: &ast::Block) -> Option<&ast::Expr> { + let mut it = block.stmts.iter(); + + if let (Some(stmt), None) = (it.next(), it.next()) { + match stmt.node { + ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => Some(expr), + _ => None, } } else { None } } - -fn single_stmt_of_expr(expr: &ast::Expr) -> Option<&ast::Expr> { - if let ast::ExprKind::Block(ref block) = expr.node { - single_stmt_of_block(block) - } else { - Some(expr) - } -} diff --git a/tests/compile-fail/collapsible_if.rs b/tests/compile-fail/collapsible_if.rs index ea2ef284f38f..e51e31c69379 100644 --- a/tests/compile-fail/collapsible_if.rs +++ b/tests/compile-fail/collapsible_if.rs @@ -139,4 +139,9 @@ fn main() { println!("world!") } } + + if true { + } else { + assert!(true); // assert! is just an `if` + } } From f609ac5b587f92947b3341a75e663b77ca73037c Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 2 Jul 2016 16:02:27 +0200 Subject: [PATCH 3/3] Bump to 0.0.78 --- CHANGELOG.md | 3 ++- Cargo.toml | 4 ++-- clippy_lints/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fece611c1dac..7fedd3749487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Change Log All notable changes to this project will be documented in this file. -## 0.0.78 - TBA +## 0.0.78 - 2016-07-02 +* Rustup to *rustc 1.11.0-nightly (01411937f 2016-07-01)* * New lints: [`wrong_transmute`, `double_neg`] * For compatibility, `cargo clippy` does not defines the `clippy` feature introduced in 0.0.76 anymore diff --git a/Cargo.toml b/Cargo.toml index 5dc1363f3f53..f8b63a56257c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.0.77" +version = "0.0.78" authors = [ "Manish Goregaokar ", "Andre Bogus ", @@ -25,7 +25,7 @@ test = false [dependencies] # begin automatic update -clippy_lints = { version = "0.0.77", path = "clippy_lints" } +clippy_lints = { version = "0.0.78", path = "clippy_lints" } # end automatic update [dev-dependencies] diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index a1012e94ee5a..e74db2b17a64 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_lints" # begin automatic update -version = "0.0.77" +version = "0.0.78" # end automatic update authors = [ "Manish Goregaokar ",