Skip to content

Commit ed984b9

Browse files
poliorceticspoliorcetics
committed
lints: constant_bool_expr: detect constant booleans expressions using && or ||
For now this only detects simple cases using literals.
1 parent a1d390a commit ed984b9

9 files changed

Lines changed: 190 additions & 14 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6825,6 +6825,7 @@ Released 2018-09-13
68256825
[`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
68266826
[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
68276827
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
6828+
[`constant_bool_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#constant_bool_expr
68286829
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
68296830
[`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
68306831
[`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
602602
crate::operators::ASSIGN_OP_PATTERN_INFO,
603603
crate::operators::BAD_BIT_MASK_INFO,
604604
crate::operators::CMP_OWNED_INFO,
605+
crate::operators::CONSTANT_BOOL_EXPR_INFO,
605606
crate::operators::DECIMAL_BITWISE_OPERANDS_INFO,
606607
crate::operators::DOUBLE_COMPARISONS_INFO,
607608
crate::operators::DURATION_SUBSEC_INFO,

‎clippy_lints/src/operators/constant_bool_expr.rs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::eq_expr_value;
3+
use rustc_hir::{BinOpKind, Expr, ExprKind};
4+
use rustc_lint::LateContext;
5+
6+
use super::CONSTANT_BOOL_EXPR;
7+
8+
pub(crate) fn check<'tcx>(
9+
cx: &LateContext<'tcx>,
10+
e: &'tcx Expr<'_>,
11+
op: BinOpKind,
12+
left: &'tcx Expr<'_>,
13+
right: &'tcx Expr<'_>,
14+
) {
15+
match op {
16+
BinOpKind::Or => detect_always_true_or(cx, e, left, right),
17+
BinOpKind::And => detect_always_false_and(cx, e, left, right),
18+
_ => (),
19+
}
20+
}
21+
22+
// `left_lhs != left_rhs || right_lhs != right_rhs`
23+
fn detect_always_true_or<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, left: &'tcx Expr<'_>, right: &'tcx Expr<'_>) {
24+
let ExprKind::Binary(left_op, left_lhs, left_rhs) = left.kind else {
25+
return;
26+
};
27+
let ExprKind::Binary(right_op, right_lhs, right_rhs) = right.kind else {
28+
return;
29+
};
30+
if left_op.node != BinOpKind::Ne || right_op.node != BinOpKind::Ne {
31+
return;
32+
}
33+
34+
if detect_variants(cx, e, left_lhs, right_lhs, left_rhs, right_rhs) {
35+
span_lint(cx, CONSTANT_BOOL_EXPR, e.span, "expression always evaluates to `true`");
36+
}
37+
}
38+
39+
// `left_lhs == left_rhs && right_lhs == right_rhs`
40+
fn detect_always_false_and<'tcx>(
41+
cx: &LateContext<'tcx>,
42+
e: &'tcx Expr<'_>,
43+
left: &'tcx Expr<'_>,
44+
right: &'tcx Expr<'_>,
45+
) {
46+
let ExprKind::Binary(left_op, left_lhs, left_rhs) = left.kind else {
47+
return;
48+
};
49+
let ExprKind::Binary(right_op, right_lhs, right_rhs) = right.kind else {
50+
return;
51+
};
52+
if left_op.node != BinOpKind::Eq || right_op.node != BinOpKind::Eq {
53+
return;
54+
}
55+
56+
if detect_variants(cx, e, left_lhs, right_lhs, left_rhs, right_rhs) {
57+
span_lint(cx, CONSTANT_BOOL_EXPR, e.span, "expression always evaluates to `false`");
58+
}
59+
}
60+
61+
fn detect_variants<'tcx>(
62+
cx: &LateContext<'tcx>,
63+
e: &'tcx Expr<'_>,
64+
left_lhs: &'tcx Expr<'_>,
65+
right_lhs: &'tcx Expr<'_>,
66+
left_rhs: &'tcx Expr<'_>,
67+
right_rhs: &'tcx Expr<'_>,
68+
) -> bool {
69+
let ctxt = e.span.ctxt();
70+
71+
let detect_variant = |left_a, right_a, left_literal: &Expr<'_>, right_literal: &Expr<'_>| {
72+
matches!(left_literal.kind, ExprKind::Lit(_))
73+
&& matches!(right_literal.kind, ExprKind::Lit(_))
74+
&& !eq_expr_value(cx, ctxt, left_literal, right_literal)
75+
&& eq_expr_value(cx, ctxt, left_a, right_a)
76+
};
77+
78+
// (a CMP_OP _) BIN_OP (a CMP_OP _)
79+
detect_variant(left_lhs, right_lhs, left_rhs, right_rhs)
80+
// (a CMP_OP _) BIN_OP (_ CMP_OP a)
81+
|| detect_variant(left_lhs, right_rhs, left_rhs, right_lhs)
82+
// (_ CMP_OP a) BIN_OP (a CMP_OP _)
83+
|| detect_variant(left_rhs, right_lhs, left_lhs, right_rhs)
84+
// (_ CMP_OP a) BIN_OP (_ CMP_OP a)
85+
|| detect_variant(left_rhs, right_rhs, left_lhs, right_lhs)
86+
}

‎clippy_lints/src/operators/mod.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod assign_op_pattern;
33
mod bit_mask;
44
mod cmp_owned;
55
mod const_comparisons;
6+
mod constant_bool_expr;
67
mod decimal_bitwise_operands;
78
mod double_comparison;
89
mod duration_subsec;
@@ -206,6 +207,33 @@ declare_clippy_lint! {
206207
"creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`"
207208
}
208209

210+
declare_clippy_lint! {
211+
/// ### What it does
212+
/// Checks for boolean expressions that end up constant, always `true` or `false`.
213+
///
214+
/// ### Why is this bad?
215+
/// This is most likely a logic bug.
216+
///
217+
/// ### Limitations
218+
/// This lint only checks against literals.
219+
///
220+
/// ### Example
221+
/// ```no_run
222+
/// let a = 99;
223+
/// if a != 100 || a != 101 { println!("Hello {a}"); } // Always true
224+
/// if a == 100 && a == 101 { println!("Hello {a}"); } // Always false
225+
/// ```
226+
/// Use instead:
227+
/// ```no_run
228+
/// let a = 99;
229+
/// println!("Hello {a}");
230+
/// ```
231+
#[clippy::version = "1.99.0"]
232+
pub CONSTANT_BOOL_EXPR,
233+
correctness,
234+
"checks for boolean expressions that end up constant"
235+
}
236+
209237
declare_clippy_lint! {
210238
/// ### What it does
211239
/// Checks for decimal literals used as bit masks in bitwise operations.
@@ -998,6 +1026,7 @@ impl_lint_pass!(Operators => [
9981026
ASSIGN_OP_PATTERN,
9991027
BAD_BIT_MASK,
10001028
CMP_OWNED,
1029+
CONSTANT_BOOL_EXPR,
10011030
DECIMAL_BITWISE_OPERANDS,
10021031
DOUBLE_COMPARISONS,
10031032
DURATION_SUBSEC,
@@ -1054,6 +1083,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
10541083
absurd_extreme_comparisons::check(cx, e, op.node, lhs, rhs);
10551084
if !(macro_with_not_op(lhs) || macro_with_not_op(rhs)) {
10561085
eq_op::check(cx, e, op.node, lhs, rhs);
1086+
constant_bool_expr::check(cx, e, op.node, lhs, rhs);
10571087
op_ref::check(cx, e, op.node, lhs, rhs);
10581088
}
10591089
erasing_op::check(cx, e, op.node, lhs, rhs);

‎tests/ui/collapsible_if.fixed‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#![allow(clippy::eq_op, clippy::needless_ifs)]
2-
#![expect(clippy::assertions_on_constants, clippy::redundant_pattern_matching)]
2+
#![expect(
3+
clippy::assertions_on_constants,
4+
clippy::constant_bool_expr,
5+
clippy::redundant_pattern_matching
6+
)]
37

48
#[rustfmt::skip]
59
#[warn(clippy::collapsible_if)]

‎tests/ui/collapsible_if.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#![allow(clippy::eq_op, clippy::needless_ifs)]
2-
#![expect(clippy::assertions_on_constants, clippy::redundant_pattern_matching)]
2+
#![expect(
3+
clippy::assertions_on_constants,
4+
clippy::constant_bool_expr,
5+
clippy::redundant_pattern_matching
6+
)]
37

48
#[rustfmt::skip]
59
#[warn(clippy::collapsible_if)]

‎tests/ui/collapsible_if.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` statement can be collapsed
2-
--> tests/ui/collapsible_if.rs:9:5
2+
--> tests/ui/collapsible_if.rs:13:5
33
|
44
LL | / if x == "hello" {
55
LL | | if y == "world" {
@@ -19,7 +19,7 @@ LL ~ }
1919
|
2020

2121
error: this `if` statement can be collapsed
22-
--> tests/ui/collapsible_if.rs:16:5
22+
--> tests/ui/collapsible_if.rs:20:5
2323
|
2424
LL | / if x == "hello" || x == "world" {
2525
LL | | if y == "world" || y == "hello" {
@@ -37,7 +37,7 @@ LL ~ }
3737
|
3838

3939
error: this `if` statement can be collapsed
40-
--> tests/ui/collapsible_if.rs:23:5
40+
--> tests/ui/collapsible_if.rs:27:5
4141
|
4242
LL | / if x == "hello" && x == "world" {
4343
LL | | if y == "world" || y == "hello" {
@@ -55,7 +55,7 @@ LL ~ }
5555
|
5656

5757
error: this `if` statement can be collapsed
58-
--> tests/ui/collapsible_if.rs:30:5
58+
--> tests/ui/collapsible_if.rs:34:5
5959
|
6060
LL | / if x == "hello" || x == "world" {
6161
LL | | if y == "world" && y == "hello" {
@@ -73,7 +73,7 @@ LL ~ }
7373
|
7474

7575
error: this `if` statement can be collapsed
76-
--> tests/ui/collapsible_if.rs:37:5
76+
--> tests/ui/collapsible_if.rs:41:5
7777
|
7878
LL | / if x == "hello" && x == "world" {
7979
LL | | if y == "world" && y == "hello" {
@@ -91,7 +91,7 @@ LL ~ }
9191
|
9292

9393
error: this `if` statement can be collapsed
94-
--> tests/ui/collapsible_if.rs:44:5
94+
--> tests/ui/collapsible_if.rs:48:5
9595
|
9696
LL | / if 42 == 1337 {
9797
LL | | if 'a' != 'A' {
@@ -109,7 +109,7 @@ LL ~ }
109109
|
110110

111111
error: this `if` statement can be collapsed
112-
--> tests/ui/collapsible_if.rs:80:5
112+
--> tests/ui/collapsible_if.rs:84:5
113113
|
114114
LL | / if x == "hello" {
115115
LL | | if y == "world" { // Collapsible
@@ -127,7 +127,7 @@ LL ~ }
127127
|
128128

129129
error: this `if` statement can be collapsed
130-
--> tests/ui/collapsible_if.rs:108:5
130+
--> tests/ui/collapsible_if.rs:112:5
131131
|
132132
LL | / if matches!(true, true) {
133133
LL | | if matches!(true, true) {}
@@ -141,7 +141,7 @@ LL ~ && matches!(true, true) {}
141141
|
142142

143143
error: this `if` statement can be collapsed
144-
--> tests/ui/collapsible_if.rs:114:5
144+
--> tests/ui/collapsible_if.rs:118:5
145145
|
146146
LL | / if matches!(true, true) && truth() {
147147
LL | | if matches!(true, true) {}
@@ -155,7 +155,7 @@ LL ~ && matches!(true, true) {}
155155
|
156156

157157
error: this `if` statement can be collapsed
158-
--> tests/ui/collapsible_if.rs:126:5
158+
--> tests/ui/collapsible_if.rs:130:5
159159
|
160160
LL | / if true {
161161
LL | | if true {
@@ -173,7 +173,7 @@ LL ~ }
173173
|
174174

175175
error: this `if` statement can be collapsed
176-
--> tests/ui/collapsible_if.rs:143:5
176+
--> tests/ui/collapsible_if.rs:147:5
177177
|
178178
LL | / if true {
179179
LL | | if true {
@@ -191,7 +191,7 @@ LL ~ ; 3
191191
|
192192

193193
error: this `if` statement can be collapsed
194-
--> tests/ui/collapsible_if.rs:190:5
194+
--> tests/ui/collapsible_if.rs:194:5
195195
|
196196
LL | / if true {
197197
LL | | (if true {

‎tests/ui/constant_bool_expr.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![warn(clippy::constant_bool_expr)]
2+
#![expect(clippy::needless_ifs)]
3+
4+
fn main() {
5+
let a = 99;
6+
7+
if a != 100 || a != 101 {}
8+
//~^ constant_bool_expr
9+
if a == 100 && a == 101 {}
10+
//~^ constant_bool_expr
11+
12+
let _b = a != 100 || a != 101;
13+
//~^ constant_bool_expr
14+
let _b = a == 100 && a == 101;
15+
//~^ constant_bool_expr
16+
17+
const A: i32 = 100;
18+
19+
let _b = a != A || a != 101;
20+
let _b = a == A && a == 101;
21+
}

‎tests/ui/constant_bool_expr.stderr‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
error: expression always evaluates to `true`
2+
--> tests/ui/constant_bool_expr.rs:7:8
3+
|
4+
LL | if a != 100 || a != 101 {}
5+
| ^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::constant-bool-expr` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::constant_bool_expr)]`
9+
10+
error: expression always evaluates to `false`
11+
--> tests/ui/constant_bool_expr.rs:9:8
12+
|
13+
LL | if a == 100 && a == 101 {}
14+
| ^^^^^^^^^^^^^^^^^^^^
15+
16+
error: expression always evaluates to `true`
17+
--> tests/ui/constant_bool_expr.rs:12:14
18+
|
19+
LL | let _b = a != 100 || a != 101;
20+
| ^^^^^^^^^^^^^^^^^^^^
21+
22+
error: expression always evaluates to `false`
23+
--> tests/ui/constant_bool_expr.rs:14:14
24+
|
25+
LL | let _b = a == 100 && a == 101;
26+
| ^^^^^^^^^^^^^^^^^^^^
27+
28+
error: aborting due to 4 previous errors
29+

0 commit comments

Comments
 (0)