Skip to content

Commit b7b69b1

Browse files
AlexendooAlexendoo
authored
Remove unneeded parentheses in unnecessary_map_or lint output (#13932)
When the expression is transformed into an equality, parentheses are needed only if the resulting equality is used: - as a receiver in a method call - as part of a binary or unary expression - as part of a cast In other cases, which will be the majority, no parentheses are required. This makes the lint suggestions cleaner. changelog: `none`
2 parents 4ef9177 + 9c46e11 commit b7b69b1

File tree

4 files changed

+74
-34
lines changed

4 files changed

+74
-34
lines changed

‎clippy_lints/src/methods/unnecessary_map_or.rs

+19-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clippy_utils::source::snippet_opt;
77
use clippy_utils::sugg::{Sugg, make_binop};
88
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
99
use clippy_utils::visitors::is_local_used;
10-
use clippy_utils::{is_from_proc_macro, path_to_local_id};
10+
use clippy_utils::{get_parent_expr, is_from_proc_macro, path_to_local_id};
1111
use rustc_ast::LitKind::Bool;
1212
use rustc_errors::Applicability;
1313
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
@@ -96,11 +96,25 @@ pub(super) fn check<'a>(
9696
Sugg::hir(cx, non_binding_location, "")
9797
)));
9898

99-
let binop = make_binop(op.node, &Sugg::hir(cx, recv, ".."), &inner_non_binding)
100-
.maybe_par()
101-
.into_string();
99+
let mut app = Applicability::MachineApplicable;
100+
let binop = make_binop(
101+
op.node,
102+
&Sugg::hir_with_applicability(cx, recv, "..", &mut app),
103+
&inner_non_binding,
104+
);
102105

103-
(binop, "a standard comparison", Applicability::MaybeIncorrect)
106+
let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) {
107+
match parent_expr.kind {
108+
ExprKind::Binary(..) | ExprKind::Unary(..) | ExprKind::Cast(..) => binop.maybe_par(),
109+
ExprKind::MethodCall(_, receiver, _, _) if receiver.hir_id == expr.hir_id => binop.maybe_par(),
110+
_ => binop,
111+
}
112+
} else {
113+
binop
114+
}
115+
.into_string();
116+
117+
(sugg, "a standard comparison", app)
104118
} else if !def_bool
105119
&& msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
106120
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())

‎tests/ui/unnecessary_map_or.fixed

+9-5
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33
#![allow(clippy::no_effect)]
44
#![allow(clippy::eq_op)]
55
#![allow(clippy::unnecessary_lazy_evaluations)]
6+
#![allow(clippy::nonminimal_bool)]
67
#[clippy::msrv = "1.70.0"]
78
#[macro_use]
89
extern crate proc_macros;
910

1011
fn main() {
1112
// should trigger
12-
let _ = (Some(5) == Some(5));
13-
let _ = (Some(5) != Some(5));
14-
let _ = (Some(5) == Some(5));
13+
let _ = Some(5) == Some(5);
14+
let _ = Some(5) != Some(5);
15+
let _ = Some(5) == Some(5);
1516
let _ = Some(5).is_some_and(|n| {
1617
let _ = n;
1718
6 >= 5
@@ -21,10 +22,13 @@ fn main() {
2122
let _ = Some(5).is_some_and(|n| n == n);
2223
let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
2324
let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
24-
let _ = (Ok::<i32, i32>(5) == Ok(5));
25+
let _ = Ok::<i32, i32>(5) == Ok(5);
2526
let _ = (Some(5) == Some(5)).then(|| 1);
2627
let _ = Some(5).is_none_or(|n| n == 5);
2728
let _ = Some(5).is_none_or(|n| 5 == n);
29+
let _ = !(Some(5) == Some(5));
30+
let _ = (Some(5) == Some(5)) || false;
31+
let _ = (Some(5) == Some(5)) as usize;
2832

2933
macro_rules! x {
3034
() => {
@@ -60,7 +64,7 @@ fn main() {
6064
#[derive(PartialEq)]
6165
struct S2;
6266
let r: Result<i32, S2> = Ok(4);
63-
let _ = (r == Ok(8));
67+
let _ = r == Ok(8);
6468

6569
// do not lint `Result::map_or(true, …)`
6670
let r: Result<i32, S2> = Ok(4);

‎tests/ui/unnecessary_map_or.rs

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#![allow(clippy::no_effect)]
44
#![allow(clippy::eq_op)]
55
#![allow(clippy::unnecessary_lazy_evaluations)]
6+
#![allow(clippy::nonminimal_bool)]
67
#[clippy::msrv = "1.70.0"]
78
#[macro_use]
89
extern crate proc_macros;
@@ -28,6 +29,9 @@ fn main() {
2829
let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
2930
let _ = Some(5).map_or(true, |n| n == 5);
3031
let _ = Some(5).map_or(true, |n| 5 == n);
32+
let _ = !Some(5).map_or(false, |n| n == 5);
33+
let _ = Some(5).map_or(false, |n| n == 5) || false;
34+
let _ = Some(5).map_or(false, |n| n == 5) as usize;
3135

3236
macro_rules! x {
3337
() => {

‎tests/ui/unnecessary_map_or.stderr

+42-24
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
11
error: this `map_or` can be simplified
2-
--> tests/ui/unnecessary_map_or.rs:12:13
2+
--> 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+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) == Some(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)]`
99

1010
error: this `map_or` can be simplified
11-
--> tests/ui/unnecessary_map_or.rs:13:13
11+
--> tests/ui/unnecessary_map_or.rs:14:13
1212
|
1313
LL | let _ = Some(5).map_or(true, |n| n != 5);
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) != Some(5))`
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) != Some(5)`
1515

1616
error: this `map_or` can be simplified
17-
--> tests/ui/unnecessary_map_or.rs:14:13
17+
--> tests/ui/unnecessary_map_or.rs:15:13
1818
|
1919
LL | let _ = Some(5).map_or(false, |n| {
2020
| _____________^
2121
LL | | let _ = 1;
2222
LL | | n == 5
2323
LL | | });
24-
| |______^ help: use a standard comparison instead: `(Some(5) == Some(5))`
24+
| |______^ help: use a standard comparison instead: `Some(5) == Some(5)`
2525

2626
error: this `map_or` can be simplified
27-
--> tests/ui/unnecessary_map_or.rs:18:13
27+
--> tests/ui/unnecessary_map_or.rs:19:13
2828
|
2929
LL | let _ = Some(5).map_or(false, |n| {
3030
| _____________^
@@ -42,88 +42,106 @@ LL ~ });
4242
|
4343

4444
error: this `map_or` can be simplified
45-
--> tests/ui/unnecessary_map_or.rs:22:13
45+
--> tests/ui/unnecessary_map_or.rs:23:13
4646
|
4747
LL | let _ = Some(vec![5]).map_or(false, |n| n == [5]);
4848
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![5]).is_some_and(|n| n == [5])`
4949

5050
error: this `map_or` can be simplified
51-
--> tests/ui/unnecessary_map_or.rs:23:13
51+
--> tests/ui/unnecessary_map_or.rs:24:13
5252
|
5353
LL | let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
5454
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
5555

5656
error: this `map_or` can be simplified
57-
--> tests/ui/unnecessary_map_or.rs:24:13
57+
--> tests/ui/unnecessary_map_or.rs:25:13
5858
|
5959
LL | let _ = Some(5).map_or(false, |n| n == n);
6060
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == n)`
6161

6262
error: this `map_or` can be simplified
63-
--> tests/ui/unnecessary_map_or.rs:25:13
63+
--> tests/ui/unnecessary_map_or.rs:26:13
6464
|
6565
LL | let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
6666
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
6767

6868
error: this `map_or` can be simplified
69-
--> tests/ui/unnecessary_map_or.rs:26:13
69+
--> tests/ui/unnecessary_map_or.rs:27:13
7070
|
7171
LL | let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
7272
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
7373

7474
error: this `map_or` can be simplified
75-
--> tests/ui/unnecessary_map_or.rs:27:13
75+
--> tests/ui/unnecessary_map_or.rs:28:13
7676
|
7777
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))`
78+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Ok::<i32, i32>(5) == Ok(5)`
7979

8080
error: this `map_or` can be simplified
81-
--> tests/ui/unnecessary_map_or.rs:28:13
81+
--> tests/ui/unnecessary_map_or.rs:29:13
8282
|
8383
LL | let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
8484
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
8585

8686
error: this `map_or` can be simplified
87-
--> tests/ui/unnecessary_map_or.rs:29:13
87+
--> tests/ui/unnecessary_map_or.rs:30:13
8888
|
8989
LL | let _ = Some(5).map_or(true, |n| n == 5);
9090
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| n == 5)`
9191

9292
error: this `map_or` can be simplified
93-
--> tests/ui/unnecessary_map_or.rs:30:13
93+
--> tests/ui/unnecessary_map_or.rs:31:13
9494
|
9595
LL | let _ = Some(5).map_or(true, |n| 5 == n);
9696
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| 5 == n)`
9797

9898
error: this `map_or` can be simplified
99-
--> tests/ui/unnecessary_map_or.rs:54:13
99+
--> tests/ui/unnecessary_map_or.rs:32:14
100+
|
101+
LL | let _ = !Some(5).map_or(false, |n| n == 5);
102+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
103+
104+
error: this `map_or` can be simplified
105+
--> tests/ui/unnecessary_map_or.rs:33:13
106+
|
107+
LL | let _ = Some(5).map_or(false, |n| n == 5) || false;
108+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
109+
110+
error: this `map_or` can be simplified
111+
--> tests/ui/unnecessary_map_or.rs:34:13
112+
|
113+
LL | let _ = Some(5).map_or(false, |n| n == 5) as usize;
114+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
115+
116+
error: this `map_or` can be simplified
117+
--> tests/ui/unnecessary_map_or.rs:58:13
100118
|
101119
LL | let _ = r.map_or(false, |x| x == 7);
102120
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(|x| x == 7)`
103121

104122
error: this `map_or` can be simplified
105-
--> tests/ui/unnecessary_map_or.rs:59:13
123+
--> tests/ui/unnecessary_map_or.rs:63:13
106124
|
107125
LL | let _ = r.map_or(false, func);
108126
| ^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(func)`
109127

110128
error: this `map_or` can be simplified
111-
--> tests/ui/unnecessary_map_or.rs:60:13
129+
--> tests/ui/unnecessary_map_or.rs:64:13
112130
|
113131
LL | let _ = Some(5).map_or(false, func);
114132
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(func)`
115133

116134
error: this `map_or` can be simplified
117-
--> tests/ui/unnecessary_map_or.rs:61:13
135+
--> tests/ui/unnecessary_map_or.rs:65:13
118136
|
119137
LL | let _ = Some(5).map_or(true, func);
120138
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(func)`
121139

122140
error: this `map_or` can be simplified
123-
--> tests/ui/unnecessary_map_or.rs:66:13
141+
--> tests/ui/unnecessary_map_or.rs:70:13
124142
|
125143
LL | let _ = r.map_or(false, |x| x == 8);
126-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(r == Ok(8))`
144+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `r == Ok(8)`
127145

128-
error: aborting due to 18 previous errors
146+
error: aborting due to 21 previous errors
129147

0 commit comments

Comments
 (0)