Skip to content

Commit 32b35e7

Browse files
mira-eandafranciscoBSalgueiro
mira-eanda
and
franciscoBSalgueiro
committed
Implement manual_and and manual_or lints
Suggests replacing if-else expressions where one branch is a boolean literal with && / || operators. Co-authored-by: Francisco Salgueiro <[email protected]>
1 parent 0dc265f commit 32b35e7

29 files changed

+473
-146
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5509,6 +5509,7 @@ Released 2018-09-13
55095509
[`macro_metavars_in_unsafe`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe
55105510
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
55115511
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
5512+
[`manual_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_and
55125513
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
55135514
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
55145515
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
@@ -5532,6 +5533,7 @@ Released 2018-09-13
55325533
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
55335534
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
55345535
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
5536+
[`manual_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_or
55355537
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
55365538
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
55375539
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns

‎clippy_lints/src/declared_lints.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
298298
crate::macro_metavars_in_unsafe::MACRO_METAVARS_IN_UNSAFE_INFO,
299299
crate::macro_use::MACRO_USE_IMPORTS_INFO,
300300
crate::main_recursion::MAIN_RECURSION_INFO,
301+
crate::manual_and_or::MANUAL_AND_INFO,
302+
crate::manual_and_or::MANUAL_OR_INFO,
301303
crate::manual_assert::MANUAL_ASSERT_INFO,
302304
crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
303305
crate::manual_bits::MANUAL_BITS_INFO,

‎clippy_lints/src/escape.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,6 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
183183
impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {
184184
fn is_large_box(&self, ty: Ty<'tcx>) -> bool {
185185
// Large types need to be boxed to avoid stack overflows.
186-
if ty.is_box() {
187-
self.cx.layout_of(ty.boxed_ty()).map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
188-
} else {
189-
false
190-
}
186+
ty.is_box() && self.cx.layout_of(ty.boxed_ty()).map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
191187
}
192188
}

‎clippy_lints/src/functions/must_use.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,8 @@ fn is_mutable_pat(cx: &LateContext<'_>, pat: &hir::Pat<'_>, tys: &mut DefIdSet)
185185
if let hir::PatKind::Wild = pat.kind {
186186
return false; // ignore `_` patterns
187187
}
188-
if cx.tcx.has_typeck_results(pat.hir_id.owner.def_id) {
189-
is_mutable_ty(cx, cx.tcx.typeck(pat.hir_id.owner.def_id).pat_ty(pat), tys)
190-
} else {
191-
false
192-
}
188+
cx.tcx.has_typeck_results(pat.hir_id.owner.def_id)
189+
&& is_mutable_ty(cx, cx.tcx.typeck(pat.hir_id.owner.def_id).pat_ty(pat), tys)
193190
}
194191

195192
static KNOWN_WRAPPER_TYS: &[Symbol] = &[sym::Rc, sym::Arc];

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ mod loops;
197197
mod macro_metavars_in_unsafe;
198198
mod macro_use;
199199
mod main_recursion;
200+
mod manual_and_or;
200201
mod manual_assert;
201202
mod manual_async_fn;
202203
mod manual_bits;
@@ -1169,6 +1170,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11691170
})
11701171
});
11711172
store.register_late_pass(|_| Box::new(string_patterns::StringPatterns));
1173+
store.register_late_pass(|_| Box::new(manual_and_or::ManualAndOr));
11721174
// add lints here, do not remove this comment, it's used in `new_lint`
11731175
}
11741176

‎clippy_lints/src/literal_representation.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,7 @@ impl LiteralDigitGrouping {
374374
}
375375

376376
let group_sizes: Vec<usize> = num_lit.integer.split('_').map(str::len).collect();
377-
if UUID_GROUP_LENS.len() == group_sizes.len() {
378-
iter::zip(&UUID_GROUP_LENS, &group_sizes).all(|(&a, &b)| a == b)
379-
} else {
380-
false
381-
}
377+
UUID_GROUP_LENS.len() == group_sizes.len() && iter::zip(&UUID_GROUP_LENS, &group_sizes).all(|(&a, &b)| a == b)
382378
}
383379

384380
/// Given the sizes of the digit groups of both integral and fractional

‎clippy_lints/src/manual_and_or.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::visitors::for_each_expr;
3+
use clippy_utils::{get_parent_expr, higher, peel_blocks};
4+
use core::ops::ControlFlow;
5+
use rustc_ast::ast::LitKind;
6+
use rustc_ast::BinOpKind;
7+
use rustc_errors::Applicability;
8+
use rustc_hir::{Expr, ExprKind};
9+
use rustc_lint::{LateContext, LateLintPass, LintContext};
10+
use rustc_session::declare_lint_pass;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Detects `if`-then-`else` that can be replaced with `&&`.
15+
///
16+
/// ### Why is this bad?
17+
/// `&&` is simpler than `if`-then-`else`.
18+
///
19+
/// ### Example
20+
/// ```ignore
21+
/// if a {
22+
/// b
23+
/// } else {
24+
/// false
25+
/// }
26+
/// ```
27+
/// Use instead:
28+
/// ```ignore
29+
/// a && b
30+
/// ```
31+
#[clippy::version = "1.80.0"]
32+
pub MANUAL_AND,
33+
complexity,
34+
"this `if`-then-`else` that can be replaced with `&&`."
35+
}
36+
37+
declare_clippy_lint! {
38+
/// ### What it does
39+
/// Detects `if`-then-`else` that can be replaced with `||`.
40+
///
41+
/// ### Why is this bad?
42+
/// `||` is simpler than `if`-then-`else`.
43+
///
44+
/// ### Example
45+
/// ```ignore
46+
/// if a {
47+
/// true
48+
/// } else {
49+
/// b
50+
/// }
51+
/// ```
52+
/// Use instead:
53+
/// ```ignore
54+
/// a || b
55+
/// ```
56+
#[clippy::version = "1.80.0"]
57+
pub MANUAL_OR,
58+
complexity,
59+
"this `if`-then-`else` expression can be simplified with `||`"
60+
}
61+
62+
declare_lint_pass!(ManualAndOr => [MANUAL_AND, MANUAL_OR]);
63+
64+
fn extract_final_expression_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<String> {
65+
if let ExprKind::Block(block, _) = expr.kind {
66+
if let Some(final_expr) = block.expr {
67+
return cx.sess().source_map().span_to_snippet(final_expr.span).ok();
68+
}
69+
}
70+
cx.sess().source_map().span_to_snippet(expr.span).ok()
71+
}
72+
73+
fn fetch_bool_expr(expr: &Expr<'_>) -> Option<bool> {
74+
if let ExprKind::Lit(lit_ptr) = peel_blocks(expr).kind {
75+
if let LitKind::Bool(value) = lit_ptr.node {
76+
return Some(value);
77+
}
78+
}
79+
None
80+
}
81+
82+
fn contains_or(cond: &Expr<'_>) -> bool {
83+
for_each_expr(cond, |e| {
84+
if let ExprKind::Binary(ref n, _, _) = e.kind {
85+
if n.node == BinOpKind::Or {
86+
ControlFlow::Break(())
87+
} else {
88+
ControlFlow::Continue(())
89+
}
90+
} else {
91+
ControlFlow::Continue(())
92+
}
93+
})
94+
.is_some()
95+
}
96+
97+
fn check_and<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, cond: &Expr<'tcx>, then: &Expr<'tcx>) {
98+
if let Some(parent) = get_parent_expr(cx, expr) {
99+
if let ExprKind::If(_, _, _) = parent.kind {
100+
return;
101+
}
102+
}
103+
if contains_or(cond) || contains_or(then) || fetch_bool_expr(then).is_some() {
104+
return;
105+
}
106+
if match then.kind {
107+
ExprKind::Block(block, _) => !block.stmts.is_empty(),
108+
_ => false,
109+
} {
110+
return;
111+
}
112+
113+
let applicability = Applicability::MachineApplicable;
114+
let cond_snippet = cx
115+
.sess()
116+
.source_map()
117+
.span_to_snippet(cond.span)
118+
.unwrap_or_else(|_| "..".to_string());
119+
120+
let then_snippet = extract_final_expression_snippet(cx, then).unwrap_or_else(|| "..".to_string());
121+
let suggestion = format!("{cond_snippet} && {then_snippet}");
122+
span_lint_and_sugg(
123+
cx,
124+
MANUAL_AND,
125+
expr.span,
126+
"this `if`-then-`else` expression can be simplified with `&&`",
127+
"try",
128+
suggestion,
129+
applicability,
130+
);
131+
}
132+
133+
fn check_or<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, cond: &Expr<'tcx>, else_expr: &Expr<'tcx>) {
134+
if matches!(else_expr.kind, ExprKind::If(..)) || fetch_bool_expr(else_expr).is_some() {
135+
return;
136+
}
137+
if match else_expr.kind {
138+
ExprKind::Block(block, _) => !block.stmts.is_empty(),
139+
_ => false,
140+
} {
141+
return;
142+
}
143+
144+
let applicability = Applicability::MachineApplicable;
145+
let cond_snippet = cx
146+
.sess()
147+
.source_map()
148+
.span_to_snippet(cond.span)
149+
.unwrap_or_else(|_| "..".to_string());
150+
151+
let else_snippet = extract_final_expression_snippet(cx, else_expr).unwrap_or_else(|| "..".to_string());
152+
let suggestion = format!("{cond_snippet} || {else_snippet}");
153+
span_lint_and_sugg(
154+
cx,
155+
MANUAL_OR,
156+
expr.span,
157+
"this `if`-then-`else` expression can be simplified with `||`",
158+
"try",
159+
suggestion,
160+
applicability,
161+
);
162+
}
163+
164+
impl<'tcx> LateLintPass<'tcx> for ManualAndOr {
165+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
166+
if let Some(higher::If {
167+
cond,
168+
then,
169+
r#else: Some(else_expr),
170+
}) = higher::If::hir(expr)
171+
{
172+
if let Some(false) = fetch_bool_expr(else_expr) {
173+
check_and(cx, expr, cond, then);
174+
} else if let Some(true) = fetch_bool_expr(then) {
175+
check_or(cx, expr, cond, else_expr);
176+
}
177+
}
178+
}
179+
}

‎clippy_lints/src/methods/bind_instead_of_map.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,8 @@ pub(crate) trait BindInsteadOfMap {
144144
let closure_body = cx.tcx.hir().body(body);
145145
let closure_expr = peel_blocks(closure_body.value);
146146

147-
if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
148-
true
149-
} else {
150-
Self::lint_closure(cx, expr, closure_expr)
151-
}
147+
Self::lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span)
148+
|| Self::lint_closure(cx, expr, closure_expr)
152149
},
153150
// `_.and_then(Some)` case, which is no-op.
154151
hir::ExprKind::Path(QPath::Resolved(_, path)) if Self::is_variant(cx, path.res) => {

‎clippy_lints/src/methods/chars_last_cmp.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@ use super::CHARS_LAST_CMP;
55

66
/// Checks for the `CHARS_LAST_CMP` lint.
77
pub(super) fn check(cx: &LateContext<'_>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
8-
if chars_cmp::check(cx, info, &["chars", "last"], CHARS_LAST_CMP, "ends_with") {
9-
true
10-
} else {
11-
chars_cmp::check(cx, info, &["chars", "next_back"], CHARS_LAST_CMP, "ends_with")
12-
}
8+
chars_cmp::check(cx, info, &["chars", "last"], CHARS_LAST_CMP, "ends_with")
9+
|| chars_cmp::check(cx, info, &["chars", "next_back"], CHARS_LAST_CMP, "ends_with")
1310
}

‎clippy_lints/src/methods/chars_last_cmp_with_unwrap.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@ use super::CHARS_LAST_CMP;
55

66
/// Checks for the `CHARS_LAST_CMP` lint with `unwrap()`.
77
pub(super) fn check(cx: &LateContext<'_>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
8-
if chars_cmp_with_unwrap::check(cx, info, &["chars", "last", "unwrap"], CHARS_LAST_CMP, "ends_with") {
9-
true
10-
} else {
11-
chars_cmp_with_unwrap::check(cx, info, &["chars", "next_back", "unwrap"], CHARS_LAST_CMP, "ends_with")
12-
}
8+
chars_cmp_with_unwrap::check(cx, info, &["chars", "last", "unwrap"], CHARS_LAST_CMP, "ends_with")
9+
|| chars_cmp_with_unwrap::check(cx, info, &["chars", "next_back", "unwrap"], CHARS_LAST_CMP, "ends_with")
1310
}

‎clippy_lints/src/methods/search_is_some.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,8 @@ pub(super) fn check<'tcx>(
110110
else if search_method == "find" {
111111
let is_string_or_str_slice = |e| {
112112
let self_ty = cx.typeck_results().expr_ty(e).peel_refs();
113-
if is_type_lang_item(cx, self_ty, hir::LangItem::String) {
114-
true
115-
} else {
116-
self_ty.is_str()
117-
}
113+
114+
is_type_lang_item(cx, self_ty, hir::LangItem::String) || self_ty.is_str()
118115
};
119116
if is_string_or_str_slice(search_recv) && is_string_or_str_slice(search_arg) {
120117
let msg = format!("called `{option_check_method}()` after calling `find()` on a string");

‎clippy_lints/src/non_send_fields_in_send_ty.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,13 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
207207
.all(|ty| ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)),
208208
ty::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, *ty, send_trait),
209209
ty::Adt(_, args) => {
210-
if contains_pointer_like(cx, ty) {
210+
contains_pointer_like(cx, ty)
211211
// descends only if ADT contains any raw pointers
212-
args.iter().all(|generic_arg| match generic_arg.unpack() {
212+
&& args.iter().all(|generic_arg| match generic_arg.unpack() {
213213
GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
214214
// Lifetimes and const generics are not solid part of ADT and ignored
215215
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => true,
216216
})
217-
} else {
218-
false
219-
}
220217
},
221218
// Raw pointers are `!Send` but allowed by the heuristic
222219
ty::RawPtr(_, _) => true,

0 commit comments

Comments
 (0)