needless_else: new lint to check for empty else clauses

CHANGELOG.md

@@ -4874,6 +4874,7 @@ Released 2018-09-13
 [`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
 [`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
+[`needless_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_else
 [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each
 [`needless_late_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init
 [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

clippy_lints/src/declared_lints.rs

@@ -449,6 +449,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_bool::NEEDLESS_BOOL_ASSIGN_INFO,
     crate::needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE_INFO,
     crate::needless_continue::NEEDLESS_CONTINUE_INFO,
+    crate::needless_else::NEEDLESS_ELSE_INFO,
     crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
     crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
     crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,

clippy_lints/src/lib.rs

@@ -218,6 +218,7 @@ mod needless_arbitrary_self_type;
 mod needless_bool;
 mod needless_borrowed_ref;
 mod needless_continue;
+mod needless_else;
 mod needless_for_each;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
@@ -992,6 +993,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule));
     store.register_early_pass(|| Box::new(ref_patterns::RefPatterns));
     store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
+    store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/needless_else.rs

@@ -0,0 +1,57 @@
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::trim_span, span_extract_comment};
+use rustc_ast::ast::{Expr, ExprKind};
+use rustc_errors::Applicability;
+use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for empty `else` branches.
+    ///
+    /// ### Why is this bad?
+    /// An empty else branch does nothing and can be removed.
+    ///
+    /// ### Example
+    /// ```rust
+    ///# fn check() -> bool { true }
+    /// if check() {
+    ///     println!("Check successful!");
+    /// } else {
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    ///# fn check() -> bool { true }
+    /// if check() {
+    ///     println!("Check successful!");
+    /// }
+    /// ```
+    #[clippy::version = "1.71.0"]
+    pub NEEDLESS_ELSE,
+    style,
+    "empty else branch"
+}
+declare_lint_pass!(NeedlessElse => [NEEDLESS_ELSE]);
+
+impl EarlyLintPass for NeedlessElse {
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
+        if let ExprKind::If(_, then_block, Some(else_clause)) = &expr.kind &&
+            let ExprKind::Block(block, _) = &else_clause.kind &&
+            !expr.span.from_expansion() &&
+            !else_clause.span.from_expansion() &&
+            block.stmts.is_empty() {
+                let span = trim_span(cx.sess().source_map(), expr.span.trim_start(then_block.span).unwrap());
+                if span_extract_comment(cx.sess().source_map(), span).is_empty() {
+                    span_lint_and_sugg(
+                        cx,
+                        NEEDLESS_ELSE,
+                        span,
+                        "this else branch is empty",
+                        "you can remove it",
+                        String::new(),
+                        Applicability::MachineApplicable,
+                    );
+                }
+            }
+    }
+}

tests/ui-toml/ifs_same_cond/ifs_same_cond.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::ifs_same_cond)]
-#![allow(clippy::if_same_then_else, clippy::comparison_chain)]
+#![allow(clippy::if_same_then_else, clippy::comparison_chain, clippy::needless_else)]
 
 fn main() {}
 

tests/ui/branches_sharing_code/valid_if_blocks.rs

@@ -1,6 +1,10 @@
 #![deny(clippy::branches_sharing_code, clippy::if_same_then_else)]
 #![allow(dead_code)]
-#![allow(clippy::mixed_read_write_in_expression, clippy::uninlined_format_args)]
+#![allow(
+    clippy::mixed_read_write_in_expression,
+    clippy::uninlined_format_args,
+    clippy::needless_else
+)]
 
 // This tests valid if blocks that shouldn't trigger the lint
 

tests/ui/branches_sharing_code/valid_if_blocks.stderr

@@ -1,13 +1,13 @@
 error: this `if` has identical blocks
-  --> $DIR/valid_if_blocks.rs:105:14
+  --> $DIR/valid_if_blocks.rs:109:14
    |
 LL |       if false {
    |  ______________^
 LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/valid_if_blocks.rs:106:12
+  --> $DIR/valid_if_blocks.rs:110:12
    |
 LL |       } else {
    |  ____________^
@@ -20,7 +20,7 @@ LL | #![deny(clippy::branches_sharing_code, clippy::if_same_then_else)]
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this `if` has identical blocks
-  --> $DIR/valid_if_blocks.rs:116:15
+  --> $DIR/valid_if_blocks.rs:120:15
    |
 LL |       if x == 0 {
    |  _______________^
@@ -31,7 +31,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/valid_if_blocks.rs:120:12
+  --> $DIR/valid_if_blocks.rs:124:12
    |
 LL |       } else {
    |  ____________^
@@ -42,19 +42,19 @@ LL | |     }
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/valid_if_blocks.rs:127:23
+  --> $DIR/valid_if_blocks.rs:131:23
    |
 LL |     let _ = if x == 6 { 7 } else { 7 };
    |                       ^^^^^
    |
 note: same as this
-  --> $DIR/valid_if_blocks.rs:127:34
+  --> $DIR/valid_if_blocks.rs:131:34
    |
 LL |     let _ = if x == 6 { 7 } else { 7 };
    |                                  ^^^^^
 
 error: this `if` has identical blocks
-  --> $DIR/valid_if_blocks.rs:133:23
+  --> $DIR/valid_if_blocks.rs:137:23
    |
 LL |       } else if x == 68 {
    |  _______________________^
@@ -66,7 +66,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/valid_if_blocks.rs:138:12
+  --> $DIR/valid_if_blocks.rs:142:12
    |
 LL |       } else {
    |  ____________^
@@ -78,7 +78,7 @@ LL | |     };
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/valid_if_blocks.rs:147:23
+  --> $DIR/valid_if_blocks.rs:151:23
    |
 LL |       } else if x == 68 {
    |  _______________________^
@@ -88,7 +88,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/valid_if_blocks.rs:150:12
+  --> $DIR/valid_if_blocks.rs:154:12
    |
 LL |       } else {
    |  ____________^

tests/ui/crashes/ice-7410.rs

@@ -6,6 +6,7 @@
 #![no_std]
 #![allow(clippy::if_same_then_else)]
 #![allow(clippy::redundant_pattern_matching)]
+#![allow(clippy::needless_else)]
 
 use core::panic::PanicInfo;
 

tests/ui/ifs_same_cond.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::ifs_same_cond)]
-#![allow(clippy::if_same_then_else, clippy::comparison_chain)] // all empty blocks
+#![allow(clippy::if_same_then_else, clippy::comparison_chain, clippy::needless_else)] // all empty blocks
 
 fn ifs_same_cond() {
     let a = 0;

tests/ui/needless_else.fixed

@@ -0,0 +1,40 @@
+//@run-rustfix
+#![allow(unused)]
+#![warn(clippy::needless_else)]
+#![allow(clippy::suspicious_else_formatting)]
+
+macro_rules! mac {
+    ($test:expr) => {
+        if $test {
+            println!("Test successful!");
+        } else {
+        }
+    };
+}
+
+fn main() {
+    let b = std::hint::black_box(true);
+
+    if b {
+        println!("Foobar");
+    } 
+
+    if b {
+        println!("Foobar");
+    } else {
+        // Do not lint because this comment might be important
+    }
+
+    if b {
+        println!("Foobar");
+    } else
+    /* Do not lint because this comment might be important */
+    {
+    }
+
+    // Do not lint because of the expression
+    let _ = if b { 1 } else { 2 };
+
+    // Do not lint because inside a macro
+    mac!(b);
+}

tests/ui/needless_else.rs

@@ -0,0 +1,41 @@
+//@run-rustfix
+#![allow(unused)]
+#![warn(clippy::needless_else)]
+#![allow(clippy::suspicious_else_formatting)]
+
+macro_rules! mac {
+    ($test:expr) => {
+        if $test {
+            println!("Test successful!");
+        } else {
+        }
+    };
+}
+
+fn main() {
+    let b = std::hint::black_box(true);
+
+    if b {
+        println!("Foobar");
+    } else {
+    }
+
+    if b {
+        println!("Foobar");
+    } else {
+        // Do not lint because this comment might be important
+    }
+
+    if b {
+        println!("Foobar");
+    } else
+    /* Do not lint because this comment might be important */
+    {
+    }
+
+    // Do not lint because of the expression
+    let _ = if b { 1 } else { 2 };
+
+    // Do not lint because inside a macro
+    mac!(b);
+}

tests/ui/needless_else.stderr

@@ -0,0 +1,12 @@
+error: this else branch is empty
+  --> $DIR/needless_else.rs:20:7
+   |
+LL |       } else {
+   |  _______^
+LL | |     }
+   | |_____^ help: you can remove it
+   |
+   = note: `-D clippy::needless-else` implied by `-D warnings`
+
+error: aborting due to previous error
+

tests/ui/needless_return.fixed

@@ -7,7 +7,8 @@
     clippy::if_same_then_else,
     clippy::single_match,
     clippy::needless_bool,
-    clippy::equatable_if_let
+    clippy::equatable_if_let,
+    clippy::needless_else
 )]
 #![warn(clippy::needless_return)]
 

tests/ui/needless_return.rs

@@ -7,7 +7,8 @@
     clippy::if_same_then_else,
     clippy::single_match,
     clippy::needless_bool,
-    clippy::equatable_if_let
+    clippy::equatable_if_let,
+    clippy::needless_else
 )]
 #![warn(clippy::needless_return)]