Unnamed repository; edit this file 'description' to name the repository.
Diagnose unresolved method calls
| -rw-r--r-- | crates/hir-ty/src/infer.rs | 63 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/expr.rs | 121 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 9 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 22 | ||||
| -rw-r--r-- | crates/ide-db/src/source_change.rs | 8 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs | 13 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/unresolved_method.rs | 130 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 2 |
8 files changed, 320 insertions, 48 deletions
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index a663a568b9..22dcea8fcd 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -164,14 +164,45 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>; #[derive(Debug, PartialEq, Eq, Clone)] pub enum InferenceDiagnostic { - NoSuchField { expr: ExprId }, - PrivateField { expr: ExprId, field: FieldId }, - PrivateAssocItem { id: ExprOrPatId, item: AssocItemId }, - UnresolvedField { expr: ExprId, receiver: Ty, name: Name, method_with_same_name_exists: bool }, + NoSuchField { + expr: ExprId, + }, + PrivateField { + expr: ExprId, + field: FieldId, + }, + PrivateAssocItem { + id: ExprOrPatId, + item: AssocItemId, + }, + UnresolvedField { + expr: ExprId, + receiver: Ty, + name: Name, + method_with_same_name_exists: bool, + }, + UnresolvedMethodCall { + expr: ExprId, + receiver: Ty, + name: Name, + /// Contains the type the field resolves to + field_with_same_name: Option<Ty>, + }, // FIXME: Make this proper - BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool }, - MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, - ExpectedFunction { call_expr: ExprId, found: Ty }, + BreakOutsideOfLoop { + expr: ExprId, + is_break: bool, + bad_value_break: bool, + }, + MismatchedArgCount { + call_expr: ExprId, + expected: usize, + found: usize, + }, + ExpectedFunction { + call_expr: ExprId, + found: Ty, + }, } /// A mismatch between an expected and an inferred type. @@ -509,12 +540,28 @@ impl<'a> InferenceContext<'a> { } result.diagnostics.retain_mut(|diagnostic| { if let InferenceDiagnostic::ExpectedFunction { found: ty, .. } - | InferenceDiagnostic::UnresolvedField { receiver: ty, .. } = diagnostic + | InferenceDiagnostic::UnresolvedField { receiver: ty, .. } + | InferenceDiagnostic::UnresolvedMethodCall { receiver: ty, .. } = diagnostic { *ty = table.resolve_completely(ty.clone()); + // FIXME: Remove this when we are on par with rustc in terms of inference if ty.is_unknown() { return false; } + + if let InferenceDiagnostic::UnresolvedMethodCall { field_with_same_name, .. } = + diagnostic + { + let clear = if let Some(ty) = field_with_same_name { + *ty = table.resolve_completely(ty.clone()); + ty.is_unknown() + } else { + false + }; + if clear { + *field_with_same_name = None; + } + } } true }); diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 531a359a96..02024e1ea7 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -1212,12 +1212,14 @@ impl<'a> InferenceContext<'a> { } } - fn infer_field_access(&mut self, tgt_expr: ExprId, expr: ExprId, name: &Name) -> Ty { - let receiver_ty = self.infer_expr_inner(expr, &Expectation::none()); - + fn lookup_field( + &mut self, + receiver_ty: &Ty, + name: &Name, + ) -> Option<(Ty, Option<FieldId>, Vec<Adjustment>, bool)> { let mut autoderef = Autoderef::new(&mut self.table, receiver_ty.clone()); let mut private_field = None; - let ty = autoderef.by_ref().find_map(|(derefed_ty, _)| { + let res = autoderef.by_ref().find_map(|(derefed_ty, _)| { let (field_id, parameters) = match derefed_ty.kind(Interner) { TyKind::Tuple(_, substs) => { return name.as_tuple_index().and_then(|idx| { @@ -1226,6 +1228,7 @@ impl<'a> InferenceContext<'a> { .get(idx) .map(|a| a.assert_ty_ref(Interner)) .cloned() + .map(|ty| (None, ty)) }); } TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => { @@ -1244,58 +1247,81 @@ impl<'a> InferenceContext<'a> { .is_visible_from(self.db.upcast(), self.resolver.module()); if !is_visible { if private_field.is_none() { - private_field = Some(field_id); + private_field = Some((field_id, parameters)); } return None; } - // can't have `write_field_resolution` here because `self.table` is borrowed :( - self.result.field_resolutions.insert(tgt_expr, field_id); let ty = self.db.field_types(field_id.parent)[field_id.local_id] .clone() .substitute(Interner, ¶meters); - Some(ty) + Some((Some(field_id), ty)) }); - let ty = match ty { - Some(ty) => { + + Some(match res { + Some((field_id, ty)) => { + let adjustments = auto_deref_adjust_steps(&autoderef); + let ty = self.insert_type_vars(ty); + let ty = self.normalize_associated_types_in(ty); + + (ty, field_id, adjustments, true) + } + None => { + let (field_id, subst) = private_field?; let adjustments = auto_deref_adjust_steps(&autoderef); - self.write_expr_adj(expr, adjustments); + let ty = self.db.field_types(field_id.parent)[field_id.local_id] + .clone() + .substitute(Interner, &subst); let ty = self.insert_type_vars(ty); let ty = self.normalize_associated_types_in(ty); + + (ty, Some(field_id), adjustments, false) + } + }) + } + + fn infer_field_access(&mut self, tgt_expr: ExprId, receiver: ExprId, name: &Name) -> Ty { + let receiver_ty = self.infer_expr_inner(receiver, &Expectation::none()); + match self.lookup_field(&receiver_ty, name) { + Some((ty, field_id, adjustments, is_public)) => { + self.write_expr_adj(receiver, adjustments); + if let Some(field_id) = field_id { + self.result.field_resolutions.insert(tgt_expr, field_id); + } + if !is_public { + if let Some(field) = field_id { + // FIXME: Merge this diagnostic into UnresolvedField? + self.result + .diagnostics + .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field }); + } + } ty } - _ => { - // Write down the first private field resolution if we found no field - // This aids IDE features for private fields like goto def - if let Some(field) = private_field { - self.result.field_resolutions.insert(tgt_expr, field); - // FIXME: Merge this diagnostic into UnresolvedField - self.result - .diagnostics - .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field }); - } else { - // no field found, try looking for a method of the same name + None => { + // no field found, + let method_with_same_name_exists = { let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast()); - let resolved = method_resolution::lookup_method( + method_resolution::lookup_method( self.db, &canonicalized_receiver.value, self.trait_env.clone(), &traits_in_scope, VisibleFromModule::Filter(self.resolver.module()), name, - ); - self.result.diagnostics.push(InferenceDiagnostic::UnresolvedField { - expr: tgt_expr, - receiver: receiver_ty, - name: name.clone(), - method_with_same_name_exists: resolved.is_some(), - }); - } + ) + .is_some() + }; + self.result.diagnostics.push(InferenceDiagnostic::UnresolvedField { + expr: tgt_expr, + receiver: receiver_ty, + name: name.clone(), + method_with_same_name_exists, + }); self.err_ty() } - }; - ty + } } fn infer_method_call( @@ -1335,11 +1361,30 @@ impl<'a> InferenceContext<'a> { } (ty, self.db.value_ty(func.into()), substs) } - None => ( - receiver_ty, - Binders::empty(Interner, self.err_ty()), - Substitution::empty(Interner), - ), + None => { + let field_with_same_name_exists = match self.lookup_field(&receiver_ty, method_name) + { + Some((ty, field_id, adjustments, _public)) => { + self.write_expr_adj(receiver, adjustments); + if let Some(field_id) = field_id { + self.result.field_resolutions.insert(tgt_expr, field_id); + } + Some(ty) + } + None => None, + }; + self.result.diagnostics.push(InferenceDiagnostic::UnresolvedMethodCall { + expr: tgt_expr, + receiver: receiver_ty.clone(), + name: method_name.clone(), + field_with_same_name: field_with_same_name_exists, + }); + ( + receiver_ty, + Binders::empty(Interner, self.err_ty()), + Substitution::empty(Interner), + ) + } }; let method_ty = method_ty.substitute(Interner, &substs); self.register_obligations_for_call(&method_ty); diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 58d02479e5..b30c664e24 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -51,6 +51,7 @@ diagnostics![ UnresolvedField, UnresolvedImport, UnresolvedMacroCall, + UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, ]; @@ -147,6 +148,14 @@ pub struct UnresolvedField { } #[derive(Debug)] +pub struct UnresolvedMethodCall { + pub expr: InFile<AstPtr<ast::Expr>>, + pub receiver: Type, + pub name: Name, + pub field_with_same_name: Option<Type>, +} + +#[derive(Debug)] pub struct PrivateField { pub expr: InFile<AstPtr<ast::Expr>>, pub field: Field, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index bac6d4cd85..269c45943e 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -89,7 +89,7 @@ pub use crate::{ MissingMatchArms, MissingUnsafe, NoSuchField, PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall, - UnresolvedModule, UnresolvedProcMacro, + UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, }, has_source::HasSource, semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits}, @@ -1441,6 +1441,26 @@ impl DefWithBody { .into(), ) } + hir_ty::InferenceDiagnostic::UnresolvedMethodCall { + expr, + receiver, + name, + field_with_same_name, + } => { + let expr = expr_syntax(*expr); + + acc.push( + UnresolvedMethodCall { + expr, + name: name.clone(), + receiver: Type::new(db, DefWithBodyId::from(self), receiver.clone()), + field_with_same_name: field_with_same_name + .clone() + .map(|ty| Type::new(db, DefWithBodyId::from(self), ty)), + } + .into(), + ) + } } } for (pat_or_expr, mismatch) in infer.type_mismatches() { diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 8e338061df..936354f296 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -83,6 +83,14 @@ impl From<NoHashHashMap<FileId, TextEdit>> for SourceChange { } } +impl FromIterator<(FileId, TextEdit)> for SourceChange { + fn from_iter<T: IntoIterator<Item = (FileId, TextEdit)>>(iter: T) -> Self { + let mut this = SourceChange::default(); + this.extend(iter); + this + } +} + pub struct SourceChangeBuilder { pub edit: TextEditBuilder, pub file_id: FileId, diff --git a/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs b/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs index 9826e1c707..a0c276cc33 100644 --- a/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs +++ b/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs @@ -55,7 +55,18 @@ fn fixes( #[cfg(test)] mod tests { - use crate::tests::{check_diagnostics, check_fix}; + use crate::{ + tests::{check_diagnostics_with_config, check_fix}, + DiagnosticsConfig, + }; + + #[track_caller] + pub(crate) fn check_diagnostics(ra_fixture: &str) { + let mut config = DiagnosticsConfig::test_sample(); + config.disabled.insert("inactive-code".to_string()); + config.disabled.insert("unresolved-method".to_string()); + check_diagnostics_with_config(config, ra_fixture) + } #[test] fn replace_filter_map_next_with_find_map2() { diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs new file mode 100644 index 0000000000..0d1f91f02c --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -0,0 +1,130 @@ +use hir::{db::AstDatabase, HirDisplay}; +use ide_db::{ + assists::{Assist, AssistId, AssistKind}, + base_db::FileRange, + label::Label, + source_change::SourceChange, +}; +use syntax::{ast, AstNode, TextRange}; +use text_edit::TextEdit; + +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-method +// +// This diagnostic is triggered if a method does not exist on a given type. +pub(crate) fn unresolved_method( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedMethodCall, +) -> Diagnostic { + let field_suffix = if d.field_with_same_name.is_some() { + ", but a field with a similar name exists" + } else { + "" + }; + Diagnostic::new( + "unresolved-method", + format!( + "no method `{}` on type `{}`{field_suffix}", + d.name, + d.receiver.display(ctx.sema.db) + ), + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option<Vec<Assist>> { + if let Some(ty) = &d.field_with_same_name { + field_fix(ctx, d, ty) + } else { + // FIXME: add quickfix + None + } +} + +fn field_fix( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedMethodCall, + ty: &hir::Type, +) -> Option<Vec<Assist>> { + if !ty.impls_fnonce(ctx.sema.db) { + return None; + } + let expr_ptr = &d.expr; + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); + let (file_id, range) = match expr { + ast::Expr::MethodCallExpr(mcall) => { + let FileRange { range, file_id } = + ctx.sema.original_range_opt(mcall.receiver()?.syntax())?; + let FileRange { range: range2, file_id: file_id2 } = + ctx.sema.original_range_opt(mcall.name_ref()?.syntax())?; + if file_id != file_id2 { + return None; + } + (file_id, TextRange::new(range.start(), range2.end())) + } + _ => return None, + }; + Some(vec![Assist { + id: AssistId("expected-method-found-field-fix", AssistKind::QuickFix), + label: Label::new("Use parentheses to call the value of the field".to_string()), + group: None, + target: range, + source_change: Some(SourceChange::from_iter([ + (file_id, TextEdit::insert(range.start(), "(".to_owned())), + (file_id, TextEdit::insert(range.end(), ")".to_owned())), + ])), + trigger_signature_help: false, + }]) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_diagnostics, check_fix}; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn main() { + ().foo(); + // ^^^^^^^^ error: no method `foo` on type `()` +} +"#, + ); + } + + #[test] + fn field() { + check_diagnostics( + r#" +struct Foo { bar: i32 } +fn foo() { + Foo { bar: i32 }.bar(); + // ^^^^^^^^^^^^^^^^^^^^^^ error: no method `bar` on type `Foo`, but a field with a similar name exists +} +"#, + ); + } + + #[test] + fn callable_field() { + check_fix( + r#" +//- minicore: fn +struct Foo { bar: fn() } +fn foo() { + Foo { bar: foo }.b$0ar(); +} +"#, + r#" +struct Foo { bar: fn() } +fn foo() { + (Foo { bar: foo }.bar)(); +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index a0e8bca5cb..c8635ff801 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -45,6 +45,7 @@ mod handlers { pub(crate) mod unimplemented_builtin_macro; pub(crate) mod unresolved_extern_crate; pub(crate) mod unresolved_field; + pub(crate) mod unresolved_method; pub(crate) mod unresolved_import; pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_module; @@ -271,6 +272,7 @@ pub fn diagnostics( AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d, config.proc_macros_enabled, config.proc_attr_macros_enabled), AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d), AnyDiagnostic::UnresolvedField(d) => handlers::unresolved_field::unresolved_field(&ctx, &d), + AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { Some(it) => it, |