Repository: rust-analyzer/rust-analyzer
Pull request #8875 (2021-05-19 07:01:53)

fix: false positive "Missing match arm" when an or-pattern has mismatched types
![Screenshot_20210519_114510](https://user-images.githubusercontent.com/7803845/118768935-19e12c00-b86f-11eb-90c4-1eed3f2bf57f.jpg)
`InferenceResult` now records pattern type mismatches.


Patch:
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index 47709c1e8f..b9a69b79c8 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -211,7 +211,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
 
         // FIXME: Due to shortcomings in the current type system implementation, only emit this
         // diagnostic if there are no type mismatches in the containing function.
-        if self.infer.type_mismatches.iter().next().is_some() {
+        if self.infer.expr_type_mismatches().next().is_some() {
             return;
         }
 
@@ -311,11 +311,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
                 // necessary.
                 //
                 // FIXME we should use the type checker for this.
-                if pat_ty == match_expr_ty
+                if (pat_ty == match_expr_ty
                     || match_expr_ty
                         .as_reference()
                         .map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
-                        .unwrap_or(false)
+                        .unwrap_or(false))
+                    && types_of_subpatterns_do_match(pat, &cx.body, &infer)
                 {
                     // If we had a NotUsefulMatchArm diagnostic, we could
                     // check the usefulness of each pattern as we added it
@@ -496,6 +497,21 @@ pub fn record_pattern_missing_fields(
     Some((variant_def, missed_fields, exhaustive))
 }
 
+fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResult) -> bool {
+    fn walk(pat: PatId, body: &Body, infer: &InferenceResult, has_type_mismatches: &mut bool) {
+        match infer.type_mismatch_for_pat(pat) {
+            Some(_) => *has_type_mismatches = true,
+            None => {
+                body[pat].walk_child_pats(|subpat| walk(subpat, body, infer, has_type_mismatches))
+            }
+        }
+    }
+
+    let mut has_type_mismatches = false;
+    walk(pat, body, infer, &mut has_type_mismatches);
+    !has_type_mismatches
+}
+
 #[cfg(test)]
 mod tests {
     use crate::diagnostics::tests::check_diagnostics;
diff --git a/crates/hir_ty/src/diagnostics/match_check.rs b/crates/hir_ty/src/diagnostics/match_check.rs
index 6ee0529c62..e8dd669bf6 100644
--- a/crates/hir_ty/src/diagnostics/match_check.rs
+++ b/crates/hir_ty/src/diagnostics/match_check.rs
@@ -1127,6 +1127,18 @@ fn main() {
         );
     }
 
+    #[test]
+    fn mismatched_types_in_or_patterns() {
+        check_diagnostics(
+            r#"
+fn main() {
+    match false { true | () => {} }
+    match (false,) { (true | (),) => {} }
+}
+"#,
+        );
+    }
+
     #[test]
     fn malformed_match_arm_tuple_enum_missing_pattern() {
         // We are testing to be sure we don't panic here when the match
diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs
index bf2da2d4ae..0ee851a74c 100644
--- a/crates/hir_ty/src/infer.rs
+++ b/crates/hir_ty/src/infer.rs
@@ -137,8 +137,12 @@ pub struct InferenceResult {
     assoc_resolutions: FxHashMap<ExprOrPatId, AssocItemId>,
     diagnostics: Vec<InferenceDiagnostic>,
     pub type_of_expr: ArenaMap<ExprId, Ty>,
+    /// For each pattern record the type it resolves to.
+    ///
+    /// **Note**: When a pattern type is resolved it may still contain
+    /// unresolved or missing subpatterns or subpatterns of mismatched types.
     pub type_of_pat: ArenaMap<PatId, Ty>,
-    pub(super) type_mismatches: ArenaMap<ExprId, TypeMismatch>,
+    type_mismatches: FxHashMap<ExprOrPatId, TypeMismatch>,
     /// Interned Unknown to return references to.
     standard_types: InternedStandardTypes,
 }
@@ -163,7 +167,22 @@ impl InferenceResult {
         self.assoc_resolutions.get(&id.into()).copied()
     }
     pub fn type_mismatch_for_expr(&self, expr: ExprId) -> Option<&TypeMismatch> {
-        self.type_mismatches.get(expr)
+        self.type_mismatches.get(&expr.into())
+    }
+    pub fn type_mismatch_for_pat(&self, pat: PatId) -> Option<&TypeMismatch> {
+        self.type_mismatches.get(&pat.into())
+    }
+    pub fn expr_type_mismatches(&self) -> impl Iterator<Item = (ExprId, &TypeMismatch)> {
+        self.type_mismatches.iter().filter_map(|(expr_or_pat, mismatch)| match *expr_or_pat {
+            ExprOrPatId::ExprId(expr) => Some((expr, mismatch)),
+            _ => None,
+        })
+    }
+    pub fn pat_type_mismatches(&self) -> impl Iterator<Item = (PatId, &TypeMismatch)> {
+        self.type_mismatches.iter().filter_map(|(expr_or_pat, mismatch)| match *expr_or_pat {
+            ExprOrPatId::PatId(pat) => Some((pat, mismatch)),
+            _ => None,
+        })
     }
     pub fn add_diagnostics(
         &self,
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index b6b5a1b750..7278faeec0 100644
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -42,7 +42,7 @@ impl<'a> InferenceContext<'a> {
         let could_unify = self.unify(&ty, &expected.ty);
         if !could_unify {
             self.result.type_mismatches.insert(
-                tgt_expr,
+                tgt_expr.into(),
                 TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() },
             );
         }
@@ -54,9 +54,10 @@ impl<'a> InferenceContext<'a> {
     pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty {
         let ty = self.infer_expr_inner(expr, &expected);
         let ty = if !self.coerce(&ty, &expected.coercion_target()) {
-            self.result
-                .type_mismatches
-                .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() });
+            self.result.type_mismatches.insert(
+                expr.into(),
+                TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() },
+            );
             // Return actual type when type mismatch.
             // This is needed for diagnostic when return type mismatch.
             ty
diff --git a/crates/hir_ty/src/infer/pat.rs b/crates/hir_ty/src/infer/pat.rs
index 60b94a6422..b15f4977dd 100644
--- a/crates/hir_ty/src/infer/pat.rs
+++ b/crates/hir_ty/src/infer/pat.rs
@@ -10,7 +10,7 @@ use hir_def::{
 };
 use hir_expand::name::Name;
 
-use super::{BindingMode, Expectation, InferenceContext};
+use super::{BindingMode, Expectation, InferenceContext, TypeMismatch};
 use crate::{
     lower::lower_to_chalk_mutability, static_lifetime, Interner, Substitution, Ty, TyBuilder,
     TyExt, TyKind,
@@ -266,7 +266,10 @@ impl<'a> InferenceContext<'a> {
         // use a new type variable if we got error type here
         let ty = self.insert_type_vars_shallow(ty);
         if !self.unify(&ty, expected) {
-            // FIXME record mismatch, we need to change the type of self.type_mismatches for that
+            self.result.type_mismatches.insert(
+                pat.into(),
+                TypeMismatch { expected: expected.clone(), actual: ty.clone() },
+            );
         }
         let ty = self.resolve_ty_as_possible(ty);
         self.write_pat_ty(pat, ty.clone());


Test patch:
diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs
index ccfb88c52d..cc819373c6 100644
--- a/crates/hir_ty/src/tests.rs
+++ b/crates/hir_ty/src/tests.rs
@@ -130,7 +130,10 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String {
                 }
                 Err(SyntheticSyntax) => continue,
             };
-            types.push((syntax_ptr, ty));
+            types.push((syntax_ptr.clone(), ty));
+            if let Some(mismatch) = inference_result.type_mismatch_for_pat(pat) {
+                mismatches.push((syntax_ptr, mismatch));
+            }
         }
 
         for (expr, ty) in inference_result.type_of_expr.iter() {
diff --git a/crates/hir_ty/src/tests/patterns.rs b/crates/hir_ty/src/tests/patterns.rs
index 787647e9f3..ddbadbe400 100644
--- a/crates/hir_ty/src/tests/patterns.rs
+++ b/crates/hir_ty/src/tests/patterns.rs
@@ -546,7 +546,9 @@ fn infer_const_pattern() {
             273..276 'Bar': usize
             280..283 'Bar': usize
             200..223: expected (), got Foo
+            211..214: expected (), got Foo
             262..285: expected (), got usize
+            273..276: expected (), got usize
         "#]],
     );
 }
@@ -703,7 +705,7 @@ fn box_pattern() {
 
 #[test]
 fn tuple_ellipsis_pattern() {
-    check_infer(
+    check_infer_with_mismatches(
         r#"
 fn foo(tuple: (u8, i16, f32)) {
     match tuple {
@@ -744,6 +746,8 @@ fn foo(tuple: (u8, i16, f32)) {
             186..200 '{/*too long*/}': ()
             209..210 '_': (u8, i16, f32)
             214..216 '{}': ()
+            136..142: expected (u8, i16, f32), got (u8, i16)
+            170..182: expected (u8, i16, f32), got (u8, i16, f32, _)
         "#]],
     );
 }
@@ -851,3 +855,55 @@ fn f(e: Enum) {
     "#,
     )
 }
+
+#[test]
+fn type_mismatch_in_or_pattern() {
+    check_infer_with_mismatches(
+        r#"
+fn main() {
+    match (false,) {
+        (true | (),) => {}
+        (() | true,) => {}
+        (_ | (),) => {}
+        (() | _,) => {}
+    }
+}
+"#,
+        expect![[r#"
+            10..142 '{     ...   } }': ()
+            16..140 'match ...     }': ()
+            22..30 '(false,)': (bool,)
+            23..28 'false': bool
+            41..53 '(true | (),)': (bool,)
+            42..46 'true': bool
+            42..46 'true': bool
+            42..51 'true | ()': bool
+            49..51 '()': ()
+            57..59 '{}': ()
+            68..80 '(() | true,)': ((),)
+            69..71 '()': ()
+            69..78 '() | true': ()
+            74..78 'true': bool
+            74..78 'true': bool
+            84..86 '{}': ()
+            95..104 '(_ | (),)': (bool,)
+            96..97 '_': bool
+            96..102 '_ | ()': bool
+            100..102 '()': ()
+            108..110 '{}': ()
+            119..128 '(() | _,)': ((),)
+            120..122 '()': ()
+            120..126 '() | _': ()
+            125..126 '_': bool
+            132..134 '{}': ()
+            49..51: expected bool, got ()
+            68..80: expected (bool,), got ((),)
+            69..71: expected bool, got ()
+            69..78: expected bool, got ()
+            100..102: expected bool, got ()
+            119..128: expected (bool,), got ((),)
+            120..122: expected bool, got ()
+            120..126: expected bool, got ()
+        "#]],
+    );
+}