Add needless_if lint

CHANGELOG.md

@@ -5008,6 +5008,7 @@ Released 2018-09-13
 [`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_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_if
 [`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
 [`needless_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_match

clippy_lints/src/declared_lints.rs

@@ -459,6 +459,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_continue::NEEDLESS_CONTINUE_INFO,
     crate::needless_else::NEEDLESS_ELSE_INFO,
     crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
+    crate::needless_if::NEEDLESS_IF_INFO,
     crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
     crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
     crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,

clippy_lints/src/lib.rs

@@ -223,6 +223,7 @@ mod needless_borrowed_ref;
 mod needless_continue;
 mod needless_else;
 mod needless_for_each;
+mod needless_if;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_value;
@@ -1029,6 +1030,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(endian_bytes::EndianBytes));
     store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations));
     store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync));
+    store.register_late_pass(|_| Box::new(needless_if::NeedlessIf));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/needless_if.rs

@@ -0,0 +1,103 @@
+use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, source::snippet_with_applicability};
+use rustc_errors::Applicability;
+use rustc_hir::{
+    intravisit::{walk_expr, Visitor},
+    Expr, ExprKind, Node,
+};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for empty `if` branches with no else branch.
+    ///
+    /// ### Why is this bad?
+    /// It can be entirely omitted, and often the condition too.
+    ///
+    /// ### Known issues
+    /// This will usually only suggest to remove the `if` statement, not the condition. Other lints
+    /// such as `no_effect` will take care of removing the condition if it's unnecessary.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// if really_expensive_condition(&i) {}
+    /// if really_expensive_condition_with_side_effects(&mut i) {}
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// // <omitted>
+    /// really_expensive_condition_with_side_effects(&mut i);
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub NEEDLESS_IF,
+    complexity,
+    "checks for empty if branches"
+}
+declare_lint_pass!(NeedlessIf => [NEEDLESS_IF]);
+
+impl LateLintPass<'_> for NeedlessIf {
+    fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if let ExprKind::If(if_expr, block, else_expr) = &expr.kind
+            && let ExprKind::Block(block, ..) = block.kind
+            && block.stmts.is_empty()
+            && block.expr.is_none()
+            && else_expr.is_none()
+            && !in_external_macro(cx.sess(), expr.span)
+        {
+            // Ignore `else if`
+            if let Some(parent_id) = cx.tcx.hir().opt_parent_id(expr.hir_id)
+                && let Some(Node::Expr(Expr {
+                    kind: ExprKind::If(_, _, Some(else_expr)),
+                    ..
+                })) = cx.tcx.hir().find(parent_id)
+                && else_expr.hir_id == expr.hir_id
+            {
+                return;
+            }
+
+            if is_any_if_let(if_expr) || is_from_proc_macro(cx, expr) {
+                return;
+            }
+
+            let mut app = Applicability::MachineApplicable;
+            let snippet = snippet_with_applicability(cx, if_expr.span, "{ ... }", &mut app);
+
+            span_lint_and_sugg(
+                cx,
+                NEEDLESS_IF,
+                expr.span,
+                "this `if` branch is empty",
+                "you can remove it",
+                if if_expr.can_have_side_effects() {
+                    format!("{snippet};")
+                } else {
+                    String::new()
+                },
+                app,
+            );
+        }
+    }
+}
+
+/// Returns true if any `Expr` contained within this `Expr` is a `Let`, else false.
+///
+/// Really wish `Expr` had a `walk` method...
+fn is_any_if_let(expr: &Expr<'_>) -> bool {
+    let mut v = IsAnyLetVisitor(false);
+
+    v.visit_expr(expr);
+    v.0
+}
+
+struct IsAnyLetVisitor(bool);
+
+impl Visitor<'_> for IsAnyLetVisitor {
+    fn visit_expr(&mut self, expr: &Expr<'_>) {
+        if matches!(expr.kind, ExprKind::Let(..)) {
+            self.0 = true;
+        } else {
+            walk_expr(self, expr);
+        }
+    }
+}

tests/ui-internal/if_chain_style.rs

@@ -1,5 +1,10 @@
 #![warn(clippy::if_chain_style)]
-#![allow(clippy::no_effect, clippy::nonminimal_bool, clippy::missing_clippy_version_attribute)]
+#![allow(
+    clippy::needless_if,
+    clippy::no_effect,
+    clippy::nonminimal_bool,
+    clippy::missing_clippy_version_attribute
+)]
 
 extern crate if_chain;
 

tests/ui-internal/if_chain_style.stderr

@@ -1,5 +1,5 @@
 error: this `if` can be part of the inner `if_chain!`
-  --> $DIR/if_chain_style.rs:9:5
+  --> $DIR/if_chain_style.rs:14:5
    |
 LL | /     if true {
 LL | |         let x = "";
@@ -11,14 +11,14 @@ LL | |     }
    | |_____^
    |
 help: this `let` statement can also be in the `if_chain!`
-  --> $DIR/if_chain_style.rs:10:9
+  --> $DIR/if_chain_style.rs:15:9
    |
 LL |         let x = "";
    |         ^^^^^^^^^^^
    = note: `-D clippy::if-chain-style` implied by `-D warnings`
 
 error: `if a && b;` should be `if a; if b;`
-  --> $DIR/if_chain_style.rs:19:12
+  --> $DIR/if_chain_style.rs:24:12
    |
 LL |           if true
    |  ____________^
@@ -27,25 +27,25 @@ LL | |             && false;
    | |____________________^
 
 error: `let` expression should be inside `then { .. }`
-  --> $DIR/if_chain_style.rs:24:9
+  --> $DIR/if_chain_style.rs:29:9
    |
 LL |         let x = "";
    |         ^^^^^^^^^^^
 
 error: this `if` can be part of the outer `if_chain!`
-  --> $DIR/if_chain_style.rs:35:13
+  --> $DIR/if_chain_style.rs:40:13
    |
 LL |             if true {}
    |             ^^^^^^^^^^
    |
 help: this `let` statement can also be in the `if_chain!`
-  --> $DIR/if_chain_style.rs:33:13
+  --> $DIR/if_chain_style.rs:38:13
    |
 LL |             let x = "";
    |             ^^^^^^^^^^^
 
 error: `if_chain!` only has one `if`
-  --> $DIR/if_chain_style.rs:29:5
+  --> $DIR/if_chain_style.rs:34:5
    |
 LL | /     if_chain! {
 LL | |         // single `if` condition
@@ -59,13 +59,13 @@ LL | |     }
    = note: this error originates in the macro `__if_chain` which comes from the expansion of the macro `if_chain` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: `let` expression should be above the `if_chain!`
-  --> $DIR/if_chain_style.rs:40:9
+  --> $DIR/if_chain_style.rs:45:9
    |
 LL |         let x = "";
    |         ^^^^^^^^^^^
 
 error: this `if_chain!` can be merged with the outer `if_chain!`
-  --> $DIR/if_chain_style.rs:46:13
+  --> $DIR/if_chain_style.rs:51:13
    |
 LL | /             if_chain! {
 LL | |                 if true;
@@ -75,7 +75,7 @@ LL | |             }
    | |_____________^
    |
 help: these `let` statements can also be in the `if_chain!`
-  --> $DIR/if_chain_style.rs:43:13
+  --> $DIR/if_chain_style.rs:48:13
    |
 LL | /             let x = "";
 LL | |             let x = "";

tests/ui-toml/excessive_nesting/excessive_nesting.rs

@@ -7,6 +7,7 @@
 #![allow(clippy::no_effect)]
 #![allow(clippy::unnecessary_operation)]
 #![allow(clippy::never_loop)]
+#![allow(clippy::needless_if)]
 #![warn(clippy::excessive_nesting)]
 #![allow(clippy::collapsible_if)]