Skip to content

Commit 80292c9

Browse files
committed
add additional cases for map_or lint
1 parent bd7226d commit 80292c9

File tree

1 file changed

+19
-6
lines changed

1 file changed

+19
-6
lines changed

clippy_lints/src/methods/unnecessary_pattern_matching.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
23
use clippy_utils::path_to_local_id;
34
use clippy_utils::source::snippet;
45
use clippy_utils::ty::is_type_diagnostic_item;
@@ -19,35 +20,47 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def:
1920
|| is_type_diagnostic_item(cx, recv_ty, sym::Result))
2021
// check we are dealing with boolean map_or
2122
&& let Bool(def_bool) = def_kind.node
22-
&& def_bool == false
2323
&& let ExprKind::Closure(map_closure) = map.kind
2424
&& let closure_body = cx.tcx.hir().body(map_closure.body)
2525
&& let closure_body_value = closure_body.value.peel_blocks()
26-
// check we are dealing with equality statement in the closure
26+
// switching from closure to direct comparison changes the comparison
27+
// from lazy (when map_or evaluates to first param) to eager, so we force eager here
28+
// this stops the lint running whenever the closure has a potentially expensive
29+
// computation inside of it
30+
&& switch_to_eager_eval(cx, closure_body_value)
2731
&& let ExprKind::Binary(op, l, r) = closure_body_value.kind
2832
&& let Some(param) = closure_body.params.first()
2933
&& let PatKind::Binding(_, hir_id, _, _) = param.pat.kind
30-
&& BinOpKind::Eq == op.node
31-
&& (path_to_local_id(l, hir_id) || path_to_local_id(r, hir_id))
34+
// checking that map_or is either:
35+
// .map_or(false, |x| x == y) OR .map_or(true, |x| x != y)
36+
&& ((BinOpKind::Eq == op.node && !def_bool) || (BinOpKind::Ne == op.node && def_bool))
37+
// check that either the left or right side of the comparison
38+
// is the binding param to the closure
39+
// xor, because if its both then thats a strange edge case and
40+
// we can just ignore it, since by default clippy will error on this
41+
&& (path_to_local_id(l, hir_id) ^ path_to_local_id(r, hir_id))
3242
{
3343
let wrap = if is_type_diagnostic_item(cx, recv_ty, sym::Option) {
3444
"Some"
3545
} else {
3646
"Result"
3747
};
3848

49+
let comparator = if BinOpKind::Eq == op.node { "==" } else { "!=" };
50+
3951
// we already checked either l or r is the local id earlier
4052
let non_binding_location = if path_to_local_id(l, hir_id) { r } else { l };
4153

4254
span_lint_and_sugg(
4355
cx,
4456
UNNECESSARY_PATTERN_MATCHING,
4557
expr.span,
46-
"`map_or` here is redundant, use typical equality instead",
58+
"`map_or` here is redundant, use standard comparison instead",
4759
"try",
4860
format!(
49-
"{} == {}({})",
61+
"{} {} {}({})",
5062
snippet(cx, recv.span, ".."),
63+
comparator,
5164
wrap,
5265
snippet(cx, non_binding_location.span.source_callsite(), "..")
5366
),

0 commit comments

Comments
 (0)