Merge remote-tracking branch 'origin/rustup' into sugg

This commit is contained in:
mcarton 2016-07-01 17:48:57 +02:00
commit d6182b365c
No known key found for this signature in database
GPG key ID: 5E427C794CBA45E8
18 changed files with 155 additions and 110 deletions

View file

@ -2,7 +2,7 @@
All notable changes to this project will be documented in this file.
## 0.0.78 - TBA
* New lints: [`wrong_transmute`]
* New lints: [`wrong_transmute`, `double_neg`]
* For compatibility, `cargo clippy` does not defines the `clippy` feature
introduced in 0.0.76 anymore
* [`collapsible_if`] now considers `if let`
@ -153,6 +153,7 @@ All notable changes to this project will be documented in this file.
[`deprecated_semver`]: https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver
[`derive_hash_xor_eq`]: https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq
[`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown
[`double_neg`]: https://github.com/Manishearth/rust-clippy/wiki#double_neg
[`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref
[`duplicate_underscore_argument`]: https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument
[`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop

View file

@ -17,7 +17,7 @@ Table of contents:
## Lints
There are 156 lints included in this crate:
There are 157 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -49,6 +49,7 @@ name
[deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver
[derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly
[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | checks for the presence of `_`, `::` or camel-case outside ticks in documentation
[double_neg](https://github.com/Manishearth/rust-clippy/wiki#double_neg) | warn | `--x` is a double negation of `x` and not a pre-decrement as in C or C++
[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value
[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore
[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected

View file

@ -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()),
}
};

View file

@ -71,8 +71,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(..) => {
@ -95,7 +95,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 {
@ -113,28 +113,16 @@ fn check_collapsible_no_if_let(
}}
}
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)
}
}

View file

@ -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) {

View file

@ -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");
}
}
}

View file

@ -370,6 +370,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
misc::MODULO_ONE,
misc::REDUNDANT_PATTERN,
misc::TOPLEVEL_REF_ARG,
misc_early::DOUBLE_NEG,
misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
misc_early::REDUNDANT_CLOSURE_CALL,
misc_early::UNNEEDED_FIELD_PATTERN,

View file

@ -40,12 +40,25 @@ declare_lint! {
"Closures should not be called in the expression they are defined"
}
/// **What it does:** This lint detects expressions of the form `--x`
///
/// **Why is this bad?** It can mislead C/C++ programmers to think `x` was decremented.
///
/// **Known problems:** None.
///
/// **Example:** `--x;`
declare_lint! {
pub DOUBLE_NEG, Warn,
"`--x` is a double negation of `x` and not a pre-decrement as in C or C++"
}
#[derive(Copy, Clone)]
pub struct MiscEarly;
impl LintPass for MiscEarly {
fn get_lints(&self) -> LintArray {
lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL)
lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL, DOUBLE_NEG)
}
}
@ -126,36 +139,46 @@ impl EarlyLintPass for MiscEarly {
}
fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
if let ExprKind::Call(ref paren, _) = expr.node {
if let ExprKind::Paren(ref closure) = paren.node {
if let ExprKind::Closure(_, ref decl, ref block, _) = closure.node {
span_lint_and_then(cx,
REDUNDANT_CLOSURE_CALL,
expr.span,
"Try not to call a closure in the expression where it is declared.",
|db| {
if decl.inputs.is_empty() {
let hint = format!("{}", snippet(cx, block.span, ".."));
db.span_suggestion(expr.span, "Try doing something like: ", hint);
}
});
match expr.node {
ExprKind::Call(ref paren, _) => {
if let ExprKind::Paren(ref closure) = paren.node {
if let ExprKind::Closure(_, ref decl, ref block, _) = closure.node {
span_lint_and_then(cx,
REDUNDANT_CLOSURE_CALL,
expr.span,
"Try not to call a closure in the expression where it is declared.",
|db| {
if decl.inputs.is_empty() {
let hint = format!("{}", snippet(cx, block.span, ".."));
db.span_suggestion(expr.span, "Try doing something like: ", hint);
}
});
}
}
}
ExprKind::Unary(UnOp::Neg, ref inner) => {
if let ExprKind::Unary(UnOp::Neg, _) = inner.node {
span_lint(cx,
DOUBLE_NEG,
expr.span,
"`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op");
}
}
_ => ()
}
}
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");

View file

@ -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
}
}

View file

@ -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,

View file

@ -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;

View file

@ -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<PathBuf>,
ofile: &Option<PathBuf>,
descriptions: &diagnostics::registry::Registry)
descriptions: &rustc_errors::registry::Registry)
-> Option<(Input, Option<PathBuf>)> {
self.0.no_input(matches, sopts, odir, ofile, descriptions)
}

View file

@ -175,4 +175,9 @@ fn main() {
println!("world!")
}
}
if true {
} else {
assert!(true); // assert! is just an `if`
}
}

View file

@ -0,0 +1,10 @@
#![feature(plugin)]
#![plugin(clippy)]
#[deny(double_neg)]
fn main() {
let x = 1;
-x;
-(-x);
--x; //~ERROR: `--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op
}

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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: T) -> Spanned<T> {
Spanned {
@ -27,7 +28,7 @@ fn expr(n: Expr_) -> Expr {
id: 1,
node: n,
span: COMMAND_LINE_SP,
attrs: None,
attrs: ThinVec::new(),
}
}