mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-24 07:44:10 +00:00
lint ! and != in if expressions with else branches
This commit is contained in:
parent
35e00e20c9
commit
3b7720f992
@ -8,7 +8,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
|
||||
[Jump to usage instructions](#usage)
|
||||
|
||||
##Lints
|
||||
There are 128 lints included in this crate:
|
||||
There are 129 lints included in this crate:
|
||||
|
||||
name | default | meaning
|
||||
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
||||
@ -52,6 +52,7 @@ name
|
||||
[for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option) | warn | for-looping over an `Option`, which is more clearly expressed as an `if let`
|
||||
[for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result) | warn | for-looping over a `Result`, which is more clearly expressed as an `if let`
|
||||
[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1`
|
||||
[if_not_else](https://github.com/Manishearth/rust-clippy/wiki#if_not_else) | warn | finds if branches that could be swapped so no negation operation is necessary on the condition
|
||||
[if_same_then_else](https://github.com/Manishearth/rust-clippy/wiki#if_same_then_else) | warn | if with the same *then* and *else* blocks
|
||||
[ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond) | warn | consecutive `ifs` with the same condition
|
||||
[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`
|
||||
|
@ -45,9 +45,7 @@ impl<'v> Visitor<'v> for ExVisitor<'v> {
|
||||
fn visit_expr(&mut self, expr: &'v Expr) {
|
||||
if let ExprClosure(_, _, ref block) = expr.node {
|
||||
let complex = {
|
||||
if !block.stmts.is_empty() {
|
||||
true
|
||||
} else {
|
||||
if block.stmts.is_empty() {
|
||||
if let Some(ref ex) = block.expr {
|
||||
match ex.node {
|
||||
ExprBlock(_) => true,
|
||||
@ -56,6 +54,8 @@ impl<'v> Visitor<'v> for ExVisitor<'v> {
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
true
|
||||
}
|
||||
};
|
||||
if complex {
|
||||
|
@ -159,10 +159,10 @@ impl PartialOrd for Constant {
|
||||
fn partial_cmp(&self, other: &Constant) -> Option<Ordering> {
|
||||
match (self, other) {
|
||||
(&Constant::Str(ref ls, ref lsty), &Constant::Str(ref rs, ref rsty)) => {
|
||||
if lsty != rsty {
|
||||
None
|
||||
} else {
|
||||
if lsty == rsty {
|
||||
Some(ls.cmp(rs))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
(&Constant::Byte(ref l), &Constant::Byte(ref r)) => Some(l.cmp(r)),
|
||||
|
@ -89,12 +89,10 @@ impl EarlyLintPass for EnumVariantNames {
|
||||
let post_camel = camel_case_from(&post);
|
||||
post = &post[post_camel..];
|
||||
}
|
||||
let (what, value) = if !pre.is_empty() {
|
||||
("pre", pre)
|
||||
} else if !post.is_empty() {
|
||||
("post", post)
|
||||
} else {
|
||||
return;
|
||||
let (what, value) = match (pre.is_empty(), post.is_empty()) {
|
||||
(true, true) => return,
|
||||
(false, _) => ("pre", pre),
|
||||
(true, false) => ("post", post),
|
||||
};
|
||||
span_help_and_lint(cx,
|
||||
ENUM_VARIANT_NAMES,
|
||||
|
53
src/if_not_else.rs
Normal file
53
src/if_not_else.rs
Normal file
@ -0,0 +1,53 @@
|
||||
//! lint on if branches that could be swapped so no `!` operation is necessary on the condition
|
||||
|
||||
use rustc::lint::*;
|
||||
use syntax::attr::*;
|
||||
use syntax::ast::*;
|
||||
|
||||
use utils::span_help_and_lint;
|
||||
|
||||
/// **What it does:** Warns on the use of `!` or `!=` in an if condition with an else branch
|
||||
///
|
||||
/// **Why is this bad?** Negations reduce the readability of statements
|
||||
///
|
||||
/// **Known problems:** None
|
||||
///
|
||||
/// **Example:** if !v.is_empty() { a() } else { b() }
|
||||
declare_lint! {
|
||||
pub IF_NOT_ELSE, Warn,
|
||||
"finds if branches that could be swapped so no negation operation is necessary on the condition"
|
||||
}
|
||||
|
||||
pub struct IfNotElse;
|
||||
|
||||
impl LintPass for IfNotElse {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(IF_NOT_ELSE)
|
||||
}
|
||||
}
|
||||
|
||||
impl EarlyLintPass for IfNotElse {
|
||||
fn check_expr(&mut self, cx: &EarlyContext, item: &Expr) {
|
||||
if let ExprKind::If(ref cond, _, Some(ref els)) = item.node {
|
||||
if let ExprKind::Block(..) = els.node {
|
||||
match cond.node {
|
||||
ExprKind::Unary(UnOp::Not, _) => {
|
||||
span_help_and_lint(cx,
|
||||
IF_NOT_ELSE,
|
||||
item.span,
|
||||
"Unnecessary boolean `not` operation",
|
||||
"remove the `!` and swap the blocks of the if/else");
|
||||
},
|
||||
ExprKind::Binary(ref kind, _, _) if kind.node == BinOpKind::Ne => {
|
||||
span_help_and_lint(cx,
|
||||
IF_NOT_ELSE,
|
||||
item.span,
|
||||
"Unnecessary `!=` operation",
|
||||
"change to `==` and swap the blocks of the if/else");
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
@ -60,6 +60,7 @@ pub mod eta_reduction;
|
||||
pub mod format;
|
||||
pub mod formatting;
|
||||
pub mod identity_op;
|
||||
pub mod if_not_else;
|
||||
pub mod items_after_statements;
|
||||
pub mod len_zero;
|
||||
pub mod lifetimes;
|
||||
@ -171,6 +172,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
||||
reg.register_late_lint_pass(box format::FormatMacLint);
|
||||
reg.register_early_lint_pass(box formatting::Formatting);
|
||||
reg.register_late_lint_pass(box swap::Swap);
|
||||
reg.register_early_lint_pass(box if_not_else::IfNotElse);
|
||||
|
||||
reg.register_lint_group("clippy_pedantic", vec![
|
||||
enum_glob_use::ENUM_GLOB_USE,
|
||||
@ -222,6 +224,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
||||
formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
|
||||
formatting::SUSPICIOUS_ELSE_FORMATTING,
|
||||
identity_op::IDENTITY_OP,
|
||||
if_not_else::IF_NOT_ELSE,
|
||||
items_after_statements::ITEMS_AFTER_STATEMENTS,
|
||||
len_zero::LEN_WITHOUT_IS_EMPTY,
|
||||
len_zero::LEN_ZERO,
|
||||
|
@ -357,10 +357,10 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
|
||||
};
|
||||
|
||||
let take: Cow<_> = if let Some(ref r) = *r {
|
||||
if !is_len_call(&r, &indexed) {
|
||||
format!(".take({})", snippet(cx, r.span, "..")).into()
|
||||
} else {
|
||||
if is_len_call(&r, &indexed) {
|
||||
"".into()
|
||||
} else {
|
||||
format!(".take({})", snippet(cx, r.span, "..")).into()
|
||||
}
|
||||
} else {
|
||||
"".into()
|
||||
|
@ -249,23 +249,21 @@ fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
|
||||
};
|
||||
|
||||
if let Some((ref true_expr, ref false_expr)) = exprs {
|
||||
if !is_unit_expr(true_expr) {
|
||||
if !is_unit_expr(false_expr) {
|
||||
match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
|
||||
(false, false) =>
|
||||
Some(format!("if {} {} else {}",
|
||||
snippet(cx, ex.span, "b"),
|
||||
expr_block(cx, true_expr, None, ".."),
|
||||
expr_block(cx, false_expr, None, "..")))
|
||||
} else {
|
||||
expr_block(cx, false_expr, None, ".."))),
|
||||
(false, true) =>
|
||||
Some(format!("if {} {}",
|
||||
snippet(cx, ex.span, "b"),
|
||||
expr_block(cx, true_expr, None, "..")))
|
||||
}
|
||||
} else if !is_unit_expr(false_expr) {
|
||||
Some(format!("try\nif !{} {}",
|
||||
snippet(cx, ex.span, "b"),
|
||||
expr_block(cx, false_expr, None, "..")))
|
||||
} else {
|
||||
None
|
||||
expr_block(cx, true_expr, None, ".."))),
|
||||
(true, false) =>
|
||||
Some(format!("try\nif !{} {}",
|
||||
snippet(cx, ex.span, "b"),
|
||||
expr_block(cx, false_expr, None, ".."))),
|
||||
(true, true) => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
|
@ -1,6 +1,6 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![allow(unused)]
|
||||
#![allow(unused, if_not_else)]
|
||||
|
||||
#![deny(map_entry)]
|
||||
|
||||
|
18
tests/compile-fail/if_not_else.rs
Normal file
18
tests/compile-fail/if_not_else.rs
Normal file
@ -0,0 +1,18 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![deny(clippy)]
|
||||
|
||||
fn bla() -> bool { unimplemented!() }
|
||||
|
||||
fn main() {
|
||||
if !bla() { //~ ERROR: Unnecessary boolean `not` operation
|
||||
println!("Bugs");
|
||||
} else {
|
||||
println!("Bunny");
|
||||
}
|
||||
if 4 != 5 { //~ ERROR: Unnecessary `!=` operation
|
||||
println!("Bugs");
|
||||
} else {
|
||||
println!("Bunny");
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user