From 42bed035001b9a0bd6282326a243a926ea3d0424 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 23:05:47 -0400 Subject: [PATCH] Pass on the DefId so rustdoc can name it in suggestions Look at this beauty: ```rust error: unresolved link to `S::h` --> intra-link-errors.rs:51:6 | 51 | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | = note: this link resolves to the associated function `h`, which is not in the type namespace ``` --- .../passes/collect_intra_doc_links.rs | 52 ++++++++++--------- src/test/rustdoc-ui/intra-link-errors.rs | 7 +++ src/test/rustdoc-ui/intra-link-errors.stderr | 27 ++++++---- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 50132577367..287f5fcf805 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -121,7 +121,7 @@ struct LinkCollector<'a, 'tcx> { /// This is used to store the kind of associated items, /// because `clean` and the disambiguator code expect them to be different. /// See the code for associated items on inherent impls for details. - kind_side_channel: Cell>, + kind_side_channel: Cell>, } impl<'a, 'tcx> LinkCollector<'a, 'tcx> { @@ -381,7 +381,7 @@ fn resolve<'path>( ) => { debug!("looking for associated item named {} for item {:?}", item_name, did); // Checks if item_name belongs to `impl SomeItem` - let kind = cx + let assoc_item = cx .tcx .inherent_impls(did) .iter() @@ -393,7 +393,7 @@ fn resolve<'path>( imp, ) }) - .map(|item| item.kind) + .map(|item| (item.kind, item.def_id)) // There should only ever be one associated item that matches from any inherent impl .next() // Check if item_name belongs to `impl SomeTrait for SomeItem` @@ -409,7 +409,7 @@ fn resolve<'path>( kind }); - if let Some(kind) = kind { + if let Some((kind, id)) = assoc_item { let out = match kind { ty::AssocKind::Fn => "method", ty::AssocKind::Const => "associatedconstant", @@ -425,7 +425,7 @@ fn resolve<'path>( // HACK(jynelson): `clean` expects the type, not the associated item. // but the disambiguator logic expects the associated item. // Store the kind in a side channel so that only the disambiguator logic looks at it. - self.kind_side_channel.set(Some(kind.as_def_kind())); + self.kind_side_channel.set(Some((kind.as_def_kind(), id))); Ok((ty_res, Some(format!("{}.{}", out, item_name)))) }) } else if ns == Namespace::ValueNS { @@ -525,7 +525,7 @@ fn resolve_associated_trait_item( item_name: Symbol, ns: Namespace, cx: &DocContext<'_>, -) -> Option { +) -> Option<(ty::AssocKind, DefId)> { let ty = cx.tcx.type_of(did); // First consider automatic impls: `impl From for T` let implicit_impls = crate::clean::get_auto_trait_and_blanket_impls(cx, ty, did); @@ -553,7 +553,7 @@ fn resolve_associated_trait_item( // but provided methods come directly from `tcx`. // Fortunately, we don't need the whole method, we just need to know // what kind of associated item it is. - Some((assoc.def_id, kind)) + Some((kind, assoc.def_id)) }); let assoc = items.next(); debug_assert_eq!(items.count(), 0); @@ -575,7 +575,7 @@ fn resolve_associated_trait_item( ns, trait_, ) - .map(|assoc| (assoc.def_id, assoc.kind)) + .map(|assoc| (assoc.kind, assoc.def_id)) } } _ => panic!("get_impls returned something that wasn't an impl"), @@ -592,12 +592,12 @@ fn resolve_associated_trait_item( cx.tcx .associated_items(trait_) .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, trait_) - .map(|assoc| (assoc.def_id, assoc.kind)) + .map(|assoc| (assoc.kind, assoc.def_id)) })); } // FIXME: warn about ambiguity debug!("the candidates were {:?}", candidates); - candidates.pop().map(|(_, kind)| kind) + candidates.pop() } /// Given a type, return all traits in scope in `module` implemented by that type. @@ -851,18 +851,21 @@ fn fold_item(&mut self, mut item: Item) -> Option { // used for reporting better errors let check_full_res = |this: &mut Self, ns| { - match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { - Ok(res) => { - debug!( - "check_full_res: saw res for {} in {:?} ns: {:?}", - path_str, ns, res.0 - ); - Some(res.0) - } - Err(ErrorKind::Resolve(kind)) => kind.full_res(), - // TODO: add `Res` to AnchorFailure - Err(ErrorKind::AnchorFailure(_)) => None, - } + let res = + match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) + { + Ok(res) => { + debug!( + "check_full_res: saw res for {} in {:?} ns: {:?}", + path_str, ns, res.0 + ); + Some(res.0) + } + Err(ErrorKind::Resolve(kind)) => kind.full_res(), + // TODO: add `Res` to AnchorFailure + Err(ErrorKind::AnchorFailure(_)) => None, + }; + this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) }; match disambiguator.map(Disambiguator::ns) { @@ -876,7 +879,8 @@ fn fold_item(&mut self, mut item: Item) -> Option { if kind.full_res().is_none() { let other_ns = if ns == ValueNS { TypeNS } else { ValueNS }; if let Some(res) = check_full_res(self, other_ns) { - kind = ResolutionFailure::WrongNamespace(res, other_ns); + // recall that this stores the _expected_ namespace + kind = ResolutionFailure::WrongNamespace(res, ns); } } resolution_failure( @@ -1092,7 +1096,7 @@ fn fold_item(&mut self, mut item: Item) -> Option { // Disallow e.g. linking to enums with `struct@` if let Res::Def(kind, _) = res { debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator); - match (self.kind_side_channel.take().unwrap_or(kind), disambiguator) { + match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) { | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const))) // NOTE: this allows 'method' to mean both normal functions and associated functions // This can't cause ambiguity because both are in the same namespace. diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 99d080fb324..8c42a38ff4e 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -49,11 +49,18 @@ pub fn f() {} pub enum E { A, B, C } /// [type@S::h] +//~^ ERROR unresolved link +//~| HELP to link to the associated function +//~| NOTE not in the type namespace impl S { pub fn h() {} } /// [type@T::g] +//~^ ERROR unresolved link +//~| HELP to link to the associated function +//~| NOTE not in the type namespace + /// [T::h!] pub trait T { fn g() {} diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index baf4ab7a9bc..bb9db68e0d5 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -80,27 +80,34 @@ error: unresolved link to `S` --> $DIR/intra-link-errors.rs:41:6 | LL | /// [S!] - | ^^ help: to link to the unit struct, use its disambiguator: `value@S` + | ^^ help: to link to the struct, use its disambiguator: `struct@S` | - = note: this link resolves to the unit struct `S`, which is not in the macro namespace + = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:56:6 + --> $DIR/intra-link-errors.rs:59:6 | LL | /// [type@T::g] - | ^^^^^^^^^ + | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` | - = note: this link partially resolves to the trait `T` - = note: `T` has no field, variant, or associated item named `g` + = note: this link resolves to the associated function `g`, which is not in the type namespace + +error: unresolved link to `T::h` + --> $DIR/intra-link-errors.rs:64:6 + | +LL | /// [T::h!] + | ^^^^^ + | + = note: no item named `T::h` is in scope + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` --> $DIR/intra-link-errors.rs:51:6 | LL | /// [type@S::h] - | ^^^^^^^^^ + | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | - = note: this link partially resolves to the struct `S` - = note: `S` has no field, variant, or associated item named `h` + = note: this link resolves to the associated function `h`, which is not in the type namespace -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors -- 2.44.0