Skip to content

Commit f0563ad

Browse files
ada4aada4a
committed
extract bool_comparison to a separate file
1 parent 3a3d3e4 commit f0563ad

File tree

4 files changed

+185
-167
lines changed

4 files changed

+185
-167
lines changed

‎clippy_lints/src/bool_comparison.rs

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::sugg::Sugg;
3+
use clippy_utils::{is_expn_of, peel_blocks, sym};
4+
use rustc_ast::ast::LitKind;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{BinOpKind, Expr, ExprKind};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::declare_lint_pass;
9+
use rustc_span::Span;
10+
use rustc_span::source_map::Spanned;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for expressions of the form `x == true`,
15+
/// `x != true` and order comparisons such as `x < true` (or vice versa) and
16+
/// suggest using the variable directly.
17+
///
18+
/// ### Why is this bad?
19+
/// Unnecessary code.
20+
///
21+
/// ### Example
22+
/// ```rust,ignore
23+
/// if x == true {}
24+
/// if y == false {}
25+
/// ```
26+
/// use `x` directly:
27+
/// ```rust,ignore
28+
/// if x {}
29+
/// if !y {}
30+
/// ```
31+
#[clippy::version = "pre 1.29.0"]
32+
pub BOOL_COMPARISON,
33+
complexity,
34+
"comparing a variable to a boolean, e.g., `if x == true` or `if x != true`"
35+
}
36+
37+
declare_lint_pass!(BoolComparison => [BOOL_COMPARISON]);
38+
39+
impl<'tcx> LateLintPass<'tcx> for BoolComparison {
40+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
41+
if e.span.from_expansion() {
42+
return;
43+
}
44+
45+
if let ExprKind::Binary(Spanned { node, .. }, left_side, right_side) = e.kind
46+
&& is_expn_of(left_side.span, sym::cfg).is_none()
47+
&& is_expn_of(right_side.span, sym::cfg).is_none()
48+
&& cx.typeck_results().expr_ty(left_side).is_bool()
49+
&& cx.typeck_results().expr_ty(right_side).is_bool()
50+
{
51+
let ignore_case = None::<(fn(_) -> _, &str)>;
52+
let ignore_no_literal = None::<(fn(_, _) -> _, &str)>;
53+
match node {
54+
BinOpKind::Eq => {
55+
let true_case = Some((|h| h, "equality checks against true are unnecessary"));
56+
let false_case = Some((
57+
|h: Sugg<'tcx>| !h,
58+
"equality checks against false can be replaced by a negation",
59+
));
60+
check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
61+
},
62+
BinOpKind::Ne => {
63+
let true_case = Some((
64+
|h: Sugg<'tcx>| !h,
65+
"inequality checks against true can be replaced by a negation",
66+
));
67+
let false_case = Some((|h| h, "inequality checks against false are unnecessary"));
68+
check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
69+
},
70+
BinOpKind::Lt => check_comparison(
71+
cx,
72+
e,
73+
ignore_case,
74+
Some((|h| h, "greater than checks against false are unnecessary")),
75+
Some((
76+
|h: Sugg<'tcx>| !h,
77+
"less than comparison against true can be replaced by a negation",
78+
)),
79+
ignore_case,
80+
Some((
81+
|l: Sugg<'tcx>, r: Sugg<'tcx>| (!l).bit_and(&r),
82+
"order comparisons between booleans can be simplified",
83+
)),
84+
),
85+
BinOpKind::Gt => check_comparison(
86+
cx,
87+
e,
88+
Some((
89+
|h: Sugg<'tcx>| !h,
90+
"less than comparison against true can be replaced by a negation",
91+
)),
92+
ignore_case,
93+
ignore_case,
94+
Some((|h| h, "greater than checks against false are unnecessary")),
95+
Some((
96+
|l: Sugg<'tcx>, r: Sugg<'tcx>| l.bit_and(&(!r)),
97+
"order comparisons between booleans can be simplified",
98+
)),
99+
),
100+
_ => (),
101+
}
102+
}
103+
}
104+
}
105+
106+
fn check_comparison<'a, 'tcx>(
107+
cx: &LateContext<'tcx>,
108+
e: &'tcx Expr<'_>,
109+
left_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
110+
left_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
111+
right_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
112+
right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
113+
no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &'static str)>,
114+
) {
115+
if let ExprKind::Binary(_, left_side, right_side) = e.kind {
116+
let mut applicability = Applicability::MachineApplicable;
117+
// Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs,
118+
// calling `source_callsite` make sure macros are handled correctly, see issue #9907
119+
let binop_span = left_side.span.source_callsite().to(right_side.span.source_callsite());
120+
121+
match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
122+
(Some(true), None) => left_true.map_or((), |(h, m)| {
123+
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
124+
}),
125+
(None, Some(true)) => right_true.map_or((), |(h, m)| {
126+
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
127+
}),
128+
(Some(false), None) => left_false.map_or((), |(h, m)| {
129+
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
130+
}),
131+
(None, Some(false)) => right_false.map_or((), |(h, m)| {
132+
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
133+
}),
134+
(None, None) => no_literal.map_or((), |(h, m)| {
135+
let left_side = Sugg::hir_with_context(cx, left_side, binop_span.ctxt(), "..", &mut applicability);
136+
let right_side = Sugg::hir_with_context(cx, right_side, binop_span.ctxt(), "..", &mut applicability);
137+
span_lint_and_sugg(
138+
cx,
139+
BOOL_COMPARISON,
140+
binop_span,
141+
m,
142+
"try",
143+
h(left_side, right_side).into_string(),
144+
applicability,
145+
);
146+
}),
147+
_ => (),
148+
}
149+
}
150+
}
151+
152+
fn suggest_bool_comparison<'a, 'tcx>(
153+
cx: &LateContext<'tcx>,
154+
span: Span,
155+
expr: &Expr<'_>,
156+
mut app: Applicability,
157+
message: &'static str,
158+
conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
159+
) {
160+
let hint = Sugg::hir_with_context(cx, expr, span.ctxt(), "..", &mut app);
161+
span_lint_and_sugg(
162+
cx,
163+
BOOL_COMPARISON,
164+
span,
165+
message,
166+
"try",
167+
conv_hint(hint).into_string(),
168+
app,
169+
);
170+
}
171+
172+
fn fetch_bool_expr(expr: &Expr<'_>) -> Option<bool> {
173+
if let ExprKind::Lit(lit_ptr) = peel_blocks(expr).kind
174+
&& let LitKind::Bool(value) = lit_ptr.node
175+
{
176+
return Some(value);
177+
}
178+
None
179+
}

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
3535
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
3636
crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO,
3737
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
38+
crate::bool_comparison::BOOL_COMPARISON_INFO,
3839
crate::bool_to_int_with_if::BOOL_TO_INT_WITH_IF_INFO,
3940
crate::booleans::NONMINIMAL_BOOL_INFO,
4041
crate::booleans::OVERLY_COMPLEX_BOOL_EXPR_INFO,
@@ -538,7 +539,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
538539
crate::mutex_atomic::MUTEX_ATOMIC_INFO,
539540
crate::mutex_atomic::MUTEX_INTEGER_INFO,
540541
crate::needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE_INFO,
541-
crate::needless_bool::BOOL_COMPARISON_INFO,
542542
crate::needless_bool::NEEDLESS_BOOL_INFO,
543543
crate::needless_bool::NEEDLESS_BOOL_ASSIGN_INFO,
544544
crate::needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE_INFO,

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ mod attrs;
8484
mod await_holding_invalid;
8585
mod blocks_in_conditions;
8686
mod bool_assert_comparison;
87+
mod bool_comparison;
8788
mod bool_to_int_with_if;
8889
mod booleans;
8990
mod borrow_deref_ref;
@@ -477,7 +478,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
477478
store.register_late_pass(|_| Box::new(float_literal::FloatLiteral));
478479
store.register_late_pass(|_| Box::new(ptr::Ptr));
479480
store.register_late_pass(|_| Box::new(needless_bool::NeedlessBool));
480-
store.register_late_pass(|_| Box::new(needless_bool::BoolComparison));
481+
store.register_late_pass(|_| Box::new(bool_comparison::BoolComparison));
481482
store.register_late_pass(|_| Box::new(needless_for_each::NeedlessForEach));
482483
store.register_late_pass(|_| Box::new(misc::LintPass));
483484
store.register_late_pass(|_| Box::new(eta_reduction::EtaReduction));

0 commit comments

Comments
 (0)