Add new type-mismatch diagnostic

Author: flodiebold

crates/hir/src/diagnostics.rs

@@ -28,7 +28,6 @@ macro_rules! diagnostics {
 }
 
 diagnostics![
-    AddReferenceHere,
     BreakOutsideOfLoop,
     InactiveCode,
     IncorrectCase,
@@ -38,11 +37,10 @@ diagnostics![
     MismatchedArgCount,
     MissingFields,
     MissingMatchArms,
-    MissingOkOrSomeInTailExpr,
     MissingUnsafe,
     NoSuchField,
-    RemoveThisSemicolon,
     ReplaceFilterMapNextWithFindMap,
+    TypeMismatch,
     UnimplementedBuiltinMacro,
     UnresolvedExternCrate,
     UnresolvedImport,
@@ -147,29 +145,17 @@ pub struct MismatchedArgCount {
     pub found: usize,
 }
 
-#[derive(Debug)]
-pub struct RemoveThisSemicolon {
-    pub expr: InFile<AstPtr<ast::Expr>>,
-}
-
-#[derive(Debug)]
-pub struct MissingOkOrSomeInTailExpr {
-    pub expr: InFile<AstPtr<ast::Expr>>,
-    // `Some` or `Ok` depending on whether the return type is Result or Option
-    pub required: String,
-    pub expected: Type,
-}
-
 #[derive(Debug)]
 pub struct MissingMatchArms {
     pub file: HirFileId,
     pub match_expr: AstPtr<ast::Expr>,
 }
 
 #[derive(Debug)]
-pub struct AddReferenceHere {
+pub struct TypeMismatch {
     pub expr: InFile<AstPtr<ast::Expr>>,
-    pub mutability: Mutability,
+    pub expected: Type,
+    pub actual: Type,
 }
 
 pub use hir_ty::diagnostics::IncorrectCase;

crates/hir/src/lib.rs

@@ -85,12 +85,11 @@ use crate::db::{DefDatabase, HirDatabase};
 pub use crate::{
     attrs::{HasAttrs, Namespace},
     diagnostics::{
-        AddReferenceHere, AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase,
-        InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields,
-        MissingMatchArms, MissingOkOrSomeInTailExpr, MissingUnsafe, NoSuchField,
-        RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro,
-        UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule,
-        UnresolvedProcMacro,
+        AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InvalidDeriveTarget,
+        MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms,
+        MissingUnsafe, NoSuchField, ReplaceFilterMapNextWithFindMap, TypeMismatch,
+        UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
+        UnresolvedModule, UnresolvedProcMacro,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo},
@@ -1163,6 +1162,28 @@ impl DefWithBody {
                 }
             }
         }
+        for (expr, mismatch) in infer.expr_type_mismatches() {
+            let expr =
+                source_map.expr_syntax(expr).expect("break outside of loop in synthetic syntax");
+            acc.push(
+                TypeMismatch {
+                    expr,
+                    expected: Type::new(
+                        db,
+                        krate,
+                        DefWithBodyId::from(self),
+                        mismatch.expected.clone(),
+                    ),
+                    actual: Type::new(
+                        db,
+                        krate,
+                        DefWithBodyId::from(self),
+                        mismatch.actual.clone(),
+                    ),
+                }
+                .into(),
+            );
+        }
 
         for expr in hir_ty::diagnostics::missing_unsafe(db, self.into()) {
             match source_map.expr_syntax(expr) {
@@ -1259,25 +1280,6 @@ impl DefWithBody {
                         Err(SyntheticSyntax) => (),
                     }
                 }
-                BodyValidationDiagnostic::RemoveThisSemicolon { expr } => {
-                    match source_map.expr_syntax(expr) {
-                        Ok(expr) => acc.push(RemoveThisSemicolon { expr }.into()),
-                        Err(SyntheticSyntax) => (),
-                    }
-                }
-                BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr, required } => {
-                    match source_map.expr_syntax(expr) {
-                        Ok(expr) => acc.push(
-                            MissingOkOrSomeInTailExpr {
-                                expr,
-                                required,
-                                expected: self.body_type(db),
-                            }
-                            .into(),
-                        ),
-                        Err(SyntheticSyntax) => (),
-                    }
-                }
                 BodyValidationDiagnostic::MissingMatchArms { match_expr } => {
                     match source_map.expr_syntax(match_expr) {
                         Ok(source_ptr) => {
@@ -1299,12 +1301,7 @@ impl DefWithBody {
                         Err(SyntheticSyntax) => (),
                     }
                 }
-                BodyValidationDiagnostic::AddReferenceHere { arg_expr, mutability } => {
-                    match source_map.expr_syntax(arg_expr) {
-                        Ok(expr) => acc.push(AddReferenceHere { expr, mutability }.into()),
-                        Err(SyntheticSyntax) => (),
-                    }
-                }
+                _ => {} // TODO fixme
             }
         }
 
@@ -2618,6 +2615,14 @@ impl Type {
         Type { krate, env: environment, ty }
     }
 
+    pub fn reference(inner: &Type, m: Mutability) -> Type {
+        inner.derived(TyKind::Ref(
+            if m.is_mut() { hir_ty::Mutability::Mut } else { hir_ty::Mutability::Not },
+            hir_ty::static_lifetime(),
+            inner.ty.clone(),
+        ).intern(Interner))
+    }
+
     fn new(db: &dyn HirDatabase, krate: CrateId, lexical_env: impl HasResolver, ty: Ty) -> Type {
         let resolver = lexical_env.resolver(db.upcast());
         let environment = resolver
@@ -2659,6 +2664,12 @@ impl Type {
         matches!(self.ty.kind(Interner), TyKind::Ref(..))
     }
 
+    pub fn as_reference(&self) -> Option<(Type, Mutability)> {
+        let (ty, _lt, m) = self.ty.as_reference()?;
+        let m = Mutability::from_mutable(matches!(m, hir_ty::Mutability::Mut));
+        Some((self.derived(ty.clone()), m))
+    }
+
     pub fn is_slice(&self) -> bool {
         matches!(self.ty.kind(Interner), TyKind::Slice(..))
     }

crates/hir_def/src/type_ref.rs

@@ -38,6 +38,22 @@ impl Mutability {
             Mutability::Mut => "mut ",
         }
     }
+
+    /// Returns `true` if the mutability is [`Mut`].
+    ///
+    /// [`Mut`]: Mutability::Mut
+    #[must_use]
+    pub fn is_mut(&self) -> bool {
+        matches!(self, Self::Mut)
+    }
+
+    /// Returns `true` if the mutability is [`Shared`].
+    ///
+    /// [`Shared`]: Mutability::Shared
+    #[must_use]
+    pub fn is_shared(&self) -> bool {
+        matches!(self, Self::Shared)
+    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]

crates/ide_diagnostics/src/handlers/field_shorthand.rs

@@ -108,13 +108,13 @@ mod tests {
         check_diagnostics(
             r#"
 struct A { a: &'static str }
-fn main() { A { a: "hello" } }
+fn main() { A { a: "hello" }; }
 "#,
         );
         check_diagnostics(
             r#"
 struct A(usize);
-fn main() { A { 0: 0 } }
+fn main() { A { 0: 0 }; }
 "#,
         );
 
@@ -123,14 +123,14 @@ fn main() { A { 0: 0 } }
 struct A { a: &'static str }
 fn main() {
     let a = "haha";
-    A { a$0: a }
+    A { a$0: a };
 }
 "#,
             r#"
 struct A { a: &'static str }
 fn main() {
     let a = "haha";
-    A { a }
+    A { a };
 }
 "#,
         );
@@ -141,15 +141,15 @@ struct A { a: &'static str, b: &'static str }
 fn main() {
     let a = "haha";
     let b = "bb";
-    A { a$0: a, b }
+    A { a$0: a, b };
 }
 "#,
             r#"
 struct A { a: &'static str, b: &'static str }
 fn main() {
     let a = "haha";
     let b = "bb";
-    A { a, b }
+    A { a, b };
 }
 "#,
         );

crates/ide_diagnostics/src/handlers/missing_match_arms.rs

@@ -278,6 +278,7 @@ fn main() {
     match (true, false) {
         (true, false, true) => (),
         (true) => (),
+      // ^^^^  error: expected (bool, bool), found bool
     }
     match (true, false) { (true,) => {} }
     match (0) { () => () }

crates/ide_diagnostics/src/handlers/add_reference_here.rs renamed to crates/ide_diagnostics/src/handlers/type_mismatch.rs

@@ -1,37 +1,62 @@
-use hir::db::AstDatabase;
+use hir::{db::AstDatabase, HirDisplay, Type};
 use ide_db::source_change::SourceChange;
-use syntax::AstNode;
+use syntax::{AstNode, TextRange};
 use text_edit::TextEdit;
 
 use crate::{fix, Assist, Diagnostic, DiagnosticsContext};
 
-// Diagnostic: add-reference-here
+// Diagnostic: type-mismatch
 //
-// This diagnostic is triggered when there's a missing referencing of expression.
-pub(crate) fn add_reference_here(
-    ctx: &DiagnosticsContext<'_>,
-    d: &hir::AddReferenceHere,
-) -> Diagnostic {
-    Diagnostic::new(
-        "add-reference-here",
-        "add reference here",
+// This diagnostic is triggered when the type of an expression does not match
+// the expected type.
+pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Diagnostic {
+    let mut diag = Diagnostic::new(
+        "type-mismatch",
+        format!(
+            "expected {}, found {}",
+            d.expected.display(ctx.sema.db),
+            d.actual.display(ctx.sema.db)
+        ),
         ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
     )
-    .with_fixes(fixes(ctx, d))
+    .with_fixes(fixes(ctx, d));
+    if diag.fixes.is_none() {
+        diag.experimental = true;
+    }
+    diag
 }
 
-fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::AddReferenceHere) -> Option<Vec<Assist>> {
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Option<Vec<Assist>> {
+    let mut fixes = Vec::new();
+
+    add_reference(ctx, d, &mut fixes);
+
+    if fixes.is_empty() {
+        None
+    } else {
+        Some(fixes)
+    }
+}
+
+fn add_reference(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch, acc: &mut Vec<Assist>) -> Option<()> {
     let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
-    let arg_expr = d.expr.value.to_node(&root);
+    let expr_node = d.expr.value.to_node(&root);
+
+    let range = ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range;
 
-    let arg_with_ref = format!("&{}{}", d.mutability.as_keyword_for_ref(), arg_expr.syntax());
+    let (_, mutability) = d.expected.as_reference()?;
+    let actual_with_ref = Type::reference(&d.actual, mutability);
+    if !actual_with_ref.could_coerce_to(ctx.sema.db, &d.expected) {
+        return None;
+    }
+
+    let ampersands = format!("&{}", mutability.as_keyword_for_ref());
 
-    let arg_range = arg_expr.syntax().text_range();
-    let edit = TextEdit::replace(arg_range, arg_with_ref);
+    let edit = TextEdit::insert(expr_node.syntax().text_range().start(), ampersands);
     let source_change =
         SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
-
-    Some(vec![fix("add_reference_here", "Add reference here", source_change, arg_range)])
+    acc.push(fix("add_reference_here", "Add reference here", source_change, range));
+    Some(())
 }
 
 #[cfg(test)]
@@ -44,7 +69,7 @@ mod tests {
             r#"
 fn main() {
     test(123);
-       //^^^ 💡 error: add reference here
+       //^^^ 💡 error: expected &i32, found i32
 }
 fn test(arg: &i32) {}
 "#,
@@ -91,6 +116,7 @@ fn test(arg: &mut i32) {}
     fn test_add_reference_to_array() {
         check_fix(
             r#"
+//- minicore: coerce_unsized
 fn main() {
     test($0[1, 2, 3]);
 }
@@ -105,6 +131,37 @@ fn test(arg: &[i32]) {}
         );
     }
 
+    #[test]
+    fn test_add_reference_with_autoderef() {
+        check_fix(
+            r#"
+//- minicore: coerce_unsized, deref
+struct Foo;
+struct Bar;
+impl core::ops::Deref for Foo {
+    type Target = Bar;
+}
+
+fn main() {
+    test($0Foo);
+}
+fn test(arg: &Bar) {}
+            "#,
+            r#"
+struct Foo;
+struct Bar;
+impl core::ops::Deref for Foo {
+    type Target = Bar;
+}
+
+fn main() {
+    test(&Foo);
+}
+fn test(arg: &Bar) {}
+            "#,
+        );
+    }
+
     #[test]
     fn test_add_reference_to_method_call() {
         check_fix(