From dd7b8c85a6a2bffb2cce1c40ba72680a1d7be93b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 20 Sep 2020 00:32:00 -0400 Subject: [PATCH 1/3] Perform most diagnostic lookups in `resolution_failure` Previously, these were spread throughout the codebase. This had two drawbacks: 1. It caused the fast path to be slower: even if a link resolved, rustdoc would still perform various lookups for the error diagnostic. 2. It was inconsistent and didn't always give all diagnostics (https://github.com/rust-lang/rust/issues/76925) Now, diagnostics only perform expensive lookups in the error case. Additionally, the error handling is much more consistent, both in wording and behavior. - Remove `CannotHaveAssociatedItems`, `NotInScope`, `NoAssocItem`, and `NotAVariant` in favor of the more general `NotResolved` `resolution_failure` will now look up which of the four above categories is relevant, instead of requiring the rest of the code to be consistent and accurate in which it picked. - Remove unnecessary lookups throughout the intra-doc link pass. These are now done by `resolution_failure`. + Remove unnecessary `extra_fragment` argument to `variant_field()`; it was only used to do lookups on failure. + Remove various lookups related to associated items + Remove distinction between 'not in scope' and 'no associated item' - Don't perform unnecessary copies - Remove unused variables and code - Update tests - Note why looking at other namespaces is still necessary - 'has no inner item' -> 'contains no item' bless tests --- .../passes/collect_intra_doc_links.rs | 397 +++++++++--------- .../deny-intra-link-resolution-failure.stderr | 2 +- src/test/rustdoc-ui/intra-link-errors.rs | 16 +- src/test/rustdoc-ui/intra-link-errors.stderr | 51 ++- .../intra-link-span-ice-55723.stderr | 2 +- .../intra-links-warning-crlf.stderr | 8 +- .../rustdoc-ui/intra-links-warning.stderr | 36 +- src/test/rustdoc-ui/lint-group.stderr | 2 +- .../rustdoc/intra-link-associated-items.rs | 2 + 9 files changed, 263 insertions(+), 253 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 5a9eeec4dfe..71fd5deca54 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -60,14 +60,6 @@ impl<'a> From> for ErrorKind<'a> { #[derive(Debug)] enum ResolutionFailure<'a> { - /// This resolved, but with the wrong namespace. - /// `Namespace` is the expected namespace (as opposed to the actual). - WrongNamespace(Res, Namespace), - /// This has a partial resolution, but is not in the TypeNS and so cannot - /// have associated items or fields. - CannotHaveAssociatedItems(Res, Namespace), - /// `name` is the base name of the path (not necessarily the whole link) - NotInScope { module_id: DefId, name: Cow<'a, str> }, /// this is a primitive type without an impls (no associated methods) /// when will this actually happen? /// the `Res` is the primitive it resolved to @@ -75,33 +67,19 @@ enum ResolutionFailure<'a> { /// `[u8::not_found]` /// the `Res` is the primitive it resolved to NoPrimitiveAssocItem { res: Res, prim_name: &'a str, assoc_item: Symbol }, - /// `[S::not_found]` - /// the `String` is the associated item that wasn't found - NoAssocItem(Res, Symbol), + /// This resolved, but with the wrong namespace. + /// `Namespace` is the expected namespace (as opposed to the actual). + WrongNamespace(Res, Namespace), + /// The link failed to resolve. `resolution_failure` should look to see if there's + /// a more helpful error that can be given. + NotResolved { module_id: DefId, partial_res: Option, unresolved: Cow<'a, str> }, /// should not ever happen NoParentItem, - /// this could be an enum variant, but the last path fragment wasn't resolved. - /// the `String` is the variant that didn't exist - NotAVariant(Res, Symbol), /// used to communicate that this should be ignored, but shouldn't be reported to the user Dummy, } impl ResolutionFailure<'a> { - // A partial or full resolution - fn res(&self) -> Option { - use ResolutionFailure::*; - match self { - NoPrimitiveAssocItem { res, .. } - | NoAssocItem(res, _) - | NoPrimitiveImpl(res, _) - | NotAVariant(res, _) - | WrongNamespace(res, _) - | CannotHaveAssociatedItems(res, _) => Some(*res), - NotInScope { .. } | NoParentItem | Dummy => None, - } - } - // This resolved fully (not just partially) but is erroneous for some other reason fn full_res(&self) -> Option { match self { @@ -136,22 +114,25 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { path_str: &'path str, current_item: &Option, module_id: DefId, - extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; + let no_res = || ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: path_str.into(), + }; debug!("looking for enum variant {}", path_str); let mut split = path_str.rsplitn(3, "::"); - let variant_field_name = split + let (variant_field_str, variant_field_name) = split .next() - .map(|f| Symbol::intern(f)) + .map(|f| (f, Symbol::intern(f))) .expect("fold_item should ensure link is non-empty"); - let variant_name = + let (variant_str, variant_name) = // we're not sure this is a variant at all, so use the full string - split.next().map(|f| Symbol::intern(f)).ok_or_else(|| ResolutionFailure::NotInScope { - module_id, - name: path_str.into(), - })?; + // If there's no second component, the link looks like `[path]`. + // So there's no partial res and we should say the whole link failed to resolve. + split.next().map(|f| (f, Symbol::intern(f))).ok_or_else(no_res)?; let path = split .next() .map(|f| { @@ -162,10 +143,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - .ok_or_else(|| ResolutionFailure::NotInScope { - module_id, - name: variant_name.to_string().into(), - })?; + // If there's no third component, we saw `[a::b]` before and it failed to resolve. + // So there's no partial res. + .ok_or_else(no_res)?; let ty_res = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) @@ -173,7 +153,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .map(|(_, res)| res) .unwrap_or(Res::Err); if let Res::Err = ty_res { - return Err(ResolutionFailure::NotInScope { module_id, name: path.into() }.into()); + return Err(no_res().into()); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { @@ -196,38 +176,27 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty_res, Some(format!( "variant.{}.field.{}", - variant_name, variant_field_name + variant_str, variant_field_name )), )) } else { - Err(ResolutionFailure::NotAVariant(ty_res, variant_field_name).into()) + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: Some(Res::Def(DefKind::Enum, def.did)), + unresolved: variant_field_str.into(), + } + .into()) } } _ => unreachable!(), } } - // `variant_field` looks at 3 different path segments in a row. - // But `NoAssocItem` assumes there are only 2. Check to see if there's - // an intermediate segment that resolves. - _ => { - let intermediate_path = format!("{}::{}", path, variant_name); - // NOTE: we have to be careful here, because we're already in `resolve`. - // We know this doesn't recurse forever because we use a shorter path each time. - // NOTE: this uses `TypeNS` because nothing else has a valid path segment after - let kind = if let Some(intermediate) = self.check_full_res( - TypeNS, - &intermediate_path, - module_id, - current_item, - extra_fragment, - ) { - ResolutionFailure::NoAssocItem(intermediate, variant_field_name) - } else { - // Even with the shorter path, it didn't resolve, so say that. - ResolutionFailure::NoAssocItem(ty_res, variant_name) - }; - Err(kind.into()) + _ => Err(ResolutionFailure::NotResolved { + module_id, + partial_res: Some(ty_res), + unresolved: variant_str.into(), } + .into()), } } @@ -248,11 +217,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { false, ) { if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind { - return Some(Ok(res.map_id(|_| panic!("unexpected id")))); + return Ok(res.map_id(|_| panic!("unexpected id"))); } } if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) { - return Some(Ok(res.map_id(|_| panic!("unexpected id")))); + return Ok(res.map_id(|_| panic!("unexpected id"))); } debug!("resolving {} as a macro in the module {:?}", path_str, module_id); if let Ok((_, res)) = @@ -261,28 +230,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // don't resolve builtins like `#[derive]` if let Res::Def(..) = res { let res = res.map_id(|_| panic!("unexpected node_id")); - return Some(Ok(res)); + return Ok(res); } } - None - }) - // This weird control flow is so we don't borrow the resolver more than once at a time - .unwrap_or_else(|| { - let mut split = path_str.rsplitn(2, "::"); - if let Some((parent, base)) = split.next().and_then(|x| Some((split.next()?, x))) { - if let Some(res) = self.check_full_res(TypeNS, parent, module_id, &None, &None) { - return Err(if matches!(res, Res::PrimTy(_)) { - ResolutionFailure::NoPrimitiveAssocItem { - res, - prim_name: parent, - assoc_item: Symbol::intern(base), - } - } else { - ResolutionFailure::NoAssocItem(res, Symbol::intern(base)) - }); - } - } - Err(ResolutionFailure::NotInScope { module_id, name: path_str.into() }) + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: path_str.into(), + }) }) } @@ -347,7 +302,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Try looking for methods and associated items. let mut split = path_str.rsplitn(2, "::"); // this can be an `unwrap()` because we ensure the link is never empty - let item_name = Symbol::intern(split.next().unwrap()); + let (item_str, item_name) = split.next().map(|i| (i, Symbol::intern(i))).unwrap(); let path_root = split .next() .map(|f| { @@ -362,7 +317,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved. .ok_or_else(|| { debug!("found no `::`, assumming {} was correctly not in scope", item_name); - ResolutionFailure::NotInScope { module_id, name: item_name.to_string().into() } + ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: item_str.into(), + } })?; if let Some((path, prim)) = is_primitive(&path_root, TypeNS) { @@ -383,7 +342,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }) - .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name)))); + .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_str)))); if let Some(link) = link { return Ok(link); } @@ -411,25 +370,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let ty_res = match ty_res { Err(()) | Ok(Res::Err) => { return if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id, extra_fragment) + self.variant_field(path_str, current_item, module_id) } else { - // See if it only broke because of the namespace. - let kind = cx.enter_resolver(|resolver| { - // NOTE: this doesn't use `check_full_res` because we explicitly want to ignore `TypeNS` (we already checked it) - for &ns in &[MacroNS, ValueNS] { - match resolver - .resolve_str_path_error(DUMMY_SP, &path_root, ns, module_id) - { - Ok((_, Res::Err)) | Err(()) => {} - Ok((_, res)) => { - let res = res.map_id(|_| panic!("unexpected node_id")); - return ResolutionFailure::CannotHaveAssociatedItems(res, ns); - } - } - } - ResolutionFailure::NotInScope { module_id, name: path_root.into() } - }); - Err(kind.into()) + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: path_root.into(), + } + .into()) }; } Ok(res) => res, @@ -479,7 +427,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // 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(), id))); - Ok((ty_res, Some(format!("{}.{}", out, item_name)))) + Ok((ty_res, Some(format!("{}.{}", out, item_str)))) }) } else if ns == Namespace::ValueNS { debug!("looking for variants or fields named {} for {:?}", item_name, did); @@ -522,7 +470,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } else { // We already know this isn't in ValueNS, so no need to check variant_field - return Err(ResolutionFailure::NoAssocItem(ty_res, item_name).into()); + return Err(ResolutionFailure::NotResolved { + module_id, + partial_res: Some(ty_res), + unresolved: item_str.into(), + } + .into()); } } Res::Def(DefKind::Trait, did) => cx @@ -546,16 +499,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res))) } else { let res = Res::Def(item.kind.as_def_kind(), item.def_id); - Ok((res, Some(format!("{}.{}", kind, item_name)))) + Ok((res, Some(format!("{}.{}", kind, item_str)))) } }), _ => None, }; res.unwrap_or_else(|| { if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id, extra_fragment) + self.variant_field(path_str, current_item, module_id) } else { - Err(ResolutionFailure::NoAssocItem(ty_res, item_name).into()) + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: Some(ty_res), + unresolved: item_str.into(), + } + .into()) } }) } @@ -1133,6 +1091,8 @@ impl LinkCollector<'_, '_> { // We only looked in one namespace. Try to give a better error if possible. if kind.full_res().is_none() { let other_ns = if ns == ValueNS { TypeNS } else { ValueNS }; + // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator` + // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach for &new_ns in &[other_ns, MacroNS] { if let Some(res) = self.check_full_res( new_ns, @@ -1535,7 +1495,6 @@ fn resolution_failure( dox, &link_range, |diag, sp| { - let in_scope = kinds.iter().any(|kind| kind.res().is_some()); let item = |res: Res| { format!( "the {} `{}`", @@ -1556,53 +1515,142 @@ fn resolution_failure( // ignore duplicates let mut variants_seen = SmallVec::<[_; 3]>::new(); for mut failure in kinds { - // Check if _any_ parent of the path gets resolved. - // If so, report it and say the first which failed; if not, say the first path segment didn't resolve. - if let ResolutionFailure::NotInScope { module_id, name } = &mut failure { - let mut current = name.as_ref(); - loop { - current = match current.rsplitn(2, "::").nth(1) { - Some(p) => p, - None => { - *name = current.to_owned().into(); - break; - } - }; - if let Some(res) = - collector.check_full_res(TypeNS, ¤t, *module_id, &None, &None) - { - failure = ResolutionFailure::NoAssocItem(res, Symbol::intern(current)); - break; - } - } - } let variant = std::mem::discriminant(&failure); if variants_seen.contains(&variant) { continue; } variants_seen.push(variant); - let note = match failure { - ResolutionFailure::NotInScope { module_id, name, .. } => { - if in_scope { - continue; + + if let ResolutionFailure::NotResolved { module_id, partial_res, unresolved } = + &mut failure + { + use DefKind::*; + + let module_id = *module_id; + // FIXME(jynelson): this might conflict with my `Self` fix in #76467 + fn split(path: &str) -> Option<(&str, &str)> { + let mut splitter = path.rsplitn(2, "::"); + splitter.next().and_then(|right| splitter.next().map(|left| (left, right))) + } + + // Check if _any_ parent of the path gets resolved. + // If so, report it and say the first which failed; if not, say the first path segment didn't resolve. + let mut name = path_str; + 'outer: loop { + let (start, end) = if let Some(x) = split(name) { + x + } else { + // avoid bug that marked [Quux::Z] as missing Z, not Quux + if partial_res.is_none() { + *unresolved = name.into(); + } + break; + }; + name = start; + for &ns in &[TypeNS, ValueNS, MacroNS] { + if let Some(res) = + collector.check_full_res(ns, &start, module_id, &None, &None) + { + debug!("found partial_res={:?}", res); + *partial_res = Some(res); + *unresolved = end.into(); + break 'outer; + } } + *unresolved = end.into(); + } + + let last_found_module = match *partial_res { + Some(Res::Def(DefKind::Mod, id)) => Some(id), + None => Some(module_id), + _ => None, + }; + // See if this was a module: `[path]` or `[std::io::nope]` + if let Some(module) = last_found_module { // NOTE: uses an explicit `continue` so the `note:` will come before the `help:` - let module_name = collector.cx.tcx.item_name(module_id); - let note = format!("no item named `{}` in `{}`", name, module_name); + let module_name = collector.cx.tcx.item_name(module); + let note = format!( + "the module `{}` contains no item named `{}`", + module_name, unresolved + ); if let Some(span) = sp { diag.span_label(span, ¬e); } else { diag.note(¬e); } - // If the link has `::` in the path, assume it's meant to be an intra-doc link + // If the link has `::` in it, assume it was meant to be an intra-doc link. + // Otherwise, the `[]` might be unrelated. + // FIXME: don't show this for autolinks (`<>`), `()` style links, or reference links if !path_str.contains("::") { - // Otherwise, the `[]` might be unrelated. - // FIXME(https://github.com/raphlinus/pulldown-cmark/issues/373): - // don't show this for autolinks (`<>`), `()` style links, or reference links diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); } continue; } + + // Otherwise, it must be an associated item or variant + let res = partial_res.expect("None case was handled by `last_found_module`"); + let diagnostic_name; + let (kind, name) = match res { + Res::Def(kind, def_id) => { + diagnostic_name = collector.cx.tcx.item_name(def_id).as_str(); + (Some(kind), &*diagnostic_name) + } + Res::PrimTy(_) => (None, name), + _ => unreachable!("only ADTs and primitives are in scope at module level"), + }; + let path_description = if let Some(kind) = kind { + match kind { + Mod | ForeignMod => "inner item", + Struct => "field or associated item", + Enum | Union => "variant or associated item", + Variant + | Field + | Closure + | Generator + | AssocTy + | AssocConst + | AssocFn + | Fn + | Macro(_) + | Const + | ConstParam + | ExternCrate + | Use + | LifetimeParam + | Ctor(_, _) + | AnonConst => { + let note = assoc_item_not_allowed(res); + if let Some(span) = sp { + diag.span_label(span, ¬e); + } else { + diag.note(¬e); + } + return; + } + Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam + | Static => "associated item", + Impl | GlobalAsm => unreachable!("not a path"), + } + } else { + res.descr() + }; + let note = format!( + "the {} `{}` has no {} named `{}`", + res.descr(), + name, + disambiguator.map_or(path_description, |d| d.descr()), + unresolved, + ); + if let Some(span) = sp { + diag.span_label(span, ¬e); + } else { + diag.note(¬e); + } + + continue; + } + let note = match failure { + ResolutionFailure::NotResolved { .. } => unreachable!("handled above"), ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace(res, expected_ns) => { if let Res::Def(kind, _) = res { @@ -1637,69 +1685,6 @@ fn resolution_failure( prim_name, assoc_item ) } - ResolutionFailure::NoAssocItem(res, assoc_item) => { - use DefKind::*; - - let (kind, def_id) = match res { - Res::Def(kind, def_id) => (kind, def_id), - x => unreachable!( - "primitives are covered above and other `Res` variants aren't possible at module scope: {:?}", - x, - ), - }; - let name = collector.cx.tcx.item_name(def_id); - let path_description = if let Some(disambiguator) = disambiguator { - disambiguator.descr() - } else { - match kind { - Mod | ForeignMod => "inner item", - Struct => "field or associated item", - Enum | Union => "variant or associated item", - Variant - | Field - | Closure - | Generator - | AssocTy - | AssocConst - | AssocFn - | Fn - | Macro(_) - | Const - | ConstParam - | ExternCrate - | Use - | LifetimeParam - | Ctor(_, _) - | AnonConst => { - let note = assoc_item_not_allowed(res); - if let Some(span) = sp { - diag.span_label(span, ¬e); - } else { - diag.note(¬e); - } - return; - } - Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam - | Static => "associated item", - Impl | GlobalAsm => unreachable!("not a path"), - } - }; - format!( - "the {} `{}` has no {} named `{}`", - res.descr(), - name, - path_description, - assoc_item - ) - } - ResolutionFailure::CannotHaveAssociatedItems(res, _) => { - assoc_item_not_allowed(res) - } - ResolutionFailure::NotAVariant(res, variant) => format!( - "this link partially resolves to {}, but there is no variant named {}", - item(res), - variant - ), }; if let Some(span) = sp { diag.span_label(span, ¬e); diff --git a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr index 5020b97b2f2..33260fa0e1e 100644 --- a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr +++ b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr @@ -2,7 +2,7 @@ error: unresolved link to `v2` --> $DIR/deny-intra-link-resolution-failure.rs:3:6 | LL | /// [v2] - | ^^ no item named `v2` in `deny_intra_link_resolution_failure` + | ^^ the module `deny_intra_link_resolution_failure` contains no item named `v2` | note: the lint level is defined here --> $DIR/deny-intra-link-resolution-failure.rs:1:9 diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 26b629b1313..8c250fbc58b 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -6,19 +6,23 @@ /// [path::to::nonexistent::module] //~^ ERROR unresolved link -//~| NOTE no item named `path` in `intra_link_errors` +//~| NOTE `intra_link_errors` contains no item named `path` /// [path::to::nonexistent::macro!] //~^ ERROR unresolved link -//~| NOTE no item named `path` in `intra_link_errors` +//~| NOTE `intra_link_errors` contains no item named `path` /// [type@path::to::nonexistent::type] //~^ ERROR unresolved link -//~| NOTE no item named `path` in `intra_link_errors` +//~| NOTE `intra_link_errors` contains no item named `path` /// [std::io::not::here] //~^ ERROR unresolved link -//~| NOTE the module `io` has no inner item +//~| NOTE `io` contains no item named `not` + +/// [type@std::io::not::here] +//~^ ERROR unresolved link +//~| NOTE `io` contains no item named `not` /// [std::io::Error::x] //~^ ERROR unresolved link @@ -32,6 +36,10 @@ //~^ ERROR unresolved link //~| NOTE `f` is a function, not a module +/// [f::A!] +//~^ ERROR unresolved link +//~| NOTE `f` is a function, not a module + /// [S::A] //~^ ERROR unresolved link //~| NOTE struct `S` has no field or associated item diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index fbf3dcbbec2..cd06ee6f798 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -2,7 +2,7 @@ error: unresolved link to `path::to::nonexistent::module` --> $DIR/intra-link-errors.rs:7:6 | LL | /// [path::to::nonexistent::module] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `path` in `intra_link_errors` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` | note: the lint level is defined here --> $DIR/intra-link-errors.rs:1:9 @@ -14,64 +14,79 @@ error: unresolved link to `path::to::nonexistent::macro` --> $DIR/intra-link-errors.rs:11:6 | LL | /// [path::to::nonexistent::macro!] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `path` in `intra_link_errors` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` error: unresolved link to `path::to::nonexistent::type` --> $DIR/intra-link-errors.rs:15:6 | LL | /// [type@path::to::nonexistent::type] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `path` in `intra_link_errors` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` error: unresolved link to `std::io::not::here` --> $DIR/intra-link-errors.rs:19:6 | LL | /// [std::io::not::here] - | ^^^^^^^^^^^^^^^^^^ the module `io` has no inner item named `not` + | ^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` + +error: unresolved link to `std::io::not::here` + --> $DIR/intra-link-errors.rs:23:6 + | +LL | /// [type@std::io::not::here] + | ^^^^^^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` error: unresolved link to `std::io::Error::x` - --> $DIR/intra-link-errors.rs:23:6 + --> $DIR/intra-link-errors.rs:27:6 | LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` - --> $DIR/intra-link-errors.rs:27:6 + --> $DIR/intra-link-errors.rs:31:6 | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:31:6 + --> $DIR/intra-link-errors.rs:35:6 | LL | /// [f::A] | ^^^^ `f` is a function, not a module or type, and cannot have associated items +error: unresolved link to `f::A` + --> $DIR/intra-link-errors.rs:39:6 + | +LL | /// [f::A!] + | ^^^^^ `f` is a function, not a module or type, and cannot have associated items + error: unresolved link to `S::A` - --> $DIR/intra-link-errors.rs:35:6 + --> $DIR/intra-link-errors.rs:43:6 | LL | /// [S::A] | ^^^^ the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:39:6 + --> $DIR/intra-link-errors.rs:47:6 | LL | /// [S::fmt] | ^^^^^^ the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:43:6 + --> $DIR/intra-link-errors.rs:51:6 | LL | /// [E::D] | ^^^^ the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:47:6 + --> $DIR/intra-link-errors.rs:55:6 | LL | /// [u8::not_found] - | ^^^^^^^^^^^^^ the builtin type `u8` does not have an associated item named `not_found` + | ^^^^^^^^^^^^^ + | | + | the builtin type `u8` does not have an associated item named `not_found` + | the builtin type `u8` has no builtin type named `not_found` error: unresolved link to `S` - --> $DIR/intra-link-errors.rs:51:6 + --> $DIR/intra-link-errors.rs:59:6 | LL | /// [S!] | ^^ @@ -80,7 +95,7 @@ LL | /// [S!] | help: to link to the struct, prefix with `struct@`: `struct@S` error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:69:6 + --> $DIR/intra-link-errors.rs:77:6 | LL | /// [type@T::g] | ^^^^^^^^^ @@ -89,13 +104,13 @@ LL | /// [type@T::g] | help: to link to the associated function, add parentheses: `T::g()` error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:74:6 + --> $DIR/intra-link-errors.rs:82:6 | LL | /// [T::h!] | ^^^^^ the trait `T` has no macro named `h` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:61:6 + --> $DIR/intra-link-errors.rs:69:6 | LL | /// [type@S::h] | ^^^^^^^^^ @@ -104,7 +119,7 @@ LL | /// [type@S::h] | help: to link to the associated function, add parentheses: `S::h()` error: unresolved link to `m` - --> $DIR/intra-link-errors.rs:81:6 + --> $DIR/intra-link-errors.rs:89:6 | LL | /// [m()] | ^^^ @@ -112,5 +127,5 @@ LL | /// [m()] | this link resolves to the macro `m`, which is not in the value namespace | help: to link to the macro, add an exclamation mark: `m!` -error: aborting due to 16 previous errors +error: aborting due to 18 previous errors diff --git a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr index 3c13df20588..d946aa93980 100644 --- a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr +++ b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr @@ -2,7 +2,7 @@ error: unresolved link to `i` --> $DIR/intra-link-span-ice-55723.rs:9:10 | LL | /// （arr[i]） - | ^ no item named `i` in `intra_link_span_ice_55723` + | ^ the module `intra_link_span_ice_55723` contains no item named `i` | note: the lint level is defined here --> $DIR/intra-link-span-ice-55723.rs:1:9 diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr index 351f8fafa64..76a2ac0c8cf 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr @@ -2,7 +2,7 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:7:6 | LL | /// [error] - | ^^^^^ no item named `error` in `intra_links_warning_crlf` + | ^^^^^ the module `intra_links_warning_crlf` contains no item named `error` | = note: `#[warn(broken_intra_doc_links)]` on by default = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -11,7 +11,7 @@ warning: unresolved link to `error1` --> $DIR/intra-links-warning-crlf.rs:12:11 | LL | /// docs [error1] - | ^^^^^^ no item named `error1` in `intra_links_warning_crlf` + | ^^^^^^ the module `intra_links_warning_crlf` contains no item named `error1` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -19,7 +19,7 @@ warning: unresolved link to `error2` --> $DIR/intra-links-warning-crlf.rs:15:11 | LL | /// docs [error2] - | ^^^^^^ no item named `error2` in `intra_links_warning_crlf` + | ^^^^^^ the module `intra_links_warning_crlf` contains no item named `error2` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -27,7 +27,7 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:23:20 | LL | * It also has an [error]. - | ^^^^^ no item named `error` in `intra_links_warning_crlf` + | ^^^^^ the module `intra_links_warning_crlf` contains no item named `error` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 0832e00d35a..09db465df59 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -10,37 +10,37 @@ warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ no item named `Bar` in `intra_links_warning` + | ^^^^^^^^ the module `intra_links_warning` contains no item named `Bar` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:6:13 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ no item named `Uniooon` in `intra_links_warning` + | ^^^^^^^^^^ the module `intra_links_warning` contains no item named `Uniooon` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:6:30 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ no item named `Qux` in `intra_links_warning` + | ^^^^^^ the module `intra_links_warning` contains no item named `Qux` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:10:14 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ no item named `Uniooon` in `intra_links_warning` + | ^^^^^^^^^^ the module `intra_links_warning` contains no item named `Uniooon` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:10:31 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ no item named `Qux` in `intra_links_warning` + | ^^^^^^ the module `intra_links_warning` contains no item named `Qux` warning: unresolved link to `Qux:Y` --> $DIR/intra-links-warning.rs:14:13 | LL | /// [Qux:Y] - | ^^^^^ no item named `Qux:Y` in `intra_links_warning` + | ^^^^^ the module `intra_links_warning` contains no item named `Qux:Y` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -48,7 +48,7 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:58:30 | LL | * time to introduce a link [error]*/ - | ^^^^^ no item named `error` in `intra_links_warning` + | ^^^^^ the module `intra_links_warning` contains no item named `error` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -56,7 +56,7 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:64:30 | LL | * time to introduce a link [error] - | ^^^^^ no item named `error` in `intra_links_warning` + | ^^^^^ the module `intra_links_warning` contains no item named `error` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -70,7 +70,7 @@ LL | #[doc = "single line [error]"] single line [error] ^^^^^ - = note: no item named `error` in `intra_links_warning` + = note: the module `intra_links_warning` contains no item named `error` = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -83,7 +83,7 @@ LL | #[doc = "single line with \"escaping\" [error]"] single line with "escaping" [error] ^^^^^ - = note: no item named `error` in `intra_links_warning` + = note: the module `intra_links_warning` contains no item named `error` = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -98,14 +98,14 @@ LL | | /// [error] [error] ^^^^^ - = note: no item named `error` in `intra_links_warning` + = note: the module `intra_links_warning` contains no item named `error` = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error1` --> $DIR/intra-links-warning.rs:80:11 | LL | /// docs [error1] - | ^^^^^^ no item named `error1` in `intra_links_warning` + | ^^^^^^ the module `intra_links_warning` contains no item named `error1` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -113,7 +113,7 @@ warning: unresolved link to `error2` --> $DIR/intra-links-warning.rs:82:11 | LL | /// docs [error2] - | ^^^^^^ no item named `error2` in `intra_links_warning` + | ^^^^^^ the module `intra_links_warning` contains no item named `error2` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -121,7 +121,7 @@ warning: unresolved link to `BarA` --> $DIR/intra-links-warning.rs:21:10 | LL | /// bar [BarA] bar - | ^^^^ no item named `BarA` in `intra_links_warning` + | ^^^^ the module `intra_links_warning` contains no item named `BarA` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -129,7 +129,7 @@ warning: unresolved link to `BarB` --> $DIR/intra-links-warning.rs:27:9 | LL | * bar [BarB] bar - | ^^^^ no item named `BarB` in `intra_links_warning` + | ^^^^ the module `intra_links_warning` contains no item named `BarB` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -137,7 +137,7 @@ warning: unresolved link to `BarC` --> $DIR/intra-links-warning.rs:34:6 | LL | bar [BarC] bar - | ^^^^ no item named `BarC` in `intra_links_warning` + | ^^^^ the module `intra_links_warning` contains no item named `BarC` | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` @@ -151,7 +151,7 @@ LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] bar [BarD] bar ^^^^ - = note: no item named `BarD` in `intra_links_warning` + = note: the module `intra_links_warning` contains no item named `BarD` = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarF` @@ -167,7 +167,7 @@ LL | f!("Foo\nbar [BarF] bar\nbaz"); bar [BarF] bar ^^^^ - = note: no item named `BarF` in `intra_links_warning` + = note: the module `intra_links_warning` contains no item named `BarF` = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/src/test/rustdoc-ui/lint-group.stderr b/src/test/rustdoc-ui/lint-group.stderr index 550b79f6e89..4e9134ea469 100644 --- a/src/test/rustdoc-ui/lint-group.stderr +++ b/src/test/rustdoc-ui/lint-group.stderr @@ -32,7 +32,7 @@ error: unresolved link to `error` --> $DIR/lint-group.rs:9:29 | LL | /// what up, let's make an [error] - | ^^^^^ no item named `error` in `lint_group` + | ^^^^^ the module `lint_group` contains no item named `error` | note: the lint level is defined here --> $DIR/lint-group.rs:7:9 diff --git a/src/test/rustdoc/intra-link-associated-items.rs b/src/test/rustdoc/intra-link-associated-items.rs index 16a21e33748..dec0deae5df 100644 --- a/src/test/rustdoc/intra-link-associated-items.rs +++ b/src/test/rustdoc/intra-link-associated-items.rs @@ -3,8 +3,10 @@ /// [`std::collections::BTreeMap::into_iter`] /// [`String::from`] is ambiguous as to which `From` impl +/// [type@Vec::into_iter] uses a disambiguator // @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter' // @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from"]' 'String::from' +// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/vec/struct.Vec.html#method.into_iter"]' 'Vec::into_iter' pub fn foo() {} /// Link to [MyStruct], [link from struct][MyStruct::method], [MyStruct::clone], [MyStruct::Input] From 472e52e5a03790becdbe21be1002a90dd2d7d3d4 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 20 Sep 2020 02:18:09 -0400 Subject: [PATCH 2/3] Fix intra-doc links for primitives - Add `PrimTy::name` and `PrimTy::name_str` - Use those new functions to distinguish between the name in scope and the canonical name - Fix diagnostics for primitive types - Add tests for primitives --- compiler/rustc_hir/src/hir.rs | 24 +++++++++++++++++++ .../passes/collect_intra_doc_links.rs | 20 +++++++++++----- src/test/rustdoc-ui/intra-link-errors.stderr | 5 +--- src/test/rustdoc/primitive-link.rs | 7 ++++++ 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index cd4185226dc..636f67a77c8 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2004,6 +2004,30 @@ pub enum PrimTy { Char, } +impl PrimTy { + pub fn name_str(self) -> &'static str { + match self { + PrimTy::Int(i) => i.name_str(), + PrimTy::Uint(u) => u.name_str(), + PrimTy::Float(f) => f.name_str(), + PrimTy::Str => "str", + PrimTy::Bool => "bool", + PrimTy::Char => "char", + } + } + + pub fn name(self) -> Symbol { + match self { + PrimTy::Int(i) => i.name(), + PrimTy::Uint(u) => u.name(), + PrimTy::Float(f) => f.name(), + PrimTy::Str => sym::str, + PrimTy::Bool => sym::bool, + PrimTy::Char => sym::char, + } + } +} + #[derive(Debug, HashStable_Generic)] pub struct BareFnTy<'hir> { pub unsafety: Unsafety, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 71fd5deca54..af077e0f5eb 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -273,13 +273,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { return handle_variant(cx, res, extra_fragment); } // Not a trait item; just return what we found. - Res::PrimTy(..) => { + Res::PrimTy(ty) => { if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure( AnchorFailure::RustdocAnchorConflict(res), )); } - return Ok((res, Some(path_str.to_owned()))); + return Ok((res, Some(ty.name_str().to_owned()))); } Res::Def(DefKind::Mod, _) => { return Ok((res, extra_fragment.clone())); @@ -292,6 +292,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if value != (ns == ValueNS) { return Err(ResolutionFailure::WrongNamespace(res, ns).into()); } + // FIXME: why is this necessary? } else if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(prim))); @@ -1008,12 +1009,12 @@ impl LinkCollector<'_, '_> { suggest_disambiguator(resolved, diag, path_str, dox, sp, &link_range); }); }; - if let Res::PrimTy(_) = res { + if let Res::PrimTy(ty) = res { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { item.attrs.links.push(ItemLink { link: ori_link, - link_text: path_str.to_owned(), + link_text, did: None, fragment, }); @@ -1488,6 +1489,10 @@ fn resolution_failure( link_range: Option>, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { + let has_primitive = kinds.iter().any(|err| + matches!(err, ResolutionFailure::NoPrimitiveAssocItem{..} | ResolutionFailure::NoPrimitiveImpl(_, _)) + ); + report_diagnostic( collector.cx, &format!("unresolved link to `{}`", path_str), @@ -1528,6 +1533,7 @@ fn resolution_failure( let module_id = *module_id; // FIXME(jynelson): this might conflict with my `Self` fix in #76467 + // FIXME: use itertools `collect_tuple` instead fn split(path: &str) -> Option<(&str, &str)> { let mut splitter = path.rsplitn(2, "::"); splitter.next().and_then(|right| splitter.next().map(|left| (left, right))) @@ -1567,7 +1573,6 @@ fn resolution_failure( }; // See if this was a module: `[path]` or `[std::io::nope]` if let Some(module) = last_found_module { - // NOTE: uses an explicit `continue` so the `note:` will come before the `help:` let module_name = collector.cx.tcx.item_name(module); let note = format!( "the module `{}` contains no item named `{}`", @@ -1595,7 +1600,10 @@ fn resolution_failure( diagnostic_name = collector.cx.tcx.item_name(def_id).as_str(); (Some(kind), &*diagnostic_name) } - Res::PrimTy(_) => (None, name), + Res::PrimTy(_) => { + assert!(has_primitive); + continue; + } _ => unreachable!("only ADTs and primitives are in scope at module level"), }; let path_description = if let Some(kind) = kind { diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index cd06ee6f798..fab8c105a49 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -80,10 +80,7 @@ error: unresolved link to `u8::not_found` --> $DIR/intra-link-errors.rs:55:6 | LL | /// [u8::not_found] - | ^^^^^^^^^^^^^ - | | - | the builtin type `u8` does not have an associated item named `not_found` - | the builtin type `u8` has no builtin type named `not_found` + | ^^^^^^^^^^^^^ the builtin type `u8` does not have an associated item named `not_found` error: unresolved link to `S` --> $DIR/intra-link-errors.rs:59:6 diff --git a/src/test/rustdoc/primitive-link.rs b/src/test/rustdoc/primitive-link.rs index 819ef05174a..8f69b894a22 100644 --- a/src/test/rustdoc/primitive-link.rs +++ b/src/test/rustdoc/primitive-link.rs @@ -4,6 +4,13 @@ // @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.u32.html"]' 'u32' // @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i64.html"]' 'i64' +// @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i32.html"]' 'std::primitive::i32' +// @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html"]' 'std::primitive::str' + +// FIXME: this doesn't resolve +// @ has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i32.html#associatedconstant.MAX"]' 'std::primitive::i32::MAX' /// It contains [`u32`] and [i64]. +/// It also links to [std::primitive::i32], [std::primitive::str], +/// and [`std::primitive::i32::MAX`]. pub struct Foo; From 049d29bac580b958cdff6e898dad22de09a27958 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 20 Sep 2020 02:39:19 -0400 Subject: [PATCH 3/3] Unify primitive errors with other intra-link errors Now that `PrimTy::name()` exists, there's no need to carry around the name of the primitive that failed to resolve. This removes the variants special-casing primitives in favor of `NotResolved`. - Remove `NoPrimitiveImpl` and `NoPrimitiveAssocItem` - Remove hacky `has_primitive` check in `resolution_failure()` - Fixup a couple tests that I forgot to `--bless` before --- .../passes/collect_intra_doc_links.rs | 48 ++++++------------- src/test/rustdoc-ui/intra-link-errors.rs | 11 ++++- src/test/rustdoc-ui/intra-link-errors.stderr | 29 ++++++++--- .../rustdoc/intra-link-associated-items.rs | 2 +- 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index af077e0f5eb..cb2de05598f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -60,13 +60,6 @@ impl<'a> From> for ErrorKind<'a> { #[derive(Debug)] enum ResolutionFailure<'a> { - /// this is a primitive type without an impls (no associated methods) - /// when will this actually happen? - /// the `Res` is the primitive it resolved to - NoPrimitiveImpl(Res, String), - /// `[u8::not_found]` - /// the `Res` is the primitive it resolved to - NoPrimitiveAssocItem { res: Res, prim_name: &'a str, assoc_item: Symbol }, /// This resolved, but with the wrong namespace. /// `Namespace` is the expected namespace (as opposed to the actual). WrongNamespace(Res, Namespace), @@ -326,8 +319,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { })?; if let Some((path, prim)) = is_primitive(&path_root, TypeNS) { - let impls = primitive_impl(cx, &path) - .ok_or_else(|| ResolutionFailure::NoPrimitiveImpl(prim, path_root.into()))?; + let impls = + primitive_impl(cx, &path).ok_or_else(|| ResolutionFailure::NotResolved { + module_id, + partial_res: Some(prim), + unresolved: item_str.into(), + })?; for &impl_ in impls { let link = cx .tcx @@ -354,10 +351,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { item_name, ns.descr() ); - return Err(ResolutionFailure::NoPrimitiveAssocItem { - res: prim, - prim_name: path, - assoc_item: item_name, + return Err(ResolutionFailure::NotResolved { + module_id, + partial_res: Some(prim), + unresolved: item_str.into(), } .into()); } @@ -1009,7 +1006,7 @@ impl LinkCollector<'_, '_> { suggest_disambiguator(resolved, diag, path_str, dox, sp, &link_range); }); }; - if let Res::PrimTy(ty) = res { + if let Res::PrimTy(..) = res { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { item.attrs.links.push(ItemLink { @@ -1489,10 +1486,6 @@ fn resolution_failure( link_range: Option>, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { - let has_primitive = kinds.iter().any(|err| - matches!(err, ResolutionFailure::NoPrimitiveAssocItem{..} | ResolutionFailure::NoPrimitiveImpl(_, _)) - ); - report_diagnostic( collector.cx, &format!("unresolved link to `{}`", path_str), @@ -1533,7 +1526,7 @@ fn resolution_failure( let module_id = *module_id; // FIXME(jynelson): this might conflict with my `Self` fix in #76467 - // FIXME: use itertools `collect_tuple` instead + // FIXME: maybe use itertools `collect_tuple` instead? fn split(path: &str) -> Option<(&str, &str)> { let mut splitter = path.rsplitn(2, "::"); splitter.next().and_then(|right| splitter.next().map(|left| (left, right))) @@ -1600,10 +1593,7 @@ fn resolution_failure( diagnostic_name = collector.cx.tcx.item_name(def_id).as_str(); (Some(kind), &*diagnostic_name) } - Res::PrimTy(_) => { - assert!(has_primitive); - continue; - } + Res::PrimTy(ty) => (None, ty.name_str()), _ => unreachable!("only ADTs and primitives are in scope at module level"), }; let path_description = if let Some(kind) = kind { @@ -1640,7 +1630,7 @@ fn resolution_failure( Impl | GlobalAsm => unreachable!("not a path"), } } else { - res.descr() + "associated item" }; let note = format!( "the {} `{}` has no {} named `{}`", @@ -1683,16 +1673,6 @@ fn resolution_failure( diag.level = rustc_errors::Level::Bug; "all intra doc links should have a parent item".to_owned() } - ResolutionFailure::NoPrimitiveImpl(res, _) => format!( - "this link partially resolves to {}, which does not have any associated items", - item(res), - ), - ResolutionFailure::NoPrimitiveAssocItem { prim_name, assoc_item, .. } => { - format!( - "the builtin type `{}` does not have an associated item named `{}`", - prim_name, assoc_item - ) - } }; if let Some(span) = sp { diag.span_label(span, ¬e); diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 8c250fbc58b..0278caf3087 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -54,7 +54,16 @@ /// [u8::not_found] //~^ ERROR unresolved link -//~| NOTE the builtin type `u8` does not have an associated item named `not_found` +//~| NOTE the builtin type `u8` has no associated item named `not_found` + +/// [std::primitive::u8::not_found] +//~^ ERROR unresolved link +//~| NOTE the builtin type `u8` has no associated item named `not_found` + +/// [type@Vec::into_iter] +//~^ ERROR unresolved link +//~| HELP to link to the associated function, add parentheses +//~| NOTE this link resolves to the associated function `into_iter` /// [S!] //~^ ERROR unresolved link diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index fab8c105a49..b63f799535a 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -80,10 +80,25 @@ error: unresolved link to `u8::not_found` --> $DIR/intra-link-errors.rs:55:6 | LL | /// [u8::not_found] - | ^^^^^^^^^^^^^ the builtin type `u8` does not have an associated item named `not_found` + | ^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` + +error: unresolved link to `std::primitive::u8::not_found` + --> $DIR/intra-link-errors.rs:59:6 + | +LL | /// [std::primitive::u8::not_found] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` + +error: unresolved link to `Vec::into_iter` + --> $DIR/intra-link-errors.rs:63:6 + | +LL | /// [type@Vec::into_iter] + | ^^^^^^^^^^^^^^^^^^^ + | | + | this link resolves to the associated function `into_iter`, which is not in the type namespace + | help: to link to the associated function, add parentheses: `Vec::into_iter()` error: unresolved link to `S` - --> $DIR/intra-link-errors.rs:59:6 + --> $DIR/intra-link-errors.rs:68:6 | LL | /// [S!] | ^^ @@ -92,7 +107,7 @@ LL | /// [S!] | help: to link to the struct, prefix with `struct@`: `struct@S` error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:77:6 + --> $DIR/intra-link-errors.rs:86:6 | LL | /// [type@T::g] | ^^^^^^^^^ @@ -101,13 +116,13 @@ LL | /// [type@T::g] | help: to link to the associated function, add parentheses: `T::g()` error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:82:6 + --> $DIR/intra-link-errors.rs:91:6 | LL | /// [T::h!] | ^^^^^ the trait `T` has no macro named `h` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:69:6 + --> $DIR/intra-link-errors.rs:78:6 | LL | /// [type@S::h] | ^^^^^^^^^ @@ -116,7 +131,7 @@ LL | /// [type@S::h] | help: to link to the associated function, add parentheses: `S::h()` error: unresolved link to `m` - --> $DIR/intra-link-errors.rs:89:6 + --> $DIR/intra-link-errors.rs:98:6 | LL | /// [m()] | ^^^ @@ -124,5 +139,5 @@ LL | /// [m()] | this link resolves to the macro `m`, which is not in the value namespace | help: to link to the macro, add an exclamation mark: `m!` -error: aborting due to 18 previous errors +error: aborting due to 20 previous errors diff --git a/src/test/rustdoc/intra-link-associated-items.rs b/src/test/rustdoc/intra-link-associated-items.rs index dec0deae5df..daf7075a917 100644 --- a/src/test/rustdoc/intra-link-associated-items.rs +++ b/src/test/rustdoc/intra-link-associated-items.rs @@ -3,7 +3,7 @@ /// [`std::collections::BTreeMap::into_iter`] /// [`String::from`] is ambiguous as to which `From` impl -/// [type@Vec::into_iter] uses a disambiguator +/// [Vec::into_iter()] uses a disambiguator // @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter' // @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from"]' 'String::from' // @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/vec/struct.Vec.html#method.into_iter"]' 'Vec::into_iter'