Merge pull request #1054 from sbillig/diag-improvements

Diagnostic improvements

crates/driver/src/diagnostics.rs

@@ -52,7 +52,7 @@ where
             code,
             message,
             labels,
-            notes: vec![],
+            notes: complete.notes,
         }
     }
 }

crates/hir-analysis/src/diagnostics.rs

@@ -10,6 +10,7 @@ use crate::{
             BodyDiag, FuncBodyDiag, ImplDiag, TraitConstraintDiag, TraitLowerDiag,
             TyDiagCollection, TyLowerDiag,
         },
+        ty_check::RecordLike,
         ty_def::{TyData, TyVarSort},
     },
     HirAnalysisDb,
@@ -1255,6 +1256,45 @@ impl<'db> DiagnosticVoucher<'db> for BodyDiag<'db> {
                 }
             }
 
+            Self::NotAMethod {
+                span,
+                receiver_ty,
+                func_name,
+                func_ty,
+            } => CompleteDiagnostic {
+                severity: Severity::Error,
+                message: format!("`{}` is not a method", func_name.data(adb)),
+                sub_diagnostics: vec![
+                    SubDiagnostic {
+                        style: LabelStyle::Primary,
+                        message: format!(
+                            "`{}` is an associated function, not a method",
+                            func_name.data(adb),
+                        ),
+                        span: span.method_name().resolve(db),
+                    },
+                    SubDiagnostic {
+                        style: LabelStyle::Primary,
+                        message: format!(
+                            "help: use associated function syntax instead: `{}::{}`",
+                            receiver_ty.pretty_print(adb),
+                            func_name.data(adb)
+                        ),
+                        span: span.resolve(db),
+                    },
+                    SubDiagnostic {
+                        style: LabelStyle::Secondary,
+                        message: "function defined here".to_string(),
+                        span: func_ty.name_span(adb).unwrap().resolve(db),
+                    },
+                ],
+                notes: vec![
+                    "note: to be used as a method, a function must have a `self` parameter"
+                        .to_string(),
+                ],
+                error_code,
+            },
+
             Self::AmbiguousInherentMethodCall {
                 primary,
                 method_name,
@@ -1369,22 +1409,51 @@ impl<'db> DiagnosticVoucher<'db> for BodyDiag<'db> {
                 method_name,
                 receiver,
             } => {
-                let receiver = match receiver {
-                    Either::Left(ty) => ty.pretty_print(adb),
-                    Either::Right(trait_) => trait_
-                        .trait_(db)
-                        .name(db.as_hir_db())
-                        .unwrap()
-                        .data(db.as_hir_db()),
+                let (recv_name, recv_ty, recv_kind) = match receiver {
+                    Either::Left(ty) => (ty.pretty_print(adb), Some(ty), ty.kind_name(adb)),
+                    Either::Right(trait_) => {
+                        let name = trait_
+                            .trait_(db)
+                            .name(db.as_hir_db())
+                            .unwrap()
+                            .data(db.as_hir_db());
+                        (name, None, "trait".to_string())
+                    }
                 };
 
-                let method_name = method_name.data(db.as_hir_db());
+                let method_str = method_name.data(db.as_hir_db());
+                let message = format!(
+                    "no method named `{}` found for {} `{}`",
+                    method_str, recv_kind, recv_name
+                );
+
+                if let Some(ty) = recv_ty {
+                    if let Some(field_ty) = ty.record_field_ty(adb, *method_name) {
+                        return CompleteDiagnostic {
+                            severity: Severity::Error,
+                            message,
+                            sub_diagnostics: vec![SubDiagnostic {
+                                style: LabelStyle::Primary,
+                                message: format!(
+                                    "field `{}` in `{}` has type `{}`",
+                                    method_str,
+                                    recv_name,
+                                    field_ty.pretty_print(adb)
+                                ),
+                                span: primary.resolve(db),
+                            }],
+                            notes: vec![],
+                            error_code,
+                        };
+                    }
+                }
+
                 CompleteDiagnostic {
                     severity: Severity::Error,
-                    message: format!("`{}` is not found", method_name),
+                    message,
                     sub_diagnostics: vec![SubDiagnostic {
                         style: LabelStyle::Primary,
-                        message: format!("`{}` is not found in `{}`", method_name, receiver),
+                        message: format!("method not found in `{}`", recv_name),
                         span: primary.resolve(db),
                     }],
                     notes: vec![],

crates/hir-analysis/src/ty/diagnostics.rs

@@ -3,7 +3,7 @@ use hir::{
     hir_def::{
         FieldIndex, Func, IdentId, ImplTrait, ItemKind, PathId, Trait, TypeAlias as HirTypeAlias,
     },
-    span::DynLazySpan,
+    span::{expr::LazyMethodCallExprSpan, DynLazySpan},
 };
 use smallvec2::SmallVec;
 
@@ -263,6 +263,13 @@ pub enum BodyDiag<'db> {
 
     NotCallable(DynLazySpan<'db>, TyId<'db>),
 
+    NotAMethod {
+        span: LazyMethodCallExprSpan<'db>,
+        receiver_ty: TyId<'db>,
+        func_name: IdentId<'db>,
+        func_ty: TyId<'db>,
+    },
+
     CallGenericArgNumMismatch {
         primary: DynLazySpan<'db>,
         def_span: DynLazySpan<'db>,
@@ -444,6 +451,7 @@ impl<'db> BodyDiag<'db> {
             Self::NotValue { .. } => 30,
             Self::TypeAnnotationNeeded { .. } => 31,
             Self::DuplicatedBinding { .. } => 32,
+            Self::NotAMethod { .. } => 33,
         }
     }
 }

crates/hir-analysis/src/ty/ty_check/callable.rs

@@ -23,7 +23,7 @@ use crate::{
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Callable<'db> {
-    func_def: FuncDef<'db>,
+    pub func_def: FuncDef<'db>,
     generic_args: Vec<TyId<'db>>,
 }
 

crates/hir-analysis/src/ty/ty_check/expr.rs

@@ -14,8 +14,8 @@ use super::{
 };
 use crate::{
     name_resolution::{
-        diagnostics::NameResDiag, is_scope_visible_from, resolve_name_res, resolve_path,
-        resolve_query, EarlyNameQueryId, NameDomain, NameResBucket, PathRes, QueryDirective,
+        diagnostics::NameResDiag, is_scope_visible_from, resolve_name_res, resolve_query,
+        EarlyNameQueryId, NameDomain, NameResBucket, PathRes, QueryDirective,
     },
     ty::{
         canonical::Canonicalized,
@@ -370,6 +370,17 @@ impl<'db> TyChecker<'db> {
             return ExprProp::invalid(self.db);
         }
 
+        if !callable.func_def.is_method(self.db) {
+            let diag = BodyDiag::NotAMethod {
+                span: call_span,
+                receiver_ty: receiver_prop.ty,
+                func_name: method_name,
+                func_ty,
+            };
+            self.push_diag(diag);
+            return ExprProp::invalid(self.db);
+        }
+
         callable.check_args(
             self,
             args,
@@ -467,9 +478,18 @@ impl<'db> TyChecker<'db> {
                 }
                 PathRes::Const(ty) => ExprProp::new(ty, true),
                 PathRes::TypeMemberTbd(parent_ty) => {
-                    let ty = self
-                        .select_method_candidate_for_path(parent_ty, *path, span.path())
-                        .unwrap_or_else(|| TyId::invalid(self.db, InvalidCause::Other));
+                    let ty = if parent_ty.has_invalid(self.db) {
+                        let span = span
+                            .path()
+                            .segment(path.segment_index(self.db.as_hir_db()) - 1);
+                        if let Some(diag) = parent_ty.emit_diag(self.db, span.into()) {
+                            self.diags.push(diag.into());
+                        }
+                        TyId::invalid(self.db, InvalidCause::Other)
+                    } else {
+                        self.select_method_candidate_for_path(parent_ty, *path, span.path())
+                            .unwrap_or_else(|| TyId::invalid(self.db, InvalidCause::Other))
+                    };
                     ExprProp::new(self.table.instantiate_to_term(ty), true)
                 }
                 PathRes::Mod(_) | PathRes::FuncParam(..) => todo!(),
@@ -487,14 +507,13 @@ impl<'db> TyChecker<'db> {
             return ExprProp::invalid(self.db);
         };
 
-        let Ok(reso) = resolve_path(self.db, *path, self.env.scope(), true) else {
+        let Ok(reso) = self.resolve_path(*path, true) else {
             return ExprProp::invalid(self.db);
         };
 
         match reso {
             PathRes::Ty(ty) if ty.is_record(self.db) => {
-                let ty = self.table.instantiate_to_term(ty);
-                self.check_record_init_fields(ty, expr);
+                self.check_record_init_fields(&ty, expr);
                 ExprProp::new(ty, true)
             }
 
@@ -511,9 +530,8 @@ impl<'db> TyChecker<'db> {
 
             PathRes::EnumVariant(variant) => {
                 if variant.is_record(self.db) {
-                    let ty = self.table.instantiate_to_term(variant.ty);
-                    self.check_record_init_fields(variant, expr);
-                    ExprProp::new(ty, true)
+                    self.check_record_init_fields(&variant, expr);
+                    ExprProp::new(variant.ty, true)
                 } else {
                     let diag =
                         BodyDiag::record_expected::<TyId<'db>>(self.db, span.path().into(), None);
@@ -541,15 +559,15 @@ impl<'db> TyChecker<'db> {
         }
     }
 
-    fn check_record_init_fields<T: RecordLike<'db>>(&mut self, mut record_like: T, expr: ExprId) {
+    fn check_record_init_fields<T: RecordLike<'db>>(&mut self, record_like: &T, expr: ExprId) {
         let hir_db = self.db.as_hir_db();
 
         let Partial::Present(Expr::RecordInit(_, fields)) = expr.data(hir_db, self.body()) else {
             unreachable!()
         };
         let span = expr.lazy_span(self.body()).into_record_init_expr();
 
-        let mut rec_checker = RecordInitChecker::new(self, &mut record_like);
+        let mut rec_checker = RecordInitChecker::new(self, record_like);
 
         for (i, field) in fields.iter().enumerate() {
             let label = field.label_eagerly(rec_checker.tc.db.as_hir_db(), rec_checker.tc.body());

crates/hir-analysis/src/ty/ty_check/mod.rs

@@ -15,7 +15,7 @@ use hir::{
     span::{expr::LazyExprSpan, pat::LazyPatSpan, DynLazySpan},
     visitor::{walk_expr, walk_pat, Visitor, VisitorCtxt},
 };
-pub(super) use path::RecordLike;
+pub(crate) use path::RecordLike;
 
 use rustc_hash::{FxHashMap, FxHashSet};
 

crates/hir-analysis/src/ty/ty_check/pat.rs

@@ -381,7 +381,7 @@ impl<'db> TyChecker<'db> {
         }
     }
 
-    fn check_record_pat_fields<T>(&mut self, mut record_like: T, pat: PatId)
+    fn check_record_pat_fields<T>(&mut self, record_like: T, pat: PatId)
     where
         T: RecordLike<'db>,
     {
@@ -394,7 +394,7 @@ impl<'db> TyChecker<'db> {
         let mut contains_rest = false;
 
         let pat_span = pat.lazy_span(self.body()).into_record_pat();
-        let mut rec_checker = RecordInitChecker::new(self, &mut record_like);
+        let mut rec_checker = RecordInitChecker::new(self, &record_like);
 
         for (i, field_pat) in fields.iter().enumerate() {
             let field_pat_span = pat_span.fields().field(i);

crates/hir-analysis/src/ty/ty_check/path.rs

@@ -47,7 +47,7 @@ pub(super) enum ResolvedPathInBody<'db> {
 
 pub(super) struct RecordInitChecker<'tc, 'db, 'a, T> {
     pub(super) tc: &'tc mut TyChecker<'db>,
-    data: &'a mut T,
+    data: &'a T,
     already_given: FxHashMap<IdentId<'db>, DynLazySpan<'db>>,
     invalid_field_given: bool,
 }
@@ -60,7 +60,7 @@ where
     ///
     /// ## Panics
     /// Panics if the given `data` is not a record.
-    pub(super) fn new(tc: &'tc mut TyChecker<'db>, data: &'a mut T) -> Self {
+    pub(super) fn new(tc: &'tc mut TyChecker<'db>, data: &'a T) -> Self {
         assert!(data.is_record(tc.db));
 
         Self {

crates/hir-analysis/test_files/ty_check/record_init.fe

@@ -3,12 +3,12 @@ struct Foo {
     y: String<10>
 }
 
-fn foo() { 
+fn foo() {
     let x = 1
     let y = "FOO"
 
     let f = Foo {x, y}
-    
+
     let f2 = Foo {x: 1, y: "FOO"}
 
     let f3 = Foo {y: "FOO", x: 1}
@@ -23,7 +23,7 @@ fn foo2<Z>(b: bool, z: Z) {
     let t = false
     let u = "Bar"
     let f = Bar {t, u}
-    
+
     let f2 = Bar {t: z, u: f}
 }
 
@@ -36,4 +36,12 @@ where T: * -> * -> *
 fn foo3() {
     let bar = Bar { t: 1, u: false }
     let x = Wrapper { t: bar }
-}
+}
+
+enum G<T> {
+    V1 { val: T }
+}
+
+fn generic_path() {
+    let g = G<u32>::V1 { val: 10 }
+}