From 3797f29aadb51ed038e8b9eaf1b2098cfa26d547 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 11:41:18 -0400 Subject: [PATCH] [WIP] give better errors for broken intra doc links --- compiler/rustc_hir/src/def.rs | 9 + compiler/rustc_hir/src/lib.rs | 1 + .../passes/collect_intra_doc_links.rs | 359 ++++++++++++++---- .../deny-intra-link-resolution-failure.stderr | 3 +- .../rustdoc-ui/intra-doc-alias-ice.stderr | 5 +- src/test/rustdoc-ui/intra-link-errors.rs | 59 +++ src/test/rustdoc-ui/intra-link-errors.stderr | 68 ++++ .../intra-link-span-ice-55723.stderr | 3 +- .../intra-links-warning-crlf.stderr | 12 +- .../rustdoc-ui/intra-links-warning.stderr | 49 ++- 10 files changed, 475 insertions(+), 93 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-link-errors.rs create mode 100644 src/test/rustdoc-ui/intra-link-errors.stderr diff --git a/compiler/rustc_hir/src/def.rs b/compiler/rustc_hir/src/def.rs index 0d61dc037c6..b019e518d0c 100644 --- a/compiler/rustc_hir/src/def.rs +++ b/compiler/rustc_hir/src/def.rs @@ -6,6 +6,7 @@ use rustc_ast::NodeId; use rustc_macros::HashStable_Generic; use rustc_span::hygiene::MacroKind; +use std::array::IntoIter; use std::fmt::Debug; /// Encodes if a `DefKind::Ctor` is the constructor of an enum variant or a struct. @@ -291,6 +292,14 @@ impl PerNS { pub fn map U>(self, mut f: F) -> PerNS { PerNS { value_ns: f(self.value_ns), type_ns: f(self.type_ns), macro_ns: f(self.macro_ns) } } + + pub fn into_iter(self) -> IntoIter { + IntoIter::new([self.value_ns, self.type_ns, self.macro_ns]) + } + + pub fn iter(&self) -> IntoIter<&T, 3> { + IntoIter::new([&self.value_ns, &self.type_ns, &self.macro_ns]) + } } impl ::std::ops::Index for PerNS { diff --git a/compiler/rustc_hir/src/lib.rs b/compiler/rustc_hir/src/lib.rs index c69a9b063ae..9d931b3a9e1 100644 --- a/compiler/rustc_hir/src/lib.rs +++ b/compiler/rustc_hir/src/lib.rs @@ -2,6 +2,7 @@ //! //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/hir.html +#![feature(array_value_iter)] #![feature(crate_visibility_modifier)] #![feature(const_fn)] // For the unsizing cast on `&[]` #![feature(const_panic)] diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 5d10e2e149b..f6f01028ee2 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -17,7 +17,7 @@ use rustc_span::hygiene::MacroKind; use rustc_span::symbol::Ident; use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; -use smallvec::SmallVec; +use smallvec::{smallvec, SmallVec}; use std::cell::Cell; use std::ops::Range; @@ -47,10 +47,53 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { } enum ErrorKind { - ResolutionFailure, + Resolve(ResolutionFailure), AnchorFailure(AnchorFailure), } +#[derive(Debug)] +enum ResolutionFailure { + /// This resolved, but with the wrong namespace. + /// `Namespace` is the expected namespace (as opposed to the actual). + WrongNamespace(Res, Namespace), + /// `String` is the base name of the path (not necessarily the whole link) + NotInScope(String), + /// 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: String, assoc_item: String }, + /// `[S::not_found]` + /// the `String` is the associated item that wasn't found + NoAssocItem(Res, String), + /// should not ever happen + NoParentItem, + /// the root of this path resolved, but it was not an enum. + NotAnEnum(Res), + /// 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, String), + /// used to communicate that this should be ignored, but shouldn't be reported to the user + Dummy, +} + +impl ResolutionFailure { + fn res(&self) -> Option { + use ResolutionFailure::*; + match self { + NoPrimitiveAssocItem { res, .. } + | NoAssocItem(res, _) + | NoPrimitiveImpl(res, _) + | NotAnEnum(res) + | NotAVariant(res, _) + | WrongNamespace(res, _) => Some(*res), + NotInScope(_) | NoParentItem | Dummy => None, + } + } +} + enum AnchorFailure { MultipleAnchors, Primitive, @@ -85,10 +128,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let cx = self.cx; let mut split = path_str.rsplitn(3, "::"); - let variant_field_name = - split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::ResolutionFailure)?; + let variant_field_name = split + .next() + .map(|f| Symbol::intern(f)) + .expect("fold_item should ensure link is non-empty"); let variant_name = - split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::ResolutionFailure)?; + // we're not sure this is a variant at all, so use the full string + split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.to_string())))?; + // TODO: this looks very wrong, why are we requiring 3 fields? let path = split .next() .map(|f| { @@ -99,14 +146,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - .ok_or(ErrorKind::ResolutionFailure)?; + // TODO: is this right? + .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(variant_name.to_string())))?; let (_, ty_res) = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) }) - .map_err(|_| ErrorKind::ResolutionFailure)?; + .map_err(|_| ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string())))?; if let Res::Err = ty_res { - return Err(ErrorKind::ResolutionFailure); + return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string()))); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { @@ -118,7 +166,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .flat_map(|imp| cx.tcx.associated_items(*imp).in_definition_order()) .any(|item| item.ident.name == variant_name) { - return Err(ErrorKind::ResolutionFailure); + // This is just to let `fold_item` know that this shouldn't be considered; + // it's a bug for the error to make it to the user + return Err(ErrorKind::Resolve(ResolutionFailure::Dummy)); } match cx.tcx.type_of(did).kind() { ty::Adt(def, _) if def.is_enum() => { @@ -131,18 +181,25 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { )), )) } else { - Err(ErrorKind::ResolutionFailure) + Err(ErrorKind::Resolve(ResolutionFailure::NotAVariant( + ty_res, + variant_field_name.to_string(), + ))) } } - _ => Err(ErrorKind::ResolutionFailure), + _ => unreachable!(), } } - _ => Err(ErrorKind::ResolutionFailure), + _ => Err(ErrorKind::Resolve(ResolutionFailure::NotAnEnum(ty_res))), } } /// Resolves a string as a macro. - fn macro_resolve(&self, path_str: &str, parent_id: Option) -> Option { + fn macro_resolve( + &self, + path_str: &str, + parent_id: Option, + ) -> Result { let cx = self.cx; let path = ast::Path::from_ident(Ident::from_str(path_str)); cx.enter_resolver(|resolver| { @@ -154,11 +211,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { false, ) { if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind { - return Some(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(res.map_id(|_| panic!("unexpected id"))); + return Ok(res.map_id(|_| panic!("unexpected id"))); } if let Some(module_id) = parent_id { debug!("resolving {} as a macro in the module {:?}", path_str, module_id); @@ -168,13 +225,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(res); + return Ok(res); } } } else { debug!("attempting to resolve item without parent module: {}", path_str); + return Err(ResolutionFailure::NoParentItem); } - None + return Err(ResolutionFailure::NotInScope(path_str.to_string())); }) } /// Resolves a string as a path within a particular namespace. Also returns an optional @@ -196,8 +254,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }); debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); let result = match result { - Ok((_, Res::Err)) => Err(ErrorKind::ResolutionFailure), - _ => result.map_err(|_| ErrorKind::ResolutionFailure), + Ok((_, Res::Err)) => Err(()), + _ => result.map_err(|_| ()), }; if let Ok((_, res)) = result { @@ -226,7 +284,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if value != (ns == ValueNS) { - return Err(ErrorKind::ResolutionFailure); + return Err(ErrorKind::Resolve(ResolutionFailure::WrongNamespace(res, ns))); } } else if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { @@ -237,9 +295,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Try looking for methods and associated items. let mut split = path_str.rsplitn(2, "::"); - let item_name = - split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::ResolutionFailure)?; - let path = split + // this can be an `unwrap()` because we ensure the link is never empty + let item_name = Symbol::intern(split.next().unwrap()); + let path_root = split .next() .map(|f| { if f == "self" || f == "Self" { @@ -249,10 +307,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - .ok_or(ErrorKind::ResolutionFailure)?; + // If there's no `::`, it's not an associated item. + // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved. + .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string())))?; - if let Some((path, prim)) = is_primitive(&path, TypeNS) { - for &impl_ in primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)? { + if let Some((path, prim)) = is_primitive(&path_root, ns) { + let impls = primitive_impl(cx, &path).ok_or_else(|| { + ErrorKind::Resolve(ResolutionFailure::NoPrimitiveImpl(prim, path_root)) + })?; + for &impl_ in impls { let link = cx .tcx .associated_items(impl_) @@ -272,19 +335,25 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { return Ok(link); } } - return Err(ErrorKind::ResolutionFailure); + return Err(ErrorKind::Resolve(ResolutionFailure::NoPrimitiveAssocItem { + res: prim, + prim_name: path.to_string(), + assoc_item: item_name.to_string(), + })); } let (_, ty_res) = cx .enter_resolver(|resolver| { - resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) + resolver.resolve_str_path_error(DUMMY_SP, &path_root, TypeNS, module_id) }) - .map_err(|_| ErrorKind::ResolutionFailure)?; + .map_err(|_| { + ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.clone())) + })?; if let Res::Err = ty_res { return if ns == Namespace::ValueNS { self.variant_field(path_str, current_item, module_id) } else { - Err(ErrorKind::ResolutionFailure) + Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root))) }; } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); @@ -380,7 +449,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } else { // We already know this isn't in ValueNS, so no need to check variant_field - return Err(ErrorKind::ResolutionFailure); + return Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem( + ty_res, + item_name.to_string(), + ))); } } Res::Def(DefKind::Trait, did) => cx @@ -419,12 +491,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if ns == Namespace::ValueNS { self.variant_field(path_str, current_item, module_id) } else { - Err(ErrorKind::ResolutionFailure) + Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem( + ty_res, + item_name.to_string(), + ))) } }) } else { debug!("attempting to resolve item without parent module: {}", path_str); - Err(ErrorKind::ResolutionFailure) + // TODO: maybe this should just be an ICE? + Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)) } } } @@ -562,10 +638,10 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx /// Check for resolve collisions between a trait and its derive /// /// These are common and we should just resolve to the trait in that case -fn is_derive_trait_collision(ns: &PerNS>) -> bool { +fn is_derive_trait_collision(ns: &PerNS>) -> bool { if let PerNS { - type_ns: Some((Res::Def(DefKind::Trait, _), _)), - macro_ns: Some((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)), + type_ns: Ok((Res::Def(DefKind::Trait, _), _)), + macro_ns: Ok((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)), .. } = *ns { @@ -764,8 +840,15 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { match self.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { Ok(res) => res, - Err(ErrorKind::ResolutionFailure) => { - resolution_failure(cx, &item, path_str, &dox, link_range); + Err(ErrorKind::Resolve(kind)) => { + resolution_failure( + cx, + &item, + path_str, + &dox, + link_range, + smallvec![kind], + ); // This could just be a normal link or a broken link // we could potentially check if something is // "intra-doc-link-like" and warn in that case. @@ -792,13 +875,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { ) { Ok(res) => { debug!("got res in TypeNS: {:?}", res); - Some(res) + Ok(res) } Err(ErrorKind::AnchorFailure(msg)) => { anchor_failure(cx, &item, &ori_link, &dox, link_range, msg); continue; } - Err(ErrorKind::ResolutionFailure) => None, + Err(ErrorKind::Resolve(kind)) => Err(kind), }, value_ns: match self.resolve( path_str, @@ -807,48 +890,62 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { base_node, &extra_fragment, ) { - Ok(res) => Some(res), + Ok(res) => Ok(res), Err(ErrorKind::AnchorFailure(msg)) => { anchor_failure(cx, &item, &ori_link, &dox, link_range, msg); continue; } - Err(ErrorKind::ResolutionFailure) => None, + Err(ErrorKind::Resolve(kind)) => Err(kind), } .and_then(|(res, fragment)| { // Constructors are picked up in the type namespace. match res { - Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) => None, + Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) => { + Err(ResolutionFailure::WrongNamespace(res, TypeNS)) + } _ => match (fragment, extra_fragment) { (Some(fragment), Some(_)) => { // Shouldn't happen but who knows? - Some((res, Some(fragment))) - } - (fragment, None) | (None, fragment) => { - Some((res, fragment)) + Ok((res, Some(fragment))) } + (fragment, None) | (None, fragment) => Ok((res, fragment)), }, } }), }; - if candidates.is_empty() { - resolution_failure(cx, &item, path_str, &dox, link_range); + let mut candidates_iter = + candidates.iter().filter_map(|res| res.as_ref().ok()); + let len = candidates_iter.clone().count(); + + if len == 0 { + drop(candidates_iter); + resolution_failure( + cx, + &item, + path_str, + &dox, + link_range, + candidates.into_iter().filter_map(|res| res.err()).collect(), + ); // this could just be a normal link continue; } - let len = candidates.clone().present_items().count(); - if len == 1 { - candidates.present_items().next().unwrap() + candidates_iter.next().unwrap().clone() } else if len == 2 && is_derive_trait_collision(&candidates) { + drop(candidates_iter); candidates.type_ns.unwrap() } else { + drop(candidates_iter); if is_derive_trait_collision(&candidates) { - candidates.macro_ns = None; + candidates.macro_ns = + Err(ResolutionFailure::NotInScope(path_str.to_string())); } + // If we're reporting an ambiguity, don't mention the namespaces that failed let candidates = - candidates.map(|candidate| candidate.map(|(res, _)| res)); + candidates.map(|candidate| candidate.ok().map(|(res, _)| res)); ambiguity_error( cx, &item, @@ -861,11 +958,44 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } Some(MacroNS) => { - if let Some(res) = self.macro_resolve(path_str, base_node) { - (res, extra_fragment) - } else { - resolution_failure(cx, &item, path_str, &dox, link_range); - continue; + match self.macro_resolve(path_str, base_node) { + Ok(res) => (res, extra_fragment), + Err(mut kind) => { + // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible. + for &ns in &[TypeNS, ValueNS] { + match self.resolve( + path_str, + ns, + ¤t_item, + base_node, + &extra_fragment, + ) { + Ok(res) => { + kind = ResolutionFailure::WrongNamespace(res.0, MacroNS) + } + // This will show up in the other namespace, no need to handle it here + Err(ErrorKind::Resolve( + ResolutionFailure::WrongNamespace(..), + )) => {} + Err(ErrorKind::AnchorFailure(_)) => {} + Err(ErrorKind::Resolve(inner_kind)) => { + if let Some(res) = inner_kind.res() { + kind = + ResolutionFailure::WrongNamespace(res, MacroNS); + } + } + } + } + resolution_failure( + cx, + &item, + path_str, + &dox, + link_range, + smallvec![kind], + ); + continue; + } } } } @@ -907,7 +1037,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { let report_mismatch = |specified: Disambiguator, resolved: Disambiguator| { // The resolved item did not match the disambiguator; give a better error than 'not found' let msg = format!("incompatible link kind for `{}`", path_str); - report_diagnostic(cx, &msg, &item, &dox, link_range.clone(), |diag, sp| { + report_diagnostic(cx, &msg, &item, &dox, &link_range, |diag, sp| { let note = format!( "this link resolved to {} {}, which is not {} {}", resolved.article(), @@ -1161,7 +1291,7 @@ fn report_diagnostic( msg: &str, item: &Item, dox: &str, - link_range: Option>, + link_range: &Option>, decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), ) { let hir_id = match cx.as_local_hir_id(item.def_id) { @@ -1218,19 +1348,107 @@ fn resolution_failure( path_str: &str, dox: &str, link_range: Option>, + kinds: SmallVec<[ResolutionFailure; 3]>, ) { report_diagnostic( cx, &format!("unresolved link to `{}`", path_str), item, dox, - link_range, + &link_range, |diag, sp| { - if let Some(sp) = sp { - diag.span_label(sp, "unresolved link"); - } + let in_scope = kinds.iter().any(|kind| kind.res().is_some()); + let mut reported_not_in_scope = false; + let item = |res: Res| { + if let Some(id) = res.opt_def_id() { + (format!("the {} `{}`", res.descr(), cx.tcx.item_name(id).to_string()), ",") + } else { + (format!("{} {}", res.article(), res.descr()), "") + } + }; + for failure in kinds { + match failure { + // already handled above + ResolutionFailure::NotInScope(base) => { + if in_scope || reported_not_in_scope { + continue; + } + reported_not_in_scope = true; + diag.note(&format!("no item named `{}` is in scope", base)); + diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); + } + ResolutionFailure::Dummy => continue, + ResolutionFailure::WrongNamespace(res, expected_ns) => { + let (item, comma) = item(res); + let note = format!( + "this link resolves to {}{} which is not in the {} namespace", + item, + comma, + expected_ns.descr() + ); + diag.note(¬e); - diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); + if let Res::Def(kind, _) = res { + let disambiguator = Disambiguator::Kind(kind); + suggest_disambiguator( + disambiguator, + diag, + path_str, + dox, + sp, + &link_range, + ) + } + } + ResolutionFailure::NoParentItem => { + panic!("all intra doc links should have a parent item") + } + ResolutionFailure::NoPrimitiveImpl(res, _) => { + let (item, comma) = item(res); + let note = format!( + "this link partially resolves to {}{} which does not have any associated items", + item, comma, + ); + diag.note(¬e); + } + ResolutionFailure::NoPrimitiveAssocItem { prim_name, assoc_item, .. } => { + let note = format!( + "the builtin type `{}` does not have an associated item named `{}`", + prim_name, assoc_item + ); + diag.note(¬e); + } + ResolutionFailure::NoAssocItem(res, assoc_item) => { + let (item, _) = item(res); + diag.note(&format!("this link partially resolves to {}", item)); + // FIXME: when are items neither a primitive nor a Def? + if let Res::Def(_, def_id) = res { + let name = cx.tcx.item_name(def_id); + let note = format!( + "`{}` has no field, variant, or associated item named `{}`", + name, assoc_item + ); + diag.note(¬e); + } + } + // TODO: is there ever a case where this happens? + ResolutionFailure::NotAnEnum(res) => { + let (item, comma) = item(res); + let note = + format!("this link resolves to {}{} which is not an enum", item, comma); + diag.note(¬e); + diag.note("if this were an enum, it might have a variant which resolved"); + } + ResolutionFailure::NotAVariant(res, variant) => { + let note = format!( + "this link partially resolves to {}, but there is no variant named {}", + item(res).0, + variant + ); + diag.note(¬e); + } + } + } }, ); } @@ -1269,7 +1487,7 @@ fn anchor_failure( } }; - report_diagnostic(cx, &msg, item, dox, link_range, |diag, sp| { + report_diagnostic(cx, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "contains invalid anchor"); } @@ -1308,7 +1526,7 @@ fn ambiguity_error( } } - report_diagnostic(cx, &msg, item, dox, link_range.clone(), |diag, sp| { + report_diagnostic(cx, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "ambiguous link"); } else { @@ -1356,7 +1574,7 @@ fn privacy_error( let msg = format!("public documentation for `{}` links to private item `{}`", item_name, path_str); - report_diagnostic(cx, &msg, item, dox, link_range, |diag, sp| { + report_diagnostic(cx, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "this item is private"); } @@ -1384,7 +1602,8 @@ fn handle_variant( let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { parent } else { - return Err(ErrorKind::ResolutionFailure); + // TODO: this should just be an unwrap, there should never be `Variant`s without a parent + return Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)); }; let parent_def = Res::Def(DefKind::Enum, parent); let variant = cx.tcx.expect_variant_res(res); 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 7530e3ad0f5..4ae53e83613 100644 --- a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr +++ b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr @@ -2,13 +2,14 @@ error: unresolved link to `v2` --> $DIR/deny-intra-link-resolution-failure.rs:3:6 | LL | /// [v2] - | ^^ unresolved link + | ^^ | note: the lint level is defined here --> $DIR/deny-intra-link-resolution-failure.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ + = note: no item named `v2` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr index f1c07e31cd7..f5eb3a15abc 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -2,14 +2,15 @@ error: unresolved link to `TypeAlias::hoge` --> $DIR/intra-doc-alias-ice.rs:5:30 | LL | /// [broken cross-reference](TypeAlias::hoge) - | ^^^^^^^^^^^^^^^ unresolved link + | ^^^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/intra-doc-alias-ice.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + = note: this link partially resolves to the type alias `TypeAlias`, + = note: `TypeAlias` has no field, variant, or associated item named `hoge` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs new file mode 100644 index 00000000000..7a53a6f0793 --- /dev/null +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -0,0 +1,59 @@ +#![deny(broken_intra_doc_links)] +//~^ NOTE lint level is defined + +//! [std::io::oops] +//! [std::io::oops::not::here] + +// FIXME: this should say that it was skipped (maybe an allowed by default lint?) +/// [] + +// FIXME: this could say which path was the first to not be found (in this case, `path`) +/// [path::to::nonexistent::module] +//~^ ERROR unresolved link +//~| NOTE no item named `path::to::nonexistent` is in scope +//~| HELP to escape + +// TODO: why does this say `f` and not `f::A`?? +/// [f::A] +//~^ ERROR unresolved link +//~| NOTE no item named `f` is in scope +//~| HELP to escape + +/// [S::A] +//~^ ERROR unresolved link +//~| NOTE this link partially resolves +//~| NOTE `S` has no field + +/// [S::fmt] +//~^ ERROR unresolved link +//~| NOTE this link partially resolves +//~| NOTE `S` has no field + +/// [E::D] +//~^ ERROR unresolved link +//~| NOTE this link partially resolves +//~| NOTE `E` has no field + +/// [u8::not_found] +//~^ ERROR unresolved link +//~| NOTE the builtin type `u8` does not have an associated item named `not_found` + +/// [S!] +//~^ ERROR unresolved link +//~| HELP to link to the unit struct, use its disambiguator +//~| NOTE this link resolves to the unit struct `S` +pub fn f() {} +#[derive(Debug)] +pub struct S; + +pub enum E { A, B, C } + +/// [type@S::h] +impl S { + pub fn h() {} +} + +/// [type@T::g] +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 new file mode 100644 index 00000000000..249b27cd878 --- /dev/null +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -0,0 +1,68 @@ +error: unresolved link to `path::to::nonexistent::module` + --> $DIR/intra-link-errors.rs:8:6 + | +LL | /// [path::to::nonexistent::module] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/intra-link-errors.rs:1:9 + | +LL | #![deny(broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = note: no item named `path::to::nonexistent` is in scope + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `f::A` + --> $DIR/intra-link-errors.rs:14:6 + | +LL | /// [f::A] + | ^^^^ + | + = note: no item named `f` is in scope + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `S::A` + --> $DIR/intra-link-errors.rs:19:6 + | +LL | /// [S::A] + | ^^^^ + | + = note: this link partially resolves to the struct `S`, + = note: `S` has no field, variant, or associated item named `A` + +error: unresolved link to `S::fmt` + --> $DIR/intra-link-errors.rs:24:6 + | +LL | /// [S::fmt] + | ^^^^^^ + | + = note: this link partially resolves to the struct `S`, + = note: `S` has no field, variant, or associated item named `fmt` + +error: unresolved link to `E::D` + --> $DIR/intra-link-errors.rs:29:6 + | +LL | /// [E::D] + | ^^^^ + | + = note: this link partially resolves to the enum `E`, + = note: `E` has no field, variant, or associated item named `D` + +error: unresolved link to `u8::not_found` + --> $DIR/intra-link-errors.rs:34:6 + | +LL | /// [u8::not_found] + | ^^^^^^^^^^^^^ + | + = note: the builtin type `u8` does not have an associated item named `not_found` + +error: unresolved link to `S` + --> $DIR/intra-link-errors.rs:38:6 + | +LL | /// [S!] + | ^^ help: to link to the unit struct, use its disambiguator: `value@S` + | + = note: this link resolves to the unit struct `S`, which is not in the value namespace + +error: aborting due to 7 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 6b0ff8f1162..47b6a08baf3 100644 --- a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr +++ b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr @@ -2,13 +2,14 @@ error: unresolved link to `i` --> $DIR/intra-link-span-ice-55723.rs:9:10 | LL | /// （arr[i]） - | ^ unresolved link + | ^ | note: the lint level is defined here --> $DIR/intra-link-span-ice-55723.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ + = note: no item named `i` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr index 1e3a26fadfa..1da27b78618 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr @@ -2,33 +2,37 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:7:6 | LL | /// [error] - | ^^^^^ unresolved link + | ^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error1` --> $DIR/intra-links-warning-crlf.rs:12:11 | LL | /// docs [error1] - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `error1` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error2` --> $DIR/intra-links-warning-crlf.rs:15:11 | LL | /// docs [error2] - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `error2` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:23:20 | LL | * It also has an [error]. - | ^^^^^ unresolved link + | ^^^^^ | + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: 4 warnings emitted diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 53f2476295e..f728d3919e6 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -2,73 +2,82 @@ warning: unresolved link to `Foo::baz` --> $DIR/intra-links-warning.rs:3:23 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ unresolved link + | ^^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + = note: this link partially resolves to the struct `Foo`, + = note: `Foo` has no field, variant, or associated item named `baz` warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ unresolved link + | ^^^^^^^^ | + = note: no item named `Bar` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:6:13 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ unresolved link + | ^^^^^^^^^^ | + = note: no item named `Uniooon` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:6:30 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `Qux` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:10:14 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ unresolved link + | ^^^^^^^^^^ | + = note: no item named `Uniooon` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:10:31 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `Qux` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `Qux:Y` --> $DIR/intra-links-warning.rs:14:13 | LL | /// [Qux:Y] - | ^^^^^ unresolved link + | ^^^^^ | + = note: no item named `Qux:Y` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:58:30 | LL | * time to introduce a link [error]*/ - | ^^^^^ unresolved link + | ^^^^^ | + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:64:30 | LL | * time to introduce a link [error] - | ^^^^^ unresolved link + | ^^^^^ | + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -81,6 +90,7 @@ LL | #[doc = "single line [error]"] single line [error] ^^^^^ + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -93,6 +103,7 @@ LL | #[doc = "single line with \"escaping\" [error]"] single line with "escaping" [error] ^^^^^ + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -107,46 +118,52 @@ LL | | /// [error] [error] ^^^^^ + = note: no item named `error` is in scope = 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] - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `error1` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error2` --> $DIR/intra-links-warning.rs:82:11 | LL | /// docs [error2] - | ^^^^^^ unresolved link + | ^^^^^^ | + = note: no item named `error2` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarA` --> $DIR/intra-links-warning.rs:21:10 | LL | /// bar [BarA] bar - | ^^^^ unresolved link + | ^^^^ | + = note: no item named `BarA` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarB` --> $DIR/intra-links-warning.rs:27:9 | LL | * bar [BarB] bar - | ^^^^ unresolved link + | ^^^^ | + = note: no item named `BarB` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarC` --> $DIR/intra-links-warning.rs:34:6 | LL | bar [BarC] bar - | ^^^^ unresolved link + | ^^^^ | + = note: no item named `BarC` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarD` @@ -159,6 +176,7 @@ LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] bar [BarD] bar ^^^^ + = note: no item named `BarD` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarF` @@ -174,6 +192,7 @@ LL | f!("Foo\nbar [BarF] bar\nbaz"); bar [BarF] bar ^^^^ + = note: no item named `BarF` is in scope = 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)