Skip to content

Commit a343bb1

Browse files
apekrosapekros
committed
feat: add lint for map_or_else to map_or 
1 parent 26a5677 commit a343bb1

16 files changed

+168
-35
lines changed

‎clippy_lints/src/matches/match_single_binding.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ fn expr_in_nested_block(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
302302
if let Node::Block(block) = cx.tcx.parent_hir_node(match_expr.hir_id) {
303303
return block
304304
.expr
305-
.map_or_else(|| matches!(block.stmts, [_]), |_| block.stmts.is_empty());
305+
.map_or(matches!(block.stmts, [_]), |_| block.stmts.is_empty());
306306
}
307307
false
308308
}

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1934,6 +1934,7 @@ declare_clippy_lint! {
19341934
/// - `or_else` to `or`
19351935
/// - `get_or_insert_with` to `get_or_insert`
19361936
/// - `ok_or_else` to `ok_or`
1937+
/// - `map_or_else` to `map_or`
19371938
/// - `then` to `then_some` (for msrv >= 1.62.0)
19381939
///
19391940
/// ### Why is this bad?
@@ -5409,6 +5410,7 @@ impl Methods {
54095410
result_map_or_else_none::check(cx, expr, recv, def, map);
54105411
unnecessary_option_map_or_else::check(cx, expr, recv, def, map);
54115412
unnecessary_result_map_or_else::check(cx, expr, recv, def, map);
5413+
unnecessary_lazy_eval::check_map_or_else(cx, expr, recv, def, map);
54125414
},
54135415
(sym::next, []) => {
54145416
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {

‎clippy_lints/src/methods/unnecessary_lazy_eval.rs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,74 @@ pub(super) fn check<'tcx>(
8383
}
8484
false
8585
}
86+
87+
/// lint use of `_.map_or_else(|| value, f)` for `Option`s and `Result`s that can be
88+
/// replaced with `_.map_or(value, f)`
89+
pub(super) fn check_map_or_else<'tcx>(
90+
cx: &LateContext<'tcx>,
91+
expr: &'tcx hir::Expr<'_>,
92+
recv: &'tcx hir::Expr<'_>,
93+
def_arg: &'tcx hir::Expr<'_>,
94+
map_arg: &'tcx hir::Expr<'_>,
95+
) {
96+
let is_option = cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Option);
97+
let is_result = cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Result);
98+
99+
if !(is_option || is_result) {
100+
return;
101+
}
102+
103+
if let hir::ExprKind::Closure(&hir::Closure {
104+
body,
105+
fn_decl,
106+
kind: hir::ClosureKind::Closure,
107+
..
108+
}) = def_arg.kind
109+
{
110+
let body = cx.tcx.hir_body(body);
111+
let body_expr = &body.value;
112+
113+
if usage::BindingUsageFinder::are_params_used(cx, body) || is_from_proc_macro(cx, expr) {
114+
return;
115+
}
116+
117+
if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
118+
let msg = if is_option {
119+
"unnecessary closure used to substitute value for `Option::None`"
120+
} else {
121+
"unnecessary closure used to substitute value for `Result::Err`"
122+
};
123+
let applicability = if body
124+
.params
125+
.iter()
126+
.all(|param| matches!(param.pat.kind, hir::PatKind::Binding(..) | hir::PatKind::Wild))
127+
&& matches!(
128+
fn_decl.output,
129+
FnRetTy::DefaultReturn(_)
130+
| FnRetTy::Return(hir::Ty {
131+
kind: hir::TyKind::Infer(()),
132+
..
133+
})
134+
) {
135+
Applicability::MachineApplicable
136+
} else {
137+
Applicability::MaybeIncorrect
138+
};
139+
140+
if let hir::ExprKind::MethodCall(.., span) = expr.kind {
141+
span_lint_and_then(cx, UNNECESSARY_LAZY_EVALUATIONS, expr.span, msg, |diag| {
142+
diag.span_suggestion_verbose(
143+
span,
144+
"use `map_or` instead",
145+
format!(
146+
"map_or({}, {})",
147+
snippet(cx, body_expr.span, ".."),
148+
snippet(cx, map_arg.span, "..")
149+
),
150+
applicability,
151+
);
152+
});
153+
}
154+
}
155+
}
156+
}

‎clippy_utils/src/source.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option<usize>,
533533
/// snippet(cx, span2, "..") // -> "Vec::new()"
534534
/// ```
535535
pub fn snippet<'a>(sess: &impl HasSession, span: Span, default: &'a str) -> Cow<'a, str> {
536-
snippet_opt(sess, span).map_or_else(|| Cow::Borrowed(default), From::from)
536+
snippet_opt(sess, span).map_or(Cow::Borrowed(default), From::from)
537537
}
538538

539539
/// Same as [`snippet`], but it adapts the applicability level by following rules:

‎tests/ui/manual_ok_or.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(clippy::or_fun_call)]
33
#![allow(clippy::disallowed_names)]
44
#![allow(clippy::redundant_closure)]
5+
#![allow(clippy::unnecessary_lazy_evaluations)]
56
#![allow(dead_code)]
67
#![allow(unused_must_use)]
78

‎tests/ui/manual_ok_or.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(clippy::or_fun_call)]
33
#![allow(clippy::disallowed_names)]
44
#![allow(clippy::redundant_closure)]
5+
#![allow(clippy::unnecessary_lazy_evaluations)]
56
#![allow(dead_code)]
67
#![allow(unused_must_use)]
78

‎tests/ui/manual_ok_or.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this pattern reimplements `Option::ok_or`
2-
--> tests/ui/manual_ok_or.rs:11:5
2+
--> tests/ui/manual_ok_or.rs:12:5
33
|
44
LL | foo.map_or(Err("error"), |v| Ok(v));
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
@@ -8,19 +8,19 @@ LL | foo.map_or(Err("error"), |v| Ok(v));
88
= help: to override `-D warnings` add `#[allow(clippy::manual_ok_or)]`
99

1010
error: this pattern reimplements `Option::ok_or`
11-
--> tests/ui/manual_ok_or.rs:15:5
11+
--> tests/ui/manual_ok_or.rs:16:5
1212
|
1313
LL | foo.map_or(Err("error"), Ok);
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
1515

1616
error: this pattern reimplements `Option::ok_or`
17-
--> tests/ui/manual_ok_or.rs:19:5
17+
--> tests/ui/manual_ok_or.rs:20:5
1818
|
1919
LL | None::<i32>.map_or(Err("error"), |v| Ok(v));
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `None::<i32>.ok_or("error")`
2121

2222
error: this pattern reimplements `Option::ok_or`
23-
--> tests/ui/manual_ok_or.rs:24:5
23+
--> tests/ui/manual_ok_or.rs:25:5
2424
|
2525
LL | / foo.map_or(Err::<i32, &str>(
2626
LL | |

‎tests/ui/map_unwrap_or_fixable.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//@aux-build:option_helpers.rs
22

33
#![warn(clippy::map_unwrap_or)]
4+
#![allow(clippy::unnecessary_lazy_evaluations)]
45

56
#[macro_use]
67
extern crate option_helpers;

‎tests/ui/map_unwrap_or_fixable.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//@aux-build:option_helpers.rs
22

33
#![warn(clippy::map_unwrap_or)]
4+
#![allow(clippy::unnecessary_lazy_evaluations)]
45

56
#[macro_use]
67
extern crate option_helpers;

‎tests/ui/map_unwrap_or_fixable.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value
2-
--> tests/ui/map_unwrap_or_fixable.rs:16:13
2+
--> tests/ui/map_unwrap_or_fixable.rs:17:13
33
|
44
LL | let _ = opt.map(|x| x + 1)
55
| _____________^
@@ -11,7 +11,7 @@ LL | | .unwrap_or_else(|| 0);
1111
= help: to override `-D warnings` add `#[allow(clippy::map_unwrap_or)]`
1212

1313
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value
14-
--> tests/ui/map_unwrap_or_fixable.rs:47:13
14+
--> tests/ui/map_unwrap_or_fixable.rs:48:13
1515
|
1616
LL | let _ = res.map(|x| x + 1)
1717
| _____________^

0 commit comments

Comments
 (0)