diff options
| author | Philipp Krones <hello@philkrones.com> | 2025-07-31 17:35:23 +0200 |
|---|---|---|
| committer | Mark Rousskov <mark.simulacrum@gmail.com> | 2025-08-02 07:46:24 -0400 |
| commit | c1d016f6cd47968d6a86e576e0c4a67412a4aaf9 (patch) | |
| tree | 2243f23d83810934fc2a4139b72f1c8dd052b2a8 | |
| parent | 4c75f2749748453f06bdf65d547363c36fe5fd9e (diff) | |
| download | rust-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.
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 |
