Merge #3960

3960: ellipsis in tuple patterns r=JoshMcguigan a=JoshMcguigan

This PR lowers ellipsis in tuple patterns. It fixes a bug in the way ellipsis were previously lowered (by replacing the ellipsis with a single `Pat::Wild` no matter how many items the `..` was taking the place of).

It also uses this new information to properly handle `..` in tuple struct patterns when perform match statement exhaustiveness checks.

While this PR provides the building blocks for match statement exhaustiveness checks for tuples, there are some additional challenges there, so that is still unimplemented (unlike tuple structs).

Co-authored-by: Josh Mcguigan <[email protected]>

1 files changed, 113 insertions, 47 deletions

diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs
index c482cf619..a64be9848 100644
--- a/crates/ra_hir_ty/src/_match.rs
+++ b/crates/ra_hir_ty/src/_match.rs
@@ -289,7 +289,7 @@ impl PatStack {
         Self::from_slice(&self.0[1..])
     }
 
-    fn replace_head_with(&self, pat_ids: &[PatId]) -> PatStack {
+    fn replace_head_with<T: Into<PatIdOrWild> + Copy>(&self, pat_ids: &[T]) -> PatStack {
         let mut patterns: PatStackInner = smallvec![];
         for pat in pat_ids {
             patterns.push((*pat).into());
@@ -320,12 +320,14 @@ impl PatStack {
         constructor: &Constructor,
     ) -> MatchCheckResult<Option<PatStack>> {
         let result = match (self.head().as_pat(cx), constructor) {
-            (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => {
-                debug_assert_eq!(
-                    pat_ids.len(),
-                    *arity,
-                    "we type check before calling this code, so we should never hit this case",
-                );
+            (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => {
+                if ellipsis.is_some() {
+                    // If there are ellipsis here, we should add the correct number of
+                    // Pat::Wild patterns to `pat_ids`. We should be able to use the
+                    // constructors arity for this, but at the time of writing we aren't
+                    // correctly calculating this arity when ellipsis are present.
+                    return Err(MatchCheckErr::NotImplemented);
+                }
 
                 Some(self.replace_head_with(pat_ids))
             }
@@ -351,19 +353,47 @@ impl PatStack {
                     Some(self.to_tail())
                 }
             }
-            (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => {
+            (
+                Pat::TupleStruct { args: ref pat_ids, ellipsis, .. },
+                Constructor::Enum(enum_constructor),
+            ) => {
                 let pat_id = self.head().as_id().expect("we know this isn't a wild");
                 if !enum_variant_matches(cx, pat_id, *enum_constructor) {
                     None
                 } else {
-                    // If the enum variant matches, then we need to confirm
-                    // that the number of patterns aligns with the expected
-                    // number of patterns for that enum variant.
-                    if pat_ids.len() != constructor.arity(cx)? {
-                        return Err(MatchCheckErr::MalformedMatchArm);
+                    let constructor_arity = constructor.arity(cx)?;
+                    if let Some(ellipsis_position) = ellipsis {
+                        // If there are ellipsis in the pattern, the ellipsis must take the place
+                        // of at least one sub-pattern, so `pat_ids` should be smaller than the
+                        // constructor arity.
+                        if pat_ids.len() < constructor_arity {
+                            let mut new_patterns: Vec<PatIdOrWild> = vec![];
+
+                            for pat_id in &pat_ids[0..ellipsis_position] {
+                                new_patterns.push((*pat_id).into());
+                            }
+
+                            for _ in 0..(constructor_arity - pat_ids.len()) {
+                                new_patterns.push(PatIdOrWild::Wild);
+                            }
+
+                            for pat_id in &pat_ids[ellipsis_position..pat_ids.len()] {
+                                new_patterns.push((*pat_id).into());
+                            }
+
+                            Some(self.replace_head_with(&new_patterns))
+                        } else {
+                            return Err(MatchCheckErr::MalformedMatchArm);
+                        }
+                    } else {
+                        // If there is no ellipsis in the tuple pattern, the number
+                        // of patterns must equal the constructor arity.
+                        if pat_ids.len() == constructor_arity {
+                            Some(self.replace_head_with(pat_ids))
+                        } else {
+                            return Err(MatchCheckErr::MalformedMatchArm);
+                        }
                     }
-
-                    Some(self.replace_head_with(pat_ids))
                 }
             }
             (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented),
@@ -644,7 +674,11 @@ impl Constructor {
 fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Option<Constructor>> {
     let res = match pat.as_pat(cx) {
         Pat::Wild => None,
-        Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }),
+        // FIXME somehow create the Tuple constructor with the proper arity. If there are
+        // ellipsis, the arity is not equal to the number of patterns.
+        Pat::Tuple { args: pats, ellipsis } if ellipsis.is_none() => {
+            Some(Constructor::Tuple { arity: pats.len() })
+        }
         Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] {
             Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)),
             _ => return Err(MatchCheckErr::NotImplemented),
@@ -1507,6 +1541,67 @@ mod tests {
     }
 
     #[test]
+    fn enum_tuple_partial_ellipsis_2_no_diagnostic() {
+        let content = r"
+            enum Either {
+                A(bool, bool, bool, bool),
+                B,
+            }
+            fn test_fn() {
+                match Either::B {
+                    Either::A(true, .., true) => {},
+                    Either::A(true, .., false) => {},
+                    Either::A(.., true) => {},
+                    Either::A(.., false) => {},
+                    Either::B => {},
+                }
+            }
+        ";
+
+        check_no_diagnostic(content);
+    }
+
+    #[test]
+    fn enum_tuple_partial_ellipsis_missing_arm() {
+        let content = r"
+            enum Either {
+                A(bool, bool, bool, bool),
+                B,
+            }
+            fn test_fn() {
+                match Either::B {
+                    Either::A(true, .., true) => {},
+                    Either::A(true, .., false) => {},
+                    Either::A(false, .., false) => {},
+                    Either::B => {},
+                }
+            }
+        ";
+
+        check_diagnostic(content);
+    }
+
+    #[test]
+    fn enum_tuple_partial_ellipsis_2_missing_arm() {
+        let content = r"
+            enum Either {
+                A(bool, bool, bool, bool),
+                B,
+            }
+            fn test_fn() {
+                match Either::B {
+                    Either::A(true, .., true) => {},
+                    Either::A(true, .., false) => {},
+                    Either::A(.., true) => {},
+                    Either::B => {},
+                }
+            }
+        ";
+
+        check_diagnostic(content);
+    }
+
+    #[test]
     fn enum_tuple_ellipsis_no_diagnostic() {
         let content = r"
             enum Either {
@@ -1645,11 +1740,7 @@ mod false_negatives {
         ";
 
         // This is a false negative.
-        // The `..` pattern is currently lowered to a single `Pat::Wild`
-        // no matter how many fields the `..` pattern is covering. This
-        // causes the match arm in this test not to type check against
-        // the match expression, which causes this diagnostic not to
-        // fire.
+        // We don't currently handle tuple patterns with ellipsis.
         check_no_diagnostic(content);
     }
 
@@ -1664,32 +1755,7 @@ mod false_negatives {
         ";
 
         // This is a false negative.
-        // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`.
-        check_no_diagnostic(content);
-    }
-
-    #[test]
-    fn enum_tuple_partial_ellipsis_missing_arm() {
-        let content = r"
-            enum Either {
-                A(bool, bool, bool, bool),
-                B,
-            }
-            fn test_fn() {
-                match Either::B {
-                    Either::A(true, .., true) => {},
-                    Either::A(true, .., false) => {},
-                    Either::A(false, .., false) => {},
-                    Either::B => {},
-                }
-            }
-        ";
-
-        // This is a false negative.
-        // The `..` pattern is currently lowered to a single `Pat::Wild`
-        // no matter how many fields the `..` pattern is covering. This
-        // causes us to return a `MatchCheckErr::MalformedMatchArm` in
-        // `Pat::specialize_constructor`.
+        // We don't currently handle tuple patterns with ellipsis.
         check_no_diagnostic(content);
     }
 }