Skip to content

Commit 2280b8a

Browse files
y21y21
authored
Use clearer multipart suggestions for unnecessary_map_or lint (#13998)
A multipart suggestion will be used whenever the method call can be replaced by another one with the first argument removed. It helps place the new method call in context, especially when it is part of a larger expression. This fixes #13995 by applying a suggestion made by @y21. r? @y21 changelog: [`unnecessary_map_or`]: better representation of suggestions by placing them in context
2 parents 0707fe8 + 7f37b2a commit 2280b8a

File tree

3 files changed

+147
-46
lines changed

3 files changed

+147
-46
lines changed

‎clippy_lints/src/methods/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5070,7 +5070,7 @@ impl Methods {
50705070
option_map_or_none::check(cx, expr, recv, def, map);
50715071
manual_ok_or::check(cx, expr, recv, def, map);
50725072
option_map_or_err_ok::check(cx, expr, recv, def, map);
5073-
unnecessary_map_or::check(cx, expr, recv, def, map, &self.msrv);
5073+
unnecessary_map_or::check(cx, expr, recv, def, map, span, &self.msrv);
50745074
},
50755075
("map_or_else", [def, map]) => {
50765076
result_map_or_else_none::check(cx, expr, recv, def, map);

‎clippy_lints/src/methods/unnecessary_map_or.rs

+14-21
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use std::borrow::Cow;
22

3-
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::diagnostics::span_lint_and_then;
44
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
55
use clippy_utils::msrvs::{self, Msrv};
6-
use clippy_utils::source::snippet_opt;
76
use clippy_utils::sugg::{Sugg, make_binop};
87
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
98
use clippy_utils::visitors::is_local_used;
@@ -12,7 +11,7 @@ use rustc_ast::LitKind::Bool;
1211
use rustc_errors::Applicability;
1312
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
1413
use rustc_lint::LateContext;
15-
use rustc_span::sym;
14+
use rustc_span::{Span, sym};
1615

1716
use super::UNNECESSARY_MAP_OR;
1817

@@ -42,6 +41,7 @@ pub(super) fn check<'a>(
4241
recv: &Expr<'_>,
4342
def: &Expr<'_>,
4443
map: &Expr<'_>,
44+
method_span: Span,
4545
msrv: &Msrv,
4646
) {
4747
let ExprKind::Lit(def_kind) = def.kind else {
@@ -60,6 +60,8 @@ pub(super) fn check<'a>(
6060
Some(_) | None => return,
6161
};
6262

63+
let ext_def_span = def.span.until(map.span);
64+
6365
let (sugg, method, applicability) = if let ExprKind::Closure(map_closure) = map.kind
6466
&& let closure_body = cx.tcx.hir().body(map_closure.body)
6567
&& let closure_body_value = closure_body.value.peel_blocks()
@@ -114,26 +116,17 @@ pub(super) fn check<'a>(
114116
}
115117
.into_string();
116118

117-
(sugg, "a standard comparison", app)
118-
} else if !def_bool
119-
&& msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
120-
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
121-
&& let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
122-
{
119+
(vec![(expr.span, sugg)], "a standard comparison", app)
120+
} else if !def_bool && msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) {
123121
let suggested_name = variant.method_name();
124122
(
125-
format!("{recv_callsite}.{suggested_name}({span_callsite})",),
123+
vec![(method_span, suggested_name.into()), (ext_def_span, String::default())],
126124
suggested_name,
127125
Applicability::MachineApplicable,
128126
)
129-
} else if def_bool
130-
&& matches!(variant, Variant::Some)
131-
&& msrv.meets(msrvs::IS_NONE_OR)
132-
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
133-
&& let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
134-
{
127+
} else if def_bool && matches!(variant, Variant::Some) && msrv.meets(msrvs::IS_NONE_OR) {
135128
(
136-
format!("{recv_callsite}.is_none_or({span_callsite})"),
129+
vec![(method_span, "is_none_or".into()), (ext_def_span, String::default())],
137130
"is_none_or",
138131
Applicability::MachineApplicable,
139132
)
@@ -145,13 +138,13 @@ pub(super) fn check<'a>(
145138
return;
146139
}
147140

148-
span_lint_and_sugg(
141+
span_lint_and_then(
149142
cx,
150143
UNNECESSARY_MAP_OR,
151144
expr.span,
152145
"this `map_or` can be simplified",
153-
format!("use {method} instead"),
154-
sugg,
155-
applicability,
146+
|diag| {
147+
diag.multipart_suggestion_verbose(format!("use {method} instead"), sugg, applicability);
148+
},
156149
);
157150
}

‎tests/ui/unnecessary_map_or.stderr

+132-24
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,25 @@ error: this `map_or` can be simplified
22
--> tests/ui/unnecessary_map_or.rs:13:13
33
|
44
LL | let _ = Some(5).map_or(false, |n| n == 5);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) == Some(5)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::unnecessary-map-or` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_or)]`
9+
help: use a standard comparison instead
10+
|
11+
LL | let _ = Some(5) == Some(5);
12+
| ~~~~~~~~~~~~~~~~~~
913

1014
error: this `map_or` can be simplified
1115
--> tests/ui/unnecessary_map_or.rs:14:13
1216
|
1317
LL | let _ = Some(5).map_or(true, |n| n != 5);
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) != Some(5)`
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
|
20+
help: use a standard comparison instead
21+
|
22+
LL | let _ = Some(5) != Some(5);
23+
| ~~~~~~~~~~~~~~~~~~
1524

1625
error: this `map_or` can be simplified
1726
--> tests/ui/unnecessary_map_or.rs:15:13
@@ -21,7 +30,12 @@ LL | let _ = Some(5).map_or(false, |n| {
2130
LL | | let _ = 1;
2231
LL | | n == 5
2332
LL | | });
24-
| |______^ help: use a standard comparison instead: `Some(5) == Some(5)`
33+
| |______^
34+
|
35+
help: use a standard comparison instead
36+
|
37+
LL | let _ = Some(5) == Some(5);
38+
| ~~~~~~~~~~~~~~~~~~
2539

2640
error: this `map_or` can be simplified
2741
--> tests/ui/unnecessary_map_or.rs:19:13
@@ -35,113 +49,207 @@ LL | | });
3549
|
3650
help: use is_some_and instead
3751
|
38-
LL ~ let _ = Some(5).is_some_and(|n| {
39-
LL + let _ = n;
40-
LL + 6 >= 5
41-
LL ~ });
52+
LL - let _ = Some(5).map_or(false, |n| {
53+
LL + let _ = Some(5).is_some_and(|n| {
4254
|
4355

4456
error: this `map_or` can be simplified
4557
--> tests/ui/unnecessary_map_or.rs:23:13
4658
|
4759
LL | let _ = Some(vec![5]).map_or(false, |n| n == [5]);
48-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![5]).is_some_and(|n| n == [5])`
60+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
61+
|
62+
help: use is_some_and instead
63+
|
64+
LL - let _ = Some(vec![5]).map_or(false, |n| n == [5]);
65+
LL + let _ = Some(vec![5]).is_some_and(|n| n == [5]);
66+
|
4967

5068
error: this `map_or` can be simplified
5169
--> tests/ui/unnecessary_map_or.rs:24:13
5270
|
5371
LL | let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
54-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
72+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
73+
|
74+
help: use is_some_and instead
75+
|
76+
LL - let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
77+
LL + let _ = Some(vec![1]).is_some_and(|n| vec![2] == n);
78+
|
5579

5680
error: this `map_or` can be simplified
5781
--> tests/ui/unnecessary_map_or.rs:25:13
5882
|
5983
LL | let _ = Some(5).map_or(false, |n| n == n);
60-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == n)`
84+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
85+
|
86+
help: use is_some_and instead
87+
|
88+
LL - let _ = Some(5).map_or(false, |n| n == n);
89+
LL + let _ = Some(5).is_some_and(|n| n == n);
90+
|
6191

6292
error: this `map_or` can be simplified
6393
--> tests/ui/unnecessary_map_or.rs:26:13
6494
|
6595
LL | let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
66-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
96+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
97+
|
98+
help: use is_some_and instead
99+
|
100+
LL - let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
101+
LL + let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
102+
|
67103

68104
error: this `map_or` can be simplified
69105
--> tests/ui/unnecessary_map_or.rs:27:13
70106
|
71107
LL | let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
72-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
108+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
109+
|
110+
help: use is_ok_and instead
111+
|
112+
LL - let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
113+
LL + let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
114+
|
73115

74116
error: this `map_or` can be simplified
75117
--> tests/ui/unnecessary_map_or.rs:28:13
76118
|
77119
LL | let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
78-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Ok::<i32, i32>(5) == Ok(5)`
120+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
121+
|
122+
help: use a standard comparison instead
123+
|
124+
LL | let _ = Ok::<i32, i32>(5) == Ok(5);
125+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
79126

80127
error: this `map_or` can be simplified
81128
--> tests/ui/unnecessary_map_or.rs:29:13
82129
|
83130
LL | let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
84-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
131+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
132+
|
133+
help: use a standard comparison instead
134+
|
135+
LL | let _ = (Some(5) == Some(5)).then(|| 1);
136+
| ~~~~~~~~~~~~~~~~~~~~
85137

86138
error: this `map_or` can be simplified
87139
--> tests/ui/unnecessary_map_or.rs:30:13
88140
|
89141
LL | let _ = Some(5).map_or(true, |n| n == 5);
90-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| n == 5)`
142+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
143+
|
144+
help: use is_none_or instead
145+
|
146+
LL - let _ = Some(5).map_or(true, |n| n == 5);
147+
LL + let _ = Some(5).is_none_or(|n| n == 5);
148+
|
91149

92150
error: this `map_or` can be simplified
93151
--> tests/ui/unnecessary_map_or.rs:31:13
94152
|
95153
LL | let _ = Some(5).map_or(true, |n| 5 == n);
96-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| 5 == n)`
154+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
155+
|
156+
help: use is_none_or instead
157+
|
158+
LL - let _ = Some(5).map_or(true, |n| 5 == n);
159+
LL + let _ = Some(5).is_none_or(|n| 5 == n);
160+
|
97161

98162
error: this `map_or` can be simplified
99163
--> tests/ui/unnecessary_map_or.rs:32:14
100164
|
101165
LL | let _ = !Some(5).map_or(false, |n| n == 5);
102-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
166+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
167+
|
168+
help: use a standard comparison instead
169+
|
170+
LL | let _ = !(Some(5) == Some(5));
171+
| ~~~~~~~~~~~~~~~~~~~~
103172

104173
error: this `map_or` can be simplified
105174
--> tests/ui/unnecessary_map_or.rs:33:13
106175
|
107176
LL | let _ = Some(5).map_or(false, |n| n == 5) || false;
108-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
177+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
178+
|
179+
help: use a standard comparison instead
180+
|
181+
LL | let _ = (Some(5) == Some(5)) || false;
182+
| ~~~~~~~~~~~~~~~~~~~~
109183

110184
error: this `map_or` can be simplified
111185
--> tests/ui/unnecessary_map_or.rs:34:13
112186
|
113187
LL | let _ = Some(5).map_or(false, |n| n == 5) as usize;
114-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
188+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
189+
|
190+
help: use a standard comparison instead
191+
|
192+
LL | let _ = (Some(5) == Some(5)) as usize;
193+
| ~~~~~~~~~~~~~~~~~~~~
115194

116195
error: this `map_or` can be simplified
117196
--> tests/ui/unnecessary_map_or.rs:58:13
118197
|
119198
LL | let _ = r.map_or(false, |x| x == 7);
120-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(|x| x == 7)`
199+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
200+
|
201+
help: use is_ok_and instead
202+
|
203+
LL - let _ = r.map_or(false, |x| x == 7);
204+
LL + let _ = r.is_ok_and(|x| x == 7);
205+
|
121206

122207
error: this `map_or` can be simplified
123208
--> tests/ui/unnecessary_map_or.rs:63:13
124209
|
125210
LL | let _ = r.map_or(false, func);
126-
| ^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(func)`
211+
| ^^^^^^^^^^^^^^^^^^^^^
212+
|
213+
help: use is_ok_and instead
214+
|
215+
LL - let _ = r.map_or(false, func);
216+
LL + let _ = r.is_ok_and(func);
217+
|
127218

128219
error: this `map_or` can be simplified
129220
--> tests/ui/unnecessary_map_or.rs:64:13
130221
|
131222
LL | let _ = Some(5).map_or(false, func);
132-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(func)`
223+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
224+
|
225+
help: use is_some_and instead
226+
|
227+
LL - let _ = Some(5).map_or(false, func);
228+
LL + let _ = Some(5).is_some_and(func);
229+
|
133230

134231
error: this `map_or` can be simplified
135232
--> tests/ui/unnecessary_map_or.rs:65:13
136233
|
137234
LL | let _ = Some(5).map_or(true, func);
138-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(func)`
235+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
236+
|
237+
help: use is_none_or instead
238+
|
239+
LL - let _ = Some(5).map_or(true, func);
240+
LL + let _ = Some(5).is_none_or(func);
241+
|
139242

140243
error: this `map_or` can be simplified
141244
--> tests/ui/unnecessary_map_or.rs:70:13
142245
|
143246
LL | let _ = r.map_or(false, |x| x == 8);
144-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `r == Ok(8)`
247+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
248+
|
249+
help: use a standard comparison instead
250+
|
251+
LL | let _ = r == Ok(8);
252+
| ~~~~~~~~~~
145253

146254
error: aborting due to 21 previous errors
147255

0 commit comments

Comments
 (0)