Show pattern mismatch diagnostics

Veykril · Veykril · commit fc2b395e0095 · 2023-03-03T10:41:44.000+01:00
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
@@ -771,6 +771,15 @@ impl<T> InFile<Option<T>> {
     }
 }
 
+impl<L, R> InFile<Either<L, R>> {
+    pub fn transpose(self) -> Either<InFile<L>, InFile<R>> {
+        match self.value {
+            Either::Left(l) => Either::Left(InFile::new(self.file_id, l)),
+            Either::Right(r) => Either::Right(InFile::new(self.file_id, r)),
+        }
+    }
+}
+
 impl<'a> InFile<&'a SyntaxNode> {
     pub fn ancestors_with_macros(
         self,
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs
@@ -389,18 +389,15 @@ impl InferenceResult {
     pub fn type_mismatch_for_pat(&self, pat: PatId) -> Option<&TypeMismatch> {
         self.type_mismatches.get(&pat.into())
     }
+    pub fn type_mismatches(&self) -> impl Iterator<Item = (ExprOrPatId, &TypeMismatch)> {
+        self.type_mismatches.iter().map(|(expr_or_pat, mismatch)| (*expr_or_pat, mismatch))
+    }
     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,
-        })
-    }
 }
 
 impl Index<ExprId> for InferenceResult {
diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs
@@ -196,12 +196,7 @@ impl<'a> InferenceContext<'a> {
             Pat::Ref { pat, mutability } => {
                 let mutability = lower_to_chalk_mutability(*mutability);
                 let expectation = match expected.as_reference() {
-                    Some((inner_ty, _lifetime, exp_mut)) => {
-                        if mutability != exp_mut {
-                            // FIXME: emit type error?
-                        }
-                        inner_ty.clone()
-                    }
+                    Some((inner_ty, _lifetime, exp_mut)) => inner_ty.clone(),
                     _ => self.result.standard_types.unknown.clone(),
                 };
                 let subty = self.infer_pat(*pat, &expectation, default_bm);
diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs
@@ -191,30 +191,11 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour
             }
         }
 
-        for (pat, mismatch) in inference_result.pat_type_mismatches() {
-            let node = match pat_node(&body_source_map, pat, &db) {
-                Some(value) => value,
-                None => continue,
-            };
-            let range = node.as_ref().original_file_range(&db);
-            let actual = format!(
-                "expected {}, got {}",
-                mismatch.expected.display_test(&db),
-                mismatch.actual.display_test(&db)
-            );
-            match mismatches.remove(&range) {
-                Some(annotation) => assert_eq!(actual, annotation),
-                None => format_to!(unexpected_type_mismatches, "{:?}: {}\n", range.range, actual),
-            }
-        }
-        for (expr, mismatch) in inference_result.expr_type_mismatches() {
-            let node = match body_source_map.expr_syntax(expr) {
-                Ok(sp) => {
-                    let root = db.parse_or_expand(sp.file_id).unwrap();
-                    sp.map(|ptr| ptr.to_node(&root).syntax().clone())
-                }
-                Err(SyntheticSyntax) => continue,
-            };
+        for (expr_or_pat, mismatch) in inference_result.type_mismatches() {
+            let Some(node) = (match expr_or_pat {
+                hir_def::expr::ExprOrPatId::ExprId(expr) => expr_node(&body_source_map, expr, &db),
+                hir_def::expr::ExprOrPatId::PatId(pat) => pat_node(&body_source_map, pat, &db),
+            }) else { continue; };
             let range = node.as_ref().original_file_range(&db);
             let actual = format!(
                 "expected {}, got {}",
diff --git a/crates/hir-ty/src/tests/patterns.rs b/crates/hir-ty/src/tests/patterns.rs
@@ -1092,3 +1092,19 @@ fn my_fn(foo: ...) {}
 "#,
     );
 }
+
+#[test]
+fn ref_pat_mutability() {
+    check(
+        r#"
+fn foo() {
+    let &() = &();
+    let &mut () = &mut ();
+    let &mut () = &();
+      //^^^^^^^ expected &(), got &mut ()
+    let &() = &mut ();
+      //^^^ expected &mut (), got &()
+}
+"#,
+    );
+}
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
@@ -178,8 +178,7 @@ pub struct MissingMatchArms {
 
 #[derive(Debug)]
 pub struct TypeMismatch {
-    // FIXME: add mismatches in patterns as well
-    pub expr: InFile<AstPtr<ast::Expr>>,
+    pub expr_or_pat: Either<InFile<AstPtr<ast::Expr>>, InFile<AstPtr<ast::Pat>>>,
     pub expected: Type,
     pub actual: Type,
 }
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -1413,14 +1413,22 @@ impl DefWithBody {
                 }
             }
         }
-        for (expr, mismatch) in infer.expr_type_mismatches() {
-            let expr = match source_map.expr_syntax(expr) {
-                Ok(expr) => expr,
-                Err(SyntheticSyntax) => continue,
+        for (pat_or_expr, mismatch) in infer.type_mismatches() {
+            let expr_or_pat = match pat_or_expr {
+                ExprOrPatId::ExprId(expr) => source_map.expr_syntax(expr).map(Either::Left),
+                ExprOrPatId::PatId(pat) => source_map.pat_syntax(pat).map(Either::Right),
             };
+            let expr_or_pat = match expr_or_pat {
+                Ok(Either::Left(expr)) => Either::Left(expr),
+                Ok(Either::Right(InFile { file_id, value: Either::Left(pat) })) => {
+                    Either::Right(InFile { file_id, value: pat })
+                }
+                Ok(Either::Right(_)) | Err(SyntheticSyntax) => continue,
+            };
+
             acc.push(
                 TypeMismatch {
-                    expr,
+                    expr_or_pat,
                     expected: Type::new(db, DefWithBodyId::from(self), mismatch.expected.clone()),
                     actual: Type::new(db, DefWithBodyId::from(self), mismatch.actual.clone()),
                 }
diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs
@@ -1,28 +1,33 @@
-use hir::{db::AstDatabase, HirDisplay, Type};
+use either::Either;
+use hir::{db::AstDatabase, HirDisplay, InFile, Type};
 use ide_db::{famous_defs::FamousDefs, source_change::SourceChange};
 use syntax::{
     ast::{self, BlockExpr, ExprStmt},
-    AstNode,
+    AstNode, AstPtr,
 };
 use text_edit::TextEdit;
 
 use crate::{adjusted_display_range, fix, Assist, Diagnostic, DiagnosticsContext};
 
 // Diagnostic: type-mismatch
 //
-// This diagnostic is triggered when the type of an expression does not match
+// This diagnostic is triggered when the type of an expression or pattern does not match
 // the expected type.
 pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Diagnostic {
-    let display_range = adjusted_display_range::<ast::BlockExpr>(
-        ctx,
-        d.expr.clone().map(|it| it.into()),
-        &|block| {
-            let r_curly_range = block.stmt_list()?.r_curly_token()?.text_range();
-            cov_mark::hit!(type_mismatch_on_block);
-            Some(r_curly_range)
-        },
-    );
-
+    let display_range = match &d.expr_or_pat {
+        Either::Left(expr) => adjusted_display_range::<ast::BlockExpr>(
+            ctx,
+            expr.clone().map(|it| it.into()),
+            &|block| {
+                let r_curly_range = block.stmt_list()?.r_curly_token()?.text_range();
+                cov_mark::hit!(type_mismatch_on_block);
+                Some(r_curly_range)
+            },
+        ),
+        Either::Right(pat) => {
+            ctx.sema.diagnostics_display_range(pat.clone().map(|it| it.into())).range
+        }
+    };
     let mut diag = Diagnostic::new(
         "type-mismatch",
         format!(
@@ -42,24 +47,38 @@ pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch)
 fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Option<Vec<Assist>> {
     let mut fixes = Vec::new();
 
-    add_reference(ctx, d, &mut fixes);
-    add_missing_ok_or_some(ctx, d, &mut fixes);
-    remove_semicolon(ctx, d, &mut fixes);
-    str_ref_to_owned(ctx, d, &mut fixes);
+    match &d.expr_or_pat {
+        Either::Left(expr_ptr) => {
+            add_reference(ctx, d, expr_ptr, &mut fixes);
+            add_missing_ok_or_some(ctx, d, expr_ptr, &mut fixes);
+            remove_semicolon(ctx, d, expr_ptr, &mut fixes);
+            str_ref_to_owned(ctx, d, expr_ptr, &mut fixes);
+        }
+        Either::Right(_pat_ptr) => (),
+    }
 
     if fixes.is_empty() {
         None
     } else {
         Some(fixes)
     }
 }
+fn add_reference_pat(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::TypeMismatch,
+    expr_ptr: &InFile<AstPtr<ast::Pat>>,
+    acc: &mut Vec<Assist>,
+) -> Option<()> {
+    None
+}
 
 fn add_reference(
     ctx: &DiagnosticsContext<'_>,
     d: &hir::TypeMismatch,
+    expr_ptr: &InFile<AstPtr<ast::Expr>>,
     acc: &mut Vec<Assist>,
 ) -> Option<()> {
-    let range = ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range;
+    let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into())).range;
 
     let (_, mutability) = d.expected.as_reference()?;
     let actual_with_ref = Type::reference(&d.actual, mutability);
@@ -71,18 +90,19 @@ fn add_reference(
 
     let edit = TextEdit::insert(range.start(), ampersands);
     let source_change =
-        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
+        SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit);
     acc.push(fix("add_reference_here", "Add reference here", source_change, range));
     Some(())
 }
 
 fn add_missing_ok_or_some(
     ctx: &DiagnosticsContext<'_>,
     d: &hir::TypeMismatch,
+    expr_ptr: &InFile<AstPtr<ast::Expr>>,
     acc: &mut Vec<Assist>,
 ) -> Option<()> {
-    let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
-    let expr = d.expr.value.to_node(&root);
+    let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?;
+    let expr = expr_ptr.value.to_node(&root);
     let expr_range = expr.syntax().text_range();
     let scope = ctx.sema.scope(expr.syntax())?;
 
@@ -109,7 +129,7 @@ fn add_missing_ok_or_some(
     builder.insert(expr.syntax().text_range().start(), format!("{variant_name}("));
     builder.insert(expr.syntax().text_range().end(), ")".to_string());
     let source_change =
-        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), builder.finish());
+        SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), builder.finish());
     let name = format!("Wrap in {variant_name}");
     acc.push(fix("wrap_in_constructor", &name, source_change, expr_range));
     Some(())
@@ -118,10 +138,11 @@ fn add_missing_ok_or_some(
 fn remove_semicolon(
     ctx: &DiagnosticsContext<'_>,
     d: &hir::TypeMismatch,
+    expr_ptr: &InFile<AstPtr<ast::Expr>>,
     acc: &mut Vec<Assist>,
 ) -> Option<()> {
-    let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
-    let expr = d.expr.value.to_node(&root);
+    let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?;
+    let expr = expr_ptr.value.to_node(&root);
     if !d.actual.is_unit() {
         return None;
     }
@@ -136,7 +157,7 @@ fn remove_semicolon(
 
     let edit = TextEdit::delete(semicolon_range);
     let source_change =
-        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
+        SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit);
 
     acc.push(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon_range));
     Some(())
@@ -145,24 +166,26 @@ fn remove_semicolon(
 fn str_ref_to_owned(
     ctx: &DiagnosticsContext<'_>,
     d: &hir::TypeMismatch,
+    expr_ptr: &InFile<AstPtr<ast::Expr>>,
     acc: &mut Vec<Assist>,
 ) -> Option<()> {
     let expected = d.expected.display(ctx.sema.db);
     let actual = d.actual.display(ctx.sema.db);
 
+    // FIXME do this properly
     if expected.to_string() != "String" || actual.to_string() != "&str" {
         return None;
     }
 
-    let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
-    let expr = d.expr.value.to_node(&root);
+    let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?;
+    let expr = expr_ptr.value.to_node(&root);
     let expr_range = expr.syntax().text_range();
 
     let to_owned = format!(".to_owned()");
 
     let edit = TextEdit::insert(expr.syntax().text_range().end(), to_owned);
     let source_change =
-        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
+        SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit);
     acc.push(fix("str_ref_to_owned", "Add .to_owned() here", source_change, expr_range));
 
     Some(())
@@ -592,6 +615,24 @@ fn f() -> i32 {
     let _ = x + y;
   }
 //^ error: expected i32, found ()
+"#,
+        );
+    }
+
+    #[test]
+    fn type_mismatch_pat_smoke_test() {
+        check_diagnostics(
+            r#"
+fn f() {
+    let &() = &mut ();
+      //^^^ error: expected &mut (), found &()
+    match &() {
+        &9 => ()
+      //^^ error: expected &(), found &i32
+       //^ error: expected (), found i32
+       //^ error: expected (), found i32
+    }
+}
 "#,
         );
     }

Original file line number	Diff line number	Diff line change
`@@ -178,8 +178,7 @@ pub struct MissingMatchArms {`
`178`	`178`
`179`	`179`	`#[derive(Debug)]`
`180`	`180`	`pub struct TypeMismatch {`
`181`		`- // FIXME: add mismatches in patterns as well`
`182`		`- pub expr: InFile<AstPtr<ast::Expr>>,`
	`181`	`+ pub expr_or_pat: Either<InFile<AstPtr<ast::Expr>>, InFile<AstPtr<ast::Pat>>>,`
`183`	`182`	`pub expected: Type,`
`184`	`183`	`pub actual: Type,`
`185`	`184`	`}`