Repository: rust-lang/rust-analyzer (rust)

Issue:
Doc-Comment not resolving to field-names
**rust-analyzer version**: `0.3.1599-standalone`

**rustc version**: `rustc 1.71.0 (8ede3aae2 2023-07-12)`
**rustc version**: `rustc 1.73.0-nightly (ad963232d 2023-07-14)`

**relevant settings**: (eg. client settings, or environment variables like `CARGO`, `RUSTC`, `RUSTUP_HOME` or `CARGO_HOME`)
```jsonc
// related options from vscode settings
{
	"rust-analyzer.debug.openDebugPane": true,
	"rust-analyzer.rustfmt.extraArgs": [
		"+nightly"
	],
}
```

```txt
CARGO_HOME=/home/hasezoey/.local/cargo
CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse
RUSTUP_HOME=/home/hasezoey/.local/rustup
```

using vscode 1.79.2

---

Currently rust-analyzer (vscode at least) does not resolve to field names, but `rustdoc` does

given the following code, rust-analyzer does not link anywhere
```rust
mod testy {
	pub struct Testy {
		pub field: String,
	}
}

/// Should resolve to field [testy::Testy::field] `test`
pub fn hello() {
	todo!();
}
```

trace:
```txt
[Trace - 1:39:36 PM] Received response 'textDocument/hover - (92)' in 1ms.
Result: {
    "contents": {
        "kind": "markdown",
        "value": "\n```rust\nmh\n```\n\n```rust\nfn hello()\n```\n\n---\n\nShould resolve to field [testy::Testy::field](testy::Testy::field) `test`"
    },
    "range": {
        "start": {
            "line": 7,
            "character": 3
        },
        "end": {
            "line": 7,
            "character": 8
        }
    }
}
```

![example1](https://github.com/rust-lang/rust-analyzer/assets/10911626/16c3dd20-05f4-40b3-acea-e216c58c1222)

without the field, it would generate trace:
```txt
[Trace - 1:41:08 PM] Received response 'textDocument/hover - (130)' in 1ms.
Result: {
    "contents": {
        "kind": "markdown",
        "value": "\n```rust\ntest\n```\n\n```rust\nfn hello()\n```\n\n---\n\nShould resolve to field [testy::Testy](https://docs.rs/test/0.0.1/test/testy/struct.Testy.html) `test`"
    },
    "range": {
        "start": {
            "line": 7,
            "character": 3
        },
        "end": {
            "line": 7,
            "character": 8
        }
    }
}
```

and adding in-line code-blocks, gives a result that would mark it as a link, but it is not openable, trace:
```txt
[Trace - 1:41:44 PM] Received response 'textDocument/hover - (186)' in 1ms.
Result: {
    "contents": {
        "kind": "markdown",
        "value": "\n```rust\ntest\n```\n\n```rust\nfn hello()\n```\n\n---\n\nShould resolve to field [`testy::Testy::field`](`testy::Testy::field`) `test`"
    },
    "range": {
        "start": {
            "line": 7,
            "character": 3
        },
        "end": {
            "line": 7,
            "character": 8
        }
    }
}
```

![example2](https://github.com/rust-lang/rust-analyzer/assets/10911626/149f72a1-210a-4db9-97f2-b68456fec7c0)

but in the end `rustdoc` resolves it just fine
![rustdoc](https://github.com/rust-lang/rust-analyzer/assets/10911626/c806bb4b-92eb-4489-a7f4-17dec051c79d)

---

PS: i originally came here to report a related issue, but i cannot reproduce it in a simpler example, issue being that rust-analyzer actually creates a openable link, but the link does not open:

![Screenshot_20230724_134752](https://github.com/rust-lang/rust-analyzer/assets/10911626/da96127d-2ba5-4772-a2c0-e78ded6d49b6)


Pull request description:
Support doc links that resolve to fields
Fixes #15331

Also removes `Resolver::resolve_module_path_in_trait_assoc_items()` and reimplements it in hir with other `Resolver` methods to decouple things a bit.

Patch:
diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs
index b112c1070d..d04d2fa0e9 100644
--- a/crates/hir-def/src/resolver.rs
+++ b/crates/hir-def/src/resolver.rs
@@ -21,11 +21,11 @@ use crate::{
     path::{ModPath, Path, PathKind},
     per_ns::PerNs,
     visibility::{RawVisibility, Visibility},
-    AdtId, AssocItemId, ConstId, ConstParamId, CrateRootModuleId, DefWithBodyId, EnumId,
-    EnumVariantId, ExternBlockId, ExternCrateId, FunctionId, GenericDefId, GenericParamId,
-    HasModule, ImplId, ItemContainerId, LifetimeParamId, LocalModuleId, Lookup, Macro2Id, MacroId,
-    MacroRulesId, ModuleDefId, ModuleId, ProcMacroId, StaticId, StructId, TraitAliasId, TraitId,
-    TypeAliasId, TypeOrConstParamId, TypeOwnerId, TypeParamId, UseId, VariantId,
+    AdtId, ConstId, ConstParamId, CrateRootModuleId, DefWithBodyId, EnumId, EnumVariantId,
+    ExternBlockId, ExternCrateId, FunctionId, GenericDefId, GenericParamId, HasModule, ImplId,
+    ItemContainerId, LifetimeParamId, LocalModuleId, Lookup, Macro2Id, MacroId, MacroRulesId,
+    ModuleDefId, ModuleId, ProcMacroId, StaticId, StructId, TraitAliasId, TraitId, TypeAliasId,
+    TypeOrConstParamId, TypeOwnerId, TypeParamId, UseId, VariantId,
 };
 
 #[derive(Debug, Clone)]
@@ -148,34 +148,6 @@ impl Resolver {
         self.resolve_module_path(db, path, BuiltinShadowMode::Module)
     }
 
-    // FIXME: This shouldn't exist
-    pub fn resolve_module_path_in_trait_assoc_items(
-        &self,
-        db: &dyn DefDatabase,
-        path: &ModPath,
-    ) -> Option<PerNs> {
-        let (item_map, module) = self.item_scope();
-        let (module_res, idx) =
-            item_map.resolve_path(db, module, path, BuiltinShadowMode::Module, None);
-        match module_res.take_types()? {
-            ModuleDefId::TraitId(it) => {
-                let idx = idx?;
-                let unresolved = &path.segments()[idx..];
-                let assoc = match unresolved {
-                    [it] => it,
-                    _ => return None,
-                };
-                let &(_, assoc) = db.trait_data(it).items.iter().find(|(n, _)| n == assoc)?;
-                Some(match assoc {
-                    AssocItemId::FunctionId(it) => PerNs::values(it.into(), Visibility::Public),
-                    AssocItemId::ConstId(it) => PerNs::values(it.into(), Visibility::Public),
-                    AssocItemId::TypeAliasId(it) => PerNs::types(it.into(), Visibility::Public),
-                })
-            }
-            _ => None,
-        }
-    }
-
     pub fn resolve_path_in_type_ns(
         &self,
         db: &dyn DefDatabase,
diff --git a/crates/hir/src/attrs.rs b/crates/hir/src/attrs.rs
index 0f2fb2c811..cd0410168c 100644
--- a/crates/hir/src/attrs.rs
+++ b/crates/hir/src/attrs.rs
@@ -3,18 +3,18 @@
 use hir_def::{
     attr::{AttrsWithOwner, Documentation},
     item_scope::ItemInNs,
-    path::ModPath,
-    resolver::HasResolver,
-    AttrDefId, GenericParamId, ModuleDefId,
+    path::{ModPath, Path},
+    resolver::{HasResolver, Resolver, TypeNs},
+    AssocItemId, AttrDefId, GenericParamId, ModuleDefId,
 };
-use hir_expand::hygiene::Hygiene;
+use hir_expand::{hygiene::Hygiene, name::Name};
 use hir_ty::db::HirDatabase;
 use syntax::{ast, AstNode};
 
 use crate::{
-    Adt, AssocItem, Const, ConstParam, Enum, ExternCrateDecl, Field, Function, GenericParam, Impl,
-    LifetimeParam, Macro, Module, ModuleDef, Static, Struct, Trait, TraitAlias, TypeAlias,
-    TypeParam, Union, Variant,
+    Adt, AsAssocItem, AssocItem, BuiltinType, Const, ConstParam, Enum, ExternCrateDecl, Field,
+    Function, GenericParam, Impl, LifetimeParam, Macro, Module, ModuleDef, Static, Struct, Trait,
+    TraitAlias, TypeAlias, TypeParam, Union, Variant, VariantDef,
 };
 
 pub trait HasAttrs {
@@ -25,7 +25,7 @@ pub trait HasAttrs {
         db: &dyn HirDatabase,
         link: &str,
         ns: Option<Namespace>,
-    ) -> Option<ModuleDef>;
+    ) -> Option<DocLinkDef>;
 }
 
 #[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
@@ -35,6 +35,13 @@ pub enum Namespace {
     Macros,
 }
 
+/// Subset of `ide_db::Definition` that doc links can resolve to.
+pub enum DocLinkDef {
+    ModuleDef(ModuleDef),
+    Field(Field),
+    SelfType(Trait),
+}
+
 macro_rules! impl_has_attrs {
     ($(($def:ident, $def_id:ident),)*) => {$(
         impl HasAttrs for $def {
@@ -46,9 +53,14 @@ macro_rules! impl_has_attrs {
                 let def = AttrDefId::$def_id(self.into());
                 db.attrs(def).docs()
             }
-            fn resolve_doc_path(self, db: &dyn HirDatabase, link: &str, ns: Option<Namespace>) -> Option<ModuleDef> {
+            fn resolve_doc_path(
+                self,
+                db: &dyn HirDatabase,
+                link: &str,
+                ns: Option<Namespace>
+            ) -> Option<DocLinkDef> {
                 let def = AttrDefId::$def_id(self.into());
-                resolve_doc_path(db, def, link, ns).map(ModuleDef::from)
+                resolve_doc_path(db, def, link, ns)
             }
         }
     )*};
@@ -79,7 +91,12 @@ macro_rules! impl_has_attrs_enum {
             fn docs(self, db: &dyn HirDatabase) -> Option<Documentation> {
                 $enum::$variant(self).docs(db)
             }
-            fn resolve_doc_path(self, db: &dyn HirDatabase, link: &str, ns: Option<Namespace>) -> Option<ModuleDef> {
+            fn resolve_doc_path(
+                self,
+                db: &dyn HirDatabase,
+                link: &str,
+                ns: Option<Namespace>
+            ) -> Option<DocLinkDef> {
                 $enum::$variant(self).resolve_doc_path(db, link, ns)
             }
         }
@@ -111,7 +128,7 @@ impl HasAttrs for AssocItem {
         db: &dyn HirDatabase,
         link: &str,
         ns: Option<Namespace>,
-    ) -> Option<ModuleDef> {
+    ) -> Option<DocLinkDef> {
         match self {
             AssocItem::Function(it) => it.resolve_doc_path(db, link, ns),
             AssocItem::Const(it) => it.resolve_doc_path(db, link, ns),
@@ -147,9 +164,9 @@ impl HasAttrs for ExternCrateDecl {
         db: &dyn HirDatabase,
         link: &str,
         ns: Option<Namespace>,
-    ) -> Option<ModuleDef> {
+    ) -> Option<DocLinkDef> {
         let def = AttrDefId::ExternCrateId(self.into());
-        resolve_doc_path(db, def, link, ns).map(ModuleDef::from)
+        resolve_doc_path(db, def, link, ns)
     }
 }
 
@@ -159,7 +176,7 @@ fn resolve_doc_path(
     def: AttrDefId,
     link: &str,
     ns: Option<Namespace>,
-) -> Option<ModuleDefId> {
+) -> Option<DocLinkDef> {
     let resolver = match def {
         AttrDefId::ModuleId(it) => it.resolver(db.upcast()),
         AttrDefId::FieldId(it) => it.parent.resolver(db.upcast()),
@@ -184,8 +201,107 @@ fn resolve_doc_path(
         .resolver(db.upcast()),
     };
 
-    let modpath = {
-        // FIXME: this is not how we should get a mod path here
+    let mut modpath = modpath_from_str(db, link)?;
+
+    let resolved = resolver.resolve_module_path_in_items(db.upcast(), &modpath);
+    if resolved.is_none() {
+        let last_name = modpath.pop_segment()?;
+        resolve_assoc_or_field(db, resolver, modpath, last_name, ns)
+    } else {
+        let def = match ns {
+            Some(Namespace::Types) => resolved.take_types(),
+            Some(Namespace::Values) => resolved.take_values(),
+            Some(Namespace::Macros) => resolved.take_macros().map(ModuleDefId::MacroId),
+            None => resolved.iter_items().next().map(|it| match it {
+                ItemInNs::Types(it) => it,
+                ItemInNs::Values(it) => it,
+                ItemInNs::Macros(it) => ModuleDefId::MacroId(it),
+            }),
+        };
+        Some(DocLinkDef::ModuleDef(def?.into()))
+    }
+}
+
+fn resolve_assoc_or_field(
+    db: &dyn HirDatabase,
+    resolver: Resolver,
+    path: ModPath,
+    name: Name,
+    ns: Option<Namespace>,
+) -> Option<DocLinkDef> {
+    let path = Path::from_known_path_with_no_generic(path);
+    // FIXME: This does not handle `Self` on trait definitions, which we should resolve to the
+    // trait itself.
+    let base_def = resolver.resolve_path_in_type_ns_fully(db.upcast(), &path)?;
+
+    let ty = match base_def {
+        TypeNs::SelfType(id) => Impl::from(id).self_ty(db),
+        TypeNs::GenericParam(_) => {
+            // Even if this generic parameter has some trait bounds, rustdoc doesn't
+            // resolve `name` to trait items.
+            return None;
+        }
+        TypeNs::AdtId(id) | TypeNs::AdtSelfType(id) => Adt::from(id).ty(db),
+        TypeNs::EnumVariantId(id) => {
+            // Enum variants don't have path candidates.
+            let variant = Variant::from(id);
+            return resolve_field(db, variant.into(), name, ns);
+        }
+        TypeNs::TypeAliasId(id) => {
+            let alias = TypeAlias::from(id);
+            if alias.as_assoc_item(db).is_some() {
+                // We don't normalize associated type aliases, so we have nothing to
+                // resolve `name` to.
+                return None;
+            }
+            alias.ty(db)
+        }
+        TypeNs::BuiltinType(id) => BuiltinType::from(id).ty(db),
+        TypeNs::TraitId(id) => {
+            // Doc paths in this context may only resolve to an item of this trait
+            // (i.e. no items of its supertraits), so we need to handle them here
+            // independently of others.
+            return db.trait_data(id).items.iter().find(|it| it.0 == name).map(|(_, assoc_id)| {
+                let def = match *assoc_id {
+                    AssocItemId::FunctionId(it) => ModuleDef::Function(it.into()),
+                    AssocItemId::ConstId(it) => ModuleDef::Const(it.into()),
+                    AssocItemId::TypeAliasId(it) => ModuleDef::TypeAlias(it.into()),
+                };
+                DocLinkDef::ModuleDef(def)
+            });
+        }
+        TypeNs::TraitAliasId(_) => {
+            // XXX: Do these get resolved?
+            return None;
+        }
+    };
+
+    // FIXME: Resolve associated items here, e.g. `Option::map`. Note that associated items take
+    // precedence over fields.
+
+    let variant_def = match ty.as_adt()? {
+        Adt::Struct(it) => it.into(),
+        Adt::Union(it) => it.into(),
+        Adt::Enum(_) => return None,
+    };
+    resolve_field(db, variant_def, name, ns)
+}
+
+fn resolve_field(
+    db: &dyn HirDatabase,
+    def: VariantDef,
+    name: Name,
+    ns: Option<Namespace>,
+) -> Option<DocLinkDef> {
+    if let Some(Namespace::Types | Namespace::Macros) = ns {
+        return None;
+    }
+    def.fields(db).into_iter().find(|f| f.name(db) == name).map(DocLinkDef::Field)
+}
+
+fn modpath_from_str(db: &dyn HirDatabase, link: &str) -> Option<ModPath> {
+    // FIXME: this is not how we should get a mod path here.
+    let try_get_modpath = |link: &str| {
         let ast_path = ast::SourceFile::parse(&format!("type T = {link};"))
             .syntax_node()
             .descendants()
@@ -193,23 +309,20 @@ fn resolve_doc_path(
         if ast_path.syntax().text() != link {
             return None;
         }
-        ModPath::from_src(db.upcast(), ast_path, &Hygiene::new_unhygienic())?
+        ModPath::from_src(db.upcast(), ast_path, &Hygiene::new_unhygienic())
     };
 
-    let resolved = resolver.resolve_module_path_in_items(db.upcast(), &modpath);
-    let resolved = if resolved.is_none() {
-        resolver.resolve_module_path_in_trait_assoc_items(db.upcast(), &modpath)?
-    } else {
-        resolved
-    };
-    match ns {
-        Some(Namespace::Types) => resolved.take_types(),
-        Some(Namespace::Values) => resolved.take_values(),
-        Some(Namespace::Macros) => resolved.take_macros().map(ModuleDefId::MacroId),
-        None => resolved.iter_items().next().map(|it| match it {
-            ItemInNs::Types(it) => it,
-            ItemInNs::Values(it) => it,
-            ItemInNs::Macros(it) => ModuleDefId::MacroId(it),
-        }),
+    let full = try_get_modpath(link);
+    if full.is_some() {
+        return full;
     }
+
+    // Tuple field names cannot be a part of `ModPath` usually, but rustdoc can
+    // resolve doc paths like `TupleStruct::0`.
+    // FIXME: Find a better way to handle these.
+    let (base, maybe_tuple_field) = link.rsplit_once("::")?;
+    let tuple_field = Name::new_tuple_field(maybe_tuple_field.parse().ok()?);
+    let mut modpath = try_get_modpath(base)?;
+    modpath.push_segment(tuple_field);
+    Some(modpath)
 }
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index bf041b61f2..de60c88844 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -87,7 +87,7 @@ use triomphe::Arc;
 use crate::db::{DefDatabase, HirDatabase};
 
 pub use crate::{
-    attrs::{HasAttrs, Namespace},
+    attrs::{DocLinkDef, HasAttrs, Namespace},
     diagnostics::{
         AnyDiagnostic, BreakOutsideOfLoop, CaseType, ExpectedFunction, InactiveCode,
         IncoherentImpl, IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError,
diff --git a/crates/ide-db/src/defs.rs b/crates/ide-db/src/defs.rs
index 5e4562d9c5..4ce80532e8 100644
--- a/crates/ide-db/src/defs.rs
+++ b/crates/ide-db/src/defs.rs
@@ -7,7 +7,7 @@
 
 use arrayvec::ArrayVec;
 use hir::{
-    Adt, AsAssocItem, AssocItem, BuiltinAttr, BuiltinType, Const, Crate, DeriveHelper,
+    Adt, AsAssocItem, AssocItem, BuiltinAttr, BuiltinType, Const, Crate, DeriveHelper, DocLinkDef,
     ExternCrateDecl, Field, Function, GenericParam, HasVisibility, Impl, Label, Local, Macro,
     Module, ModuleDef, Name, PathResolution, Semantics, Static, ToolModule, Trait, TraitAlias,
     TypeAlias, Variant, Visibility,
@@ -649,3 +649,13 @@ impl From<ModuleDef> for Definition {
         }
     }
 }
+
+impl From<DocLinkDef> for Definition {
+    fn from(def: DocLinkDef) -> Self {
+        match def {
+            DocLinkDef::ModuleDef(it) => it.into(),
+            DocLinkDef::Field(it) => it.into(),
+            DocLinkDef::SelfType(it) => it.into(),
+        }
+    }
+}


Test patch:
diff --git a/crates/ide/src/doc_links/tests.rs b/crates/ide/src/doc_links/tests.rs
index 05a64b33bf..8036c77072 100644
--- a/crates/ide/src/doc_links/tests.rs
+++ b/crates/ide/src/doc_links/tests.rs
@@ -517,6 +517,62 @@ fn function();
     )
 }
 
+#[test]
+fn doc_links_field() {
+    check_doc_links(
+        r#"
+/// [`S::f`]
+/// [`S2::f`]
+/// [`T::0`]
+/// [`U::a`]
+/// [`E::A::f`]
+/// [`E::B::0`]
+struct S$0 {
+    f: i32,
+  //^ S::f
+  //^ S2::f
+}
+type S2 = S;
+struct T(i32);
+       //^^^ T::0
+union U {
+    a: i32,
+  //^ U::a
+}
+enum E {
+    A { f: i32 },
+      //^ E::A::f
+    B(i32),
+    //^^^ E::B::0
+}
+"#,
+    );
+}
+
+#[test]
+fn doc_links_field_via_self() {
+    check_doc_links(
+        r#"
+/// [`Self::f`]
+struct S$0 {
+    f: i32,
+  //^ Self::f
+}
+"#,
+    );
+}
+
+#[test]
+fn doc_links_tuple_field_via_self() {
+    check_doc_links(
+        r#"
+/// [`Self::0`]
+struct S$0(i32);
+       //^^^ Self::0
+"#,
+    );
+}
+
 #[test]
 fn rewrite_html_root_url() {
     check_rewrite(