hir: drop support for assoc type aliases, simplifying resolution of assoc items in docs

Author: 71

crates/hir-ty/src/method_resolution.rs

@@ -10,7 +10,6 @@ use hir_def::{
     data::{adt::StructFlags, ImplData},
     item_scope::ItemScope,
     nameres::DefMap,
-    resolver::HasResolver,
     AssocItemId, BlockId, ConstId, FunctionId, HasModule, ImplId, ItemContainerId, Lookup,
     ModuleDefId, ModuleId, TraitId,
 };
@@ -1003,20 +1002,6 @@ pub fn iterate_method_candidates_dyn(
     }
 }
 
-pub fn iterate_trait_item_candidates(
-    ty: &Canonical<Ty>,
-    db: &dyn HirDatabase,
-    env: Arc<TraitEnvironment>,
-    traits_in_scope: &FxHashSet<TraitId>,
-    name: Option<&Name>,
-    callback: &mut dyn FnMut(AssocItemId, bool) -> ControlFlow<()>,
-) {
-    let mut table = InferenceTable::new(db, env);
-    let self_ty = table.instantiate_canonical(ty.clone());
-
-    iterate_trait_item_candidates_(&self_ty, &mut table, traits_in_scope, name, None, callback);
-}
-
 fn iterate_method_candidates_with_autoref(
     receiver_ty: &Canonical<Ty>,
     first_adjustment: ReceiverAdjustments,
@@ -1160,26 +1145,6 @@ fn iterate_trait_method_candidates(
     receiver_ty: Option<&Ty>,
     receiver_adjustments: Option<ReceiverAdjustments>,
     callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
-) -> ControlFlow<()> {
-    iterate_trait_item_candidates_(
-        self_ty,
-        table,
-        traits_in_scope,
-        name,
-        receiver_ty,
-        &mut move |assoc_item_id, visible| {
-            callback(receiver_adjustments.clone().unwrap_or_default(), assoc_item_id, visible)
-        },
-    )
-}
-
-fn iterate_trait_item_candidates_(
-    self_ty: &Ty,
-    table: &mut InferenceTable<'_>,
-    traits_in_scope: &FxHashSet<TraitId>,
-    name: Option<&Name>,
-    receiver_ty: Option<&Ty>,
-    callback: &mut dyn FnMut(AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     let db = table.db;
     let env = table.trait_env.clone();
@@ -1222,7 +1187,7 @@ fn iterate_trait_item_candidates_(
                 }
             }
             known_implemented = true;
-            callback(item, visible)?;
+            callback(receiver_adjustments.clone().unwrap_or_default(), item, visible)?;
         }
     }
     ControlFlow::Continue(())
@@ -1417,8 +1382,6 @@ fn is_valid_candidate(
     visible_from_module: Option<ModuleId>,
 ) -> IsValidCandidate {
     let db = table.db;
-    let def_db = db.upcast();
-
     match item {
         AssocItemId::FunctionId(f) => {
             is_valid_fn_candidate(table, f, name, receiver_ty, self_ty, visible_from_module)
@@ -1434,61 +1397,23 @@ fn is_valid_candidate(
                 }
             }
             if let ItemContainerId::ImplId(impl_id) = c.lookup(db.upcast()).container {
-                if !self_ty_matches(table, db, impl_id, self_ty) {
+                let self_ty_matches = table.run_in_snapshot(|table| {
+                    let expected_self_ty = TyBuilder::impl_self_ty(db, impl_id)
+                        .fill_with_inference_vars(table)
+                        .build();
+                    table.unify(&expected_self_ty, self_ty)
+                });
+                if !self_ty_matches {
                     cov_mark::hit!(const_candidate_self_type_mismatch);
                     return IsValidCandidate::No;
                 }
             }
             IsValidCandidate::Yes
         }
-        AssocItemId::TypeAliasId(t) => {
-            // Q: should this branch be restricted to `iterate_trait_item_candidates()`?
-            //
-            // the code below does not seem to be called when adding tests to
-            // `method_resolution.rs`, so resolution of type aliases is likely performed at some
-            // other point. due to the marks below, however, the test which ensures that marks have
-            // corresponding checks will fail
-            let data = db.type_alias_data(t);
-
-            check_that!(receiver_ty.is_none());
-            check_that!(name.map_or(true, |n| data.name == *n));
-
-            if let Some(from_module) = visible_from_module {
-                // Q: should the resolved visibility be added to the database?
-                let visibility = data.visibility.resolve(def_db, &t.resolver(def_db));
-
-                if !visibility.is_visible_from(def_db, from_module) {
-                    // cov_mark::hit!(type_alias_candidate_not_visible);
-                    return IsValidCandidate::NotVisible;
-                }
-            }
-
-            // Q: is it correct to use the same check as `ConstId`?
-            if let ItemContainerId::ImplId(impl_id) = t.lookup(def_db).container {
-                if !self_ty_matches(table, db, impl_id, self_ty) {
-                    // cov_mark::hit!(type_alias_candidate_self_type_mismatch);
-                    return IsValidCandidate::No;
-                }
-            }
-
-            IsValidCandidate::Yes
-        }
+        _ => IsValidCandidate::No,
     }
 }
 
-fn self_ty_matches(
-    table: &mut InferenceTable<'_>,
-    db: &dyn HirDatabase,
-    impl_id: ImplId,
-    self_ty: &Ty,
-) -> bool {
-    table.run_in_snapshot(|table| {
-        let expected_self_ty =
-            TyBuilder::impl_self_ty(db, impl_id).fill_with_inference_vars(table).build();
-        table.unify(&expected_self_ty, self_ty)
-    })
-}
-
 enum IsValidCandidate {
     Yes,
     No,

crates/hir/src/attrs.rs

@@ -1,5 +1,7 @@
 //! Attributes & documentation for hir types.
 
+use std::ops::ControlFlow;
+
 use base_db::FileId;
 use hir_def::{
     attr::AttrsWithOwner,
@@ -13,7 +15,7 @@ use hir_expand::{
     name::Name,
     span_map::{RealSpanMap, SpanMapRef},
 };
-use hir_ty::db::HirDatabase;
+use hir_ty::{db::HirDatabase, method_resolution};
 use syntax::{ast, AstNode};
 
 use crate::{
@@ -243,23 +245,46 @@ fn resolve_impl_trait_item(
     name: &Name,
     ns: Option<Namespace>,
 ) -> Option<DocLinkDef> {
+    let canonical = ty.canonical();
+    let krate = ty.krate(db);
+    let environment = resolver.generic_def().map_or_else(
+        || crate::TraitEnvironment::empty(krate.id).into(),
+        |d| db.trait_environment(d),
+    );
     let traits_in_scope = resolver.traits_in_scope(db.upcast());
 
-    // If two traits in scope define the same item, Rustdoc links to no specific trait (for
-    // instance, given two methods `a`, Rustdoc simply links to `method.a` with no
-    // disambiguation) so we just pick the first one we find as well.
-    ty.iterate_trait_item_candidates(
+    let mut result = None;
+
+    // `ty.iterate_path_candidates()` require a scope, which is not available when resolving
+    // attributes here. Use path resolution directly instead.
+    //
+    // FIXME: resolve type aliases (which are not yielded by iterate_path_candidates)
+    method_resolution::iterate_path_candidates(
+        &canonical,
         db,
-        ty.krate(db),
-        &resolver,
+        environment,
         &traits_in_scope,
+        method_resolution::VisibleFromModule::None,
         Some(name),
-        move |assoc_item| {
+        &mut |assoc_item_id| {
+            let assoc_item: AssocItem = assoc_item_id.into();
+
             debug_assert_eq!(assoc_item.name(db).as_ref(), Some(name));
 
-            as_module_def_if_namespace_matches(assoc_item, ns)
+            // If two traits in scope define the same item, Rustdoc links to no specific trait (for
+            // instance, given two methods `a`, Rustdoc simply links to `method.a` with no
+            // disambiguation) so we just pick the first one we find as well.
+            result = as_module_def_if_namespace_matches(assoc_item, ns);
+
+            if result.is_some() {
+                ControlFlow::Break(())
+            } else {
+                ControlFlow::Continue(())
+            }
         },
-    )
+    );
+
+    result
 }
 
 fn resolve_field(
@@ -281,11 +306,7 @@ fn as_module_def_if_namespace_matches(
     let (def, expected_ns) = match assoc_item {
         AssocItem::Function(it) => (ModuleDef::Function(it), Namespace::Values),
         AssocItem::Const(it) => (ModuleDef::Const(it), Namespace::Values),
-        AssocItem::TypeAlias(it) => {
-            // Inherent associated types are supported in nightly:
-            // https://github.com/rust-lang/rust/issues/8995
-            (ModuleDef::TypeAlias(it), Namespace::Types)
-        }
+        AssocItem::TypeAlias(it) => (ModuleDef::TypeAlias(it), Namespace::Types),
     };
 
     (ns.unwrap_or(expected_ns) == expected_ns).then(|| DocLinkDef::ModuleDef(def))

crates/hir/src/lib.rs

@@ -4121,6 +4121,10 @@ impl Type {
         }
     }
 
+    pub(crate) fn canonical(&self) -> Canonical<Ty> {
+        hir_ty::replace_errors_with_variables(&self.ty)
+    }
+
     /// Returns types that this type dereferences to (including this type itself). The returned
     /// iterator won't yield the same type more than once even if the deref chain contains a cycle.
     pub fn autoderef(&self, db: &dyn HirDatabase) -> impl Iterator<Item = Type> + '_ {
@@ -4387,63 +4391,6 @@ impl Type {
         );
     }
 
-    // Q: this method takes `krate + resolver` instead of `scope` because the
-    // caller doesn't have a scope available. is there a way to make this more
-    // consistent with other `iterate_` methods?
-    pub fn iterate_trait_item_candidates<T>(
-        &self,
-        db: &dyn HirDatabase,
-        krate: Crate,
-        resolver: &Resolver,
-        traits_in_scope: &FxHashSet<TraitId>,
-        name: Option<&Name>,
-        mut callback: impl FnMut(AssocItem) -> Option<T>,
-    ) -> Option<T> {
-        let _p = profile::span("iterate_trait_item_candidates");
-        let mut slot = None;
-        self.iterate_trait_item_candidates_dyn(
-            db,
-            krate,
-            resolver,
-            traits_in_scope,
-            name,
-            &mut |assoc_item_id| {
-                if let Some(res) = callback(assoc_item_id.into()) {
-                    slot = Some(res);
-                    return ControlFlow::Break(());
-                }
-                ControlFlow::Continue(())
-            },
-        );
-        slot
-    }
-
-    fn iterate_trait_item_candidates_dyn(
-        &self,
-        db: &dyn HirDatabase,
-        krate: Crate,
-        resolver: &Resolver,
-        traits_in_scope: &FxHashSet<TraitId>,
-        name: Option<&Name>,
-        callback: &mut dyn FnMut(AssocItemId) -> ControlFlow<()>,
-    ) {
-        let canonical = hir_ty::replace_errors_with_variables(&self.ty);
-
-        let environment = resolver.generic_def().map_or_else(
-            || Arc::new(TraitEnvironment::empty(krate.id)),
-            |d| db.trait_environment(d),
-        );
-
-        method_resolution::iterate_trait_item_candidates(
-            &canonical,
-            db,
-            environment,
-            traits_in_scope,
-            name,
-            &mut |id, _| callback(id),
-        );
-    }
-
     pub fn as_adt(&self) -> Option<Adt> {
         let (adt, _subst) = self.ty.as_adt()?;
         Some(adt.into())

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

@@ -67,9 +67,22 @@ pub(crate) fn complete_expr_path(
             ctx.iterate_path_candidates(ty, |item| {
                 add_assoc_item(acc, item);
             });
+
+            // Iterate assoc types separately
+            ty.iterate_assoc_items(ctx.db, ctx.krate, |item| {
+                if let hir::AssocItem::TypeAlias(ty) = item {
+                    acc.add_type_alias(ctx, ty)
+                }
+                None::<()>
+            });
         }
         Qualified::With { resolution: None, .. } => {}
         Qualified::With { resolution: Some(resolution), .. } => {
+            // Add associated types on type parameters and `Self`.
+            ctx.scope.assoc_type_shorthand_candidates(resolution, |_, alias| {
+                acc.add_type_alias(ctx, alias);
+                None::<()>
+            });
             match resolution {
                 hir::PathResolution::Def(hir::ModuleDef::Module(module)) => {
                     let module_scope = module.scope(ctx.db, Some(ctx.module));
@@ -111,6 +124,14 @@ pub(crate) fn complete_expr_path(
                     ctx.iterate_path_candidates(&ty, |item| {
                         add_assoc_item(acc, item);
                     });
+
+                    // Iterate assoc types separately
+                    ty.iterate_assoc_items(ctx.db, ctx.krate, |item| {
+                        if let hir::AssocItem::TypeAlias(ty) = item {
+                            acc.add_type_alias(ctx, ty)
+                        }
+                        None::<()>
+                    });
                 }
                 hir::PathResolution::Def(hir::ModuleDef::Trait(t)) => {
                     // Handles `Trait::assoc` as well as `<Ty as Trait>::assoc`.

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

@@ -154,6 +154,7 @@ pub(crate) fn complete_pattern_path(
                     }
 
                     ctx.iterate_path_candidates(&ty, |item| match item {
+                        AssocItem::TypeAlias(ta) => acc.add_type_alias(ctx, ta),
                         AssocItem::Const(c) => acc.add_const(ctx, c),
                         _ => {}
                     });

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

@@ -45,7 +45,8 @@ pub(crate) fn complete_type_path(
         hir::AssocItem::Const(ct) if matches!(location, TypeLocation::GenericArg { .. }) => {
             acc.add_const(ctx, ct)
         }
-        hir::AssocItem::Function(_) | hir::AssocItem::Const(_) | hir::AssocItem::TypeAlias(_) => (),
+        hir::AssocItem::Function(_) | hir::AssocItem::Const(_) => (),
+        hir::AssocItem::TypeAlias(ty) => acc.add_type_alias(ctx, ty),
     };
 
     match qualified {

crates/ide/src/doc_links/tests.rs

@@ -482,13 +482,12 @@ fn doc_links_trait_impl_items() {
         r#"
 trait Trait {
     type Type;
-      // ^^^^ Struct::Type
     const CONST: usize;
        // ^^^^^ Struct::CONST
     fn function();
     // ^^^^^^^^ Struct::function
 }
-/// [`Struct::Type`]
+// FIXME #9694: [`Struct::Type`]
 /// [`Struct::CONST`]
 /// [`Struct::function`]
 struct Struct$0;