From 0fd4f8febf8fc0edb94481f35c44c7e9e6e1ba87 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 18 Dec 2020 23:35:44 -0500 Subject: [PATCH 1/4] Add a custom `Res` type - Don't make rustc_resolve::Res public; use a new type alias instead --- .../passes/collect_intra_doc_links.rs | 277 ++++++++++-------- 1 file changed, 161 insertions(+), 116 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ea5bf94689bc7..1f1fb9754cfed 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -10,17 +10,16 @@ use rustc_hir as hir; use rustc_hir::def::{ DefKind, Namespace::{self, *}, - PerNS, Res, + PerNS, }; use rustc_hir::def_id::{CrateNum, DefId}; -use rustc_middle::ty; +use rustc_middle::{bug, ty}; use rustc_resolve::ParentScope; use rustc_session::lint::{ builtin::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS}, Lint, }; use rustc_span::hygiene::MacroKind; -use rustc_span::symbol::sym; use rustc_span::symbol::Ident; use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; @@ -28,6 +27,7 @@ use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; use std::cell::Cell; +use std::convert::{TryFrom, TryInto}; use std::mem; use std::ops::Range; @@ -61,6 +61,71 @@ impl<'a> From> for ErrorKind<'a> { } } +#[derive(Copy, Clone, Debug, Hash)] +enum Res { + Def(DefKind, DefId), + Primitive(PrimitiveType), +} + +type ResolveRes = rustc_hir::def::Res; + +impl Res { + fn descr(self) -> &'static str { + match self { + Res::Def(kind, id) => ResolveRes::Def(kind, id).descr(), + Res::Primitive(_) => "builtin type", + } + } + + fn article(self) -> &'static str { + match self { + Res::Def(kind, id) => ResolveRes::Def(kind, id).article(), + Res::Primitive(_) => "a", + } + } + + fn name(self, tcx: ty::TyCtxt<'_>) -> String { + match self { + Res::Def(_, id) => tcx.item_name(id).to_string(), + Res::Primitive(prim) => prim.as_str().to_string(), + } + } + + fn def_id(self) -> DefId { + self.opt_def_id().expect("called def_id() on a primitive") + } + + fn opt_def_id(self) -> Option { + match self { + Res::Def(_, id) => Some(id), + Res::Primitive(_) => None, + } + } + + fn as_hir_res(self) -> Option { + match self { + Res::Def(kind, id) => Some(rustc_hir::def::Res::Def(kind, id)), + // FIXME: maybe this should handle the subset of PrimitiveType that fits into hir::PrimTy? + Res::Primitive(_) => None, + } + } +} + +impl TryFrom for Res { + type Error = (); + + fn try_from(res: ResolveRes) -> Result { + use rustc_hir::def::Res::*; + match res { + Def(kind, id) => Ok(Res::Def(kind, id)), + PrimTy(prim) => Ok(Res::Primitive(PrimitiveType::from_hir(prim))), + // e.g. `#[derive]` + NonMacroAttr(..) | Err => Result::Err(()), + other => bug!("unrecognized res {:?}", other), + } + } +} + #[derive(Debug)] /// A link failed to resolve. enum ResolutionFailure<'a> { @@ -253,12 +318,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) }) - .map(|(_, res)| res) - .unwrap_or(Res::Err); - if let Res::Err = ty_res { - return Err(no_res().into()); - } - let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); + .and_then(|(_, res)| res.try_into()) + .map_err(|()| no_res())?; + match ty_res { Res::Def(DefKind::Enum, did) => { if cx @@ -309,7 +371,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// lifetimes on `&'path` will work. fn resolve_primitive_associated_item( &self, - prim_ty: hir::PrimTy, + prim_ty: PrimitiveType, ns: Namespace, module_id: DefId, item_name: Symbol, @@ -317,7 +379,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; - PrimitiveType::from_hir(prim_ty) + prim_ty .impls(cx.tcx) .into_iter() .find_map(|&impl_| { @@ -336,21 +398,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) .map(|out| { ( - Res::PrimTy(prim_ty), - Some(format!("{}#{}.{}", prim_ty.name(), out, item_str)), + Res::Primitive(prim_ty), + Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_str)), ) }) }) .ok_or_else(|| { debug!( "returning primitive error for {}::{} in {} namespace", - prim_ty.name(), + prim_ty.as_str(), item_name, ns.descr() ); ResolutionFailure::NotResolved { module_id, - partial_res: Some(Res::PrimTy(prim_ty)), + partial_res: Some(Res::Primitive(prim_ty)), unresolved: item_str.into(), } .into() @@ -377,19 +439,18 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { false, ) { if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind { - return Ok(res.map_id(|_| panic!("unexpected id"))); + return Ok(res.try_into().unwrap()); } } - if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) { - return Ok(res.map_id(|_| panic!("unexpected id"))); + if let Some(&res) = resolver.all_macros().get(&Symbol::intern(path_str)) { + return Ok(res.try_into().unwrap()); } debug!("resolving {} as a macro in the module {:?}", path_str, module_id); if let Ok((_, res)) = resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id) { // don't resolve builtins like `#[derive]` - if let Res::Def(..) = res { - let res = res.map_id(|_| panic!("unexpected node_id")); + if let Ok(res) = res.try_into() { return Ok(res); } } @@ -408,14 +469,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// Associated items will never be resolved by this function. fn resolve_path(&self, path_str: &str, ns: Namespace, module_id: DefId) -> Option { let result = self.cx.enter_resolver(|resolver| { - resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id) + resolver + .resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id) + .and_then(|(_, res)| res.try_into()) }); debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); - match result.map(|(_, res)| res) { + match result { // resolver doesn't know about true and false so we'll have to resolve them // manually as bool - Ok(Res::Err) | Err(()) => is_bool_value(path_str, ns).map(|(_, res)| res), - Ok(res) => Some(res.map_id(|_| panic!("unexpected node_id"))), + Err(()) => is_bool_value(path_str, ns), + Ok(res) => Some(res), } } @@ -444,13 +507,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(ty) => { + Res::Primitive(ty) => { if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure( AnchorFailure::RustdocAnchorConflict(res), )); } - return Ok((res, Some(ty.name_str().to_owned()))); + return Ok((res, Some(ty.as_str().to_owned()))); } Res::Def(DefKind::Mod, _) => { return Ok((res, extra_fragment.clone())); @@ -483,7 +546,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // FIXME: are these both necessary? let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS) - .map(|(_, res)| res) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) { ty_res @@ -502,7 +564,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; let res = match ty_res { - Res::PrimTy(prim) => Some( + Res::Primitive(prim) => Some( self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str), ), Res::Def( @@ -1068,9 +1130,9 @@ impl LinkCollector<'_, '_> { if matches!( disambiguator, None | Some(Disambiguator::Namespace(Namespace::TypeNS) | Disambiguator::Primitive) - ) && !matches!(res, Res::PrimTy(_)) + ) && !matches!(res, Res::Primitive(_)) { - if let Some((path, prim)) = resolve_primitive(path_str, TypeNS) { + if let Some(prim) = resolve_primitive(path_str, TypeNS) { // `prim@char` if matches!(disambiguator, Some(Disambiguator::Primitive)) { if fragment.is_some() { @@ -1085,7 +1147,7 @@ impl LinkCollector<'_, '_> { return None; } res = prim; - fragment = Some(path.as_str().to_string()); + fragment = Some(prim.name(self.cx.tcx)); } else { // `[char]` when a `char` module is in scope let candidates = vec![res, prim]; @@ -1111,8 +1173,8 @@ impl LinkCollector<'_, '_> { }; report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, &item, dox, &link_range, callback); }; - if let Res::PrimTy(..) = res { - match disambiguator { + match res { + Res::Primitive(_) => match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { Some(ItemLink { link: ori_link, link_text, did: None, fragment }) } @@ -1120,12 +1182,11 @@ impl LinkCollector<'_, '_> { report_mismatch(other, Disambiguator::Primitive); None } - } - } else { - debug!("intra-doc link to {} resolved to {:?}", path_str, res); + }, + Res::Def(kind, id) => { + debug!("intra-doc link to {} resolved to {:?}", path_str, res); - // Disallow e.g. linking to enums with `struct@` - if let Res::Def(kind, _) = res { + // Disallow e.g. linking to enums with `struct@` debug!("saw kind {:?} with disambiguator {:?}", 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))) @@ -1144,27 +1205,26 @@ impl LinkCollector<'_, '_> { return None; } } - } - // item can be non-local e.g. when using #[doc(primitive = "pointer")] - if let Some((src_id, dst_id)) = res - .opt_def_id() - .and_then(|def_id| def_id.as_local()) - .and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id))) - { - use rustc_hir::def_id::LOCAL_CRATE; + // item can be non-local e.g. when using #[doc(primitive = "pointer")] + if let Some((src_id, dst_id)) = id + .as_local() + .and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id))) + { + use rustc_hir::def_id::LOCAL_CRATE; - let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id); - let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id); + let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id); + let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id); - if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src) - && !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst) - { - privacy_error(cx, &item, &path_str, dox, link_range); + if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src) + && !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst) + { + privacy_error(cx, &item, &path_str, dox, link_range); + } } + let id = clean::register_res(cx, rustc_hir::def::Res::Def(kind, id)); + Some(ItemLink { link: ori_link, link_text, did: Some(id), fragment }) } - let id = clean::register_res(cx, res); - Some(ItemLink { link: ori_link, link_text, did: Some(id), fragment }) } } @@ -1296,16 +1356,18 @@ impl LinkCollector<'_, '_> { .and_then(|(res, fragment)| { // Constructors are picked up in the type namespace. match res { - Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) => { + Res::Def(DefKind::Ctor(..), _) => { Err(ResolutionFailure::WrongNamespace(res, TypeNS)) } - _ => match (fragment, extra_fragment.clone()) { - (Some(fragment), Some(_)) => { - // Shouldn't happen but who knows? - Ok((res, Some(fragment))) + _ => { + match (fragment, extra_fragment.clone()) { + (Some(fragment), Some(_)) => { + // Shouldn't happen but who knows? + Ok((res, Some(fragment))) + } + (fragment, None) | (None, fragment) => Ok((res, fragment)), } - (fragment, None) | (None, fragment) => Ok((res, fragment)), - }, + } } }), }; @@ -1445,12 +1507,10 @@ impl Disambiguator { } } - /// WARNING: panics on `Res::Err` fn from_res(res: Res) -> Self { match res { Res::Def(kind, _) => Disambiguator::Kind(kind), - Res::PrimTy(_) => Disambiguator::Primitive, - _ => Disambiguator::Namespace(res.ns().expect("can't call `from_res` on Res::err")), + Res::Primitive(_) => Disambiguator::Primitive, } } @@ -1631,6 +1691,7 @@ fn resolution_failure( link_range: Option>, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { + let tcx = collector.cx.tcx; report_diagnostic( collector.cx, BROKEN_INTRA_DOC_LINKS, @@ -1639,16 +1700,9 @@ fn resolution_failure( dox, &link_range, |diag, sp| { - let item = |res: Res| { - format!( - "the {} `{}`", - res.descr(), - collector.cx.tcx.item_name(res.def_id()).to_string() - ) - }; + let item = |res: Res| format!("the {} `{}`", res.descr(), res.name(tcx),); let assoc_item_not_allowed = |res: Res| { - let def_id = res.def_id(); - let name = collector.cx.tcx.item_name(def_id); + let name = res.name(tcx); format!( "`{}` is {} {}, not a module or type, and cannot have associated items", name, @@ -1714,7 +1768,7 @@ fn resolution_failure( if let Some(module) = last_found_module { let note = if partial_res.is_some() { // Part of the link resolved; e.g. `std::io::nonexistent` - let module_name = collector.cx.tcx.item_name(module); + let module_name = tcx.item_name(module); format!("no item named `{}` in module `{}`", unresolved, module_name) } else { // None of the link resolved; e.g. `Notimported` @@ -1738,14 +1792,10 @@ fn resolution_failure( // 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(ty) => (None, ty.name_str()), - _ => unreachable!("only ADTs and primitives are in scope at module level"), + let name = res.name(tcx); + let kind = match res { + Res::Def(kind, _) => Some(kind), + Res::Primitive(_) => None, }; let path_description = if let Some(kind) = kind { match kind { @@ -2003,50 +2053,45 @@ fn handle_variant( .parent(res.def_id()) .map(|parent| { let parent_def = Res::Def(DefKind::Enum, parent); - let variant = cx.tcx.expect_variant_res(res); + let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap()); (parent_def, Some(format!("variant.{}", variant.ident.name))) }) .ok_or_else(|| ResolutionFailure::NoParentItem.into()) } -// FIXME: At this point, this is basically a copy of the PrimitiveTypeTable -const PRIMITIVES: &[(Symbol, Res)] = &[ - (sym::u8, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U8))), - (sym::u16, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U16))), - (sym::u32, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U32))), - (sym::u64, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U64))), - (sym::u128, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U128))), - (sym::usize, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::Usize))), - (sym::i8, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I8))), - (sym::i16, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I16))), - (sym::i32, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I32))), - (sym::i64, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I64))), - (sym::i128, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I128))), - (sym::isize, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::Isize))), - (sym::f32, Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F32))), - (sym::f64, Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F64))), - (sym::str, Res::PrimTy(hir::PrimTy::Str)), - (sym::bool, Res::PrimTy(hir::PrimTy::Bool)), - (sym::char, Res::PrimTy(hir::PrimTy::Char)), -]; - /// Resolve a primitive type or value. -fn resolve_primitive(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { - is_bool_value(path_str, ns).or_else(|| { - if ns == TypeNS { - // FIXME: this should be replaced by a lookup in PrimitiveTypeTable - let maybe_primitive = Symbol::intern(path_str); - PRIMITIVES.iter().find(|x| x.0 == maybe_primitive).copied() - } else { - None - } - }) +fn resolve_primitive(path_str: &str, ns: Namespace) -> Option { + if ns != TypeNS { + return None; + } + use PrimitiveType::*; + let prim = match path_str { + "u8" => U8, + "u16" => U16, + "u32" => U32, + "u64" => U64, + "u128" => U128, + "usize" => Usize, + "i8" => I8, + "i16" => I16, + "i32" => I32, + "i64" => I64, + "i128" => I128, + "isize" => Isize, + "f32" => F32, + "f64" => F64, + "str" => Str, + "bool" | "true" | "false" => Bool, + "char" => Char, + _ => return None, + }; + Some(Res::Primitive(prim)) } /// Resolve a primitive value. -fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { +fn is_bool_value(path_str: &str, ns: Namespace) -> Option { if ns == TypeNS && (path_str == "true" || path_str == "false") { - Some((sym::bool, Res::PrimTy(hir::PrimTy::Bool))) + Some(Res::Primitive(PrimitiveType::Bool)) } else { None } From 4092891a8f2ca5947d5312e1949ecf701a4f25de Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 19 Dec 2020 01:52:43 -0500 Subject: [PATCH 2/4] Fix intra-doc links for non-path primitives This does *not* currently work for associated items that are auto-implemented by the compiler (e.g. `never::eq`), because they aren't present in the source code. I plan to fix this in a follow-up PR. --- .../passes/collect_intra_doc_links.rs | 54 +++++++-------- .../rustdoc/intra-doc/non-path-primitives.rs | 66 +++++++++++++++++++ 2 files changed, 94 insertions(+), 26 deletions(-) create mode 100644 src/test/rustdoc/intra-doc/non-path-primitives.rs diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1f1fb9754cfed..b8db0d5a4d63c 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -475,9 +475,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }); debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); match result { - // resolver doesn't know about true and false so we'll have to resolve them + // resolver doesn't know about true, false, and types that aren't paths (e.g. `()`) // manually as bool - Err(()) => is_bool_value(path_str, ns), + Err(()) => resolve_primitive(path_str, ns), Ok(res) => Some(res), } } @@ -1020,7 +1020,7 @@ impl LinkCollector<'_, '_> { (link.trim(), None) }; - if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !".contains(ch))) { + if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*()[]&;".contains(ch))) { return None; } @@ -1108,9 +1108,8 @@ impl LinkCollector<'_, '_> { // Sanity check to make sure we don't have any angle brackets after stripping generics. assert!(!path_str.contains(['<', '>'].as_slice())); - // The link is not an intra-doc link if it still contains commas or spaces after - // stripping generics. - if path_str.contains([',', ' '].as_slice()) { + // The link is not an intra-doc link if it still contains spaces after stripping generics. + if path_str.contains(' ') { return None; } @@ -1476,8 +1475,11 @@ impl Disambiguator { ("!", DefKind::Macro(MacroKind::Bang)), ]; for &(suffix, kind) in &suffixes { - if link.ends_with(suffix) { - return Ok((Kind(kind), link.trim_end_matches(suffix))); + if let Some(link) = link.strip_suffix(suffix) { + // Avoid turning `!` or `()` into an empty string + if !link.is_empty() { + return Ok((Kind(kind), link)); + } } } Err(()) @@ -2066,37 +2068,37 @@ fn resolve_primitive(path_str: &str, ns: Namespace) -> Option { } use PrimitiveType::*; let prim = match path_str { - "u8" => U8, - "u16" => U16, - "u32" => U32, - "u64" => U64, - "u128" => U128, - "usize" => Usize, + "isize" => Isize, "i8" => I8, "i16" => I16, "i32" => I32, "i64" => I64, "i128" => I128, - "isize" => Isize, + "usize" => Usize, + "u8" => U8, + "u16" => U16, + "u32" => U32, + "u64" => U64, + "u128" => U128, "f32" => F32, "f64" => F64, - "str" => Str, - "bool" | "true" | "false" => Bool, "char" => Char, + "bool" | "true" | "false" => Bool, + "str" => Str, + "slice" | "&[]" | "[T]" => Slice, + "array" | "[]" | "[T;N]" => Array, + "tuple" | "(,)" => Tuple, + "unit" | "()" => Unit, + "pointer" | "*" | "*const" | "*mut" => RawPointer, + "reference" | "&" | "&mut" => Reference, + "fn" => Fn, + "never" | "!" => Never, _ => return None, }; + debug!("resolved primitives {:?}", prim); Some(Res::Primitive(prim)) } -/// Resolve a primitive value. -fn is_bool_value(path_str: &str, ns: Namespace) -> Option { - if ns == TypeNS && (path_str == "true" || path_str == "false") { - Some(Res::Primitive(PrimitiveType::Bool)) - } else { - None - } -} - fn strip_generics_from_path(path_str: &str) -> Result> { let mut stripped_segments = vec![]; let mut path = path_str.chars().peekable(); diff --git a/src/test/rustdoc/intra-doc/non-path-primitives.rs b/src/test/rustdoc/intra-doc/non-path-primitives.rs new file mode 100644 index 0000000000000..83d081824ff58 --- /dev/null +++ b/src/test/rustdoc/intra-doc/non-path-primitives.rs @@ -0,0 +1,66 @@ +// ignore-tidy-linelength +#![crate_name = "foo"] +#![deny(broken_intra_doc_links)] + +// @has foo/index.html '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'slice::rotate_left' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'X' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'Y' +//! [slice::rotate_left] +//! [X]([T]::rotate_left) +//! [Y](&[]::rotate_left) +// These don't work because markdown syntax doesn't allow it. +// [[T]::rotate_left] +//! [&[]::rotate_left] + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'array::map' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'X' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'Y' +//! [array::map] +//! [X]([]::map) +//! [Y]([T;N]::map) +// These don't work because markdown syntax doesn't allow it. +// [Z]([T; N]::map) +//! [`[T; N]::map`] +//! [[]::map] +// [Z][] +// +// [Z]: [T; N]::map + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' 'pointer::is_null' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' '*const::is_null' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' '*mut::is_null' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' '*::is_null' +//! [pointer::is_null] +//! [*const::is_null] +//! [*mut::is_null] +//! [*::is_null] + +// FIXME: Associated items on some primitives aren't working, because the impls +// are part of the compiler instead of being part of the source code. + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.unit.html"]' 'unit' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.unit.html"]' '()' +//! [unit] +//! [()] + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' 'tuple' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' 'X' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' '(,)' +//! [tuple] +//! [X]((,)) +//! [(,)] + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.reference.html"]' 'reference' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.reference.html"]' '&' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.reference.html"]' '&mut' +//! [reference] +//! [&] +//! [&mut] + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.fn.html"]' 'fn' +//! [fn] + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.never.html"]' 'never' +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.never.html"]' '!' +//! [never] +//! [!] From 8842c1ccf33649f00e72a6f0be284919a847e69b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 19 Dec 2020 09:23:34 -0500 Subject: [PATCH 3/4] Fix new ambiguity in the standard library This caught several bugs where people expected `slice` to link to the primitive, but it linked to the module instead. This also uses `cfg_attr(bootstrap)` since the ambiguity only occurs when compiling with stage 1. --- library/std/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 39b0ca63301e2..b7f9bea7c2af7 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -175,7 +175,7 @@ //! [`str`]: prim@str //! [`mpsc`]: sync::mpsc //! [`std::cmp`]: cmp -//! [`std::slice`]: slice +//! [`std::slice`]: mod@slice //! [`use std::env`]: env/index.html //! [`use`]: ../book/ch07-02-defining-modules-to-control-scope-and-privacy.html //! [crates.io]: https://crates.io @@ -185,7 +185,8 @@ //! [other]: #what-is-in-the-standard-library-documentation //! [primitive types]: ../book/ch03-02-data-types.html //! [rust-discord]: https://discord.gg/rust-lang - +#![cfg_attr(not(bootstrap), doc = "[array]: prim@array")] +#![cfg_attr(not(bootstrap), doc = "[slice]: prim@slice")] #![cfg_attr(not(feature = "restricted-std"), stable(feature = "rust1", since = "1.0.0"))] #![cfg_attr(feature = "restricted-std", unstable(feature = "restricted_std", issue = "none"))] #![doc( From 6ac52f0d9dc2f3f8a423e3b54663f42ac4ebab05 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 24 Dec 2020 18:55:05 -0500 Subject: [PATCH 4/4] Don't process `[]` and `()` in intra-doc links These caused several false positives when documenting rustc, which means there will likely be many more false positives in the rest of the ecosystem. --- .../passes/collect_intra_doc_links.rs | 11 +-- .../intra-doc/non-path-primitives.rs | 34 +++++++++ .../intra-doc/non-path-primitives.stderr | 69 +++++++++++++++++++ .../rustdoc/intra-doc/non-path-primitives.rs | 27 -------- 4 files changed, 109 insertions(+), 32 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-doc/non-path-primitives.rs create mode 100644 src/test/rustdoc-ui/intra-doc/non-path-primitives.stderr diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index b8db0d5a4d63c..1fd0b3a293190 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1020,7 +1020,7 @@ impl LinkCollector<'_, '_> { (link.trim(), None) }; - if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*()[]&;".contains(ch))) { + if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*&;".contains(ch))) { return None; } @@ -2085,10 +2085,11 @@ fn resolve_primitive(path_str: &str, ns: Namespace) -> Option { "char" => Char, "bool" | "true" | "false" => Bool, "str" => Str, - "slice" | "&[]" | "[T]" => Slice, - "array" | "[]" | "[T;N]" => Array, - "tuple" | "(,)" => Tuple, - "unit" | "()" => Unit, + // See #80181 for why these don't have symbols associated. + "slice" => Slice, + "array" => Array, + "tuple" => Tuple, + "unit" => Unit, "pointer" | "*" | "*const" | "*mut" => RawPointer, "reference" | "&" | "&mut" => Reference, "fn" => Fn, diff --git a/src/test/rustdoc-ui/intra-doc/non-path-primitives.rs b/src/test/rustdoc-ui/intra-doc/non-path-primitives.rs new file mode 100644 index 0000000000000..114502b0ddf4e --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/non-path-primitives.rs @@ -0,0 +1,34 @@ +#![deny(broken_intra_doc_links)] +// These are links that could reasonably expected to work, but don't. + +// `[]` isn't supported because it had too many false positives. +//! [X]([T]::not_here) +//! [Y](&[]::not_here) +//! [X]([]::not_here) +//! [Y]([T;N]::not_here) + +// These don't work because markdown syntax doesn't allow it. +//! [[T]::rotate_left] //~ ERROR unresolved link to `T` +//! [&[]::not_here] +//![Z]([T; N]::map) //~ ERROR unresolved link to `Z` +//! [`[T; N]::map`] +//! [[]::map] +//! [Z][] //~ ERROR unresolved link to `Z` +//! +//! [Z]: [T; N]::map //~ ERROR unresolved link to `Z` + +// `()` isn't supported because it had too many false positives. +//! [()::not_here] +//! [X]((,)::not_here) +//! [(,)::not_here] + +// FIXME: Associated items on some primitives aren't working, because the impls +// are part of the compiler instead of being part of the source code. +//! [unit::eq] //~ ERROR unresolved +//! [tuple::eq] //~ ERROR unresolved +//! [fn::eq] //~ ERROR unresolved +//! [never::eq] //~ ERROR unresolved + +// FIXME(#78800): This breaks because it's a blanket impl +// (I think? Might break for other reasons too.) +//! [reference::deref] //~ ERROR unresolved diff --git a/src/test/rustdoc-ui/intra-doc/non-path-primitives.stderr b/src/test/rustdoc-ui/intra-doc/non-path-primitives.stderr new file mode 100644 index 0000000000000..ea831e648f638 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/non-path-primitives.stderr @@ -0,0 +1,69 @@ +error: unresolved link to `T` + --> $DIR/non-path-primitives.rs:11:7 + | +LL | //! [[T]::rotate_left] + | ^ no item named `T` in scope + | +note: the lint level is defined here + --> $DIR/non-path-primitives.rs:1:9 + | +LL | #![deny(broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `Z` + --> $DIR/non-path-primitives.rs:13:5 + | +LL | //![Z]([T; N]::map) + | ^ no item named `Z` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `Z` + --> $DIR/non-path-primitives.rs:16:6 + | +LL | //! [Z][] + | ^ no item named `Z` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `Z` + --> $DIR/non-path-primitives.rs:18:6 + | +LL | //! [Z]: [T; N]::map + | ^ no item named `Z` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `unit::eq` + --> $DIR/non-path-primitives.rs:27:6 + | +LL | //! [unit::eq] + | ^^^^^^^^ the builtin type `unit` has no associated item named `eq` + +error: unresolved link to `tuple::eq` + --> $DIR/non-path-primitives.rs:28:6 + | +LL | //! [tuple::eq] + | ^^^^^^^^^ the builtin type `tuple` has no associated item named `eq` + +error: unresolved link to `fn::eq` + --> $DIR/non-path-primitives.rs:29:6 + | +LL | //! [fn::eq] + | ^^^^^^ the builtin type `fn` has no associated item named `eq` + +error: unresolved link to `never::eq` + --> $DIR/non-path-primitives.rs:30:6 + | +LL | //! [never::eq] + | ^^^^^^^^^ the builtin type `never` has no associated item named `eq` + +error: unresolved link to `reference::deref` + --> $DIR/non-path-primitives.rs:34:6 + | +LL | //! [reference::deref] + | ^^^^^^^^^^^^^^^^ the builtin type `reference` has no associated item named `deref` + +error: aborting due to 9 previous errors + diff --git a/src/test/rustdoc/intra-doc/non-path-primitives.rs b/src/test/rustdoc/intra-doc/non-path-primitives.rs index 83d081824ff58..ad4f6ddd9de69 100644 --- a/src/test/rustdoc/intra-doc/non-path-primitives.rs +++ b/src/test/rustdoc/intra-doc/non-path-primitives.rs @@ -3,28 +3,10 @@ #![deny(broken_intra_doc_links)] // @has foo/index.html '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'slice::rotate_left' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'X' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.rotate_left"]' 'Y' //! [slice::rotate_left] -//! [X]([T]::rotate_left) -//! [Y](&[]::rotate_left) -// These don't work because markdown syntax doesn't allow it. -// [[T]::rotate_left] -//! [&[]::rotate_left] // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'array::map' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'X' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.array.html#method.map"]' 'Y' //! [array::map] -//! [X]([]::map) -//! [Y]([T;N]::map) -// These don't work because markdown syntax doesn't allow it. -// [Z]([T; N]::map) -//! [`[T; N]::map`] -//! [[]::map] -// [Z][] -// -// [Z]: [T; N]::map // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' 'pointer::is_null' // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.is_null"]' '*const::is_null' @@ -35,20 +17,11 @@ //! [*mut::is_null] //! [*::is_null] -// FIXME: Associated items on some primitives aren't working, because the impls -// are part of the compiler instead of being part of the source code. - // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.unit.html"]' 'unit' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.unit.html"]' '()' //! [unit] -//! [()] // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' 'tuple' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' 'X' -// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.tuple.html"]' '(,)' //! [tuple] -//! [X]((,)) -//! [(,)] // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.reference.html"]' 'reference' // @has - '//a[@href="https://doc.rust-lang.org/nightly/std/primitive.reference.html"]' '&'