Skip to content

Commit 9d0b00d

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 680256f commit 9d0b00d

30 files changed

+528
-146
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5451,6 +5451,7 @@ Released 2018-09-13
54515451
[`macro_metavars_in_unsafe`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe
54525452
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
54535453
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
5454+
[`manual_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_and
54545455
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
54555456
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
54565457
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
@@ -5474,6 +5475,7 @@ Released 2018-09-13
54745475
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
54755476
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
54765477
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
5478+
[`manual_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_or
54775479
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
54785480
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
54795481
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid

‎clippy_lints/src/declared_lints.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
299299
crate::macro_metavars_in_unsafe::MACRO_METAVARS_IN_UNSAFE_INFO,
300300
crate::macro_use::MACRO_USE_IMPORTS_INFO,
301301
crate::main_recursion::MAIN_RECURSION_INFO,
302+
crate::manual_and::MANUAL_AND_INFO,
302303
crate::manual_assert::MANUAL_ASSERT_INFO,
303304
crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
304305
crate::manual_bits::MANUAL_BITS_INFO,
@@ -310,6 +311,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
310311
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
311312
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
312313
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
314+
crate::manual_or::MANUAL_OR_INFO,
313315
crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
314316
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
315317
crate::manual_retain::MANUAL_RETAIN_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: 4 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;
200201
mod manual_assert;
201202
mod manual_async_fn;
202203
mod manual_bits;
@@ -207,6 +208,7 @@ mod manual_is_ascii_check;
207208
mod manual_let_else;
208209
mod manual_main_separator_str;
209210
mod manual_non_exhaustive;
211+
mod manual_or;
210212
mod manual_range_patterns;
211213
mod manual_rem_euclid;
212214
mod manual_retain;
@@ -1165,6 +1167,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11651167
..Default::default()
11661168
})
11671169
});
1170+
store.register_late_pass(|_| Box::new(manual_or::ManualOr));
1171+
store.register_late_pass(|_| Box::new(manual_and::ManualAnd));
11681172
// add lints here, do not remove this comment, it's used in `new_lint`
11691173
}
11701174

‎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.rs

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::visitors::for_each_expr;
3+
use clippy_utils::{get_parent_expr, 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+
/// ```no_run
21+
/// if a {
22+
/// b
23+
/// } else {
24+
/// false
25+
/// }
26+
/// ```
27+
/// Use instead:
28+
/// ```no_run
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_lint_pass!(ManualAnd => [MANUAL_AND]);
38+
39+
fn extract_final_expression_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<String> {
40+
if let ExprKind::Block(block, _) = expr.kind {
41+
if let Some(final_expr) = block.expr {
42+
return cx.sess().source_map().span_to_snippet(final_expr.span).ok();
43+
}
44+
}
45+
cx.sess().source_map().span_to_snippet(expr.span).ok()
46+
}
47+
48+
fn fetch_bool_expr(expr: &Expr<'_>) -> Option<bool> {
49+
if let ExprKind::Lit(lit_ptr) = peel_blocks(expr).kind {
50+
if let LitKind::Bool(value) = lit_ptr.node {
51+
return Some(value);
52+
}
53+
}
54+
None
55+
}
56+
57+
fn contains_let(cond: &Expr<'_>) -> bool {
58+
for_each_expr(cond, |e| {
59+
if matches!(e.kind, ExprKind::Let(_)) {
60+
ControlFlow::Break(())
61+
} else {
62+
ControlFlow::Continue(())
63+
}
64+
})
65+
.is_some()
66+
}
67+
68+
fn contains_or(cond: &Expr<'_>) -> bool {
69+
for_each_expr(cond, |e| {
70+
if let ExprKind::Binary(ref n, _, _) = e.kind {
71+
if n.node == BinOpKind::Or {
72+
ControlFlow::Break(())
73+
} else {
74+
ControlFlow::Continue(())
75+
}
76+
} else {
77+
ControlFlow::Continue(())
78+
}
79+
})
80+
.is_some()
81+
}
82+
83+
impl<'tcx> LateLintPass<'tcx> for ManualAnd {
84+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
85+
if let Some(parent) = get_parent_expr(cx, expr) {
86+
if let ExprKind::If(_, _, _) = parent.kind {
87+
return;
88+
}
89+
}
90+
if let ExprKind::If(cond, then, Some(else_expr)) = expr.kind {
91+
if contains_let(cond) || contains_or(cond) || contains_or(then) {
92+
return;
93+
}
94+
if fetch_bool_expr(then).is_some()
95+
|| match then.kind {
96+
ExprKind::Block(block, _) => !block.stmts.is_empty(),
97+
_ => false,
98+
}
99+
{
100+
return;
101+
}
102+
if let Some(false) = fetch_bool_expr(else_expr) {
103+
let applicability = Applicability::MachineApplicable;
104+
let cond_snippet = cx
105+
.sess()
106+
.source_map()
107+
.span_to_snippet(cond.span)
108+
.unwrap_or_else(|_| "..".to_string());
109+
110+
let then_snippet = extract_final_expression_snippet(cx, then).unwrap_or_else(|| "..".to_string());
111+
span_lint_and_then(
112+
cx,
113+
MANUAL_AND,
114+
expr.span,
115+
"this `if`-then-`else` expression can be simplified with `&&`",
116+
|diag| {
117+
diag.span_suggestion(
118+
expr.span,
119+
"try",
120+
format!("{cond_snippet} && {then_snippet}"),
121+
applicability,
122+
);
123+
},
124+
);
125+
}
126+
}
127+
}
128+
}

‎clippy_lints/src/manual_or.rs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::peel_blocks;
3+
use clippy_utils::visitors::for_each_expr;
4+
use core::ops::ControlFlow;
5+
use rustc_ast::ast::LitKind;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Expr, ExprKind};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_session::declare_lint_pass;
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Detects `if`-then-`else` that can be replaced with `||`.
14+
///
15+
/// ### Why is this bad?
16+
/// `||` is simpler than `if`-then-`else`.
17+
///
18+
/// ### Example
19+
/// ```no_run
20+
/// if a {
21+
/// true
22+
/// } else {
23+
/// b
24+
/// }
25+
/// ```
26+
/// Use instead:
27+
/// ```no_run
28+
/// a || b
29+
/// ```
30+
#[clippy::version = "1.80.0"]
31+
pub MANUAL_OR,
32+
complexity,
33+
"this `if`-then-`else` expression can be simplified with `||`"
34+
}
35+
36+
declare_lint_pass!(ManualOr => [MANUAL_OR]);
37+
38+
fn extract_final_expression_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<String> {
39+
if let ExprKind::Block(block, _) = expr.kind {
40+
if let Some(final_expr) = block.expr {
41+
return cx.sess().source_map().span_to_snippet(final_expr.span).ok();
42+
}
43+
}
44+
cx.sess().source_map().span_to_snippet(expr.span).ok()
45+
}
46+
47+
fn fetch_bool_expr(expr: &Expr<'_>) -> Option<bool> {
48+
if let ExprKind::Lit(lit_ptr) = peel_blocks(expr).kind {
49+
if let LitKind::Bool(value) = lit_ptr.node {
50+
return Some(value);
51+
}
52+
}
53+
None
54+
}
55+
56+
fn contains_let(cond: &Expr<'_>) -> bool {
57+
for_each_expr(cond, |e| {
58+
if matches!(e.kind, ExprKind::Let(_)) {
59+
ControlFlow::Break(())
60+
} else {
61+
ControlFlow::Continue(())
62+
}
63+
})
64+
.is_some()
65+
}
66+
67+
impl<'tcx> LateLintPass<'tcx> for ManualOr {
68+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
69+
if let ExprKind::If(cond, then, Some(else_expr)) = expr.kind {
70+
if contains_let(cond) {
71+
return;
72+
}
73+
if matches!(else_expr.kind, ExprKind::If(..))
74+
|| match else_expr.kind {
75+
ExprKind::Block(block, _) => !block.stmts.is_empty(),
76+
_ => false,
77+
}
78+
|| fetch_bool_expr(else_expr).is_some()
79+
{
80+
return;
81+
}
82+
if let Some(true) = fetch_bool_expr(then) {
83+
let applicability = Applicability::MachineApplicable;
84+
let cond_snippet = cx
85+
.sess()
86+
.source_map()
87+
.span_to_snippet(cond.span)
88+
.unwrap_or_else(|_| "..".to_string());
89+
90+
let else_snippet = extract_final_expression_snippet(cx, else_expr).unwrap_or_else(|| "..".to_string());
91+
let suggestion = format!("{cond_snippet} || {else_snippet}");
92+
span_lint_and_sugg(
93+
cx,
94+
MANUAL_OR,
95+
expr.span,
96+
"this `if`-then-`else` expression can be simplified with `||`",
97+
"try",
98+
suggestion,
99+
applicability,
100+
);
101+
}
102+
}
103+
}
104+
}

‎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");

0 commit comments

Comments
 (0)