Skip to content

Commit

Permalink
Add field@ and variant@ doc-link disambiguators
Browse files Browse the repository at this point in the history
  • Loading branch information
coolreader18 committed Sep 19, 2024
1 parent 749f80a commit 9fd4eaf
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ fn Foo() {}

These prefixes will be stripped when displayed in the documentation, so `[struct@Foo]` will be
rendered as `Foo`. The following prefixes are available: `struct`, `enum`, `trait`, `union`,
`mod`, `module`, `const`, `constant`, `fn`, `function`, `method`, `derive`, `type`, `value`,
`macro`, `prim` or `primitive`.
`mod`, `module`, `const`, `constant`, `fn`, `function`, `field`, `variant`, `method`, `derive`,
`type`, `value`, `macro`, `prim` or `primitive`.

You can also disambiguate for functions by adding `()` after the function name,
or for macros by adding `!` after the macro name. The macro `!` can be followed by `()`, `{}`,
Expand Down
95 changes: 54 additions & 41 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ impl Res {

let prefix = match kind {
DefKind::Fn | DefKind::AssocFn => return Suggestion::Function,
DefKind::Field => return Suggestion::RemoveDisambiguator,
DefKind::Macro(MacroKind::Bang) => return Suggestion::Macro,

DefKind::Macro(MacroKind::Derive) => "derive",
Expand All @@ -123,6 +122,8 @@ impl Res {
"const"
}
DefKind::Static { .. } => "static",
DefKind::Field => "field",
DefKind::Variant | DefKind::Ctor(..) => "variant",
// Now handle things that don't have a specific disambiguator
_ => match kind
.ns()
Expand Down Expand Up @@ -415,6 +416,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&mut self,
path_str: &'path str,
ns: Namespace,
disambiguator: Option<Disambiguator>,
item_id: DefId,
module_id: DefId,
) -> Result<Vec<(Res, Option<DefId>)>, UnresolvedPath<'path>> {
Expand Down Expand Up @@ -454,7 +456,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
match resolve_primitive(path_root, TypeNS)
.or_else(|| self.resolve_path(path_root, TypeNS, item_id, module_id))
.map(|ty_res| {
self.resolve_associated_item(ty_res, item_name, ns, module_id)
self.resolve_associated_item(ty_res, item_name, ns, disambiguator, module_id)
.into_iter()
.map(|(res, def_id)| (res, Some(def_id)))
.collect::<Vec<_>>()
Expand Down Expand Up @@ -557,6 +559,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
root_res: Res,
item_name: Symbol,
ns: Namespace,
disambiguator: Option<Disambiguator>,
module_id: DefId,
) -> Vec<(Res, DefId)> {
let tcx = self.cx.tcx;
Expand All @@ -583,7 +586,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
// FIXME: if the associated item is defined directly on the type alias,
// it will show up on its documentation page, we should link there instead.
let Some(res) = self.def_id_to_res(did) else { return Vec::new() };
self.resolve_associated_item(res, item_name, ns, module_id)
self.resolve_associated_item(res, item_name, ns, disambiguator, module_id)
}
Res::Def(
def_kind @ (DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::ForeignTy),
Expand All @@ -604,6 +607,39 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
}

let search_for_field = || {
let (DefKind::Struct | DefKind::Union) = def_kind else { return vec![] };
debug!("looking for fields named {item_name} for {did:?}");
// FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?)
// NOTE: it's different from variant_field because it only resolves struct fields,
// not variant fields (2 path segments, not 3).
//
// We need to handle struct (and union) fields in this code because
// syntactically their paths are identical to associated item paths:
// `module::Type::field` and `module::Type::Assoc`.
//
// On the other hand, variant fields can't be mistaken for associated
// items because they look like this: `module::Type::Variant::field`.
//
// Variants themselves don't need to be handled here, even though
// they also look like associated items (`module::Type::Variant`),
// because they are real Rust syntax (unlike the intra-doc links
// field syntax) and are handled by the compiler's resolver.
let ty::Adt(def, _) = tcx.type_of(did).instantiate_identity().kind() else {
unreachable!()
};
def.non_enum_variant()
.fields
.iter()
.filter(|field| field.name == item_name)
.map(|field| (root_res, field.did))
.collect::<Vec<_>>()
};

if let Some(Disambiguator::Kind(DefKind::Field)) = disambiguator {
return search_for_field();
}

// Checks if item_name belongs to `impl SomeItem`
let mut assoc_items: Vec<_> = tcx
.inherent_impls(did)
Expand Down Expand Up @@ -647,32 +683,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
if ns != Namespace::ValueNS {
return Vec::new();
}
debug!("looking for fields named {item_name} for {did:?}");
// FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?)
// NOTE: it's different from variant_field because it only resolves struct fields,
// not variant fields (2 path segments, not 3).
//
// We need to handle struct (and union) fields in this code because
// syntactically their paths are identical to associated item paths:
// `module::Type::field` and `module::Type::Assoc`.
//
// On the other hand, variant fields can't be mistaken for associated
// items because they look like this: `module::Type::Variant::field`.
//
// Variants themselves don't need to be handled here, even though
// they also look like associated items (`module::Type::Variant`),
// because they are real Rust syntax (unlike the intra-doc links
// field syntax) and are handled by the compiler's resolver.
let def = match tcx.type_of(did).instantiate_identity().kind() {
ty::Adt(def, _) if !def.is_enum() => def,
_ => return Vec::new(),
};
def.non_enum_variant()
.fields
.iter()
.filter(|field| field.name == item_name)
.map(|field| (root_res, field.did))
.collect::<Vec<_>>()

search_for_field()
}
Res::Def(DefKind::Trait, did) => filter_assoc_items_by_name_and_namespace(
tcx,
Expand Down Expand Up @@ -1298,7 +1310,7 @@ impl LinkCollector<'_, '_> {

match disambiguator.map(Disambiguator::ns) {
Some(expected_ns) => {
match self.resolve(path_str, expected_ns, item_id, module_id) {
match self.resolve(path_str, expected_ns, disambiguator, item_id, module_id) {
Ok(candidates) => candidates,
Err(err) => {
// We only looked in one namespace. Try to give a better error if possible.
Expand All @@ -1307,8 +1319,9 @@ impl LinkCollector<'_, '_> {
let mut err = ResolutionFailure::NotResolved(err);
for other_ns in [TypeNS, ValueNS, MacroNS] {
if other_ns != expected_ns {
if let Ok(&[res, ..]) =
self.resolve(path_str, other_ns, item_id, module_id).as_deref()
if let Ok(&[res, ..]) = self
.resolve(path_str, other_ns, None, item_id, module_id)
.as_deref()
{
err = ResolutionFailure::WrongNamespace {
res: full_res(self.cx.tcx, res),
Expand All @@ -1328,7 +1341,7 @@ impl LinkCollector<'_, '_> {
None => {
// Try everything!
let mut candidate = |ns| {
self.resolve(path_str, ns, item_id, module_id)
self.resolve(path_str, ns, None, item_id, module_id)
.map_err(ResolutionFailure::NotResolved)
};

Expand Down Expand Up @@ -1532,6 +1545,8 @@ impl Disambiguator {
}),
"function" | "fn" | "method" => Kind(DefKind::Fn),
"derive" => Kind(DefKind::Macro(MacroKind::Derive)),
"field" => Kind(DefKind::Field),
"variant" => Kind(DefKind::Variant),
"type" => NS(Namespace::TypeNS),
"value" => NS(Namespace::ValueNS),
"macro" => NS(Namespace::MacroNS),
Expand Down Expand Up @@ -1570,6 +1585,8 @@ impl Disambiguator {
fn ns(self) -> Namespace {
match self {
Self::Namespace(n) => n,
// for purposes of link resolution, fields are in the value namespace.
Self::Kind(DefKind::Field) => ValueNS,
Self::Kind(k) => {
k.ns().expect("only DefKinds with a valid namespace can be disambiguators")
}
Expand Down Expand Up @@ -1604,8 +1621,6 @@ enum Suggestion {
Function,
/// `m!`
Macro,
/// `foo` without any disambiguator
RemoveDisambiguator,
}

impl Suggestion {
Expand All @@ -1614,7 +1629,6 @@ impl Suggestion {
Self::Prefix(x) => format!("prefix with `{x}@`").into(),
Self::Function => "add parentheses".into(),
Self::Macro => "add an exclamation mark".into(),
Self::RemoveDisambiguator => "remove the disambiguator".into(),
}
}

Expand All @@ -1624,13 +1638,11 @@ impl Suggestion {
Self::Prefix(prefix) => format!("{prefix}@{path_str}"),
Self::Function => format!("{path_str}()"),
Self::Macro => format!("{path_str}!"),
Self::RemoveDisambiguator => path_str.into(),
}
}

fn as_help_span(
&self,
path_str: &str,
ori_link: &str,
sp: rustc_span::Span,
) -> Vec<(rustc_span::Span, String)> {
Expand Down Expand Up @@ -1678,7 +1690,6 @@ impl Suggestion {
}
sugg
}
Self::RemoveDisambiguator => vec![(sp, path_str.into())],
}
}
}
Expand Down Expand Up @@ -1827,7 +1838,9 @@ fn resolution_failure(
};
name = start;
for ns in [TypeNS, ValueNS, MacroNS] {
if let Ok(v_res) = collector.resolve(start, ns, item_id, module_id) {
if let Ok(v_res) =
collector.resolve(start, ns, None, item_id, module_id)
{
debug!("found partial_res={v_res:?}");
if let Some(&res) = v_res.first() {
*partial_res = Some(full_res(tcx, res));
Expand Down Expand Up @@ -2165,7 +2178,7 @@ fn suggest_disambiguator(
};

if let (Some(sp), Some(ori_link)) = (sp, ori_link) {
let mut spans = suggestion.as_help_span(path_str, ori_link, sp);
let mut spans = suggestion.as_help_span(ori_link, sp);
if spans.len() > 1 {
diag.multipart_suggestion(help, spans, Applicability::MaybeIncorrect);
} else {
Expand Down
18 changes: 17 additions & 1 deletion tests/rustdoc-ui/intra-doc/disambiguator-mismatch.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![deny(rustdoc::broken_intra_doc_links)]
//~^ NOTE lint level is defined
pub enum S {}
pub enum S {
A,
}
fn S() {}

#[macro_export]
Expand All @@ -13,6 +15,10 @@ const c: usize = 0;

trait T {}

struct X {
y: usize,
}

/// Link to [struct@S]
//~^ ERROR incompatible link kind for `S`
//~| NOTE this link resolved
Expand Down Expand Up @@ -78,4 +84,14 @@ trait T {}
//~^ ERROR unresolved link to `std`
//~| NOTE this link resolves to the crate `std`
//~| HELP to link to the crate, prefix with `mod@`

/// Link to [method@X::y]
//~^ ERROR incompatible link kind for `X::y`
//~| NOTE this link resolved
//~| HELP prefix with `field@`

/// Link to [field@S::A]
//~^ ERROR incompatible link kind for `S::A`
//~| NOTE this link resolved
//~| HELP prefix with `variant@`
pub fn f() {}
Loading

0 comments on commit 9fd4eaf

Please sign in to comment.