fix: Make value_ty query fallible

Author: Veykril

crates/hir-def/src/db.rs

@@ -95,21 +95,7 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
     #[salsa::invoke(DefMap::crate_def_map_query)]
     fn crate_def_map_query(&self, krate: CrateId) -> Arc<DefMap>;
 
-    /// Computes the block-level `DefMap`, returning `None` when `block` doesn't contain any inner
-    /// items directly.
-    ///
-    /// For example:
-    ///
-    /// ```
-    /// fn f() { // (0)
-    ///     { // (1)
-    ///         fn inner() {}
-    ///     }
-    /// }
-    /// ```
-    ///
-    /// The `block_def_map` for block 0 would return `None`, while `block_def_map` of block 1 would
-    /// return a `DefMap` containing `inner`.
+    /// Computes the block-level `DefMap`.
     #[salsa::invoke(DefMap::block_def_map_query)]
     fn block_def_map(&self, block: BlockId) -> Arc<DefMap>;
 

crates/hir-ty/src/db.rs

@@ -86,8 +86,10 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
     #[salsa::cycle(crate::lower::ty_recover)]
     fn ty(&self, def: TyDefId) -> Binders<Ty>;
 
+    /// Returns the type of the value of the given constant, or `None` if the the `ValueTyDefId` is
+    /// a `StructId` or `EnumVariantId` with a record constructor.
     #[salsa::invoke(crate::lower::value_ty_query)]
-    fn value_ty(&self, def: ValueTyDefId) -> Binders<Ty>;
+    fn value_ty(&self, def: ValueTyDefId) -> Option<Binders<Ty>>;
 
     #[salsa::invoke(crate::lower::impl_self_ty_query)]
     #[salsa::cycle(crate::lower::impl_self_ty_recover)]

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

@@ -1245,7 +1245,7 @@ impl InferenceContext<'_> {
             .build();
         self.write_method_resolution(tgt_expr, func, subst.clone());
 
-        let method_ty = self.db.value_ty(func.into()).substitute(Interner, &subst);
+        let method_ty = self.db.value_ty(func.into()).unwrap().substitute(Interner, &subst);
         self.register_obligations_for_call(&method_ty);
 
         self.infer_expr_coerce(rhs, &Expectation::has_type(rhs_ty.clone()));
@@ -1541,7 +1541,7 @@ impl InferenceContext<'_> {
                         self.check_method_call(
                             tgt_expr,
                             &[],
-                            self.db.value_ty(func.into()),
+                            self.db.value_ty(func.into()).unwrap(),
                             substs,
                             ty,
                             expected,
@@ -1586,7 +1586,7 @@ impl InferenceContext<'_> {
                         item: func.into(),
                     })
                 }
-                (ty, self.db.value_ty(func.into()), substs)
+                (ty, self.db.value_ty(func.into()).unwrap(), substs)
             }
             None => {
                 let field_with_same_name_exists = match self.lookup_field(&receiver_ty, method_name)

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

@@ -34,7 +34,7 @@ impl InferenceContext<'_> {
 
         self.add_required_obligations_for_value_path(generic_def, &substs);
 
-        let ty = self.db.value_ty(value_def).substitute(Interner, &substs);
+        let ty = self.db.value_ty(value_def)?.substitute(Interner, &substs);
         let ty = self.normalize_associated_types_in(ty);
         Some(ty)
     }
@@ -98,7 +98,7 @@ impl InferenceContext<'_> {
         let Some(generic_def) = value_def.to_generic_def_id() else {
             // `value_def` is the kind of item that can never be generic (i.e. statics, at least
             // currently). We can just skip the binders to get its type.
-            let (ty, binders) = self.db.value_ty(value_def).into_value_and_skipped_binders();
+            let (ty, binders) = self.db.value_ty(value_def)?.into_value_and_skipped_binders();
             stdx::always!(
                 parent_substs.is_none() && binders.is_empty(Interner),
                 "non-empty binders for non-generic def",

crates/hir-ty/src/lower.rs

@@ -1727,19 +1727,19 @@ fn fn_sig_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> PolyFnS
 }
 
 /// Build the type of a tuple struct constructor.
-fn type_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> Binders<Ty> {
+fn type_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> Option<Binders<Ty>> {
     let struct_data = db.struct_data(def);
     match struct_data.variant_data.kind() {
-        StructKind::Record => unreachable!("callers check for valueness of variant"),
-        StructKind::Unit => return type_for_adt(db, def.into()),
+        StructKind::Record => None,
+        StructKind::Unit => Some(type_for_adt(db, def.into())),
         StructKind::Tuple => {
             let generics = generics(db.upcast(), AdtId::from(def).into());
             let substs = generics.bound_vars_subst(db, DebruijnIndex::INNERMOST);
-            make_binders(
+            Some(make_binders(
                 db,
                 &generics,
                 TyKind::FnDef(CallableDefId::StructId(def).to_chalk(db), substs).intern(Interner),
-            )
+            ))
         }
     }
 }
@@ -1757,20 +1757,23 @@ fn fn_sig_for_enum_variant_constructor(db: &dyn HirDatabase, def: EnumVariantId)
 }
 
 /// Build the type of a tuple enum variant constructor.
-fn type_for_enum_variant_constructor(db: &dyn HirDatabase, def: EnumVariantId) -> Binders<Ty> {
+fn type_for_enum_variant_constructor(
+    db: &dyn HirDatabase,
+    def: EnumVariantId,
+) -> Option<Binders<Ty>> {
     let e = def.lookup(db.upcast()).parent;
     match db.enum_variant_data(def).variant_data.kind() {
-        StructKind::Record => unreachable!("callers check for valueness of variant"),
-        StructKind::Unit => return type_for_adt(db, e.into()),
+        StructKind::Record => None,
+        StructKind::Unit => Some(type_for_adt(db, e.into())),
         StructKind::Tuple => {
             let generics = generics(db.upcast(), e.into());
             let substs = generics.bound_vars_subst(db, DebruijnIndex::INNERMOST);
-            make_binders(
+            Some(make_binders(
                 db,
                 &generics,
                 TyKind::FnDef(CallableDefId::EnumVariantId(def).to_chalk(db), substs)
                     .intern(Interner),
-            )
+            ))
         }
     }
 }
@@ -1889,14 +1892,14 @@ pub(crate) fn ty_recover(db: &dyn HirDatabase, _cycle: &Cycle, def: &TyDefId) ->
     make_binders(db, &generics, TyKind::Error.intern(Interner))
 }
 
-pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Binders<Ty> {
+pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Option<Binders<Ty>> {
     match def {
-        ValueTyDefId::FunctionId(it) => type_for_fn(db, it),
+        ValueTyDefId::FunctionId(it) => Some(type_for_fn(db, it)),
         ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it),
-        ValueTyDefId::UnionId(it) => type_for_adt(db, it.into()),
+        ValueTyDefId::UnionId(it) => Some(type_for_adt(db, it.into())),
         ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it),
-        ValueTyDefId::ConstId(it) => type_for_const(db, it),
-        ValueTyDefId::StaticId(it) => type_for_static(db, it),
+        ValueTyDefId::ConstId(it) => Some(type_for_const(db, it)),
+        ValueTyDefId::StaticId(it) => Some(type_for_static(db, it)),
     }
 }
 

crates/hir-ty/src/tests/regression.rs

@@ -2012,3 +2012,31 @@ fn rustc_test_issue_52437() {
     "#,
     );
 }
+
+#[test]
+fn incorrect_variant_form_through_alias_caught() {
+    check_types(
+        r#"
+enum Enum { Braced {}, Unit, Tuple() }
+type Alias = Enum;
+
+fn main() {
+    Alias::Braced;
+  //^^^^^^^^^^^^^ {unknown}
+    let Alias::Braced = loop {};
+      //^^^^^^^^^^^^^ !
+  let Alias::Braced(..) = loop {};
+    //^^^^^^^^^^^^^^^^^ Enum
+
+    Alias::Unit();
+  //^^^^^^^^^^^^^ {unknown}
+    Alias::Unit{};
+  //^^^^^^^^^^^^^ Enum
+    let Alias::Unit() = loop {};
+      //^^^^^^^^^^^^^ Enum
+    let Alias::Unit{} = loop {};
+      //^^^^^^^^^^^^^ Enum
+}
+"#,
+    )
+}

crates/hir/src/lib.rs

@@ -3786,7 +3786,9 @@ impl Type {
     }
 
     fn from_value_def(db: &dyn HirDatabase, def: impl Into<ValueTyDefId> + HasResolver) -> Type {
-        let ty = db.value_ty(def.into());
+        let Some(ty) = db.value_ty(def.into()) else {
+            return Type::new(db, def, TyKind::Error.intern(Interner));
+        };
         let substs = TyBuilder::unknown_subst(
             db,
             match def.into() {

crates/hir/src/source_analyzer.rs

@@ -268,7 +268,7 @@ impl SourceAnalyzer {
     ) -> Option<Callable> {
         let expr_id = self.expr_id(db, &call.clone().into())?;
         let (func, substs) = self.infer.as_ref()?.method_resolution(expr_id)?;
-        let ty = db.value_ty(func.into()).substitute(Interner, &substs);
+        let ty = db.value_ty(func.into())?.substitute(Interner, &substs);
         let ty = Type::new_with_resolver(db, &self.resolver, ty);
         let mut res = ty.as_callable(db)?;
         res.is_bound_method = true;