Unnamed repository; edit this file 'description' to name the repository.
Diagnose unresolved field accesses
| -rw-r--r-- | crates/hir-ty/src/infer.rs | 18 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/expr.rs | 152 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 9 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 47 | ||||
| -rw-r--r-- | crates/ide-completion/src/completions/flyimport.rs | 5 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/unresolved_field.rs | 134 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 2 |
7 files changed, 273 insertions, 94 deletions
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 0c7529cffe..a663a568b9 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -31,7 +31,7 @@ use hir_def::{ AdtId, AssocItemId, DefWithBodyId, EnumVariantId, FieldId, FunctionId, HasModule, ItemContainerId, Lookup, TraitId, TypeAliasId, VariantId, }; -use hir_expand::name::name; +use hir_expand::name::{name, Name}; use la_arena::ArenaMap; use rustc_hash::FxHashMap; use stdx::always; @@ -167,6 +167,7 @@ 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 }, // FIXME: Make this proper BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool }, MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, @@ -506,14 +507,17 @@ impl<'a> InferenceContext<'a> { mismatch.expected = table.resolve_completely(mismatch.expected.clone()); mismatch.actual = table.resolve_completely(mismatch.actual.clone()); } - for diagnostic in &mut result.diagnostics { - match diagnostic { - InferenceDiagnostic::ExpectedFunction { found, .. } => { - *found = table.resolve_completely(found.clone()) + result.diagnostics.retain_mut(|diagnostic| { + if let InferenceDiagnostic::ExpectedFunction { found: ty, .. } + | InferenceDiagnostic::UnresolvedField { receiver: ty, .. } = diagnostic + { + *ty = table.resolve_completely(ty.clone()); + if ty.is_unknown() { + return false; } - _ => (), } - } + true + }); for (_, subst) in result.method_resolutions.values_mut() { *subst = table.resolve_completely(subst.clone()); } diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 9d75b67bc7..531a359a96 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -552,71 +552,7 @@ impl<'a> InferenceContext<'a> { } ty } - Expr::Field { expr, name } => { - let receiver_ty = self.infer_expr_inner(*expr, &Expectation::none()); - - let mut autoderef = Autoderef::new(&mut self.table, receiver_ty); - let mut private_field = None; - let ty = 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| { - substs - .as_slice(Interner) - .get(idx) - .map(|a| a.assert_ty_ref(Interner)) - .cloned() - }); - } - TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => { - let local_id = self.db.struct_data(*s).variant_data.field(name)?; - let field = FieldId { parent: (*s).into(), local_id }; - (field, parameters.clone()) - } - TyKind::Adt(AdtId(hir_def::AdtId::UnionId(u)), parameters) => { - let local_id = self.db.union_data(*u).variant_data.field(name)?; - let field = FieldId { parent: (*u).into(), local_id }; - (field, parameters.clone()) - } - _ => return None, - }; - let is_visible = self.db.field_visibilities(field_id.parent)[field_id.local_id] - .is_visible_from(self.db.upcast(), self.resolver.module()); - if !is_visible { - if private_field.is_none() { - private_field = Some(field_id); - } - 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) - }); - let ty = match ty { - Some(ty) => { - let adjustments = auto_deref_adjust_steps(&autoderef); - self.write_expr_adj(*expr, adjustments); - let ty = self.insert_type_vars(ty); - let ty = self.normalize_associated_types_in(ty); - 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); - self.result - .diagnostics - .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field }); - } - self.err_ty() - } - }; - ty - } + Expr::Field { expr, name } => self.infer_field_access(tgt_expr, *expr, name), Expr::Await { expr } => { let inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); self.resolve_associated_type(inner_ty, self.resolve_future_future_output()) @@ -1276,6 +1212,92 @@ 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()); + + 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 (field_id, parameters) = match derefed_ty.kind(Interner) { + TyKind::Tuple(_, substs) => { + return name.as_tuple_index().and_then(|idx| { + substs + .as_slice(Interner) + .get(idx) + .map(|a| a.assert_ty_ref(Interner)) + .cloned() + }); + } + TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => { + let local_id = self.db.struct_data(*s).variant_data.field(name)?; + let field = FieldId { parent: (*s).into(), local_id }; + (field, parameters.clone()) + } + TyKind::Adt(AdtId(hir_def::AdtId::UnionId(u)), parameters) => { + let local_id = self.db.union_data(*u).variant_data.field(name)?; + let field = FieldId { parent: (*u).into(), local_id }; + (field, parameters.clone()) + } + _ => return None, + }; + let is_visible = self.db.field_visibilities(field_id.parent)[field_id.local_id] + .is_visible_from(self.db.upcast(), self.resolver.module()); + if !is_visible { + if private_field.is_none() { + private_field = Some(field_id); + } + 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) + }); + let ty = match ty { + Some(ty) => { + let adjustments = auto_deref_adjust_steps(&autoderef); + self.write_expr_adj(expr, adjustments); + let ty = self.insert_type_vars(ty); + let ty = self.normalize_associated_types_in(ty); + 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 + 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( + 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(), + }); + } + self.err_ty() + } + }; + ty + } + fn infer_method_call( &mut self, tgt_expr: ExprId, diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 3a6f26a4ea..58d02479e5 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -48,6 +48,7 @@ diagnostics![ TypeMismatch, UnimplementedBuiltinMacro, UnresolvedExternCrate, + UnresolvedField, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, @@ -138,6 +139,14 @@ pub struct ExpectedFunction { } #[derive(Debug)] +pub struct UnresolvedField { + pub expr: InFile<AstPtr<ast::Expr>>, + pub receiver: Type, + pub name: Name, + pub method_with_same_name_exists: bool, +} + +#[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 08ccf38b65..bac6d4cd85 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -88,8 +88,8 @@ pub use crate::{ InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, MissingUnsafe, NoSuchField, PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro, - UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, - UnresolvedProcMacro, + UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall, + UnresolvedModule, UnresolvedProcMacro, }, has_source::HasSource, semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits}, @@ -1375,6 +1375,7 @@ impl DefWithBody { let infer = db.infer(self.into()); let source_map = Lazy::new(|| db.body_with_source_map(self.into()).1); + let expr_syntax = |expr| source_map.expr_syntax(expr).expect("unexpected synthetic"); for d in &infer.diagnostics { match d { &hir_ty::InferenceDiagnostic::NoSuchField { expr } => { @@ -1386,30 +1387,23 @@ impl DefWithBody { is_break, bad_value_break, } => { - let expr = source_map - .expr_syntax(expr) - .expect("break outside of loop in synthetic syntax"); + let expr = expr_syntax(expr); acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into()) } &hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => { - match source_map.expr_syntax(call_expr) { - Ok(source_ptr) => acc.push( - MismatchedArgCount { call_expr: source_ptr, expected, found }.into(), - ), - Err(SyntheticSyntax) => (), - } + acc.push( + MismatchedArgCount { call_expr: expr_syntax(call_expr), expected, found } + .into(), + ) } &hir_ty::InferenceDiagnostic::PrivateField { expr, field } => { - let expr = source_map.expr_syntax(expr).expect("unexpected synthetic"); + let expr = expr_syntax(expr); let field = field.into(); acc.push(PrivateField { expr, field }.into()) } &hir_ty::InferenceDiagnostic::PrivateAssocItem { id, item } => { let expr_or_pat = match id { - ExprOrPatId::ExprId(expr) => source_map - .expr_syntax(expr) - .expect("unexpected synthetic") - .map(Either::Left), + ExprOrPatId::ExprId(expr) => expr_syntax(expr).map(Either::Left), ExprOrPatId::PatId(pat) => source_map .pat_syntax(pat) .expect("unexpected synthetic") @@ -1419,8 +1413,7 @@ impl DefWithBody { acc.push(PrivateAssocItem { expr_or_pat, item }.into()) } hir_ty::InferenceDiagnostic::ExpectedFunction { call_expr, found } => { - let call_expr = - source_map.expr_syntax(*call_expr).expect("unexpected synthetic"); + let call_expr = expr_syntax(*call_expr); acc.push( ExpectedFunction { @@ -1430,6 +1423,24 @@ impl DefWithBody { .into(), ) } + hir_ty::InferenceDiagnostic::UnresolvedField { + expr, + receiver, + name, + method_with_same_name_exists, + } => { + let expr = expr_syntax(*expr); + + acc.push( + UnresolvedField { + expr, + name: name.clone(), + receiver: Type::new(db, DefWithBodyId::from(self), receiver.clone()), + method_with_same_name_exists: *method_with_same_name_exists, + } + .into(), + ) + } } } for (pat_or_expr, mismatch) in infer.type_mismatches() { diff --git a/crates/ide-completion/src/completions/flyimport.rs b/crates/ide-completion/src/completions/flyimport.rs index 364969af9c..0979f6a6df 100644 --- a/crates/ide-completion/src/completions/flyimport.rs +++ b/crates/ide-completion/src/completions/flyimport.rs @@ -5,10 +5,7 @@ use ide_db::imports::{ insert_use::ImportScope, }; use itertools::Itertools; -use syntax::{ - ast::{self}, - AstNode, SyntaxNode, T, -}; +use syntax::{ast, AstNode, SyntaxNode, T}; use crate::{ context::{ diff --git a/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/crates/ide-diagnostics/src/handlers/unresolved_field.rs new file mode 100644 index 0000000000..33c39de085 --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/unresolved_field.rs @@ -0,0 +1,134 @@ +use hir::{db::AstDatabase, HirDisplay, InFile}; +use ide_db::{ + assists::{Assist, AssistId, AssistKind}, + base_db::FileRange, + label::Label, + source_change::SourceChange, +}; +use syntax::{ast, AstNode, AstPtr}; +use text_edit::TextEdit; + +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-field +// +// This diagnostic is triggered if a field does not exist on a given type. +pub(crate) fn unresolved_field( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedField, +) -> Diagnostic { + let method_suffix = if d.method_with_same_name_exists { + ", but a method with a similar name exists" + } else { + "" + }; + Diagnostic::new( + "unresolved-field", + format!( + "no field `{}` on type `{}`{method_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::UnresolvedField) -> Option<Vec<Assist>> { + if d.method_with_same_name_exists { + method_fix(ctx, &d.expr) + } else { + // FIXME: add quickfix + + None + } +} + +// FIXME: We should fill out the call here, mvoe the cursor and trigger signature help +fn method_fix( + ctx: &DiagnosticsContext<'_>, + expr_ptr: &InFile<AstPtr<ast::Expr>>, +) -> Option<Vec<Assist>> { + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); + let FileRange { range, file_id } = ctx.sema.original_range_opt(expr.syntax())?; + Some(vec![Assist { + id: AssistId("expected-field-found-method-call-fix", AssistKind::QuickFix), + label: Label::new("Use parentheses to call the method".to_string()), + group: None, + target: range, + source_change: Some(SourceChange::from_text_edit( + file_id, + TextEdit::insert(range.end(), "()".to_owned()), + )), + trigger_signature_help: false, + }]) +} +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn main() { + ().foo; + // ^^^^^^ error: no field `foo` on type `()` +} +"#, + ); + } + + #[test] + fn method_clash() { + check_diagnostics( + r#" +struct Foo; +impl Foo { + fn bar(&self) {} +} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } + + #[test] + fn method_trait_() { + check_diagnostics( + r#" +struct Foo; +trait Bar { + fn bar(&self) {} +} +impl Bar for Foo {} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } + + #[test] + fn method_trait_2() { + check_diagnostics( + r#" +struct Foo; +trait Bar { + fn bar(&self); +} +impl Bar for Foo { + fn bar(&self) {} +} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index b878119fee..a0e8bca5cb 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -44,6 +44,7 @@ mod handlers { pub(crate) mod type_mismatch; pub(crate) mod unimplemented_builtin_macro; pub(crate) mod unresolved_extern_crate; + pub(crate) mod unresolved_field; pub(crate) mod unresolved_import; pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_module; @@ -269,6 +270,7 @@ pub fn diagnostics( AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d), 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::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { Some(it) => it, |