Diagnose unresolved field accesses

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 {
+        result.diagnostics.retain_mut(|diagnostic| {
-            match diagnostic {
+            if let InferenceDiagnostic::ExpectedFunction { found: ty, .. }
-                InferenceDiagnostic::ExpectedFunction { found, .. } => {
+            | InferenceDiagnostic::UnresolvedField { receiver: ty, .. } = diagnostic
-                    *found = table.resolve_completely(found.clone())
+            {
+                *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());
         }

crates/hir-ty/src/infer/expr.rs

@@ -552,71 +552,7 @@ impl<'a> InferenceContext<'a> {
                 }
                 ty
             }
-            Expr::Field { expr, name } => {
+            Expr::Field { expr, name } => self.infer_field_access(tgt_expr, *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, &parameters);
-                    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::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, &parameters);
+            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,

crates/hir/src/diagnostics.rs

@@ -48,6 +48,7 @@ diagnostics![
     TypeMismatch,
     UnimplementedBuiltinMacro,
     UnresolvedExternCrate,
+    UnresolvedField,
     UnresolvedImport,
     UnresolvedMacroCall,
     UnresolvedModule,
@@ -137,6 +138,14 @@ pub struct ExpectedFunction {
     pub found: Type,
 }
 
+#[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>>,

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,
+        UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedProcMacro,
+        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
+                    let expr = expr_syntax(expr);
-                        .expr_syntax(expr)
-                        .expect("break outside of loop in synthetic syntax");
                     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) {
+                    acc.push(
-                        Ok(source_ptr) => acc.push(
+                        MismatchedArgCount { call_expr: expr_syntax(call_expr), expected, found }
-                            MismatchedArgCount { call_expr: source_ptr, expected, found }.into(),
+                            .into(),
-                        ),
+                    )
-                        Err(SyntheticSyntax) => (),
-                    }
                 }
                 &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
+                        ExprOrPatId::ExprId(expr) => expr_syntax(expr).map(Either::Left),
-                            .expr_syntax(expr)
-                            .expect("unexpected synthetic")
-                            .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 =
+                    let call_expr = expr_syntax(*call_expr);
-                        source_map.expr_syntax(*call_expr).expect("unexpected synthetic");
 
                     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() {

crates/ide-completion/src/completions/flyimport.rs

@@ -5,10 +5,7 @@ use ide_db::imports::{
     insert_use::ImportScope,
 };
 use itertools::Itertools;
-use syntax::{
+use syntax::{ast, AstNode, SyntaxNode, T};
-    ast::{self},
-    AstNode, SyntaxNode, T,
-};
 
 use crate::{
     context::{

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
+}
+"#,
+        );
+    }
+}

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,