From e91e01509e1e034a71c545d42410d2ea4f7db89e Mon Sep 17 00:00:00 2001 From: Noa Date: Thu, 19 Sep 2024 16:42:55 -0500 Subject: [PATCH] Add `field@` and `variant@` doc-link disambiguators --- .../linking-to-items-by-name.md | 4 +- .../passes/collect_intra_doc_links.rs | 95 +++++++++++-------- .../intra-doc/disambiguator-mismatch.rs | 18 +++- .../intra-doc/disambiguator-mismatch.stderr | 50 +++++++--- tests/rustdoc-ui/intra-doc/field-ice.rs | 4 +- tests/rustdoc-ui/intra-doc/field-ice.stderr | 7 +- .../issue-108653-associated-items.stderr | 6 +- tests/rustdoc/intra-doc/field.rs | 20 ++++ 8 files changed, 138 insertions(+), 66 deletions(-) diff --git a/src/doc/rustdoc/src/write-documentation/linking-to-items-by-name.md b/src/doc/rustdoc/src/write-documentation/linking-to-items-by-name.md index 1a367b8274b..5e785483402 100644 --- a/src/doc/rustdoc/src/write-documentation/linking-to-items-by-name.md +++ b/src/doc/rustdoc/src/write-documentation/linking-to-items-by-name.md @@ -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 `()`, `{}`, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 676c972529b..871f78dac5f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -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", @@ -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() @@ -415,6 +416,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &mut self, path_str: &'path str, ns: Namespace, + disambiguator: Option, item_id: DefId, module_id: DefId, ) -> Result)>, UnresolvedPath<'path>> { @@ -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::>() @@ -557,6 +559,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { root_res: Res, item_name: Symbol, ns: Namespace, + disambiguator: Option, module_id: DefId, ) -> Vec<(Res, DefId)> { let tcx = self.cx.tcx; @@ -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), @@ -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::>() + }; + + 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) @@ -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::>() + + search_for_field() } Res::Def(DefKind::Trait, did) => filter_assoc_items_by_name_and_namespace( tcx, @@ -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. @@ -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), @@ -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) }; @@ -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), @@ -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") } @@ -1604,8 +1621,6 @@ enum Suggestion { Function, /// `m!` Macro, - /// `foo` without any disambiguator - RemoveDisambiguator, } impl Suggestion { @@ -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(), } } @@ -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)> { @@ -1678,7 +1690,6 @@ impl Suggestion { } sugg } - Self::RemoveDisambiguator => vec![(sp, path_str.into())], } } } @@ -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)); @@ -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 { diff --git a/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.rs b/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.rs index 2d66566119b..8142bd83877 100644 --- a/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.rs +++ b/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.rs @@ -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] @@ -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 @@ -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() {} diff --git a/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr b/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr index ee35749ce7f..488120304fd 100644 --- a/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr +++ b/tests/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr @@ -1,5 +1,5 @@ error: incompatible link kind for `S` - --> $DIR/disambiguator-mismatch.rs:16:14 + --> $DIR/disambiguator-mismatch.rs:22:14 | LL | /// Link to [struct@S] | ^^^^^^^^ this link resolved to an enum, which is not a struct @@ -15,7 +15,7 @@ LL | /// Link to [enum@S] | ~~~~~ error: incompatible link kind for `S` - --> $DIR/disambiguator-mismatch.rs:21:14 + --> $DIR/disambiguator-mismatch.rs:27:14 | LL | /// Link to [mod@S] | ^^^^^ this link resolved to an enum, which is not a module @@ -26,7 +26,7 @@ LL | /// Link to [enum@S] | ~~~~~ error: incompatible link kind for `S` - --> $DIR/disambiguator-mismatch.rs:26:14 + --> $DIR/disambiguator-mismatch.rs:32:14 | LL | /// Link to [union@S] | ^^^^^^^ this link resolved to an enum, which is not a union @@ -37,7 +37,7 @@ LL | /// Link to [enum@S] | ~~~~~ error: incompatible link kind for `S` - --> $DIR/disambiguator-mismatch.rs:31:14 + --> $DIR/disambiguator-mismatch.rs:37:14 | LL | /// Link to [trait@S] | ^^^^^^^ this link resolved to an enum, which is not a trait @@ -48,7 +48,7 @@ LL | /// Link to [enum@S] | ~~~~~ error: incompatible link kind for `T` - --> $DIR/disambiguator-mismatch.rs:36:14 + --> $DIR/disambiguator-mismatch.rs:42:14 | LL | /// Link to [struct@T] | ^^^^^^^^ this link resolved to a trait, which is not a struct @@ -59,7 +59,7 @@ LL | /// Link to [trait@T] | ~~~~~~ error: incompatible link kind for `m` - --> $DIR/disambiguator-mismatch.rs:41:14 + --> $DIR/disambiguator-mismatch.rs:47:14 | LL | /// Link to [derive@m] | ^^^^^^^^ this link resolved to a macro, which is not a derive macro @@ -71,7 +71,7 @@ LL + /// Link to [m!] | error: unresolved link to `m` - --> $DIR/disambiguator-mismatch.rs:46:14 + --> $DIR/disambiguator-mismatch.rs:52:14 | LL | /// Link to [m()] | ^^^ this link resolves to the macro `m`, which is not in the value namespace @@ -82,7 +82,7 @@ LL | /// Link to [m!()] | + error: incompatible link kind for `s` - --> $DIR/disambiguator-mismatch.rs:52:14 + --> $DIR/disambiguator-mismatch.rs:58:14 | LL | /// Link to [const@s] | ^^^^^^^ this link resolved to a static, which is not a constant @@ -93,7 +93,7 @@ LL | /// Link to [static@s] | ~~~~~~~ error: incompatible link kind for `c` - --> $DIR/disambiguator-mismatch.rs:57:14 + --> $DIR/disambiguator-mismatch.rs:63:14 | LL | /// Link to [static@c] | ^^^^^^^^ this link resolved to a constant, which is not a static @@ -104,7 +104,7 @@ LL | /// Link to [const@c] | ~~~~~~ error: incompatible link kind for `c` - --> $DIR/disambiguator-mismatch.rs:62:14 + --> $DIR/disambiguator-mismatch.rs:68:14 | LL | /// Link to [fn@c] | ^^^^ this link resolved to a constant, which is not a function @@ -115,7 +115,7 @@ LL | /// Link to [const@c] | ~~~~~~ error: incompatible link kind for `c` - --> $DIR/disambiguator-mismatch.rs:67:14 + --> $DIR/disambiguator-mismatch.rs:73:14 | LL | /// Link to [c()] | ^^^ this link resolved to a constant, which is not a function @@ -127,7 +127,7 @@ LL + /// Link to [const@c] | error: incompatible link kind for `f` - --> $DIR/disambiguator-mismatch.rs:72:14 + --> $DIR/disambiguator-mismatch.rs:78:14 | LL | /// Link to [const@f] | ^^^^^^^ this link resolved to a function, which is not a constant @@ -139,7 +139,7 @@ LL + /// Link to [f()] | error: unresolved link to `std` - --> $DIR/disambiguator-mismatch.rs:77:14 + --> $DIR/disambiguator-mismatch.rs:83:14 | LL | /// Link to [fn@std] | ^^^^^^ this link resolves to the crate `std`, which is not in the value namespace @@ -149,5 +149,27 @@ help: to link to the crate, prefix with `mod@` LL | /// Link to [mod@std] | ~~~~ -error: aborting due to 13 previous errors +error: incompatible link kind for `X::y` + --> $DIR/disambiguator-mismatch.rs:88:14 + | +LL | /// Link to [method@X::y] + | ^^^^^^^^^^^ this link resolved to a field, which is not a function + | +help: to link to the field, prefix with `field@` + | +LL | /// Link to [field@X::y] + | ~~~~~~ + +error: incompatible link kind for `S::A` + --> $DIR/disambiguator-mismatch.rs:93:14 + | +LL | /// Link to [field@S::A] + | ^^^^^^^^^^ this link resolved to a unit variant, which is not a field + | +help: to link to the unit variant, prefix with `variant@` + | +LL | /// Link to [variant@S::A] + | ~~~~~~~~ + +error: aborting due to 15 previous errors diff --git a/tests/rustdoc-ui/intra-doc/field-ice.rs b/tests/rustdoc-ui/intra-doc/field-ice.rs index c5d501e38da..1ba865b53c2 100644 --- a/tests/rustdoc-ui/intra-doc/field-ice.rs +++ b/tests/rustdoc-ui/intra-doc/field-ice.rs @@ -4,8 +4,8 @@ /// [`Foo::bar`] /// [`Foo::bar()`] //~^ERROR incompatible link kind for `Foo::bar` -//~|HELP to link to the field, remove the disambiguator +//~|HELP to link to the field, prefix with `field@` //~|NOTE this link resolved to a field, which is not a function pub struct Foo { - pub bar: u8 + pub bar: u8, } diff --git a/tests/rustdoc-ui/intra-doc/field-ice.stderr b/tests/rustdoc-ui/intra-doc/field-ice.stderr index cc0ada873af..7321c87b790 100644 --- a/tests/rustdoc-ui/intra-doc/field-ice.stderr +++ b/tests/rustdoc-ui/intra-doc/field-ice.stderr @@ -9,10 +9,11 @@ note: the lint level is defined here | LL | #![deny(rustdoc::broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: to link to the field, remove the disambiguator +help: to link to the field, prefix with `field@` + | +LL - /// [`Foo::bar()`] +LL + /// [`field@Foo::bar`] | -LL | /// [`Foo::bar`] - | ~~~~~~~~ error: aborting due to 1 previous error diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr index ed89fa8391d..9cd855b69ff 100644 --- a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr @@ -43,10 +43,10 @@ help: to link to the associated function, add parentheses | LL | /// [`Self::IDENT()`] | ++ -help: to link to the variant, prefix with `type@` +help: to link to the variant, prefix with `variant@` | -LL | /// [`type@Self::IDENT`] - | +++++ +LL | /// [`variant@Self::IDENT`] + | ++++++++ error: `Self::IDENT2` is both an associated constant and an associated type --> $DIR/issue-108653-associated-items.rs:30:7 diff --git a/tests/rustdoc/intra-doc/field.rs b/tests/rustdoc/intra-doc/field.rs index ba6b320e560..e98419618e2 100644 --- a/tests/rustdoc/intra-doc/field.rs +++ b/tests/rustdoc/intra-doc/field.rs @@ -1,4 +1,24 @@ //@ has field/index.html '//a[@href="{{channel}}/core/ops/range/struct.Range.html#structfield.start"]' 'start' //@ has field/index.html '//a[@href="{{channel}}/std/io/error/enum.ErrorKind.html#variant.NotFound"]' 'not_found' +//@ has field/index.html '//a[@href="struct.FieldAndMethod.html#structfield.x"]' 'x' +//@ has field/index.html '//a[@href="enum.VariantAndMethod.html#variant.X"]' 'X' //! [start][std::ops::Range::start] //! [not_found][std::io::ErrorKind::NotFound] +//! [x][field@crate::FieldAndMethod::x] +//! [X][variant@crate::VariantAndMethod::X] + +pub struct FieldAndMethod { + pub x: i32, +} + +impl FieldAndMethod { + pub fn x(&self) {} +} + +pub enum VariantAndMethod { + X {}, +} + +impl VariantAndMethod { + fn X() {} +}