From 472e52e5a03790becdbe21be1002a90dd2d7d3d4 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 20 Sep 2020 02:18:09 -0400 Subject: [PATCH] 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 cd4185226dce5..636f67a77c890 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 71fd5deca54b7..af077e0f5eb6b 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 cd06ee6f798dc..fab8c105a4953 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 819ef05174a8a..8f69b894a223d 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;