Add ifs_as_logical_ops lint

Co-authored-by: Ada Alakbarova <ada.alakbarova@proton.me>

Author: barun511

CHANGELOG.md

@@ -6977,6 +6977,7 @@ Released 2018-09-13
 [`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
 [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
+[`ifs_as_logical_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_as_logical_ops
 [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
 [`ignore_without_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#ignore_without_reason
 [`ignored_unit_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns

clippy_lints/src/declared_lints.rs

@@ -214,6 +214,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::ifs::IF_SAME_THEN_ELSE_INFO,
     crate::ifs::IFS_SAME_COND_INFO,
     crate::ifs::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
+    crate::ifs_as_logical_ops::IFS_AS_LOGICAL_OPS_INFO,
     crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO,
     crate::impl_hash_with_borrow_str_and_bytes::IMPL_HASH_BORROW_WITH_STR_AND_BYTES_INFO,
     crate::implicit_hasher::IMPLICIT_HASHER_INFO,

clippy_lints/src/ifs_as_logical_ops.rs

@@ -0,0 +1,116 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher::If;
+use clippy_utils::source::{SpanExt, walk_span_to_context};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{is_else_clause, is_from_proc_macro, peel_blocks};
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Warn about cases where `x && y` could be used in place of an if condition.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// `x && y` is more standard as a construction, and makes it clearer that this is just an and.
+    /// It is also less verbose.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # fn a() -> bool { false }
+    /// fn b(b1: bool) -> bool {
+    ///     if b1 { a() } else { false}
+    /// }
+    ///
+    /// ```
+    ///
+    /// Could be written
+    ///
+    /// ```no_run
+    /// # fn a() -> bool { false }
+    /// fn b(b1: bool) -> bool {
+    ///     b1 && a()
+    /// }
+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub IFS_AS_LOGICAL_OPS,
+    pedantic,
+    "`if` conditions that can be rewritten as logical operators"
+}
+
+declare_lint_pass!(IfsAsLogicalOps => [IFS_AS_LOGICAL_OPS]);
+
+impl<'tcx> LateLintPass<'tcx> for IfsAsLogicalOps {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) {
+        // Make sure the if block is not an if-let block.
+        if let Some(If {cond: if_cond, then, r#else: Some(els)}) = If::hir(e)
+            && let ExprKind::Block(then_block, _label) = then.kind
+            // Check if the if-block has only a trailing expression
+            && then_block.stmts.is_empty()
+            && let Some(then_block_inner_expr) = then_block.expr
+            // And that the else block consists of only the boolean 'false'.
+            && let ExprKind::Block(else_block, _label) = els.kind
+            && else_block.stmts.is_empty()
+            && let Some(else_block_inner_expr) = else_block.expr
+            && let ExprKind::Lit(lit) = else_block_inner_expr.kind
+            && matches!(lit.node, LitKind::Bool(false))
+            // We do not emit this lint if the expression diverges.
+            && !cx.typeck_results().expr_ty(then_block_inner_expr).is_never()
+            // Make sure that the expression is only in a single macro context
+            && let ctxt = e.span.ctxt()
+            && ctxt == then_block.span.ctxt()
+            && ctxt == else_block.span.ctxt()
+            && ctxt == else_block_inner_expr.span.ctxt()
+            && ctxt == lit.span.ctxt()
+            && !ctxt.in_external_macro(cx.sess().source_map())
+            && !is_from_proc_macro(cx,e)
+        {
+            // Do not lint if the statement is trivially a boolean.
+            if let ExprKind::Lit(lit_ptr) = peel_blocks(then_block_inner_expr).kind
+                && let LitKind::Bool(_) = lit_ptr.node
+            {
+                return;
+            }
+
+            if let Some(walked_then_block_inner) = walk_span_to_context(then_block_inner_expr.span, ctxt)
+                && check_that_nested_spans_have_no_comments(then_block.span, walked_then_block_inner, cx)
+                && check_that_nested_spans_have_no_comments(else_block.span, else_block_inner_expr.span, cx)
+            {
+                let mut applicability = if ctxt.is_root() {
+                    Applicability::MachineApplicable
+                } else {
+                    Applicability::MaybeIncorrect
+                };
+
+                let mut sugg = Sugg::hir_with_context(cx, if_cond, ctxt, "_", &mut applicability);
+                let rhs_sugg = Sugg::hir_with_context(cx, then_block_inner_expr, ctxt, "_", &mut applicability);
+
+                sugg = sugg.and(&rhs_sugg);
+
+                if is_else_clause(cx.tcx, e) {
+                    sugg = sugg.blockify();
+                }
+
+                span_lint_and_sugg(
+                    cx,
+                    IFS_AS_LOGICAL_OPS,
+                    e.span,
+                    "if expression that could be written as a logical and expression",
+                    "try",
+                    sugg.to_string(),
+                    applicability,
+                );
+            }
+        }
+    }
+}
+
+fn check_that_nested_spans_have_no_comments(outer_span: Span, inner_span: Span, cx: &LateContext<'_>) -> bool {
+    (outer_span.lo()..inner_span.lo()).check_text(cx, |src| src.trim_end() == "{")
+        && (inner_span.hi()..outer_span.hi()).check_text(cx, |src| src.trim_start() == "}")
+}

clippy_lints/src/lib.rs

@@ -156,6 +156,7 @@ mod if_let_mutex;
 mod if_not_else;
 mod if_then_some_else_none;
 mod ifs;
+mod ifs_as_logical_ops;
 mod ignored_unit_patterns;
 mod impl_hash_with_borrow_str_and_bytes;
 mod implicit_hasher;
@@ -868,6 +869,7 @@ rustc_lint::late_lint_methods!(
         RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
         RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
         BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee,
+        IfsAsLogicalOps: ifs_as_logical_ops::IfsAsLogicalOps = ifs_as_logical_ops::IfsAsLogicalOps,
         // add late passes here, used by `cargo dev new_lint`
     ]]
 );

tests/ui/ifs_as_logical_ops.fixed

@@ -0,0 +1,183 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::ifs_as_logical_ops)]
+#![expect(clippy::needless_bool)]
+
+extern crate proc_macros;
+use proc_macros::{external, with_span};
+
+fn main() {
+    // test code goes here
+}
+
+fn basic_lint_test(b1: bool, b2: bool) -> bool {
+    b1 && b2
+    //~^ ifs_as_logical_ops
+}
+
+fn lint_test_with_cfg_as_second_argument(b1: bool, b2: bool) -> bool {
+    if b1 { b2 } else { cfg!(false) }
+}
+
+fn nested_expressions_produces_lint(b1: bool, b2: bool) -> bool {
+    b1 && { b2 }
+    //~^ ifs_as_logical_ops
+}
+
+fn diverging_does_not_produce_lint(b1: bool, b2: bool) -> bool {
+    if b1 { panic!() } else { false }
+}
+
+fn complex_expressions_do_not_produce_lint(b1: bool, b2: bool) -> bool {
+    if b1 {
+        let mut some_value = 100;
+        if some_value < 50 {
+            return true;
+        }
+        true
+    } else {
+        false
+    }
+}
+
+fn example_with_if_comment_before_expr_does_not_lint(chars: Vec<char>) -> bool {
+    if !chars.is_empty() {
+        // SAFETY: Always starts with ^[ and ends with m.
+        chars.len() > 5
+    } else {
+        false
+    }
+}
+
+fn example_with_if_comment_after_expr_does_not_lint(chars: Vec<char>) -> bool {
+    if !chars.is_empty() {
+        chars.len() > 5
+        // SAFETY: Always starts with ^[ and ends with m.
+    } else {
+        false
+    }
+}
+
+fn example_with_else_comment_before_expr_does_not_lint(chars: Vec<char>) -> bool {
+    if !chars.is_empty() {
+        chars.len() > 5
+    } else {
+        // Watch out! Comment!
+        false
+    }
+}
+
+fn example_with_else_comment_after_expr_does_not_lint(chars: Vec<char>) -> bool {
+    if !chars.is_empty() {
+        chars.len() > 5
+    } else {
+        false
+        // Watch out! Comment!
+    }
+}
+
+fn example_with_cfg_does_not_lint(chars: Vec<char>) -> bool {
+    if !chars.is_empty() {
+        #[cfg(false)]
+        return 2 * 2 == 5;
+        chars.len() > 5
+    } else {
+        false
+    }
+}
+
+fn example_with_debug_does_not_lint(b1: bool) -> bool {
+    if b1 {
+        dbg!("Something");
+        true
+    } else {
+        false
+    }
+}
+
+fn basic_if_let_does_not_lint(b1: bool, b2: Option<i32>) -> bool {
+    if let Some(30) = b2 { b1 } else { false }
+}
+
+fn if_and_let_does_not_lint(b1: bool, b2: Option<i32>, b3: bool) -> bool {
+    if b1 && let Some(b) = b2 { b3 } else { false }
+}
+
+fn if_let_complex_does_not_lint(b1: bool, b2: Option<i32>, b3: bool) -> bool {
+    if let Some(b) = b2
+        && b1
+    {
+        b3
+    } else {
+        false
+    }
+}
+
+fn else_if_does_lint(b1: bool, b2: bool, b3: bool) -> bool {
+    if b1 {
+        // There is some expansion here.
+        let _ = 40;
+        false
+    } else { b2 && b3 }
+    //~^^^^^ ifs_as_logical_ops
+}
+
+fn needless_bool_clash_does_not_lint(x: bool) -> bool {
+    if x { true } else { false }
+}
+
+macro_rules! always_return_true {
+    () => {
+        true
+    };
+}
+
+fn needless_bool_macro_clash_does_not_lint(b1: bool) -> bool {
+    if b1 { always_return_true!() } else { false }
+}
+
+macro_rules! nothing {
+    () => {};
+}
+
+fn empty_expansion_does_not_lint(b1: bool, b2: bool) -> bool {
+    if b1 {
+        nothing!();
+        b2
+    } else {
+        false
+    }
+}
+
+fn macro_with_comment_does_not_lint(b1: bool, num: i32) -> bool {
+    macro_rules! b2 {
+        () => {{ num > 33 }};
+    }
+    if b1 {
+        // watch out! Comment!
+        b2!()
+    } else {
+        false
+    }
+}
+
+fn macro_without_comment_does_lint(b1: bool, num: i32) -> bool {
+    macro_rules! b2 {
+        () => {{ num > 33 }};
+    }
+    b1 && b2!()
+    //~^ ifs_as_logical_ops
+}
+
+fn proc_macro_tests() {
+    proc_macros::external! {
+        fn would_lint_usually(b1: bool, num: i32) -> bool {
+            if b1 { num > 30 } else { false }
+        }
+    }
+    proc_macros::with_span! {
+        span
+        fn would_lint_usually_2(b1: bool, num: i32) -> bool {
+            if b1 { num > 30 } else { false }
+        }
+    }
+}