lint ! and != in if expressions with else branches

README.md

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

src/block_in_if_condition.rs

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

src/consts.rs

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

src/enum_variants.rs

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

src/if_not_else.rs

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

src/lib.rs

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

src/loops.rs

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

src/matches.rs

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

tests/compile-fail/entry.rs

@@ -1,6 +1,6 @@
 #![feature(plugin)]
 #![plugin(clippy)]
-#![allow(unused)]
+#![allow(unused, if_not_else)]
 
 #![deny(map_entry)]
 

tests/compile-fail/if_not_else.rs

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