Skip to content

Commit bec31bc

Browse files
yuxqiuyuxqiu
committed
feat: add map_unwrap_or_default lint
1 parent 3664d63 commit bec31bc

File tree

8 files changed

+289
-2
lines changed

8 files changed

+289
-2
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5260,6 +5260,7 @@ Released 2018-09-13
52605260
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
52615261
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
52625262
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
5263+
[`map_unwrap_or_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or_default
52635264
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
52645265
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
52655266
[`match_like_matches_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro

‎clippy_config/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ macro_rules! msrv_aliases {
1717
// names may refer to stabilized feature flags or library items
1818
msrv_aliases! {
1919
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
20-
1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN }
20+
1,70,0 { OPTION_IS_SOME_AND, RESULT_IS_OK_AND, BINARY_HEAP_RETAIN }
2121
1,68,0 { PATH_MAIN_SEPARATOR_STR }
2222
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
2323
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
392392
crate::methods::MAP_FLATTEN_INFO,
393393
crate::methods::MAP_IDENTITY_INFO,
394394
crate::methods::MAP_UNWRAP_OR_INFO,
395+
crate::methods::MAP_UNWRAP_OR_DEFAULT_INFO,
395396
crate::methods::MUT_MUTEX_LOCK_INFO,
396397
crate::methods::NAIVE_BYTECOUNT_INFO,
397398
crate::methods::NEEDLESS_COLLECT_INFO,

‎clippy_lints/src/methods/map_unwrap_or_default.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
use clippy_config::msrvs::{Msrv, OPTION_IS_SOME_AND, RESULT_IS_OK_AND};
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::ty::{expr_sig, is_type_diagnostic_item};
4+
use rustc_errors::Applicability;
5+
use rustc_lint::LateContext;
6+
use rustc_span::{sym, Span};
7+
8+
use super::MAP_UNWRAP_OR_DEFAULT;
9+
10+
pub(super) fn check<'tcx>(
11+
cx: &LateContext<'_>,
12+
expr: &'tcx rustc_hir::Expr<'_>,
13+
map_recv: &'tcx rustc_hir::Expr<'_>,
14+
map_arg: &'tcx rustc_hir::Expr<'_>,
15+
map_span: Span,
16+
unwrap_recv: &'tcx rustc_hir::Expr<'_>,
17+
msrv: &Msrv,
18+
) {
19+
if expr.span.from_expansion() {
20+
return;
21+
}
22+
23+
// Lint if:
24+
25+
// 1. the caller of `map()` is an `Option` or `Result`
26+
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Option);
27+
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Result);
28+
if !is_option && !is_result {
29+
return;
30+
}
31+
32+
// 2. the `map_arg` maps `T` to `bool`
33+
let map_arg_return_bool = expr_sig(cx, map_arg).is_some_and(|fn_sig| {
34+
fn_sig
35+
.output()
36+
.is_some_and(|ret| ret.no_bound_vars().is_some_and(|ty| ty.is_bool()))
37+
});
38+
if !map_arg_return_bool {
39+
return;
40+
}
41+
42+
// 3. msrv meets `OPTION_IS_SOME_AND` and `RESULT_IS_OK_AND`
43+
if !msrv.meets(OPTION_IS_SOME_AND) || !msrv.meets(RESULT_IS_OK_AND) {
44+
return;
45+
}
46+
47+
let lint_msg = if is_option {
48+
"called `map(<f>).unwrap_or_default()` on an `Option` value"
49+
} else {
50+
"called `map(<f>).unwrap_or_default()` on a `Result` value"
51+
};
52+
let suggest = if is_option { "is_some_and" } else { "is_ok_and" };
53+
54+
span_lint_and_then(
55+
cx,
56+
MAP_UNWRAP_OR_DEFAULT,
57+
expr.span.with_lo(map_span.lo()),
58+
lint_msg,
59+
|diag| {
60+
let suggestion = vec![
61+
(map_span, suggest.into()),
62+
(expr.span.with_lo(unwrap_recv.span.hi()), String::new()),
63+
];
64+
diag.multipart_suggestion(
65+
format!("use {} instead", suggest),
66+
suggestion,
67+
Applicability::MachineApplicable,
68+
);
69+
},
70+
);
71+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ mod map_err_ignore;
6161
mod map_flatten;
6262
mod map_identity;
6363
mod map_unwrap_or;
64+
mod map_unwrap_or_default;
6465
mod mut_mutex_lock;
6566
mod needless_collect;
6667
mod needless_option_as_deref;
@@ -3752,6 +3753,28 @@ declare_clippy_lint! {
37523753
"using `Option.map_or(Err(_), Ok)`, which is more succinctly expressed as `Option.ok_or(_)`"
37533754
}
37543755

3756+
declare_clippy_lint! {
3757+
/// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where f is a function or closure that returns the `bool` type.
3758+
///
3759+
/// ### Why is this bad?
3760+
/// Readability. These can be written more concisely as `option.is_some_and(f)` and `result.is_ok_and(f)`.
3761+
///
3762+
/// ### Example
3763+
/// ```no_run
3764+
/// option.map(|a| a > 10).unwrap_or_default();
3765+
/// result.map(|a| a > 10).unwrap_or_default();
3766+
/// ```
3767+
/// Use instead:
3768+
/// ```no_run
3769+
/// option.is_some_and(|a| a > 10);
3770+
/// result.is_ok_and(|a| a > 10);
3771+
/// ```
3772+
#[clippy::version = "1.76.0"]
3773+
pub MAP_UNWRAP_OR_DEFAULT,
3774+
pedantic,
3775+
"using `.map(f).unwrap_or_default()`, which is more succinctly expressed as `is_some_and(f)` or `is_ok_and(f)`"
3776+
}
3777+
37553778
pub struct Methods {
37563779
avoid_breaking_exported_api: bool,
37573780
msrv: Msrv,
@@ -3903,6 +3926,7 @@ impl_lint_pass!(Methods => [
39033926
UNNECESSARY_FALLIBLE_CONVERSIONS,
39043927
JOIN_ABSOLUTE_PATHS,
39053928
OPTION_MAP_OR_ERR_OK,
3929+
MAP_UNWRAP_OR_DEFAULT,
39063930
]);
39073931

39083932
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4567,7 +4591,16 @@ impl Methods {
45674591
}
45684592
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
45694593
},
4570-
("unwrap_or_default" | "unwrap_unchecked" | "unwrap_err_unchecked", []) => {
4594+
("unwrap_or_default", []) => {
4595+
match method_call(recv) {
4596+
Some(("map", m_recv, [arg], span, _)) => {
4597+
map_unwrap_or_default::check(cx, expr, m_recv, arg, span, recv, &self.msrv);
4598+
},
4599+
_ => {},
4600+
}
4601+
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
4602+
},
4603+
("unwrap_unchecked" | "unwrap_err_unchecked", []) => {
45714604
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
45724605
},
45734606
("unwrap_or_else", [u_arg]) => {

‎tests/ui/map_unwrap_or_default.fixed

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#![warn(clippy::map_unwrap_or_default)]
2+
3+
#[rustfmt::skip]
4+
fn option_methods() {
5+
let opt = Some(1);
6+
7+
// Check for `option.map(_).unwrap_or_default()` use.
8+
// Single line case.
9+
let _ = opt.is_some_and(|x| x > 1);
10+
// Multi-line cases.
11+
let _ = opt.is_some_and(|x| {
12+
x > 1
13+
}
14+
);
15+
let _ = opt.is_some_and(|x| x > 1);
16+
let _ = opt
17+
.is_some_and(|x| x > 1);
18+
19+
// won't fix because the return type of the closure is not `bool`
20+
let _ = opt.map(|x| x + 1).unwrap_or_default();
21+
}
22+
23+
#[rustfmt::skip]
24+
fn result_methods() {
25+
let res: Result<i32, ()> = Ok(1);
26+
27+
// multi line cases
28+
let _ = res.is_ok_and(|x| {
29+
x > 1
30+
}
31+
);
32+
let _ = res.is_ok_and(|x| x > 1);
33+
34+
// won't fix because the return type of the closure is not `bool`
35+
let _ = res.map(|x| x + 1).unwrap_or_default();
36+
}
37+
38+
fn main() {
39+
option_methods();
40+
result_methods();
41+
}

‎tests/ui/map_unwrap_or_default.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#![warn(clippy::map_unwrap_or_default)]
2+
3+
#[rustfmt::skip]
4+
fn option_methods() {
5+
let opt = Some(1);
6+
7+
// Check for `option.map(_).unwrap_or_default()` use.
8+
// Single line case.
9+
let _ = opt.map(|x| x > 1)
10+
// Should lint even though this call is on a separate line.
11+
.unwrap_or_default();
12+
// Multi-line cases.
13+
let _ = opt.map(|x| {
14+
x > 1
15+
}
16+
).unwrap_or_default();
17+
let _ = opt.map(|x| x > 1).unwrap_or_default();
18+
let _ = opt
19+
.map(|x| x > 1)
20+
.unwrap_or_default();
21+
22+
// won't fix because the return type of the closure is not `bool`
23+
let _ = opt.map(|x| x + 1).unwrap_or_default();
24+
}
25+
26+
#[rustfmt::skip]
27+
fn result_methods() {
28+
let res: Result<i32, ()> = Ok(1);
29+
30+
// multi line cases
31+
let _ = res.map(|x| {
32+
x > 1
33+
}
34+
).unwrap_or_default();
35+
let _ = res.map(|x| x > 1)
36+
.unwrap_or_default();
37+
38+
// won't fix because the return type of the closure is not `bool`
39+
let _ = res.map(|x| x + 1).unwrap_or_default();
40+
}
41+
42+
fn main() {
43+
option_methods();
44+
result_methods();
45+
}

‎tests/ui/map_unwrap_or_default.stderr

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
error: called `map(<f>).unwrap_or_default()` on an `Option` value
2+
--> $DIR/map_unwrap_or_default.rs:9:17
3+
|
4+
LL | let _ = opt.map(|x| x > 1)
5+
| _________________^
6+
LL | | // Should lint even though this call is on a separate line.
7+
LL | | .unwrap_or_default();
8+
| |____________________________^
9+
|
10+
= note: `-D clippy::map-unwrap-or-default` implied by `-D warnings`
11+
= help: to override `-D warnings` add `#[allow(clippy::map_unwrap_or_default)]`
12+
help: use is_some_and instead
13+
|
14+
LL - let _ = opt.map(|x| x > 1)
15+
LL + let _ = opt.is_some_and(|x| x > 1);
16+
|
17+
18+
error: called `map(<f>).unwrap_or_default()` on an `Option` value
19+
--> $DIR/map_unwrap_or_default.rs:13:17
20+
|
21+
LL | let _ = opt.map(|x| {
22+
| _________________^
23+
LL | | x > 1
24+
LL | | }
25+
LL | | ).unwrap_or_default();
26+
| |_________________________^
27+
|
28+
help: use is_some_and instead
29+
|
30+
LL ~ let _ = opt.is_some_and(|x| {
31+
LL | x > 1
32+
LL | }
33+
LL ~ );
34+
|
35+
36+
error: called `map(<f>).unwrap_or_default()` on an `Option` value
37+
--> $DIR/map_unwrap_or_default.rs:17:17
38+
|
39+
LL | let _ = opt.map(|x| x > 1).unwrap_or_default();
40+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
41+
|
42+
help: use is_some_and instead
43+
|
44+
LL - let _ = opt.map(|x| x > 1).unwrap_or_default();
45+
LL + let _ = opt.is_some_and(|x| x > 1);
46+
|
47+
48+
error: called `map(<f>).unwrap_or_default()` on an `Option` value
49+
--> $DIR/map_unwrap_or_default.rs:19:10
50+
|
51+
LL | .map(|x| x > 1)
52+
| __________^
53+
LL | | .unwrap_or_default();
54+
| |____________________________^
55+
|
56+
help: use is_some_and instead
57+
|
58+
LL - .map(|x| x > 1)
59+
LL + .is_some_and(|x| x > 1);
60+
|
61+
62+
error: called `map(<f>).unwrap_or_default()` on a `Result` value
63+
--> $DIR/map_unwrap_or_default.rs:31:17
64+
|
65+
LL | let _ = res.map(|x| {
66+
| _________________^
67+
LL | | x > 1
68+
LL | | }
69+
LL | | ).unwrap_or_default();
70+
| |_________________________^
71+
|
72+
help: use is_ok_and instead
73+
|
74+
LL ~ let _ = res.is_ok_and(|x| {
75+
LL | x > 1
76+
LL | }
77+
LL ~ );
78+
|
79+
80+
error: called `map(<f>).unwrap_or_default()` on a `Result` value
81+
--> $DIR/map_unwrap_or_default.rs:35:17
82+
|
83+
LL | let _ = res.map(|x| x > 1)
84+
| _________________^
85+
LL | | .unwrap_or_default();
86+
| |____________________________^
87+
|
88+
help: use is_ok_and instead
89+
|
90+
LL - let _ = res.map(|x| x > 1)
91+
LL + let _ = res.is_ok_and(|x| x > 1);
92+
|
93+
94+
error: aborting due to 6 previous errors
95+

0 commit comments

Comments
 (0)