Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
103 changes: 103 additions & 0 deletions clippy_lints/src/operators/constant_bool_expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
use clippy_utils::consts::{ConstEvalCtxt, Constant};
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_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 _)
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)
}
31 changes: 31 additions & 0 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -206,6 +207,34 @@ 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 and constant expressions since it cannot determine
/// if `a == bar() && a == foo()` will actually be different or not at runtime.
///
/// ### 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.
Expand Down Expand Up @@ -998,6 +1027,7 @@ impl_lint_pass!(Operators => [
ASSIGN_OP_PATTERN,
BAD_BIT_MASK,
CMP_OWNED,
CONSTANT_BOOL_EXPR,
DECIMAL_BITWISE_OPERANDS,
DOUBLE_COMPARISONS,
DURATION_SUBSEC,
Expand Down Expand Up @@ -1054,6 +1084,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);
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/collapsible_if.fixed
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/collapsible_if.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/collapsible_if.stderr
Original file line number Diff line number Diff line change
@@ -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" {
Expand All @@ -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" {
Expand All @@ -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" {
Expand All @@ -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" {
Expand All @@ -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" {
Expand All @@ -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' {
Expand All @@ -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
Expand All @@ -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) {}
Expand All @@ -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) {}
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
41 changes: 41 additions & 0 deletions tests/ui/constant_bool_expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#![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;
//~^ 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
Loading