single_match: Don't lint non-exhaustive matches; support tuples

This commit changes the behavior of `single_match` lint.

After that, we won't lint non-exhaustive matches like this:

```rust
match Some(v) {
    Some(a) => println!("${:?}", a),
    None => {},
}
```

The rationale is that, because the type of `a` could be changed, so the
user can get non-exhaustive match after applying the suggested lint (see
rust-lang/rust-clippy#8282 (comment)
for context).

We also will lint `match` constructions with tuples. When we see the
tuples on the both arms, we will check them both at the same time, and
if they form exhaustive match, we could display the warning.

Closes rust-lang#8282

Author: jubnzv

clippy_lints/src/matches.rs

@@ -5,7 +5,7 @@ use clippy_utils::diagnostics::{
 use clippy_utils::macros::{is_panic, root_macro_call};
 use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs};
+use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, peel_mid_ty_refs};
 use clippy_utils::visitors::is_local_used;
 use clippy_utils::{
     get_parent_expr, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs,
@@ -31,7 +31,7 @@ use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::{Span, Spanned};
 use rustc_span::sym;
-use std::cmp::Ordering;
+use std::cmp::{max, Ordering};
 use std::collections::hash_map::Entry;
 
 declare_clippy_lint! {
@@ -741,7 +741,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp
         let ty = cx.typeck_results().expr_ty(ex);
         if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) {
             check_single_match_single_pattern(cx, ex, arms, expr, els);
-            check_single_match_opt_like(cx, ex, arms, expr, ty, els);
+            check_single_match_opt_like(cx, ex, arms, expr, els);
         }
     }
 }
@@ -835,7 +835,6 @@ fn check_single_match_opt_like(
     ex: &Expr<'_>,
     arms: &[Arm<'_>],
     expr: &Expr<'_>,
-    ty: Ty<'_>,
     els: Option<&Expr<'_>>,
 ) {
     // list of candidate `Enum`s we know will never get any more members
@@ -849,25 +848,135 @@ fn check_single_match_opt_like(
         (&paths::RESULT, "Ok"),
     ];
 
-    let path = match arms[1].pat.kind {
-        PatKind::TupleStruct(ref path, inner, _) => {
-            // Contains any non wildcard patterns (e.g., `Err(err)`)?
-            if !inner.iter().all(is_wild) {
-                return;
+    // We want to suggest to exclude an arm that contains only wildcards or forms the ehaustive
+    // match with the second branch.
+    if !contains_only_wilds(arms[1].pat) && !form_exhaustive_tuples(arms[0].pat, arms[1].pat) {
+        return;
+    }
+
+    let mut paths = Vec::new();
+    if !collect_pat_paths(&mut paths, arms[1].pat) {
+        return;
+    }
+
+    let in_candidate_enum = |path: &String| -> bool {
+        for &(_, pat_path) in candidates {
+            if path == pat_path {
+                return true;
+            }
+        }
+        false
+    };
+    if paths.iter().all(in_candidate_enum) {
+        report_single_match_single_pattern(cx, ex, arms, expr, els);
+    }
+}
+
+/// Collects paths from the given paths. Returns true if the given pattern could be simplified,
+/// false otherwise.
+fn collect_pat_paths(acc: &mut Vec<String>, pat: &Pat<'_>) -> bool {
+    match pat.kind {
+        PatKind::Wild => true,
+        PatKind::Tuple(inner, _) => {
+            for p in inner {
+                if !collect_pat_paths(acc, p) {
+                    return false;
+                }
             }
-            rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
+            true
+        },
+        PatKind::TupleStruct(ref path, ..) => {
+            acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
+                s.print_qpath(path, false);
+            }));
+            true
+        },
+        PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => {
+            acc.push(ident.to_string());
+            true
         },
-        PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => ident.to_string(),
         PatKind::Path(ref path) => {
-            rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
+            acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
+                s.print_qpath(path, false);
+            }));
+            true
         },
-        _ => return,
-    };
+        _ => false,
+    }
+}
 
-    for &(ty_path, pat_path) in candidates {
-        if path == *pat_path && match_type(cx, ty, ty_path) {
-            report_single_match_single_pattern(cx, ex, arms, expr, els);
-        }
+/// Returns true if the given arm of pattern matching contains wildcard patterns.
+fn contains_only_wilds(pat: &Pat<'_>) -> bool {
+    match pat.kind {
+        PatKind::Wild => true,
+        PatKind::Tuple(inner, _) | PatKind::TupleStruct(_, inner, ..) => inner.iter().all(contains_only_wilds),
+        _ => false,
+    }
+}
+
+/// Returns true if the given patterns form the tuples that exhaustively matches all possible
+/// parameters.
+///
+/// Here are some examples:
+///
+/// ```
+/// // Doesn't form exhaustive match, because the first arm may contain not only E::V.
+/// match x {
+///     (Some(E::V), _) => todo!(),
+///     (None, _) => {}
+/// }
+///
+/// // Forms exhaustive match, because the patterns cover all possible cases at the same positions.
+/// match x {
+///     (Some(_), _) => todo!(),
+///     (None, _) => {}
+/// }
+/// ```
+fn form_exhaustive_tuples(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 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_span = 0;
+            let mut right_span = 0;
+            for i in 0..len {
+                let mut found_dotdot = false;
+                if i == left_pos {
+                    left_span += 1;
+                    if left_span < len - left_in.len() {
+                        left_pos += 1;
+                    }
+                    found_dotdot = true;
+                }
+                if i == right_pos {
+                    right_span += 1;
+                    if right_span < len - right_in.len() {
+                        right_pos += 1;
+                    }
+                    found_dotdot = true;
+                }
+                if found_dotdot {
+                    continue;
+                }
+                if !contains_only_wilds(&left_in[i - left_span]) && !contains_only_wilds(&right_in[i - right_span]) {
+                    return false;
+                }
+            }
+            true
+        },
+        _ => false,
     }
 }
 

tests/ui/single_match.rs

@@ -145,6 +145,56 @@ fn if_suggestion() {
     };
 }
 
+// See: issue #8282
+fn ranges() {
+    enum E {
+        V,
+    }
+    let x = (Some(E::V), Some(42));
+
+    // don't lint
+    match x {
+        (Some(E::V), _) => {},
+        (None, _) => {},
+    }
+
+    // lint
+    match x {
+        (Some(_), _) => {},
+        (None, _) => {},
+    }
+
+    // lint
+    match x {
+        (Some(E::V), _) => todo!(),
+        (_, _) => {},
+    }
+
+    // lint
+    match (Some(42), Some(E::V), Some(42)) {
+        (.., Some(E::V), _) => {},
+        (..) => {},
+    }
+
+    // don't lint
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (.., Some(E::V), _) => {},
+        (.., None, _) => {},
+    }
+
+    // don't lint
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (Some(E::V), ..) => {},
+        (None, ..) => {},
+    }
+
+    // don't lint
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (_, Some(E::V), ..) => {},
+        (_, None, ..) => {},
+    }
+}
+
 macro_rules! single_match {
     ($num:literal) => {
         match $num {

tests/ui/single_match.stderr

@@ -38,15 +38,6 @@ LL | |         _ => {},
 LL | |     };
    | |_____^ help: try this: `if let (2..=3, 7..=9) = z { dummy() }`
 
-error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
-  --> $DIR/single_match.rs:54:5
-   |
-LL | /     match x {
-LL | |         Some(y) => dummy(),
-LL | |         None => (),
-LL | |     };
-   | |_____^ help: try this: `if let Some(y) = x { dummy() }`
-
 error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> $DIR/single_match.rs:59:5
    |
@@ -128,5 +119,32 @@ LL | |         _ => (),
 LL | |     };
    | |_____^ help: try this: `if let None = x { println!() }`
 
-error: aborting due to 13 previous errors
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match.rs:162:5
+   |
+LL | /     match x {
+LL | |         (Some(_), _) => {},
+LL | |         (None, _) => {},
+LL | |     }
+   | |_____^ help: try this: `if let (Some(_), _) = x {}`
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match.rs:168:5
+   |
+LL | /     match x {
+LL | |         (Some(E::V), _) => todo!(),
+LL | |         (_, _) => {},
+LL | |     }
+   | |_____^ help: try this: `if let (Some(E::V), _) = x { todo!() }`
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match.rs:174:5
+   |
+LL | /     match (Some(42), Some(E::V), Some(42)) {
+LL | |         (.., Some(E::V), _) => {},
+LL | |         (..) => {},
+LL | |     }
+   | |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}`
+
+error: aborting due to 15 previous errors
 

tests/ui/single_match_else.stderr

@@ -19,45 +19,5 @@ LL +         None
 LL +     }
    |
 
-error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
-  --> $DIR/single_match_else.rs:70:5
-   |
-LL | /     match Some(1) {
-LL | |         Some(a) => println!("${:?}", a),
-LL | |         None => {
-LL | |             println!("else block");
-LL | |             return
-LL | |         },
-LL | |     }
-   | |_____^
-   |
-help: try this
-   |
-LL ~     if let Some(a) = Some(1) { println!("${:?}", a) } else {
-LL +         println!("else block");
-LL +         return
-LL +     }
-   |
-
-error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
-  --> $DIR/single_match_else.rs:79:5
-   |
-LL | /     match Some(1) {
-LL | |         Some(a) => println!("${:?}", a),
-LL | |         None => {
-LL | |             println!("else block");
-LL | |             return;
-LL | |         },
-LL | |     }
-   | |_____^
-   |
-help: try this
-   |
-LL ~     if let Some(a) = Some(1) { println!("${:?}", a) } else {
-LL +         println!("else block");
-LL +         return;
-LL +     }
-   |
-
-error: aborting due to 3 previous errors
+error: aborting due to previous error