diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c9877dbffb..261093ead9fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5558,6 +5558,7 @@ Released 2018-09-13 [`unsound_collection_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsound_collection_transmute [`unstable_as_mut_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_mut_slice [`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice +[`unusable_matches_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusable_matches_binding [`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async [`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`unused_format_specs`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_format_specs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 77438b27f900..0cb66d3aff0f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -321,6 +321,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::matches::SINGLE_MATCH_INFO, crate::matches::SINGLE_MATCH_ELSE_INFO, crate::matches::TRY_ERR_INFO, + crate::matches::UNUSABLE_MATCHES_BINDING_INFO, crate::matches::WILDCARD_ENUM_MATCH_ARM_INFO, crate::matches::WILDCARD_IN_OR_PATTERNS_INFO, crate::mem_replace::MEM_REPLACE_OPTION_WITH_NONE_INFO, diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index b5ab94b3a2f8..363ac7b64cf0 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -22,6 +22,7 @@ mod rest_pat_in_fully_bound_struct; mod significant_drop_in_scrutinee; mod single_match; mod try_err; +mod unusable_matches_binding; mod wild_in_or_pats; use clippy_utils::msrvs::{self, Msrv}; @@ -967,6 +968,39 @@ declare_clippy_lint! { "checks for unnecessary guards in match expressions" } +declare_clippy_lint! { + /// ### What it does + /// Checks for redundant and error prone bindings inside `matches!` macro. + /// + /// ### Why is this bad? + /// It could be hard to determine why the compiler complains about an unused variable, + /// which was introduced by the `matches!` macro expansion, also, it could introduce + /// behavior that was not intended by the programmer. + /// + /// ### Example + /// ```rust + /// let data_source = Some(5); + /// let unrelated_data_source = 5; + /// + /// let x = matches!(data_source, unused_binding); + /// let y = matches!(data_source, used_binding if used_binding.is_some()); + /// let z = matches!(data_source, unused_binding if unrelated_data_source > 6); + /// ``` + /// Use instead: + /// ```rust + /// let data_source = Some(5); + /// let unrelated_data_source = 5; + /// + /// let x = true; + /// let y = data_source.is_some(); + /// let z = unrelated_data_source > 6; + /// ``` + #[clippy::version = "1.75.0"] + pub UNUSABLE_MATCHES_BINDING, + correctness, + "checks for unused bindings in `matches!` macro" +} + pub struct Matches { msrv: Msrv, infallible_destructuring_match_linted: bool, @@ -1009,6 +1043,7 @@ impl_lint_pass!(Matches => [ MANUAL_MAP, MANUAL_FILTER, REDUNDANT_GUARDS, + UNUSABLE_MATCHES_BINDING, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -1021,6 +1056,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if let ExprKind::Match(ex, arms, source) = expr.kind { if is_direct_expn_of(expr.span, "matches").is_some() { redundant_pattern_match::check_match(cx, expr, ex, arms); + unusable_matches_binding::check_matches(cx, arms); } if source == MatchSource::Normal && !is_span_match(cx, expr.span) { diff --git a/clippy_lints/src/matches/unusable_matches_binding.rs b/clippy_lints/src/matches/unusable_matches_binding.rs new file mode 100644 index 000000000000..35751be046eb --- /dev/null +++ b/clippy_lints/src/matches/unusable_matches_binding.rs @@ -0,0 +1,36 @@ +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; +use clippy_utils::visitors::is_local_used; +use rustc_hir::{Arm, PatKind}; +use rustc_lint::LateContext; + +use super::UNUSABLE_MATCHES_BINDING; + +pub(crate) fn check_matches<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { + for arm in arms { + if let PatKind::Binding(_, id, _, None) = arm.pat.kind { + if !is_local_used(cx, arm.body, id) { + if let Some(guard) = arm.guard { + span_lint_and_help( + cx, + UNUSABLE_MATCHES_BINDING, + guard.body().span, + "identifier pattern in `matches!` macro always evaluates to the value of the guard", + None, + "if you meant to check predicate, then try changing `matches!` macro into predicate the guard's checking", + ); + } else { + span_lint_and_then( + cx, + UNUSABLE_MATCHES_BINDING, + arm.pat.span, + "identifier pattern in `matches!` macro always evaluates to true", + |diag| { + diag.note("the identifier pattern matches any value and creates an unusable binding in the process") + .help("if you meant to compare two values, use `x == y` or `discriminant(x) == discriminant(y)`"); + }, + ); + } + } + } + } +} diff --git a/tests/ui/unusable_matches_binding.rs b/tests/ui/unusable_matches_binding.rs new file mode 100644 index 000000000000..dbd24cbb29db --- /dev/null +++ b/tests/ui/unusable_matches_binding.rs @@ -0,0 +1,22 @@ +#![warn(clippy::unusable_matches_binding)] + +#[derive(Clone, Copy)] +struct TestingStructure(i32); + +impl TestingStructure { + fn is_valid(&self) -> bool { + self.0 > 5 + } +} + +fn main() { + let matching_data_source = TestingStructure(5); + let unrelated_data = 5; + + let _ = matches!(matching_data_source, TestingStructure(4)); + + let _ = matches!(matching_data_source, unusable_binding); + let _ = matches!(matching_data_source, used_binding if used_binding.is_valid()); + + let _ = matches!(matching_data_source, unusable_binding if unrelated_data < 4); +} diff --git a/tests/ui/unusable_matches_binding.stderr b/tests/ui/unusable_matches_binding.stderr new file mode 100644 index 000000000000..6da27321aec5 --- /dev/null +++ b/tests/ui/unusable_matches_binding.stderr @@ -0,0 +1,29 @@ +error: identifier pattern in `matches!` macro always evaluates to true + --> $DIR/unusable_matches_binding.rs:18:44 + | +LL | let _ = matches!(matching_data_source, unusable_binding); + | ^^^^^^^^^^^^^^^^ + | + = note: the identifier pattern matches any value and creates an unusable binding in the process + = help: if you meant to compare two values, use `x == y` or `discriminant(x) == discriminant(y)` + = note: `-D clippy::unusable-matches-binding` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unusable_matches_binding)]` + +error: identifier pattern in `matches!` macro always evaluates to the value of the guard + --> $DIR/unusable_matches_binding.rs:19:60 + | +LL | let _ = matches!(matching_data_source, used_binding if used_binding.is_valid()); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: if you meant to check predicate, then try changing `matches!` macro into predicate the guard's checking + +error: identifier pattern in `matches!` macro always evaluates to the value of the guard + --> $DIR/unusable_matches_binding.rs:21:64 + | +LL | let _ = matches!(matching_data_source, unusable_binding if unrelated_data < 4); + | ^^^^^^^^^^^^^^^^^^ + | + = help: if you meant to check predicate, then try changing `matches!` macro into predicate the guard's checking + +error: aborting due to 3 previous errors +