Skip to content

Commit a48bc5b

Browse files
committed
suggests is_some_and over map().unwrap_or(false)
1 parent 407bfd4 commit a48bc5b

File tree

4 files changed

+47
-7
lines changed

4 files changed

+47
-7
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ declare_clippy_lint! {
513513
/// # let result: Result<usize, ()> = Ok(1);
514514
/// # fn some_function(foo: ()) -> usize { 1 }
515515
/// option.map(|a| a + 1).unwrap_or(0);
516+
/// option.map(|a| a > 10).unwrap_or(false);
516517
/// result.map(|a| a + 1).unwrap_or_else(some_function);
517518
/// ```
518519
///
@@ -522,6 +523,7 @@ declare_clippy_lint! {
522523
/// # let result: Result<usize, ()> = Ok(1);
523524
/// # fn some_function(foo: ()) -> usize { 1 }
524525
/// option.map_or(0, |a| a + 1);
526+
/// option.is_some_and(|a| a > 10);
525527
/// result.map_or_else(some_function, |a| a + 1);
526528
/// ```
527529
#[clippy::version = "1.45.0"]

clippy_lints/src/methods/option_map_unwrap_or.rs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,32 @@ pub(super) fn check<'tcx>(
7474
return;
7575
}
7676

77+
let mut suggest_is_some_and = false;
78+
// argument to `unwrap_or` is false; should suggest using `is_some_and`
79+
if let ExprKind::Lit(unwrap_lit) = &unwrap_arg.kind {
80+
if let rustc_ast::LitKind::Bool(false) = unwrap_lit.node {
81+
suggest_is_some_and = true;
82+
}
83+
}
84+
7785
let mut applicability = Applicability::MachineApplicable;
7886
// get snippet for unwrap_or()
7987
let unwrap_snippet = snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability);
8088
// lint message
8189
// comparing the snippet from source to raw text ("None") below is safe
8290
// because we already have checked the type.
83-
let arg = if unwrap_snippet == "None" { "None" } else { "<a>" };
91+
let arg = if unwrap_snippet == "None" {
92+
"None"
93+
} else if suggest_is_some_and {
94+
"false"
95+
} else {
96+
"<a>"
97+
};
8498
let unwrap_snippet_none = unwrap_snippet == "None";
8599
let suggest = if unwrap_snippet_none {
86100
"and_then(<f>)"
101+
} else if suggest_is_some_and {
102+
"is_some_and(<f>)"
87103
} else {
88104
"map_or(<a>, <f>)"
89105
};
@@ -98,12 +114,18 @@ pub(super) fn check<'tcx>(
98114
let mut suggestion = vec![
99115
(
100116
map_span,
101-
String::from(if unwrap_snippet_none { "and_then" } else { "map_or" }),
117+
String::from(if unwrap_snippet_none {
118+
"and_then"
119+
} else if suggest_is_some_and {
120+
"is_some_and"
121+
} else {
122+
"map_or"
123+
}),
102124
),
103125
(expr.span.with_lo(unwrap_recv.span.hi()), String::new()),
104126
];
105127

106-
if !unwrap_snippet_none {
128+
if !unwrap_snippet_none && !suggest_is_some_and {
107129
suggestion.push((map_arg_span.with_hi(map_arg_span.lo()), format!("{unwrap_snippet}, ")));
108130
}
109131

tests/ui/map_unwrap_or.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ fn option_methods() {
5656
.unwrap_or_else(||
5757
0
5858
);
59+
60+
// Check for `map(f).unwrap_or(false)` use.
61+
let _ = opt.map(|x| x > 5).unwrap_or(false);
62+
5963
}
6064

6165
#[rustfmt::skip]

tests/ui/map_unwrap_or.stderr

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,20 @@ LL | | 0
126126
LL | | );
127127
| |_________^
128128

129+
error: called `map(<f>).unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and(<f>)` instead
130+
--> $DIR/map_unwrap_or.rs:61:13
131+
|
132+
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
133+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
134+
|
135+
help: use `is_some_and(<f>)` instead
136+
|
137+
LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
138+
LL + let _ = opt.is_some_and(|x| x > 5);
139+
|
140+
129141
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
130-
--> $DIR/map_unwrap_or.rs:67:13
142+
--> $DIR/map_unwrap_or.rs:71:13
131143
|
132144
LL | let _ = res.map(|x| {
133145
| _____________^
@@ -137,7 +149,7 @@ LL | | ).unwrap_or_else(|_e| 0);
137149
| |____________________________^
138150

139151
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
140-
--> $DIR/map_unwrap_or.rs:71:13
152+
--> $DIR/map_unwrap_or.rs:75:13
141153
|
142154
LL | let _ = res.map(|x| x + 1)
143155
| _____________^
@@ -147,10 +159,10 @@ LL | | });
147159
| |__________^
148160

149161
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
150-
--> $DIR/map_unwrap_or.rs:95:13
162+
--> $DIR/map_unwrap_or.rs:99:13
151163
|
152164
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
153165
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `res.map_or_else(|_e| 0, |x| x + 1)`
154166

155-
error: aborting due to 12 previous errors
167+
error: aborting due to 13 previous errors
156168

0 commit comments

Comments
 (0)