single_match: Lint structs and tuple structs

clippy_lints/src/matches/single_match.rs

@@ -1,14 +1,13 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{expr_block, snippet};
-use clippy_utils::ty::{implements_trait, match_type, peel_mid_ty_refs};
-use clippy_utils::{
-    is_lint_allowed, is_unit_expr, is_wild, paths, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs,
-};
+use clippy_utils::ty::{implements_trait, peel_mid_ty_refs};
+use clippy_utils::{is_lint_allowed, is_unit_expr, is_wild, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs};
 use core::cmp::max;
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Pat, PatKind};
+use rustc_hir::{Arm, Block, Expr, ExprKind, Pat, PatField, PatKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, Ty};
+use rustc_span::sym;
 
 use super::{MATCH_BOOL, SINGLE_MATCH, SINGLE_MATCH_ELSE};
 
@@ -140,74 +139,133 @@ fn check_opt_like<'a>(
     ty: Ty<'a>,
     els: Option<&Expr<'_>>,
 ) {
-    // list of candidate `Enum`s we know will never get any more members
-    let candidates = &[
-        (&paths::COW, "Borrowed"),
-        (&paths::COW, "Cow::Borrowed"),
-        (&paths::COW, "Cow::Owned"),
-        (&paths::COW, "Owned"),
-        (&paths::OPTION, "None"),
-        (&paths::RESULT, "Err"),
-        (&paths::RESULT, "Ok"),
-    ];
+    if check_exhaustive(cx, arms[0].pat, arms[1].pat, ty) {
+        report_single_pattern(cx, ex, arms, expr, els);
+    }
+}
 
-    // We want to suggest to exclude an arm that contains only wildcards or forms the exhaustive
-    // match with the second branch, without enum variants in matches.
-    if !contains_only_wilds(arms[1].pat) && !form_exhaustive_matches(arms[0].pat, arms[1].pat) {
-        return;
+/// Resturns true if the given type is one of the standard `Enum`s we know will never get any more
+/// members.
+fn is_known_enum(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    if let Some(adt) = ty.ty_adt_def() {
+        return matches!(
+            cx.tcx.get_diagnostic_name(adt.did),
+            Some(sym::Cow | sym::Option | sym::Result)
+        );
     }
+    false
+}
 
-    let mut paths_and_types = Vec::new();
-    if !collect_pat_paths(&mut paths_and_types, cx, arms[1].pat, ty) {
-        return;
+fn contains_only_known_enums(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
+    match pat.kind {
+        PatKind::Path(..) => is_known_enum(cx, cx.typeck_results().pat_ty(pat)),
+        PatKind::TupleStruct(..) | PatKind::Struct(..) => {
+            let ty = cx.typeck_results().pat_ty(pat);
+            (contains_only_wilds(pat) || contains_single_binding(pat)) && is_known_enum(cx, ty)
+        },
+        _ => false,
     }
+}
 
-    let in_candidate_enum = |path_info: &(String, Ty<'_>)| -> bool {
-        let (path, ty) = path_info;
-        for &(ty_path, pat_path) in candidates {
-            if path == pat_path && match_type(cx, *ty, ty_path) {
-                return true;
-            }
-        }
-        false
-    };
-    if paths_and_types.iter().all(in_candidate_enum) {
-        report_single_pattern(cx, ex, arms, expr, els);
+fn peel_subpatterns<'a>(pat: &'a Pat<'a>) -> &'a Pat<'a> {
+    if let PatKind::Binding(.., Some(sub)) = pat.kind {
+        return peel_subpatterns(sub);
     }
+    pat
 }
 
-/// Collects paths and their types from the given patterns. Returns true if the given pattern could
-/// be simplified, false otherwise.
-fn collect_pat_paths<'a>(acc: &mut Vec<(String, Ty<'a>)>, cx: &LateContext<'a>, pat: &Pat<'_>, ty: Ty<'a>) -> bool {
-    match pat.kind {
-        PatKind::Wild => true,
-        PatKind::Tuple(inner, _) => inner.iter().all(|p| {
-            let p_ty = cx.typeck_results().pat_ty(p);
-            collect_pat_paths(acc, cx, p, p_ty)
-        }),
-        PatKind::TupleStruct(ref path, ..) => {
-            let path = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
-                s.print_qpath(path, false);
-            });
-            acc.push((path, ty));
-            true
+/// Returns false if the patterns exhaustively match an enum.
+fn check_exhaustive<'a>(cx: &LateContext<'a>, left: &Pat<'_>, right: &Pat<'_>, ty: Ty<'a>) -> bool {
+    let left = peel_subpatterns(left);
+    let right = peel_subpatterns(right);
+    match (&left.kind, &right.kind) {
+        (PatKind::Wild, _) | (_, PatKind::Wild) => true,
+        (PatKind::Struct(_, left_fields, _), PatKind::Struct(_, right_fields, _)) => {
+            check_exhaustive_structs(cx, left_fields, right_fields)
         },
-        PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => {
-            acc.push((ident.to_string(), ty));
-            true
+        (PatKind::Tuple(left_in, left_pos), PatKind::Tuple(right_in, right_pos)) => {
+            check_exhaustive_tuples(cx, left_in, left_pos, right_in, right_pos)
         },
-        PatKind::Path(ref path) => {
-            let path = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
-                s.print_qpath(path, false);
-            });
-            acc.push((path, ty));
-            true
+        (PatKind::TupleStruct(_, left_in, left_pos), PatKind::TupleStruct(_, right_in, right_pos))
+            if contains_only_wilds(right) && contains_only_known_enums(cx, left) =>
+        {
+            check_exhaustive_tuples(cx, left_in, left_pos, right_in, right_pos)
         },
+        (PatKind::Binding(.., None) | PatKind::Path(_), _) if contains_only_wilds(right) => is_known_enum(cx, ty),
         _ => false,
     }
 }
 
-/// Returns true if the given arm of pattern matching contains wildcard patterns.
+fn could_be_simplified(cx: &LateContext<'_>, left_pat: &Pat<'_>, right_pat: &Pat<'_>) -> bool {
+    contains_only_wilds(left_pat)
+        || contains_only_wilds(right_pat)
+        || contains_only_known_enums(cx, left_pat) && contains_only_known_enums(cx, right_pat)
+}
+
+/// Returns true if two tuples that contain patterns `left_in` and `right_in` and `..` operators at
+/// positions `left_pos` and `right_pos` form exhaustive match. This means, that two elements of
+/// these tuples located at the same positions must have at least one wildcard (`_`) pattern.
+fn check_exhaustive_tuples<'a>(
+    cx: &LateContext<'_>,
+    left_in: &'a [Pat<'a>],
+    left_pos: &Option<usize>,
+    right_in: &'a [Pat<'a>],
+    right_pos: &Option<usize>,
+) -> bool {
+    // We don't actually know the position and the presence of the `..` (dotdot) operator
+    // in the arms, so we need to evaluate the correct offsets here in order to iterate in
+    // both arms at the same time.
+    let len = max(
+        left_in.len() + {
+            if left_pos.is_some() { 1 } else { 0 }
+        },
+        right_in.len() + {
+            if right_pos.is_some() { 1 } else { 0 }
+        },
+    );
+    let mut left_pos = left_pos.unwrap_or(usize::MAX);
+    let mut right_pos = right_pos.unwrap_or(usize::MAX);
+    let mut left_dot_space = 0;
+    let mut right_dot_space = 0;
+    for i in 0..len {
+        let mut found_dotdot = false;
+        if i == left_pos {
+            left_dot_space += 1;
+            if left_dot_space < len - left_in.len() {
+                left_pos += 1;
+            }
+            found_dotdot = true;
+        }
+        if i == right_pos {
+            right_dot_space += 1;
+            if right_dot_space < len - right_in.len() {
+                right_pos += 1;
+            }
+            found_dotdot = true;
+        }
+        if found_dotdot {
+            continue;
+        }
+
+        if !could_be_simplified(cx, &left_in[i - left_dot_space], &right_in[i - right_dot_space]) {
+            return false;
+        }
+    }
+    true
+}
+
+fn check_exhaustive_structs<'a>(
+    cx: &LateContext<'_>,
+    left_fields: &'a [PatField<'a>],
+    right_fields: &'a [PatField<'a>],
+) -> bool {
+    left_fields
+        .iter()
+        .zip(right_fields.iter())
+        .all(|(left_field, right_field)| could_be_simplified(cx, left_field.pat, right_field.pat))
+}
+
+/// Returns true if the given arm of pattern matching contains only wildcard patterns.
 fn contains_only_wilds(pat: &Pat<'_>) -> bool {
     match pat.kind {
         PatKind::Wild => true,
@@ -216,54 +274,10 @@ fn contains_only_wilds(pat: &Pat<'_>) -> bool {
     }
 }
 
-/// Returns true if the given patterns forms only exhaustive matches that don't contain enum
-/// patterns without a wildcard.
-fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool {
-    match (&left.kind, &right.kind) {
-        (PatKind::Wild, _) | (_, PatKind::Wild) => true,
-        (PatKind::Tuple(left_in, left_pos), PatKind::Tuple(right_in, right_pos)) => {
-            // We don't actually know the position and the presence of the `..` (dotdot) operator
-            // in the arms, so we need to evaluate the correct offsets here in order to iterate in
-            // both arms at the same time.
-            let len = max(
-                left_in.len() + {
-                    if left_pos.is_some() { 1 } else { 0 }
-                },
-                right_in.len() + {
-                    if right_pos.is_some() { 1 } else { 0 }
-                },
-            );
-            let mut left_pos = left_pos.unwrap_or(usize::MAX);
-            let mut right_pos = right_pos.unwrap_or(usize::MAX);
-            let mut left_dot_space = 0;
-            let mut right_dot_space = 0;
-            for i in 0..len {
-                let mut found_dotdot = false;
-                if i == left_pos {
-                    left_dot_space += 1;
-                    if left_dot_space < len - left_in.len() {
-                        left_pos += 1;
-                    }
-                    found_dotdot = true;
-                }
-                if i == right_pos {
-                    right_dot_space += 1;
-                    if right_dot_space < len - right_in.len() {
-                        right_pos += 1;
-                    }
-                    found_dotdot = true;
-                }
-                if found_dotdot {
-                    continue;
-                }
-                if !contains_only_wilds(&left_in[i - left_dot_space])
-                    && !contains_only_wilds(&right_in[i - right_dot_space])
-                {
-                    return false;
-                }
-            }
-            true
-        },
+fn contains_single_binding(pat: &Pat<'_>) -> bool {
+    match pat.kind {
+        PatKind::Binding(.., None) => true,
+        PatKind::TupleStruct(_, inner, ..) => inner.iter().all(contains_single_binding),
         _ => false,
     }
 }

tests/ui/patterns.fixed

@@ -2,12 +2,16 @@
 #![allow(unused)]
 #![warn(clippy::all)]
 
+fn dummy() {
+    dbg!("test");
+}
+
 fn main() {
     let v = Some(true);
     let s = [0, 1, 2, 3, 4];
     match v {
-        Some(x) => (),
-        y => (),
+        Some(x) => dummy(),
+        y => dummy(),
     }
     match v {
         Some(x) => (),

tests/ui/patterns.rs

@@ -2,12 +2,16 @@
 #![allow(unused)]
 #![warn(clippy::all)]
 
+fn dummy() {
+    dbg!("test");
+}
+
 fn main() {
     let v = Some(true);
     let s = [0, 1, 2, 3, 4];
     match v {
-        Some(x) => (),
-        y @ _ => (),
+        Some(x) => dummy(),
+        y @ _ => dummy(),
     }
     match v {
         Some(x) => (),

tests/ui/patterns.stderr

@@ -1,19 +1,19 @@
 error: the `y @ _` pattern can be written as just `y`
-  --> $DIR/patterns.rs:10:9
+  --> $DIR/patterns.rs:14:9
    |
-LL |         y @ _ => (),
+LL |         y @ _ => dummy(),
    |         ^^^^^ help: try: `y`
    |
    = note: `-D clippy::redundant-pattern` implied by `-D warnings`
 
 error: the `x @ _` pattern can be written as just `x`
-  --> $DIR/patterns.rs:25:9
+  --> $DIR/patterns.rs:29:9
    |
 LL |         ref mut x @ _ => {
    |         ^^^^^^^^^^^^^ help: try: `ref mut x`
 
 error: the `x @ _` pattern can be written as just `x`
-  --> $DIR/patterns.rs:33:9
+  --> $DIR/patterns.rs:37:9
    |
 LL |         ref x @ _ => println!("vec: {:?}", x),
    |         ^^^^^^^^^ help: try: `ref x`