From ab3313b1cb5e9ff79ecef0fb188873c892c193f1 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 20 Mar 2022 16:26:48 +0100 Subject: [PATCH] Add new type-mismatch diagnostic --- crates/hir/src/diagnostics.rs | 22 +---- crates/hir/src/lib.rs | 73 ++++++++------ crates/hir_def/src/type_ref.rs | 16 +++ .../src/handlers/field_shorthand.rs | 12 +-- .../src/handlers/missing_match_arms.rs | 1 + ...add_reference_here.rs => type_mismatch.rs} | 97 +++++++++++++++---- crates/ide_diagnostics/src/lib.rs | 11 +-- 7 files changed, 151 insertions(+), 81 deletions(-) rename crates/ide_diagnostics/src/handlers/{add_reference_here.rs => type_mismatch.rs} (52%) diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index b5e7d5db63..49551bf610 100644 --- a/crates/hir/src/diagnostics.rs +++ b/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,19 +145,6 @@ pub struct MismatchedArgCount { pub found: usize, } -#[derive(Debug)] -pub struct RemoveThisSemicolon { - pub expr: InFile>, -} - -#[derive(Debug)] -pub struct MissingOkOrSomeInTailExpr { - pub expr: InFile>, - // `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, @@ -167,9 +152,10 @@ pub struct MissingMatchArms { } #[derive(Debug)] -pub struct AddReferenceHere { +pub struct TypeMismatch { pub expr: InFile>, - pub mutability: Mutability, + pub expected: Type, + pub actual: Type, } pub use hir_ty::diagnostics::IncorrectCase; diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 55d2d8b7e3..035ae2d408 100644 --- a/crates/hir/src/lib.rs +++ b/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(..)) } diff --git a/crates/hir_def/src/type_ref.rs b/crates/hir_def/src/type_ref.rs index c6c521f733..8e9336a0cc 100644 --- a/crates/hir_def/src/type_ref.rs +++ b/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)] diff --git a/crates/ide_diagnostics/src/handlers/field_shorthand.rs b/crates/ide_diagnostics/src/handlers/field_shorthand.rs index 33152e2845..2b71053625 100644 --- a/crates/ide_diagnostics/src/handlers/field_shorthand.rs +++ b/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,7 +141,7 @@ 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#" @@ -149,7 +149,7 @@ struct A { a: &'static str, b: &'static str } fn main() { let a = "haha"; let b = "bb"; - A { a, b } + A { a, b }; } "#, ); diff --git a/crates/ide_diagnostics/src/handlers/missing_match_arms.rs b/crates/ide_diagnostics/src/handlers/missing_match_arms.rs index 6bdcd41a79..fe6a8683c1 100644 --- a/crates/ide_diagnostics/src/handlers/missing_match_arms.rs +++ 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) { () => () } diff --git a/crates/ide_diagnostics/src/handlers/add_reference_here.rs b/crates/ide_diagnostics/src/handlers/type_mismatch.rs similarity index 52% rename from crates/ide_diagnostics/src/handlers/add_reference_here.rs rename to crates/ide_diagnostics/src/handlers/type_mismatch.rs index db24fd6cce..2f8bda9efa 100644 --- a/crates/ide_diagnostics/src/handlers/add_reference_here.rs +++ b/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> { +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Option> { + 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) -> 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 arg_with_ref = format!("&{}{}", d.mutability.as_keyword_for_ref(), arg_expr.syntax()); + let range = ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range; - let arg_range = arg_expr.syntax().text_range(); - let edit = TextEdit::replace(arg_range, arg_with_ref); + 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 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( diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index 86d76751ad..7ea8fcde2d 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -24,7 +24,7 @@ //! don't yet have a great pattern for how to do them properly. mod handlers { - pub(crate) mod add_reference_here; + // pub(crate) mod add_reference_here; pub(crate) mod break_outside_of_loop; pub(crate) mod inactive_code; pub(crate) mod incorrect_case; @@ -34,10 +34,10 @@ mod handlers { pub(crate) mod mismatched_arg_count; pub(crate) mod missing_fields; pub(crate) mod missing_match_arms; - pub(crate) mod missing_ok_or_some_in_tail_expr; + // pub(crate) mod missing_ok_or_some_in_tail_expr; pub(crate) mod missing_unsafe; pub(crate) mod no_such_field; - pub(crate) mod remove_this_semicolon; + // pub(crate) mod remove_this_semicolon; pub(crate) mod replace_filter_map_next_with_find_map; pub(crate) mod unimplemented_builtin_macro; pub(crate) mod unresolved_extern_crate; @@ -45,6 +45,7 @@ mod handlers { pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_module; pub(crate) mod unresolved_proc_macro; + pub(crate) mod type_mismatch; // The handlers below are unusual, the implement the diagnostics as well. pub(crate) mod field_shorthand; @@ -191,7 +192,6 @@ pub fn diagnostics( for diag in diags { #[rustfmt::skip] let d = match diag { - AnyDiagnostic::AddReferenceHere(d) => handlers::add_reference_here::add_reference_here(&ctx, &d), AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d), AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d), @@ -199,11 +199,10 @@ pub fn diagnostics( AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d), AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d), - AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => handlers::missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d), AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d), - AnyDiagnostic::RemoveThisSemicolon(d) => handlers::remove_this_semicolon::remove_this_semicolon(&ctx, &d), AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), + AnyDiagnostic::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => handlers::unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => handlers::unresolved_import::unresolved_import(&ctx, &d),