Skip to content

Commit 8f6dace

Browse files
flip1995flip1995
committed
Revert "Extend manual_is_variant_and lint to check for boolean map comparisons (#14646)"
This reverts commit 551870d, reversing changes made to 3927a61.
1 parent 03aa9d7 commit 8f6dace

5 files changed

Lines changed: 15 additions & 240 deletions

File tree

‎src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs‎

Lines changed: 6 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::get_parent_expr;
32
use clippy_utils::msrvs::{self, Msrv};
4-
use clippy_utils::source::{snippet, snippet_opt};
3+
use clippy_utils::source::snippet;
54
use clippy_utils::ty::is_type_diagnostic_item;
65
use rustc_errors::Applicability;
7-
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
8-
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
96
use rustc_lint::LateContext;
10-
use rustc_middle::ty;
11-
use rustc_span::{BytePos, Span, sym};
7+
use rustc_span::{Span, sym};
128

139
use super::MANUAL_IS_VARIANT_AND;
1410

15-
pub(super) fn check(
11+
pub(super) fn check<'tcx>(
1612
cx: &LateContext<'_>,
17-
expr: &Expr<'_>,
18-
map_recv: &Expr<'_>,
19-
map_arg: &Expr<'_>,
13+
expr: &'tcx rustc_hir::Expr<'_>,
14+
map_recv: &'tcx rustc_hir::Expr<'_>,
15+
map_arg: &'tcx rustc_hir::Expr<'_>,
2016
map_span: Span,
2117
msrv: Msrv,
2218
) {
@@ -61,57 +57,3 @@ pub(super) fn check(
6157
Applicability::MachineApplicable,
6258
);
6359
}
64-
65-
fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) {
66-
if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo()))
67-
&& let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3)))
68-
{
69-
span_lint_and_sugg(
70-
cx,
71-
MANUAL_IS_VARIANT_AND,
72-
parent.span,
73-
format!(
74-
"called `.map() {}= {}()`",
75-
if op == BinOpKind::Eq { '=' } else { '!' },
76-
if is_option { "Some" } else { "Ok" },
77-
),
78-
"use",
79-
if is_option && op == BinOpKind::Ne {
80-
format!("{before_map_snippet}is_none_or{after_map_snippet}",)
81-
} else {
82-
format!(
83-
"{}{before_map_snippet}{}{after_map_snippet}",
84-
if op == BinOpKind::Eq { "" } else { "!" },
85-
if is_option { "is_some_and" } else { "is_ok_and" },
86-
)
87-
},
88-
Applicability::MachineApplicable,
89-
);
90-
}
91-
}
92-
93-
pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) {
94-
if let Some(parent_expr) = get_parent_expr(cx, expr)
95-
&& let ExprKind::Binary(op, left, right) = parent_expr.kind
96-
&& matches!(op.node, BinOpKind::Eq | BinOpKind::Ne)
97-
&& op.span.eq_ctxt(expr.span)
98-
{
99-
// Check `left` and `right` expression in any order, and for `Option` and `Result`
100-
for (expr1, expr2) in [(left, right), (right, left)] {
101-
for item in [sym::Option, sym::Result] {
102-
if let ExprKind::Call(call, ..) = expr1.kind
103-
&& let ExprKind::Path(QPath::Resolved(_, path)) = call.kind
104-
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res
105-
&& let ty = cx.typeck_results().expr_ty(expr1)
106-
&& let ty::Adt(adt, args) = ty.kind()
107-
&& cx.tcx.is_diagnostic_item(item, adt.did())
108-
&& args.type_at(0).is_bool()
109-
&& let ExprKind::MethodCall(_, recv, _, span) = expr2.kind
110-
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item)
111-
{
112-
return emit_lint(cx, op.node, parent_expr, span, item == sym::Option);
113-
}
114-
}
115-
}
116-
}
117-
}

‎src/tools/clippy/clippy_lints/src/methods/mod.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5242,7 +5242,6 @@ impl Methods {
52425242
unused_enumerate_index::check(cx, expr, recv, m_arg);
52435243
map_clone::check(cx, expr, recv, m_arg, self.msrv);
52445244
map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span);
5245-
manual_is_variant_and::check_map(cx, expr);
52465245
match method_call(recv) {
52475246
Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => {
52485247
iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv);

‎src/tools/clippy/tests/ui/manual_is_variant_and.fixed‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,44 +4,6 @@
44
#[macro_use]
55
extern crate option_helpers;
66

7-
struct Foo<T>(T);
8-
9-
impl<T> Foo<T> {
10-
fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> {
11-
Some(f(self.0))
12-
}
13-
}
14-
15-
fn foo() -> Option<bool> {
16-
Some(true)
17-
}
18-
19-
macro_rules! some_true {
20-
() => {
21-
Some(true)
22-
};
23-
}
24-
macro_rules! some_false {
25-
() => {
26-
Some(false)
27-
};
28-
}
29-
30-
macro_rules! mac {
31-
(some $e:expr) => {
32-
Some($e)
33-
};
34-
(some_map $e:expr) => {
35-
Some($e).map(|x| x % 2 == 0)
36-
};
37-
(map $e:expr) => {
38-
$e.map(|x| x % 2 == 0)
39-
};
40-
(eq $a:expr, $b:expr) => {
41-
$a == $b
42-
};
43-
}
44-
457
#[rustfmt::skip]
468
fn option_methods() {
479
let opt = Some(1);
@@ -59,30 +21,13 @@ fn option_methods() {
5921
let _ = opt
6022
.is_some_and(|x| x > 1);
6123

62-
let _ = Some(2).is_some_and(|x| x % 2 == 0);
63-
//~^ manual_is_variant_and
64-
let _ = Some(2).is_none_or(|x| x % 2 == 0);
65-
//~^ manual_is_variant_and
66-
let _ = Some(2).is_some_and(|x| x % 2 == 0);
67-
//~^ manual_is_variant_and
68-
let _ = Some(2).is_none_or(|x| x % 2 == 0);
69-
//~^ manual_is_variant_and
70-
7124
// won't fix because the return type of the closure is not `bool`
7225
let _ = opt.map(|x| x + 1).unwrap_or_default();
7326

7427
let opt2 = Some('a');
7528
let _ = opt2.is_some_and(char::is_alphanumeric); // should lint
7629
//~^ manual_is_variant_and
7730
let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
78-
79-
// Should not lint.
80-
let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true);
81-
let _ = Some(2).map(|x| x % 2 == 0) != foo();
82-
let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true));
83-
let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true);
84-
let _ = mac!(some_map 2) == Some(true);
85-
let _ = mac!(map Some(2)) == Some(true);
8631
}
8732

8833
#[rustfmt::skip]
@@ -96,13 +41,6 @@ fn result_methods() {
9641
});
9742
let _ = res.is_ok_and(|x| x > 1);
9843

99-
let _ = Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
100-
//~^ manual_is_variant_and
101-
let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
102-
//~^ manual_is_variant_and
103-
let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
104-
//~^ manual_is_variant_and
105-
10644
// won't fix because the return type of the closure is not `bool`
10745
let _ = res.map(|x| x + 1).unwrap_or_default();
10846

‎src/tools/clippy/tests/ui/manual_is_variant_and.rs‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,44 +4,6 @@
44
#[macro_use]
55
extern crate option_helpers;
66

7-
struct Foo<T>(T);
8-
9-
impl<T> Foo<T> {
10-
fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> {
11-
Some(f(self.0))
12-
}
13-
}
14-
15-
fn foo() -> Option<bool> {
16-
Some(true)
17-
}
18-
19-
macro_rules! some_true {
20-
() => {
21-
Some(true)
22-
};
23-
}
24-
macro_rules! some_false {
25-
() => {
26-
Some(false)
27-
};
28-
}
29-
30-
macro_rules! mac {
31-
(some $e:expr) => {
32-
Some($e)
33-
};
34-
(some_map $e:expr) => {
35-
Some($e).map(|x| x % 2 == 0)
36-
};
37-
(map $e:expr) => {
38-
$e.map(|x| x % 2 == 0)
39-
};
40-
(eq $a:expr, $b:expr) => {
41-
$a == $b
42-
};
43-
}
44-
457
#[rustfmt::skip]
468
fn option_methods() {
479
let opt = Some(1);
@@ -65,30 +27,13 @@ fn option_methods() {
6527
//~^ manual_is_variant_and
6628
.unwrap_or_default();
6729

68-
let _ = Some(2).map(|x| x % 2 == 0) == Some(true);
69-
//~^ manual_is_variant_and
70-
let _ = Some(2).map(|x| x % 2 == 0) != Some(true);
71-
//~^ manual_is_variant_and
72-
let _ = Some(2).map(|x| x % 2 == 0) == some_true!();
73-
//~^ manual_is_variant_and
74-
let _ = Some(2).map(|x| x % 2 == 0) != some_false!();
75-
//~^ manual_is_variant_and
76-
7730
// won't fix because the return type of the closure is not `bool`
7831
let _ = opt.map(|x| x + 1).unwrap_or_default();
7932

8033
let opt2 = Some('a');
8134
let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
8235
//~^ manual_is_variant_and
8336
let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
84-
85-
// Should not lint.
86-
let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true);
87-
let _ = Some(2).map(|x| x % 2 == 0) != foo();
88-
let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true));
89-
let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true);
90-
let _ = mac!(some_map 2) == Some(true);
91-
let _ = mac!(map Some(2)) == Some(true);
9237
}
9338

9439
#[rustfmt::skip]
@@ -105,13 +50,6 @@ fn result_methods() {
10550
//~^ manual_is_variant_and
10651
.unwrap_or_default();
10752

108-
let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true);
109-
//~^ manual_is_variant_and
110-
let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
111-
//~^ manual_is_variant_and
112-
let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
113-
//~^ manual_is_variant_and
114-
11553
// won't fix because the return type of the closure is not `bool`
11654
let _ = res.map(|x| x + 1).unwrap_or_default();
11755

‎src/tools/clippy/tests/ui/manual_is_variant_and.stderr‎

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

1313
error: called `map(<f>).unwrap_or_default()` on an `Option` value
14-
--> tests/ui/manual_is_variant_and.rs:56:17
14+
--> tests/ui/manual_is_variant_and.rs:18:17
1515
|
1616
LL | let _ = opt.map(|x| {
1717
| _________________^
@@ -30,52 +30,28 @@ LL ~ });
3030
|
3131

3232
error: called `map(<f>).unwrap_or_default()` on an `Option` value
33-
--> tests/ui/manual_is_variant_and.rs:61:17
33+
--> tests/ui/manual_is_variant_and.rs:23:17
3434
|
3535
LL | let _ = opt.map(|x| x > 1).unwrap_or_default();
3636
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)`
3737

3838
error: called `map(<f>).unwrap_or_default()` on an `Option` value
39-
--> tests/ui/manual_is_variant_and.rs:64:10
39+
--> tests/ui/manual_is_variant_and.rs:26:10
4040
|
4141
LL | .map(|x| x > 1)
4242
| __________^
4343
LL | |
4444
LL | | .unwrap_or_default();
4545
| |____________________________^ help: use: `is_some_and(|x| x > 1)`
4646

47-
error: called `.map() == Some()`
48-
--> tests/ui/manual_is_variant_and.rs:68:13
49-
|
50-
LL | let _ = Some(2).map(|x| x % 2 == 0) == Some(true);
51-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)`
52-
53-
error: called `.map() != Some()`
54-
--> tests/ui/manual_is_variant_and.rs:70:13
55-
|
56-
LL | let _ = Some(2).map(|x| x % 2 == 0) != Some(true);
57-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)`
58-
59-
error: called `.map() == Some()`
60-
--> tests/ui/manual_is_variant_and.rs:72:13
61-
|
62-
LL | let _ = Some(2).map(|x| x % 2 == 0) == some_true!();
63-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)`
64-
65-
error: called `.map() != Some()`
66-
--> tests/ui/manual_is_variant_and.rs:74:13
67-
|
68-
LL | let _ = Some(2).map(|x| x % 2 == 0) != some_false!();
69-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)`
70-
7147
error: called `map(<f>).unwrap_or_default()` on an `Option` value
72-
--> tests/ui/manual_is_variant_and.rs:81:18
48+
--> tests/ui/manual_is_variant_and.rs:34:18
7349
|
7450
LL | let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
7551
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)`
7652

7753
error: called `map(<f>).unwrap_or_default()` on a `Result` value
78-
--> tests/ui/manual_is_variant_and.rs:99:17
54+
--> tests/ui/manual_is_variant_and.rs:44:17
7955
|
8056
LL | let _ = res.map(|x| {
8157
| _________________^
@@ -94,37 +70,19 @@ LL ~ });
9470
|
9571

9672
error: called `map(<f>).unwrap_or_default()` on a `Result` value
97-
--> tests/ui/manual_is_variant_and.rs:104:17
73+
--> tests/ui/manual_is_variant_and.rs:49:17
9874
|
9975
LL | let _ = res.map(|x| x > 1)
10076
| _________________^
10177
LL | |
10278
LL | | .unwrap_or_default();
10379
| |____________________________^ help: use: `is_ok_and(|x| x > 1)`
10480

105-
error: called `.map() == Ok()`
106-
--> tests/ui/manual_is_variant_and.rs:108:13
107-
|
108-
LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true);
109-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
110-
111-
error: called `.map() != Ok()`
112-
--> tests/ui/manual_is_variant_and.rs:110:13
113-
|
114-
LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
115-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
116-
117-
error: called `.map() != Ok()`
118-
--> tests/ui/manual_is_variant_and.rs:112:13
119-
|
120-
LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
121-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
122-
12381
error: called `map(<f>).unwrap_or_default()` on a `Result` value
124-
--> tests/ui/manual_is_variant_and.rs:119:18
82+
--> tests/ui/manual_is_variant_and.rs:57:18
12583
|
12684
LL | let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
12785
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)`
12886

129-
error: aborting due to 15 previous errors
87+
error: aborting due to 8 previous errors
13088

0 commit comments

Comments
 (0)