Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 05:33:27 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #725 from oli-obk/swap_if_arms

Browse source 
lint ! and != in if expressions with else branches
This commit is contained in:
Manish Goregaokar 2016-02-29 21:34:15 +05:30
commit e256055dc4
10 changed files with 100 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
README.md
View file

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

6
src/block_in_if_condition.rs
View file

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

6
src/consts.rs
View file

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

10
src/enum_variants.rs
View file

@ -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
View 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");
},
_ => {},
}
}
}
}
}

3
src/lib.rs
View file

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

6
src/loops.rs
View file

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

22
src/matches.rs
View file

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

2
tests/compile-fail/entry.rs
View file

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