about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2025-07-31 17:35:23 +0200
committerMark Rousskov <mark.simulacrum@gmail.com>2025-08-02 07:46:24 -0400
commitc1d016f6cd47968d6a86e576e0c4a67412a4aaf9 (patch)
tree2243f23d83810934fc2a4139b72f1c8dd052b2a8
parent4c75f2749748453f06bdf65d547363c36fe5fd9e (diff)
downloadrust-c1d016f6cd47968d6a86e576e0c4a67412a4aaf9.tar.gz
rust-c1d016f6cd47968d6a86e576e0c4a67412a4aaf9.zip
Revert "Extend `manual_is_variant_and lint` to check for boolean map comparisons (#14646)"
This reverts commit 551870df96213c423c94a012c1981fc0cdc06fc2, reversing
changes made to 3927a61a546b0f134b3790d66ef73f1960e8bc8b.
-rw-r--r--src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs70
-rw-r--r--src/tools/clippy/clippy_lints/src/methods/mod.rs1
-rw-r--r--src/tools/clippy/tests/ui/manual_is_variant_and.fixed62
-rw-r--r--src/tools/clippy/tests/ui/manual_is_variant_and.rs62
-rw-r--r--src/tools/clippy/tests/ui/manual_is_variant_and.stderr60
5 files changed, 15 insertions, 240 deletions
diff --git a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs
index 4a61c223d2c..40aad03960c 100644
--- a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs
+++ b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs
@@ -1,22 +1,18 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::get_parent_expr;
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::source::{snippet, snippet_opt};
+use clippy_utils::source::snippet;
 use clippy_utils::ty::is_type_diagnostic_item;
 use rustc_errors::Applicability;
-use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
-use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
 use rustc_lint::LateContext;
-use rustc_middle::ty;
-use rustc_span::{BytePos, Span, sym};
+use rustc_span::{Span, sym};
 
 use super::MANUAL_IS_VARIANT_AND;
 
-pub(super) fn check(
+pub(super) fn check<'tcx>(
     cx: &LateContext<'_>,
-    expr: &Expr<'_>,
-    map_recv: &Expr<'_>,
-    map_arg: &Expr<'_>,
+    expr: &'tcx rustc_hir::Expr<'_>,
+    map_recv: &'tcx rustc_hir::Expr<'_>,
+    map_arg: &'tcx rustc_hir::Expr<'_>,
     map_span: Span,
     msrv: Msrv,
 ) {
@@ -61,57 +57,3 @@ pub(super) fn check(
         Applicability::MachineApplicable,
     );
 }
-
-fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) {
-    if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo()))
-        && let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3)))
-    {
-        span_lint_and_sugg(
-            cx,
-            MANUAL_IS_VARIANT_AND,
-            parent.span,
-            format!(
-                "called `.map() {}= {}()`",
-                if op == BinOpKind::Eq { '=' } else { '!' },
-                if is_option { "Some" } else { "Ok" },
-            ),
-            "use",
-            if is_option && op == BinOpKind::Ne {
-                format!("{before_map_snippet}is_none_or{after_map_snippet}",)
-            } else {
-                format!(
-                    "{}{before_map_snippet}{}{after_map_snippet}",
-                    if op == BinOpKind::Eq { "" } else { "!" },
-                    if is_option { "is_some_and" } else { "is_ok_and" },
-                )
-            },
-            Applicability::MachineApplicable,
-        );
-    }
-}
-
-pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) {
-    if let Some(parent_expr) = get_parent_expr(cx, expr)
-        && let ExprKind::Binary(op, left, right) = parent_expr.kind
-        && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne)
-        && op.span.eq_ctxt(expr.span)
-    {
-        // Check `left` and `right` expression in any order, and for `Option` and `Result`
-        for (expr1, expr2) in [(left, right), (right, left)] {
-            for item in [sym::Option, sym::Result] {
-                if let ExprKind::Call(call, ..) = expr1.kind
-                    && let ExprKind::Path(QPath::Resolved(_, path)) = call.kind
-                    && let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res
-                    && let ty = cx.typeck_results().expr_ty(expr1)
-                    && let ty::Adt(adt, args) = ty.kind()
-                    && cx.tcx.is_diagnostic_item(item, adt.did())
-                    && args.type_at(0).is_bool()
-                    && let ExprKind::MethodCall(_, recv, _, span) = expr2.kind
-                    && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item)
-                {
-                    return emit_lint(cx, op.node, parent_expr, span, item == sym::Option);
-                }
-            }
-        }
-    }
-}
diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs
index 347960e0003..6d3f59520a2 100644
--- a/src/tools/clippy/clippy_lints/src/methods/mod.rs
+++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs
@@ -5242,7 +5242,6 @@ impl Methods {
                         unused_enumerate_index::check(cx, expr, recv, m_arg);
                         map_clone::check(cx, expr, recv, m_arg, self.msrv);
                         map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span);
-                        manual_is_variant_and::check_map(cx, expr);
                         match method_call(recv) {
                             Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => {
                                 iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv);
diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed
index 18a72188ab5..c9c184561dd 100644
--- a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed
+++ b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed
@@ -4,44 +4,6 @@
 #[macro_use]
 extern crate option_helpers;
 
-struct Foo<T>(T);
-
-impl<T> Foo<T> {
-    fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> {
-        Some(f(self.0))
-    }
-}
-
-fn foo() -> Option<bool> {
-    Some(true)
-}
-
-macro_rules! some_true {
-    () => {
-        Some(true)
-    };
-}
-macro_rules! some_false {
-    () => {
-        Some(false)
-    };
-}
-
-macro_rules! mac {
-    (some $e:expr) => {
-        Some($e)
-    };
-    (some_map $e:expr) => {
-        Some($e).map(|x| x % 2 == 0)
-    };
-    (map $e:expr) => {
-        $e.map(|x| x % 2 == 0)
-    };
-    (eq $a:expr, $b:expr) => {
-        $a == $b
-    };
-}
-
 #[rustfmt::skip]
 fn option_methods() {
     let opt = Some(1);
@@ -59,15 +21,6 @@ fn option_methods() {
     let _ = opt
         .is_some_and(|x| x > 1);
 
-    let _ = Some(2).is_some_and(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-    let _ = Some(2).is_none_or(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-    let _ = Some(2).is_some_and(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-    let _ = Some(2).is_none_or(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-
     // won't fix because the return type of the closure is not `bool`
     let _ = opt.map(|x| x + 1).unwrap_or_default();
 
@@ -75,14 +28,6 @@ fn option_methods() {
     let _ = opt2.is_some_and(char::is_alphanumeric); // should lint
     //~^ manual_is_variant_and
     let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
-
-    // Should not lint.
-    let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true);
-    let _ = Some(2).map(|x| x % 2 == 0) != foo();
-    let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true));
-    let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true);
-    let _ = mac!(some_map 2) == Some(true);
-    let _ = mac!(map Some(2)) == Some(true);
 }
 
 #[rustfmt::skip]
@@ -96,13 +41,6 @@ fn result_methods() {
     });
     let _ = res.is_ok_and(|x| x > 1);
 
-    let _ = Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-    let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-    let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0);
-    //~^ manual_is_variant_and
-
     // won't fix because the return type of the closure is not `bool`
     let _ = res.map(|x| x + 1).unwrap_or_default();
 
diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.rs b/src/tools/clippy/tests/ui/manual_is_variant_and.rs
index a92f7c04369..52c7b56804c 100644
--- a/src/tools/clippy/tests/ui/manual_is_variant_and.rs
+++ b/src/tools/clippy/tests/ui/manual_is_variant_and.rs
@@ -4,44 +4,6 @@
 #[macro_use]
 extern crate option_helpers;
 
-struct Foo<T>(T);
-
-impl<T> Foo<T> {
-    fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> {
-        Some(f(self.0))
-    }
-}
-
-fn foo() -> Option<bool> {
-    Some(true)
-}
-
-macro_rules! some_true {
-    () => {
-        Some(true)
-    };
-}
-macro_rules! some_false {
-    () => {
-        Some(false)
-    };
-}
-
-macro_rules! mac {
-    (some $e:expr) => {
-        Some($e)
-    };
-    (some_map $e:expr) => {
-        Some($e).map(|x| x % 2 == 0)
-    };
-    (map $e:expr) => {
-        $e.map(|x| x % 2 == 0)
-    };
-    (eq $a:expr, $b:expr) => {
-        $a == $b
-    };
-}
-
 #[rustfmt::skip]
 fn option_methods() {
     let opt = Some(1);
@@ -65,15 +27,6 @@ fn option_methods() {
         //~^ manual_is_variant_and
         .unwrap_or_default();
 
-    let _ = Some(2).map(|x| x % 2 == 0) == Some(true);
-    //~^ manual_is_variant_and
-    let _ = Some(2).map(|x| x % 2 == 0) != Some(true);
-    //~^ manual_is_variant_and
-    let _ = Some(2).map(|x| x % 2 == 0) == some_true!();
-    //~^ manual_is_variant_and
-    let _ = Some(2).map(|x| x % 2 == 0) != some_false!();
-    //~^ manual_is_variant_and
-
     // won't fix because the return type of the closure is not `bool`
     let _ = opt.map(|x| x + 1).unwrap_or_default();
 
@@ -81,14 +34,6 @@ fn option_methods() {
     let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
     //~^ manual_is_variant_and
     let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
-
-    // Should not lint.
-    let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true);
-    let _ = Some(2).map(|x| x % 2 == 0) != foo();
-    let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true));
-    let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true);
-    let _ = mac!(some_map 2) == Some(true);
-    let _ = mac!(map Some(2)) == Some(true);
 }
 
 #[rustfmt::skip]
@@ -105,13 +50,6 @@ fn result_methods() {
     //~^ manual_is_variant_and
         .unwrap_or_default();
 
-    let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true);
-    //~^ manual_is_variant_and
-    let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
-    //~^ manual_is_variant_and
-    let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
-    //~^ manual_is_variant_and
-
     // won't fix because the return type of the closure is not `bool`
     let _ = res.map(|x| x + 1).unwrap_or_default();
 
diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr
index 1fb437a8bc7..a4fa500580d 100644
--- a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr
+++ b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr
@@ -1,5 +1,5 @@
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:51:17
+  --> tests/ui/manual_is_variant_and.rs:13:17
    |
 LL |       let _ = opt.map(|x| x > 1)
    |  _________________^
@@ -11,7 +11,7 @@ LL | |         .unwrap_or_default();
    = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]`
 
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:56:17
+  --> tests/ui/manual_is_variant_and.rs:18:17
    |
 LL |       let _ = opt.map(|x| {
    |  _________________^
@@ -30,13 +30,13 @@ LL ~     });
    |
 
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:61:17
+  --> tests/ui/manual_is_variant_and.rs:23:17
    |
 LL |     let _ = opt.map(|x| x > 1).unwrap_or_default();
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)`
 
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:64:10
+  --> tests/ui/manual_is_variant_and.rs:26:10
    |
 LL |           .map(|x| x > 1)
    |  __________^
@@ -44,38 +44,14 @@ LL | |
 LL | |         .unwrap_or_default();
    | |____________________________^ help: use: `is_some_and(|x| x > 1)`
 
-error: called `.map() == Some()`
-  --> tests/ui/manual_is_variant_and.rs:68:13
-   |
-LL |     let _ = Some(2).map(|x| x % 2 == 0) == Some(true);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)`
-
-error: called `.map() != Some()`
-  --> tests/ui/manual_is_variant_and.rs:70:13
-   |
-LL |     let _ = Some(2).map(|x| x % 2 == 0) != Some(true);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)`
-
-error: called `.map() == Some()`
-  --> tests/ui/manual_is_variant_and.rs:72:13
-   |
-LL |     let _ = Some(2).map(|x| x % 2 == 0) == some_true!();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)`
-
-error: called `.map() != Some()`
-  --> tests/ui/manual_is_variant_and.rs:74:13
-   |
-LL |     let _ = Some(2).map(|x| x % 2 == 0) != some_false!();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)`
-
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:81:18
+  --> tests/ui/manual_is_variant_and.rs:34:18
    |
 LL |     let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)`
 
 error: called `map(<f>).unwrap_or_default()` on a `Result` value
-  --> tests/ui/manual_is_variant_and.rs:99:17
+  --> tests/ui/manual_is_variant_and.rs:44:17
    |
 LL |       let _ = res.map(|x| {
    |  _________________^
@@ -94,7 +70,7 @@ LL ~     });
    |
 
 error: called `map(<f>).unwrap_or_default()` on a `Result` value
-  --> tests/ui/manual_is_variant_and.rs:104:17
+  --> tests/ui/manual_is_variant_and.rs:49:17
    |
 LL |       let _ = res.map(|x| x > 1)
    |  _________________^
@@ -102,29 +78,11 @@ LL | |
 LL | |         .unwrap_or_default();
    | |____________________________^ help: use: `is_ok_and(|x| x > 1)`
 
-error: called `.map() == Ok()`
-  --> tests/ui/manual_is_variant_and.rs:108:13
-   |
-LL |     let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
-
-error: called `.map() != Ok()`
-  --> tests/ui/manual_is_variant_and.rs:110:13
-   |
-LL |     let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
-
-error: called `.map() != Ok()`
-  --> tests/ui/manual_is_variant_and.rs:112:13
-   |
-LL |     let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)`
-
 error: called `map(<f>).unwrap_or_default()` on a `Result` value
-  --> tests/ui/manual_is_variant_and.rs:119:18
+  --> tests/ui/manual_is_variant_and.rs:57:18
    |
 LL |     let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)`
 
-error: aborting due to 15 previous errors
+error: aborting due to 8 previous errors