Skip to content

[WIP] New lint: matches_instead_of_eq #14820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6016,6 +6016,7 @@ Released 2018-09-13
[`match_str_case_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_str_case_mismatch
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
[`matches_instead_of_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#matches_instead_of_eq
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
[`maybe_misused_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_misused_cfg
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::matches::MANUAL_OK_ERR_INFO,
crate::matches::MANUAL_UNWRAP_OR_INFO,
crate::matches::MANUAL_UNWRAP_OR_DEFAULT_INFO,
crate::matches::MATCHES_INSTEAD_OF_EQ_INFO,
crate::matches::MATCH_AS_REF_INFO,
crate::matches::MATCH_BOOL_INFO,
crate::matches::MATCH_LIKE_MATCHES_MACRO_INFO,
Expand Down
39 changes: 39 additions & 0 deletions clippy_lints/src/matches/matches_instead_of_eq.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::macros::HirNode;
use clippy_utils::sym;
use clippy_utils::ty::implements_trait;
use rustc_hir::PatKind;
use rustc_lint::LateContext;

use super::MATCHES_INSTEAD_OF_EQ;

// TODO: Adjust the parameters as necessary
pub(super) fn check(
cx: &LateContext<'_>,
expr: &rustc_hir::Expr<'_>,
ex: &rustc_hir::Expr<'_>,
arms: &[rustc_hir::Arm<'_>],
) {
let Some(partialeq) = cx.tcx.get_diagnostic_item(sym::PartialEq) else {
return;
};
let expr_type = cx.typeck_results().expr_ty(ex);
if !implements_trait(cx, expr_type, partialeq, &[expr_type.into()]) {
return;
}
if arms.into_iter().all(|arm| {
!matches!(
arm.pat.kind,
PatKind::Or(..) | PatKind::Struct(..) | PatKind::TupleStruct(..)
)
}) {
span_lint_and_help(
cx,
MATCHES_INSTEAD_OF_EQ,
expr.span(),
"This expression can be replaced with a == comparison",
None,
"This type implements PartialEq, so you can use == instead of matches!()",
);
}
}
30 changes: 30 additions & 0 deletions clippy_lints/src/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod match_single_binding;
mod match_str_case_mismatch;
mod match_wild_enum;
mod match_wild_err_arm;
mod matches_instead_of_eq;
mod needless_match;
mod overlapping_arms;
mod redundant_guards;
Expand Down Expand Up @@ -1006,6 +1007,33 @@ declare_clippy_lint! {
"find manual implementations of `.ok()` or `.err()` on `Result`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the use of `matches!` with enums that implement `PartialEq`.
///
/// ### Why is this bad?
/// Using `matches!` instead of `==` or `!=` makes the code less readable.
///
/// ### Example
/// ```no_run
/// let x = Foo::Bar;
/// if matches!(x, Foo::Bar) {
/// // ...
/// }
/// ```
/// Use instead:
/// ```no_run
/// let x = Foo::Bar;
/// if x == Foo::Bar {
/// // ...
/// }
/// ```
#[clippy::version = "1.88.0"]
pub MATCHES_INSTEAD_OF_EQ,
pedantic,
"default lint description"
}

pub struct Matches {
msrv: Msrv,
infallible_destructuring_match_linted: bool,
Expand Down Expand Up @@ -1048,6 +1076,7 @@ impl_lint_pass!(Matches => [
MANUAL_FILTER,
REDUNDANT_GUARDS,
MANUAL_OK_ERR,
MATCHES_INSTEAD_OF_EQ,
]);

impl<'tcx> LateLintPass<'tcx> for Matches {
Expand All @@ -1064,6 +1093,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
{
redundant_pattern_match::check_match(cx, expr, ex, arms);
redundant_pattern_match::check_matches_true(cx, expr, arm, ex);
matches_instead_of_eq::check(cx, expr, ex, arms);
}

if source == MatchSource::Normal && !is_span_match(cx, expr.span) {
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/matches_instead_of_eq.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#![warn(clippy::matches_instead_of_eq)]

enum Foo {
Bar,
Baz,
}

#[derive(PartialEq)]
enum EvenOdd {
Even,
Odd,
Unknown,
}

#[derive(PartialEq)]
enum Foo2 {
Bar(u8, u8),
Baz { x: u8, y: u8 },
}

fn main() {
let x = Foo::Bar;
let val = matches!(x, Foo::Bar); // No error as Foo does not implement PartialEq

let x = EvenOdd::Even;
let val = matches!(x, EvenOdd::Even);
//~^ matches_instead_of_eq

let x = EvenOdd::Odd;
let val = matches!(x, EvenOdd::Even | EvenOdd::Odd); // No error

let x = Foo2::Bar(1, 2);
let val = matches!(x, Foo2::Bar(_, _)); // No error

let x = Foo2::Baz { x: 1, y: 2 };
let val = matches!(x, Foo2::Baz { .. }); // No Error

let val = matches!(x, Foo2::Bar(..) | Foo2::Baz { .. }); // No error
}
13 changes: 13 additions & 0 deletions tests/ui/matches_instead_of_eq.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: This expression can be replaced with a == comparison
--> tests/ui/matches_instead_of_eq.rs:26:15
|
LL | let val = matches!(x, EvenOdd::Even);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: This type implements PartialEq, so you can use == instead of matches!()
= note: `-D clippy::matches-instead-of-eq` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::matches_instead_of_eq)]`
= note: this error originates in the macro `matches` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

Loading