From ed984b96067feb47981df58b2cb1e1e6f0e32677 Mon Sep 17 00:00:00 2001 From: "Alexis (Poliorcetics) Bourget" Date: Mon, 13 Jul 2026 09:48:34 +0200 Subject: [PATCH 1/2] lints: constant_bool_expr: detect constant booleans expressions using `&&` or `||` For now this only detects simple cases using literals. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/operators/constant_bool_expr.rs | 86 +++++++++++++++++++ clippy_lints/src/operators/mod.rs | 30 +++++++ tests/ui/collapsible_if.fixed | 6 +- tests/ui/collapsible_if.rs | 6 +- tests/ui/collapsible_if.stderr | 24 +++--- tests/ui/constant_bool_expr.rs | 21 +++++ tests/ui/constant_bool_expr.stderr | 29 +++++++ 9 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 clippy_lints/src/operators/constant_bool_expr.rs create mode 100644 tests/ui/constant_bool_expr.rs create mode 100644 tests/ui/constant_bool_expr.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 856c229f4679..8c091b97807a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6825,6 +6825,7 @@ Released 2018-09-13 [`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime +[`constant_bool_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#constant_bool_expr [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index e77a0ff4a8ab..e34f69af925e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -602,6 +602,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::operators::ASSIGN_OP_PATTERN_INFO, crate::operators::BAD_BIT_MASK_INFO, crate::operators::CMP_OWNED_INFO, + crate::operators::CONSTANT_BOOL_EXPR_INFO, crate::operators::DECIMAL_BITWISE_OPERANDS_INFO, crate::operators::DOUBLE_COMPARISONS_INFO, crate::operators::DURATION_SUBSEC_INFO, diff --git a/clippy_lints/src/operators/constant_bool_expr.rs b/clippy_lints/src/operators/constant_bool_expr.rs new file mode 100644 index 000000000000..b608c00437f7 --- /dev/null +++ b/clippy_lints/src/operators/constant_bool_expr.rs @@ -0,0 +1,86 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::eq_expr_value; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::LateContext; + +use super::CONSTANT_BOOL_EXPR; + +pub(crate) fn check<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + op: BinOpKind, + left: &'tcx Expr<'_>, + right: &'tcx Expr<'_>, +) { + match op { + BinOpKind::Or => detect_always_true_or(cx, e, left, right), + BinOpKind::And => detect_always_false_and(cx, e, left, right), + _ => (), + } +} + +// `left_lhs != left_rhs || right_lhs != right_rhs` +fn detect_always_true_or<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, left: &'tcx Expr<'_>, right: &'tcx Expr<'_>) { + let ExprKind::Binary(left_op, left_lhs, left_rhs) = left.kind else { + return; + }; + let ExprKind::Binary(right_op, right_lhs, right_rhs) = right.kind else { + return; + }; + if left_op.node != BinOpKind::Ne || right_op.node != BinOpKind::Ne { + return; + } + + if detect_variants(cx, e, left_lhs, right_lhs, left_rhs, right_rhs) { + span_lint(cx, CONSTANT_BOOL_EXPR, e.span, "expression always evaluates to `true`"); + } +} + +// `left_lhs == left_rhs && right_lhs == right_rhs` +fn detect_always_false_and<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + left: &'tcx Expr<'_>, + right: &'tcx Expr<'_>, +) { + let ExprKind::Binary(left_op, left_lhs, left_rhs) = left.kind else { + return; + }; + let ExprKind::Binary(right_op, right_lhs, right_rhs) = right.kind else { + return; + }; + if left_op.node != BinOpKind::Eq || right_op.node != BinOpKind::Eq { + return; + } + + if detect_variants(cx, e, left_lhs, right_lhs, left_rhs, right_rhs) { + span_lint(cx, CONSTANT_BOOL_EXPR, e.span, "expression always evaluates to `false`"); + } +} + +fn detect_variants<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + left_lhs: &'tcx Expr<'_>, + right_lhs: &'tcx Expr<'_>, + left_rhs: &'tcx Expr<'_>, + right_rhs: &'tcx Expr<'_>, +) -> bool { + let ctxt = e.span.ctxt(); + + let detect_variant = |left_a, right_a, left_literal: &Expr<'_>, right_literal: &Expr<'_>| { + matches!(left_literal.kind, ExprKind::Lit(_)) + && matches!(right_literal.kind, ExprKind::Lit(_)) + && !eq_expr_value(cx, ctxt, left_literal, right_literal) + && eq_expr_value(cx, ctxt, left_a, right_a) + }; + + // (a CMP_OP _) BIN_OP (a CMP_OP _) + detect_variant(left_lhs, right_lhs, left_rhs, right_rhs) + // (a CMP_OP _) BIN_OP (_ CMP_OP a) + || detect_variant(left_lhs, right_rhs, left_rhs, right_lhs) + // (_ CMP_OP a) BIN_OP (a CMP_OP _) + || detect_variant(left_rhs, right_lhs, left_lhs, right_rhs) + // (_ CMP_OP a) BIN_OP (_ CMP_OP a) + || detect_variant(left_rhs, right_rhs, left_lhs, right_lhs) +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 120716edc203..8179e334252a 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -3,6 +3,7 @@ mod assign_op_pattern; mod bit_mask; mod cmp_owned; mod const_comparisons; +mod constant_bool_expr; mod decimal_bitwise_operands; mod double_comparison; mod duration_subsec; @@ -206,6 +207,33 @@ declare_clippy_lint! { "creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for boolean expressions that end up constant, always `true` or `false`. + /// + /// ### Why is this bad? + /// This is most likely a logic bug. + /// + /// ### Limitations + /// This lint only checks against literals. + /// + /// ### Example + /// ```no_run + /// let a = 99; + /// if a != 100 || a != 101 { println!("Hello {a}"); } // Always true + /// if a == 100 && a == 101 { println!("Hello {a}"); } // Always false + /// ``` + /// Use instead: + /// ```no_run + /// let a = 99; + /// println!("Hello {a}"); + /// ``` + #[clippy::version = "1.99.0"] + pub CONSTANT_BOOL_EXPR, + correctness, + "checks for boolean expressions that end up constant" +} + declare_clippy_lint! { /// ### What it does /// Checks for decimal literals used as bit masks in bitwise operations. @@ -998,6 +1026,7 @@ impl_lint_pass!(Operators => [ ASSIGN_OP_PATTERN, BAD_BIT_MASK, CMP_OWNED, + CONSTANT_BOOL_EXPR, DECIMAL_BITWISE_OPERANDS, DOUBLE_COMPARISONS, DURATION_SUBSEC, @@ -1054,6 +1083,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators { absurd_extreme_comparisons::check(cx, e, op.node, lhs, rhs); if !(macro_with_not_op(lhs) || macro_with_not_op(rhs)) { eq_op::check(cx, e, op.node, lhs, rhs); + constant_bool_expr::check(cx, e, op.node, lhs, rhs); op_ref::check(cx, e, op.node, lhs, rhs); } erasing_op::check(cx, e, op.node, lhs, rhs); diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index 0a52a67f4085..b0f8f05d002e 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -1,5 +1,9 @@ #![allow(clippy::eq_op, clippy::needless_ifs)] -#![expect(clippy::assertions_on_constants, clippy::redundant_pattern_matching)] +#![expect( + clippy::assertions_on_constants, + clippy::constant_bool_expr, + clippy::redundant_pattern_matching +)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index 6cb15008898f..19dc49bdf988 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -1,5 +1,9 @@ #![allow(clippy::eq_op, clippy::needless_ifs)] -#![expect(clippy::assertions_on_constants, clippy::redundant_pattern_matching)] +#![expect( + clippy::assertions_on_constants, + clippy::constant_bool_expr, + clippy::redundant_pattern_matching +)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index 78fc5e108668..14b99c30708e 100644 --- a/tests/ui/collapsible_if.stderr +++ b/tests/ui/collapsible_if.stderr @@ -1,5 +1,5 @@ error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:9:5 + --> tests/ui/collapsible_if.rs:13:5 | LL | / if x == "hello" { LL | | if y == "world" { @@ -19,7 +19,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:16:5 + --> tests/ui/collapsible_if.rs:20:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" || y == "hello" { @@ -37,7 +37,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:23:5 + --> tests/ui/collapsible_if.rs:27:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" || y == "hello" { @@ -55,7 +55,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:30:5 + --> tests/ui/collapsible_if.rs:34:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" && y == "hello" { @@ -73,7 +73,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:37:5 + --> tests/ui/collapsible_if.rs:41:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" && y == "hello" { @@ -91,7 +91,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:44:5 + --> tests/ui/collapsible_if.rs:48:5 | LL | / if 42 == 1337 { LL | | if 'a' != 'A' { @@ -109,7 +109,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:80:5 + --> tests/ui/collapsible_if.rs:84:5 | LL | / if x == "hello" { LL | | if y == "world" { // Collapsible @@ -127,7 +127,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:108:5 + --> tests/ui/collapsible_if.rs:112:5 | LL | / if matches!(true, true) { LL | | if matches!(true, true) {} @@ -141,7 +141,7 @@ LL ~ && matches!(true, true) {} | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:114:5 + --> tests/ui/collapsible_if.rs:118:5 | LL | / if matches!(true, true) && truth() { LL | | if matches!(true, true) {} @@ -155,7 +155,7 @@ LL ~ && matches!(true, true) {} | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:126:5 + --> tests/ui/collapsible_if.rs:130:5 | LL | / if true { LL | | if true { @@ -173,7 +173,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:143:5 + --> tests/ui/collapsible_if.rs:147:5 | LL | / if true { LL | | if true { @@ -191,7 +191,7 @@ LL ~ ; 3 | error: this `if` statement can be collapsed - --> tests/ui/collapsible_if.rs:190:5 + --> tests/ui/collapsible_if.rs:194:5 | LL | / if true { LL | | (if true { diff --git a/tests/ui/constant_bool_expr.rs b/tests/ui/constant_bool_expr.rs new file mode 100644 index 000000000000..c73df3b30916 --- /dev/null +++ b/tests/ui/constant_bool_expr.rs @@ -0,0 +1,21 @@ +#![warn(clippy::constant_bool_expr)] +#![expect(clippy::needless_ifs)] + +fn main() { + let a = 99; + + if a != 100 || a != 101 {} + //~^ constant_bool_expr + if a == 100 && a == 101 {} + //~^ constant_bool_expr + + let _b = a != 100 || a != 101; + //~^ constant_bool_expr + let _b = a == 100 && a == 101; + //~^ constant_bool_expr + + const A: i32 = 100; + + let _b = a != A || a != 101; + let _b = a == A && a == 101; +} diff --git a/tests/ui/constant_bool_expr.stderr b/tests/ui/constant_bool_expr.stderr new file mode 100644 index 000000000000..c7846347b021 --- /dev/null +++ b/tests/ui/constant_bool_expr.stderr @@ -0,0 +1,29 @@ +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:7:8 + | +LL | if a != 100 || a != 101 {} + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::constant-bool-expr` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::constant_bool_expr)]` + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:9:8 + | +LL | if a == 100 && a == 101 {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:12:14 + | +LL | let _b = a != 100 || a != 101; + | ^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:14:14 + | +LL | let _b = a == 100 && a == 101; + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + From 77a7856883338e7b0b6f802c39844955639e5521 Mon Sep 17 00:00:00 2001 From: "Alexis (Poliorcetics) Bourget" Date: Mon, 13 Jul 2026 14:49:34 +0200 Subject: [PATCH 2/2] lints: constant_bool_expr: add support for rudimentary const eval --- .../src/operators/constant_bool_expr.rs | 27 ++++++++-- clippy_lints/src/operators/mod.rs | 3 +- tests/ui/constant_bool_expr.rs | 20 ++++++++ tests/ui/constant_bool_expr.stderr | 50 ++++++++++++++++++- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/operators/constant_bool_expr.rs b/clippy_lints/src/operators/constant_bool_expr.rs index b608c00437f7..a7485aae6f19 100644 --- a/clippy_lints/src/operators/constant_bool_expr.rs +++ b/clippy_lints/src/operators/constant_bool_expr.rs @@ -1,3 +1,4 @@ +use clippy_utils::consts::{ConstEvalCtxt, Constant}; use clippy_utils::diagnostics::span_lint; use clippy_utils::eq_expr_value; use rustc_hir::{BinOpKind, Expr, ExprKind}; @@ -68,11 +69,27 @@ fn detect_variants<'tcx>( ) -> bool { let ctxt = e.span.ctxt(); - let detect_variant = |left_a, right_a, left_literal: &Expr<'_>, right_literal: &Expr<'_>| { - matches!(left_literal.kind, ExprKind::Lit(_)) - && matches!(right_literal.kind, ExprKind::Lit(_)) - && !eq_expr_value(cx, ctxt, left_literal, right_literal) - && eq_expr_value(cx, ctxt, left_a, right_a) + let detect_variant = |left_a, right_a, left_cmp: &Expr<'_>, right_cmp: &Expr<'_>| { + // If the `a` expression is not the same on both sides, there is no need to go further + if !eq_expr_value(cx, ctxt, left_a, right_a) { + return false; + } + + let const_ctx = ConstEvalCtxt::new(cx); + + // We try to look into constants and const blocks, but if that fails we fall back to handling + // literals only + if let Some(left_evaluated) = const_ctx.eval(left_cmp) + && let Some(right_evaluated) = const_ctx.eval(right_cmp) + && left_evaluated != Constant::Err + && right_evaluated != Constant::Err + { + left_evaluated != right_evaluated + } else { + matches!(left_cmp.kind, ExprKind::Lit(_)) + && matches!(right_cmp.kind, ExprKind::Lit(_)) + && !eq_expr_value(cx, ctxt, left_cmp, right_cmp) + } }; // (a CMP_OP _) BIN_OP (a CMP_OP _) diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 8179e334252a..1e6d2d44a87e 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -215,7 +215,8 @@ declare_clippy_lint! { /// This is most likely a logic bug. /// /// ### Limitations - /// This lint only checks against literals. + /// This lint only checks against literals and constant expressions since it cannot determine + /// if `a == bar() && a == foo()` will actually be different or not at runtime. /// /// ### Example /// ```no_run diff --git a/tests/ui/constant_bool_expr.rs b/tests/ui/constant_bool_expr.rs index c73df3b30916..f82cbc4d780e 100644 --- a/tests/ui/constant_bool_expr.rs +++ b/tests/ui/constant_bool_expr.rs @@ -17,5 +17,25 @@ fn main() { const A: i32 = 100; let _b = a != A || a != 101; + //~^ constant_bool_expr let _b = a == A && a == 101; + //~^ constant_bool_expr + + let _b = a != A || a != const { 100 + 1 }; + //~^ constant_bool_expr + let _b = a == A && a == const { 100 + 1 }; + //~^ constant_bool_expr } + +const B: i32 = 100; +const C: i32 = 99; + +const _: bool = C != B || C != const { 100 + 1 }; +//~^ constant_bool_expr +const _: bool = C == B && C == const { 100 + 1 }; +//~^ constant_bool_expr + +static D: bool = C != B || C != const { 100 + 1 }; +//~^ constant_bool_expr +static E: bool = C == B && C == const { 100 + 1 }; +//~^ constant_bool_expr diff --git a/tests/ui/constant_bool_expr.stderr b/tests/ui/constant_bool_expr.stderr index c7846347b021..ff7d7a0769f0 100644 --- a/tests/ui/constant_bool_expr.stderr +++ b/tests/ui/constant_bool_expr.stderr @@ -25,5 +25,53 @@ error: expression always evaluates to `false` LL | let _b = a == 100 && a == 101; | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:19:14 + | +LL | let _b = a != A || a != 101; + | ^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:21:14 + | +LL | let _b = a == A && a == 101; + | ^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:24:14 + | +LL | let _b = a != A || a != const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:26:14 + | +LL | let _b = a == A && a == const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:33:17 + | +LL | const _: bool = C != B || C != const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:35:17 + | +LL | const _: bool = C == B && C == const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `true` + --> tests/ui/constant_bool_expr.rs:38:18 + | +LL | static D: bool = C != B || C != const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expression always evaluates to `false` + --> tests/ui/constant_bool_expr.rs:40:18 + | +LL | static E: bool = C == B && C == const { 100 + 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors