From 3797f29aadb51ed038e8b9eaf1b2098cfa26d547 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 11:41:18 -0400 Subject: [PATCH 01/25] [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 | 361 ++++++++++++++---- .../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, 476 insertions(+), 94 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 0d61dc037c65a..b019e518d0c54 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 c69a9b063aeca..9d931b3a9e1e5 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 5d10e2e149b32..f6f01028ee2e2 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 let Some((path, prim)) = is_primitive(&path, TypeNS) { - for &impl_ in primitive_impl(cx, &path).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_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); + + 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); + } + } } - - diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); }, ); } @@ -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 7530e3ad0f551..4ae53e83613e0 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 f1c07e31cd753..f5eb3a15abcc7 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 0000000000000..7a53a6f079334 --- /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 0000000000000..249b27cd87894 --- /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 6b0ff8f116295..47b6a08baf37d 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 1e3a26fadfa9f..1da27b786180f 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 53f2476295ebb..f728d3919e670 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) From f45e7b586259b795babf49231cd7de4a4534a2e7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 21:15:32 -0400 Subject: [PATCH 02/25] Update .stderr --- .../rustdoc-ui/intra-doc-alias-ice.stderr | 2 +- src/test/rustdoc-ui/intra-link-errors.stderr | 68 +++++++++++++++---- .../rustdoc-ui/intra-links-warning.stderr | 2 +- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr index f5eb3a15abcc7..cc1343eb7b95e 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: this link partially resolves to the type alias `TypeAlias`, + = 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.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 249b27cd87894..baf4ab7a9bc46 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -1,19 +1,39 @@ -error: unresolved link to `path::to::nonexistent::module` - --> $DIR/intra-link-errors.rs:8:6 +error: unresolved link to `std::io::oops` + --> $DIR/intra-link-errors.rs:4:6 | -LL | /// [path::to::nonexistent::module] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | //! [std::io::oops] + | ^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/intra-link-errors.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ + = note: this link resolves to the crate `std`, which is not an enum + = note: if this were an enum, it might have a variant which resolved + = note: this link partially resolves to the module `io` + = note: `io` has no field, variant, or associated item named `oops` + +error: unresolved link to `std::io::oops::not::here` + --> $DIR/intra-link-errors.rs:5:6 + | +LL | //! [std::io::oops::not::here] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: no item named `std::io::oops::not` is in scope + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `path::to::nonexistent::module` + --> $DIR/intra-link-errors.rs:11:6 + | +LL | /// [path::to::nonexistent::module] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | = 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 + --> $DIR/intra-link-errors.rs:17:6 | LL | /// [f::A] | ^^^^ @@ -22,34 +42,34 @@ LL | /// [f::A] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::A` - --> $DIR/intra-link-errors.rs:19:6 + --> $DIR/intra-link-errors.rs:22:6 | LL | /// [S::A] | ^^^^ | - = note: this link partially resolves to the struct `S`, + = 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 + --> $DIR/intra-link-errors.rs:27:6 | LL | /// [S::fmt] | ^^^^^^ | - = note: this link partially resolves to the struct `S`, + = 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 + --> $DIR/intra-link-errors.rs:32:6 | LL | /// [E::D] | ^^^^ | - = note: this link partially resolves to the enum `E`, + = 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 + --> $DIR/intra-link-errors.rs:37:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -57,12 +77,30 @@ 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 + --> $DIR/intra-link-errors.rs:41: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 + = note: this link resolves to the unit struct `S`, which is not in the macro namespace + +error: unresolved link to `T::g` + --> $DIR/intra-link-errors.rs:56:6 + | +LL | /// [type@T::g] + | ^^^^^^^^^ + | + = note: this link partially resolves to the trait `T` + = note: `T` has no field, variant, or associated item named `g` + +error: unresolved link to `S::h` + --> $DIR/intra-link-errors.rs:51:6 + | +LL | /// [type@S::h] + | ^^^^^^^^^ + | + = note: this link partially resolves to the struct `S` + = note: `S` has no field, variant, or associated item named `h` -error: aborting due to 7 previous errors +error: aborting due to 11 previous errors diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index f728d3919e670..3737cad1cb187 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -5,7 +5,7 @@ LL | //! Test with [Foo::baz], [Bar::foo], ... | ^^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default - = note: this link partially resolves to the struct `Foo`, + = 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` From 4ace4e75201d5bfc187bd40cf918cd2df6009419 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 21:29:14 -0400 Subject: [PATCH 03/25] Use fewer `.to_string()`s --- .../passes/collect_intra_doc_links.rs | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f6f01028ee2e2..9bf30c71892aa 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -19,6 +19,7 @@ use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; use smallvec::{smallvec, SmallVec}; +use std::borrow::Cow; use std::cell::Cell; use std::ops::Range; @@ -46,40 +47,40 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { } } -enum ErrorKind { - Resolve(ResolutionFailure), +enum ErrorKind<'a> { + Resolve(ResolutionFailure<'a>), AnchorFailure(AnchorFailure), } #[derive(Debug)] -enum ResolutionFailure { +enum ResolutionFailure<'a> { /// 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), + NotInScope(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 NoPrimitiveImpl(Res, String), /// `[u8::not_found]` /// the `Res` is the primitive it resolved to - NoPrimitiveAssocItem { res: Res, prim_name: String, assoc_item: String }, + 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, String), + NoAssocItem(Res, Symbol), /// 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), + NotAVariant(Res, Symbol), /// used to communicate that this should be ignored, but shouldn't be reported to the user Dummy, } -impl ResolutionFailure { +impl ResolutionFailure<'a> { fn res(&self) -> Option { use ResolutionFailure::*; match self { @@ -121,10 +122,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { fn variant_field( &self, - path_str: &str, + path_str: &'path str, current_item: &Option, module_id: DefId, - ) -> Result<(Res, Option), ErrorKind> { + ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; let mut split = path_str.rsplitn(3, "::"); @@ -134,7 +135,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .expect("fold_item should ensure link is non-empty"); let 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(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.to_string())))?; + split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.into())))?; // TODO: this looks very wrong, why are we requiring 3 fields? let path = split .next() @@ -147,14 +148,18 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { f.to_owned() }) // TODO: is this right? - .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(variant_name.to_string())))?; + .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope( + variant_name.to_string().into(), + )))?; let (_, ty_res) = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) }) - .map_err(|_| ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string())))?; + .map_err(|_| { + ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string().into())) + })?; if let Res::Err = ty_res { - return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string()))); + return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string().into()))); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { @@ -183,7 +188,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } else { Err(ErrorKind::Resolve(ResolutionFailure::NotAVariant( ty_res, - variant_field_name.to_string(), + variant_field_name, ))) } } @@ -197,9 +202,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// Resolves a string as a macro. fn macro_resolve( &self, - path_str: &str, + path_str: &'a str, parent_id: Option, - ) -> Result { + ) -> Result> { let cx = self.cx; let path = ast::Path::from_ident(Ident::from_str(path_str)); cx.enter_resolver(|resolver| { @@ -232,19 +237,19 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("attempting to resolve item without parent module: {}", path_str); return Err(ResolutionFailure::NoParentItem); } - return Err(ResolutionFailure::NotInScope(path_str.to_string())); + return Err(ResolutionFailure::NotInScope(path_str.into())); }) } /// Resolves a string as a path within a particular namespace. Also returns an optional /// URL fragment in the case of variants and methods. - fn resolve( + fn resolve<'path>( &self, - path_str: &str, + path_str: &'path str, ns: Namespace, current_item: &Option, parent_id: Option, extra_fragment: &Option, - ) -> Result<(Res, Option), ErrorKind> { + ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; // In case we're in a module, try to resolve the relative path. @@ -309,11 +314,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) // 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())))?; + .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope( + item_name.to_string().into(), + )))?; 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)) + ErrorKind::Resolve(ResolutionFailure::NoPrimitiveImpl(prim, path_root.into())) })?; for &impl_ in impls { let link = cx @@ -337,8 +344,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } return Err(ErrorKind::Resolve(ResolutionFailure::NoPrimitiveAssocItem { res: prim, - prim_name: path.to_string(), - assoc_item: item_name.to_string(), + prim_name: path, + assoc_item: item_name, })); } @@ -347,13 +354,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolver.resolve_str_path_error(DUMMY_SP, &path_root, TypeNS, module_id) }) .map_err(|_| { - ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.clone())) + ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.clone().into())) })?; if let Res::Err = ty_res { return if ns == Namespace::ValueNS { self.variant_field(path_str, current_item, module_id) } else { - Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root))) + Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.into()))) }; } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); @@ -450,8 +457,7 @@ 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::Resolve(ResolutionFailure::NoAssocItem( - ty_res, - item_name.to_string(), + ty_res, item_name, ))); } } @@ -491,10 +497,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if ns == Namespace::ValueNS { self.variant_field(path_str, current_item, module_id) } else { - Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem( - ty_res, - item_name.to_string(), - ))) + Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(ty_res, item_name))) } }) } else { @@ -638,7 +641,7 @@ 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: Ok((Res::Def(DefKind::Trait, _), _)), macro_ns: Ok((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)), @@ -941,7 +944,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { drop(candidates_iter); if is_derive_trait_collision(&candidates) { candidates.macro_ns = - Err(ResolutionFailure::NotInScope(path_str.to_string())); + Err(ResolutionFailure::NotInScope(path_str.into())); } // If we're reporting an ambiguity, don't mention the namespaces that failed let candidates = @@ -1348,7 +1351,7 @@ fn resolution_failure( path_str: &str, dox: &str, link_range: Option>, - kinds: SmallVec<[ResolutionFailure; 3]>, + kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { report_diagnostic( cx, @@ -1494,10 +1497,10 @@ fn anchor_failure( }); } -fn ambiguity_error( +fn ambiguity_error<'a>( cx: &DocContext<'_>, item: &Item, - path_str: &str, + path_str: &'a str, dox: &str, link_range: Option>, candidates: Vec, @@ -1593,7 +1596,7 @@ fn handle_variant( cx: &DocContext<'_>, res: Res, extra_fragment: &Option, -) -> Result<(Res, Option), ErrorKind> { +) -> Result<(Res, Option), ErrorKind<'static>> { use rustc_middle::ty::DefIdTree; if extra_fragment.is_some() { From fcb21999a7989792c9405a89e9c788ac143e0574 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 22:31:44 -0400 Subject: [PATCH 04/25] Report if the thing exists in another namespace --- .../passes/collect_intra_doc_links.rs | 65 ++++++++++++------- src/test/rustdoc-ui/intra-link-errors.rs | 1 + 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 9bf30c71892aa..5013257736719 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -81,6 +81,7 @@ enum ResolutionFailure<'a> { } impl ResolutionFailure<'a> { + // A partial or full resolution fn res(&self) -> Option { use ResolutionFailure::*; match self { @@ -93,6 +94,14 @@ impl ResolutionFailure<'a> { NotInScope(_) | NoParentItem | Dummy => None, } } + + // This resolved fully (not just partially) but is erroneous for some other reason + fn full_res(&self) -> Option { + match self { + Self::WrongNamespace(res, _) => Some(*res), + _ => None, + } + } } enum AnchorFailure { @@ -128,6 +137,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; + debug!("looking for enum variant {}", path_str); let mut split = path_str.rsplitn(3, "::"); let variant_field_name = split .next() @@ -260,7 +270,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); let result = match result { Ok((_, Res::Err)) => Err(()), - _ => result.map_err(|_| ()), + x => x, }; if let Ok((_, res)) = result { @@ -419,6 +429,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Ok((ty_res, Some(format!("{}.{}", out, item_name)))) }) } else if ns == Namespace::ValueNS { + debug!("looking for variants or fields named {} for {:?}", item_name, did); match cx.tcx.type_of(did).kind() { ty::Adt(def, _) => { let field = if def.is_enum() { @@ -838,12 +849,36 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } + // used for reporting better errors + let check_full_res = |this: &mut Self, ns| { + match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { + Ok(res) => { + debug!( + "check_full_res: saw res for {} in {:?} ns: {:?}", + path_str, ns, res.0 + ); + Some(res.0) + } + Err(ErrorKind::Resolve(kind)) => kind.full_res(), + // TODO: add `Res` to AnchorFailure + Err(ErrorKind::AnchorFailure(_)) => None, + } + }; + match disambiguator.map(Disambiguator::ns) { Some(ns @ (ValueNS | TypeNS)) => { match self.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { Ok(res) => res, - Err(ErrorKind::Resolve(kind)) => { + Err(ErrorKind::Resolve(mut kind)) => { + // We only looked in one namespace. Try to give a better error if possible. + // TODO: handle MacroNS too + if kind.full_res().is_none() { + let other_ns = if ns == ValueNS { TypeNS } else { ValueNS }; + if let Some(res) = check_full_res(self, other_ns) { + kind = ResolutionFailure::WrongNamespace(res, other_ns); + } + } resolution_failure( cx, &item, @@ -965,30 +1000,14 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Ok(res) => (res, extra_fragment), Err(mut kind) => { // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible. + //if kind.res().is_none() { 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); - } - } + if let Some(res) = check_full_res(self, ns) { + kind = ResolutionFailure::WrongNamespace(res, MacroNS); + break; } } + //} resolution_failure( cx, &item, diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 7a53a6f079334..99d080fb324e3 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -54,6 +54,7 @@ impl S { } /// [type@T::g] +/// [T::h!] pub trait T { fn g() {} } From 42bed035001b9a0bd6282326a243a926ea3d0424 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 23:05:47 -0400 Subject: [PATCH 05/25] Pass on the DefId so rustdoc can name it in suggestions Look at this beauty: ```rust error: unresolved link to `S::h` --> intra-link-errors.rs:51:6 | 51 | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | = note: this link resolves to the associated function `h`, which is not in the type namespace ``` --- .../passes/collect_intra_doc_links.rs | 52 ++++++++++--------- src/test/rustdoc-ui/intra-link-errors.rs | 7 +++ src/test/rustdoc-ui/intra-link-errors.stderr | 27 ++++++---- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 5013257736719..287f5fcf805ef 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -121,7 +121,7 @@ struct LinkCollector<'a, 'tcx> { /// This is used to store the kind of associated items, /// because `clean` and the disambiguator code expect them to be different. /// See the code for associated items on inherent impls for details. - kind_side_channel: Cell>, + kind_side_channel: Cell>, } impl<'a, 'tcx> LinkCollector<'a, 'tcx> { @@ -381,7 +381,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) => { debug!("looking for associated item named {} for item {:?}", item_name, did); // Checks if item_name belongs to `impl SomeItem` - let kind = cx + let assoc_item = cx .tcx .inherent_impls(did) .iter() @@ -393,7 +393,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { imp, ) }) - .map(|item| item.kind) + .map(|item| (item.kind, item.def_id)) // There should only ever be one associated item that matches from any inherent impl .next() // Check if item_name belongs to `impl SomeTrait for SomeItem` @@ -409,7 +409,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { kind }); - if let Some(kind) = kind { + if let Some((kind, id)) = assoc_item { let out = match kind { ty::AssocKind::Fn => "method", ty::AssocKind::Const => "associatedconstant", @@ -425,7 +425,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // HACK(jynelson): `clean` expects the type, not the associated item. // but the disambiguator logic expects the associated item. // Store the kind in a side channel so that only the disambiguator logic looks at it. - self.kind_side_channel.set(Some(kind.as_def_kind())); + self.kind_side_channel.set(Some((kind.as_def_kind(), id))); Ok((ty_res, Some(format!("{}.{}", out, item_name)))) }) } else if ns == Namespace::ValueNS { @@ -525,7 +525,7 @@ fn resolve_associated_trait_item( item_name: Symbol, ns: Namespace, cx: &DocContext<'_>, -) -> Option { +) -> Option<(ty::AssocKind, DefId)> { let ty = cx.tcx.type_of(did); // First consider automatic impls: `impl From for T` let implicit_impls = crate::clean::get_auto_trait_and_blanket_impls(cx, ty, did); @@ -553,7 +553,7 @@ fn resolve_associated_trait_item( // but provided methods come directly from `tcx`. // Fortunately, we don't need the whole method, we just need to know // what kind of associated item it is. - Some((assoc.def_id, kind)) + Some((kind, assoc.def_id)) }); let assoc = items.next(); debug_assert_eq!(items.count(), 0); @@ -575,7 +575,7 @@ fn resolve_associated_trait_item( ns, trait_, ) - .map(|assoc| (assoc.def_id, assoc.kind)) + .map(|assoc| (assoc.kind, assoc.def_id)) } } _ => panic!("get_impls returned something that wasn't an impl"), @@ -592,12 +592,12 @@ fn resolve_associated_trait_item( cx.tcx .associated_items(trait_) .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, trait_) - .map(|assoc| (assoc.def_id, assoc.kind)) + .map(|assoc| (assoc.kind, assoc.def_id)) })); } // FIXME: warn about ambiguity debug!("the candidates were {:?}", candidates); - candidates.pop().map(|(_, kind)| kind) + candidates.pop() } /// Given a type, return all traits in scope in `module` implemented by that type. @@ -851,18 +851,21 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // used for reporting better errors let check_full_res = |this: &mut Self, ns| { - match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { - Ok(res) => { - debug!( - "check_full_res: saw res for {} in {:?} ns: {:?}", - path_str, ns, res.0 - ); - Some(res.0) - } - Err(ErrorKind::Resolve(kind)) => kind.full_res(), - // TODO: add `Res` to AnchorFailure - Err(ErrorKind::AnchorFailure(_)) => None, - } + let res = + match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) + { + Ok(res) => { + debug!( + "check_full_res: saw res for {} in {:?} ns: {:?}", + path_str, ns, res.0 + ); + Some(res.0) + } + Err(ErrorKind::Resolve(kind)) => kind.full_res(), + // TODO: add `Res` to AnchorFailure + Err(ErrorKind::AnchorFailure(_)) => None, + }; + this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) }; match disambiguator.map(Disambiguator::ns) { @@ -876,7 +879,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { if kind.full_res().is_none() { let other_ns = if ns == ValueNS { TypeNS } else { ValueNS }; if let Some(res) = check_full_res(self, other_ns) { - kind = ResolutionFailure::WrongNamespace(res, other_ns); + // recall that this stores the _expected_ namespace + kind = ResolutionFailure::WrongNamespace(res, ns); } } resolution_failure( @@ -1092,7 +1096,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // Disallow e.g. linking to enums with `struct@` if let Res::Def(kind, _) = res { debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator); - match (self.kind_side_channel.take().unwrap_or(kind), disambiguator) { + match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) { | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const))) // NOTE: this allows 'method' to mean both normal functions and associated functions // This can't cause ambiguity because both are in the same namespace. diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 99d080fb324e3..8c42a38ff4eb0 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -49,11 +49,18 @@ pub struct S; pub enum E { A, B, C } /// [type@S::h] +//~^ ERROR unresolved link +//~| HELP to link to the associated function +//~| NOTE not in the type namespace impl S { pub fn h() {} } /// [type@T::g] +//~^ ERROR unresolved link +//~| HELP to link to the associated function +//~| NOTE not in the type namespace + /// [T::h!] pub trait T { fn g() {} diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index baf4ab7a9bc46..bb9db68e0d5e1 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -80,27 +80,34 @@ error: unresolved link to `S` --> $DIR/intra-link-errors.rs:41:6 | LL | /// [S!] - | ^^ help: to link to the unit struct, use its disambiguator: `value@S` + | ^^ help: to link to the struct, use its disambiguator: `struct@S` | - = note: this link resolves to the unit struct `S`, which is not in the macro namespace + = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:56:6 + --> $DIR/intra-link-errors.rs:59:6 | LL | /// [type@T::g] - | ^^^^^^^^^ + | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` | - = note: this link partially resolves to the trait `T` - = note: `T` has no field, variant, or associated item named `g` + = note: this link resolves to the associated function `g`, which is not in the type namespace + +error: unresolved link to `T::h` + --> $DIR/intra-link-errors.rs:64:6 + | +LL | /// [T::h!] + | ^^^^^ + | + = note: no item named `T::h` is in scope + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` --> $DIR/intra-link-errors.rs:51:6 | LL | /// [type@S::h] - | ^^^^^^^^^ + | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | - = note: this link partially resolves to the struct `S` - = note: `S` has no field, variant, or associated item named `h` + = note: this link resolves to the associated function `h`, which is not in the type namespace -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors From f4e6ebd11aaea9fc412d78371fd3060468625056 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 20 Aug 2020 23:14:41 -0400 Subject: [PATCH 06/25] Fix tests and improve error message if `::` isn't found --- .../passes/collect_intra_doc_links.rs | 13 +++-- .../rustdoc-ui/intra-doc-alias-ice.stderr | 2 +- src/test/rustdoc-ui/intra-link-errors.rs | 16 +++--- src/test/rustdoc-ui/intra-link-errors.stderr | 54 ++++++------------- .../rustdoc-ui/intra-links-warning.stderr | 2 +- 5 files changed, 33 insertions(+), 54 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 287f5fcf805ef..ea3ec1b6685cc 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -324,9 +324,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) // 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().into(), - )))?; + .ok_or_else(|| { + debug!("found no `::`, assumming {} was correctly not in scope", item_name); + ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string().into())) + })?; if let Some((path, prim)) = is_primitive(&path_root, ns) { let impls = primitive_impl(cx, &path).ok_or_else(|| { @@ -361,6 +362,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let (_, ty_res) = cx .enter_resolver(|resolver| { + // only types can have associated items resolver.resolve_str_path_error(DUMMY_SP, &path_root, TypeNS, module_id) }) .map_err(|_| { @@ -1450,10 +1452,7 @@ fn resolution_failure( // 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 - ); + let note = format!("no `{}` in `{}`", assoc_item, name,); diag.note(¬e); } } diff --git a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr index cc1343eb7b95e..58abe4406a85d 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -10,7 +10,7 @@ note: the lint level is defined here LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: this link partially resolves to the type alias `TypeAlias` - = note: `TypeAlias` has no field, variant, or associated item named `hoge` + = note: no `hoge` in `TypeAlias` 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 index 8c42a38ff4eb0..79ce57c22676f 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -1,9 +1,6 @@ #![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?) /// [] @@ -22,17 +19,17 @@ /// [S::A] //~^ ERROR unresolved link //~| NOTE this link partially resolves -//~| NOTE `S` has no field +//~| NOTE no `A` in `S` /// [S::fmt] //~^ ERROR unresolved link //~| NOTE this link partially resolves -//~| NOTE `S` has no field +//~| NOTE no `fmt` in `S` /// [E::D] //~^ ERROR unresolved link //~| NOTE this link partially resolves -//~| NOTE `E` has no field +//~| NOTE no `D` in `E` /// [u8::not_found] //~^ ERROR unresolved link @@ -40,8 +37,8 @@ /// [S!] //~^ ERROR unresolved link -//~| HELP to link to the unit struct, use its disambiguator -//~| NOTE this link resolves to the unit struct `S` +//~| HELP to link to the struct, use its disambiguator +//~| NOTE this link resolves to the struct `S` pub fn f() {} #[derive(Debug)] pub struct S; @@ -62,6 +59,9 @@ impl S { //~| NOTE not in the type namespace /// [T::h!] +//~^ ERROR unresolved link +//~| NOTE no item named `T::h` +//~| HELP to escape pub trait T { fn g() {} } diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index bb9db68e0d5e1..4044c7fde66f2 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -1,39 +1,19 @@ -error: unresolved link to `std::io::oops` - --> $DIR/intra-link-errors.rs:4:6 +error: unresolved link to `path::to::nonexistent::module` + --> $DIR/intra-link-errors.rs:8:6 | -LL | //! [std::io::oops] - | ^^^^^^^^^^^^^ +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: this link resolves to the crate `std`, which is not an enum - = note: if this were an enum, it might have a variant which resolved - = note: this link partially resolves to the module `io` - = note: `io` has no field, variant, or associated item named `oops` - -error: unresolved link to `std::io::oops::not::here` - --> $DIR/intra-link-errors.rs:5:6 - | -LL | //! [std::io::oops::not::here] - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: no item named `std::io::oops::not` is in scope - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` - -error: unresolved link to `path::to::nonexistent::module` - --> $DIR/intra-link-errors.rs:11:6 - | -LL | /// [path::to::nonexistent::module] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | = 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:17:6 + --> $DIR/intra-link-errors.rs:14:6 | LL | /// [f::A] | ^^^^ @@ -42,34 +22,34 @@ LL | /// [f::A] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::A` - --> $DIR/intra-link-errors.rs:22:6 + --> $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` + = note: no `A` in `S` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:27:6 + --> $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` + = note: no `fmt` in `S` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:32:6 + --> $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` + = note: no `D` in `E` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:37:6 + --> $DIR/intra-link-errors.rs:34:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -77,7 +57,7 @@ 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:41:6 + --> $DIR/intra-link-errors.rs:38:6 | LL | /// [S!] | ^^ help: to link to the struct, use its disambiguator: `struct@S` @@ -85,7 +65,7 @@ LL | /// [S!] = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:59:6 + --> $DIR/intra-link-errors.rs:56:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` @@ -93,7 +73,7 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:64:6 + --> $DIR/intra-link-errors.rs:61:6 | LL | /// [T::h!] | ^^^^^ @@ -102,12 +82,12 @@ LL | /// [T::h!] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:51:6 + --> $DIR/intra-link-errors.rs:48:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | = note: this link resolves to the associated function `h`, which is not in the type namespace -error: aborting due to 12 previous errors +error: aborting due to 10 previous errors diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 3737cad1cb187..b51c8e89600b0 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -6,7 +6,7 @@ LL | //! Test with [Foo::baz], [Bar::foo], ... | = note: `#[warn(broken_intra_doc_links)]` on by default = note: this link partially resolves to the struct `Foo` - = note: `Foo` has no field, variant, or associated item named `baz` + = note: no `baz` in `Foo` warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 From 002d3a922c06c11a45e001d2737951a00e40687a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 00:23:23 -0400 Subject: [PATCH 07/25] Don't give misleading errors for `f::A`, where f is in the value namespace --- .../passes/collect_intra_doc_links.rs | 62 ++++++++++++++----- src/test/rustdoc-ui/intra-link-errors.rs | 6 +- src/test/rustdoc-ui/intra-link-errors.stderr | 6 +- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ea3ec1b6685cc..ed411bf2b8dab 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -57,6 +57,9 @@ 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), /// `String` is the base name of the path (not necessarily the whole link) NotInScope(Cow<'a, str>), /// this is a primitive type without an impls (no associated methods) @@ -90,7 +93,8 @@ impl ResolutionFailure<'a> { | NoPrimitiveImpl(res, _) | NotAnEnum(res) | NotAVariant(res, _) - | WrongNamespace(res, _) => Some(*res), + | WrongNamespace(res, _) + | CannotHaveAssociatedItems(res, _) => Some(*res), NotInScope(_) | NoParentItem | Dummy => None, } } @@ -360,21 +364,39 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { })); } - let (_, ty_res) = cx + let ty_res = cx .enter_resolver(|resolver| { // only types can have associated items resolver.resolve_str_path_error(DUMMY_SP, &path_root, TypeNS, module_id) }) - .map_err(|_| { - ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.clone().into())) - })?; - if let Res::Err = ty_res { - return if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id) - } else { - Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_root.into()))) - }; - } + .map(|(_, res)| res); + let ty_res = match ty_res { + Err(()) | Ok(Res::Err) => { + return if ns == Namespace::ValueNS { + 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| { + 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(path_root.into()) + }); + Err(ErrorKind::Resolve(kind)) + }; + } + Ok(res) => res, + }; let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); let res = match ty_res { Res::Def( @@ -1006,14 +1028,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Ok(res) => (res, extra_fragment), Err(mut kind) => { // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible. - //if kind.res().is_none() { for &ns in &[TypeNS, ValueNS] { if let Some(res) = check_full_res(self, ns) { kind = ResolutionFailure::WrongNamespace(res, MacroNS); break; } } - //} resolution_failure( cx, &item, @@ -1456,6 +1476,20 @@ fn resolution_failure( diag.note(¬e); } } + ResolutionFailure::CannotHaveAssociatedItems(res, _) => { + let (item, _) = item(res); + diag.note(&format!("this link partially resolves to {}", item)); + if let Res::Def(kind, def_id) = res { + let name = cx.tcx.item_name(def_id); + let note = format!( + "`{}` is {} {}, not a module or type, and cannot have associated items", + name, + kind.article(), + kind.descr(def_id) + ); + diag.note(¬e); + } + } // TODO: is there ever a case where this happens? ResolutionFailure::NotAnEnum(res) => { let (item, comma) = item(res); diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 79ce57c22676f..1a2f24cef2423 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -7,14 +7,14 @@ // 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 +//~| NOTE no item named `path::to` 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 +//~| NOTE this link partially resolves +//~| NOTE `f` is a function, not a module /// [S::A] //~^ ERROR unresolved link diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 4044c7fde66f2..0244d2bfde8d9 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: no item named `path::to::nonexistent` is in scope + = note: no item named `path::to` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `f::A` @@ -18,8 +18,8 @@ error: unresolved link to `f::A` LL | /// [f::A] | ^^^^ | - = note: no item named `f` is in scope - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + = note: this link partially resolves to the function `f` + = note: `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:19:6 From 7b8d0befd67ad2472052e3f84cf310b7866b829f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 12:31:04 -0400 Subject: [PATCH 08/25] Remove some TODOs --- src/librustdoc/passes/collect_intra_doc_links.rs | 2 -- src/test/rustdoc-ui/intra-link-errors.rs | 1 - 2 files changed, 3 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ed411bf2b8dab..0d07f8f0328b4 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -150,7 +150,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let 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(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.into())))?; - // TODO: this looks very wrong, why are we requiring 3 fields? let path = split .next() .map(|f| { @@ -161,7 +160,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - // TODO: is this right? .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope( variant_name.to_string().into(), )))?; diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 1a2f24cef2423..5bfbdcc495f29 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -10,7 +10,6 @@ //~| NOTE no item named `path::to` is in scope //~| HELP to escape -// TODO: why does this say `f` and not `f::A`?? /// [f::A] //~^ ERROR unresolved link //~| NOTE this link partially resolves From e2d69f2eb115b0b6a433977ae7c5a73c249a4f98 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 13:35:50 -0400 Subject: [PATCH 09/25] Make errors more concise and helpful Before: ``` = note: this link partially resolves to the struct `S` = note: no `fmt` in `S` ``` After: ``` = note: the struct `S` has no field or associated item named `fmt` ``` --- .../passes/collect_intra_doc_links.rs | 95 ++++++++++++------- .../rustdoc-ui/assoc-item-not-in-scope.stderr | 4 +- .../rustdoc-ui/intra-doc-alias-ice.stderr | 3 +- src/test/rustdoc-ui/intra-link-errors.rs | 10 +- src/test/rustdoc-ui/intra-link-errors.stderr | 28 +++--- .../rustdoc-ui/intra-links-warning.stderr | 3 +- src/test/rustdoc-ui/lint-group.stderr | 3 +- 7 files changed, 81 insertions(+), 65 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0d07f8f0328b4..c69850f9d9743 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1406,11 +1406,18 @@ fn resolution_failure( 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()), "") - } + format!("the {} `{}`", res.descr(), cx.tcx.item_name(res.def_id()).to_string()) + }; + let assoc_item_not_allowed = |res: Res, diag: &mut DiagnosticBuilder<'_>| { + let def_id = res.def_id(); + let name = cx.tcx.item_name(def_id); + let note = format!( + "`{}` is {} {}, not a module or type, and cannot have associated items", + name, + res.article(), + res.descr() + ); + diag.note(¬e); }; for failure in kinds { match failure { @@ -1425,11 +1432,9 @@ fn resolution_failure( } 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, + "this link resolves to {}, which is not in the {} namespace", + item(res), expected_ns.descr() ); diag.note(¬e); @@ -1450,10 +1455,9 @@ fn resolution_failure( 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, + "this link partially resolves to {}, which does not have any associated items", + item(res), ); diag.note(¬e); } @@ -1465,41 +1469,62 @@ fn resolution_failure( 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!("no `{}` in `{}`", assoc_item, name,); - diag.note(¬e); - } + use DefKind::*; + + let (kind, def_id) = match res { + Res::Def(kind, def_id) => (kind, def_id), + _ => unreachable!( + "primitives are covered above and other `Res` variants aren't possible at module scope" + ), + }; + let name = cx.tcx.item_name(def_id); + let path_description = 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 => return assoc_item_not_allowed(res, diag), + Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam + | Static => "associated item", + Impl | GlobalAsm => unreachable!("not a path"), + }; + let note = format!( + "the {} `{}` has no {} named `{}`", + res.descr(), + name, + path_description, + assoc_item + ); + diag.note(¬e); } ResolutionFailure::CannotHaveAssociatedItems(res, _) => { - let (item, _) = item(res); - diag.note(&format!("this link partially resolves to {}", item)); - if let Res::Def(kind, def_id) = res { - let name = cx.tcx.item_name(def_id); - let note = format!( - "`{}` is {} {}, not a module or type, and cannot have associated items", - name, - kind.article(), - kind.descr(def_id) - ); - diag.note(¬e); - } + assoc_item_not_allowed(res, diag) } // 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); + format!("this link resolves to {}, which is not an enum", item(res)); 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, + item(res), variant ); diag.note(¬e); diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr index 8827c9351a62d..8acfcca2139e3 100644 --- a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr @@ -2,14 +2,14 @@ error: unresolved link to `S::fmt` --> $DIR/assoc-item-not-in-scope.rs:4:14 | LL | /// Link to [`S::fmt`] - | ^^^^^^^^ unresolved link + | ^^^^^^^^ | note: the lint level is defined here --> $DIR/assoc-item-not-in-scope.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + = note: the struct `S` has no field or associated item named `fmt` 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 58abe4406a85d..5379a91883753 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -9,8 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: this link partially resolves to the type alias `TypeAlias` - = note: no `hoge` in `TypeAlias` + = note: the type alias `TypeAlias` has no 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 index 5bfbdcc495f29..ecf2b05e2818b 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -12,23 +12,19 @@ /// [f::A] //~^ ERROR unresolved link -//~| NOTE this link partially resolves //~| NOTE `f` is a function, not a module /// [S::A] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `A` in `S` +//~| NOTE struct `S` has no field or associated item /// [S::fmt] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `fmt` in `S` +//~| NOTE struct `S` has no field or associated item /// [E::D] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `D` in `E` +//~| NOTE enum `E` has no variant or associated item /// [u8::not_found] //~^ ERROR unresolved link diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 0244d2bfde8d9..ba091ff854ce6 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -13,43 +13,39 @@ LL | #![deny(broken_intra_doc_links)] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:14:6 + --> $DIR/intra-link-errors.rs:13:6 | LL | /// [f::A] | ^^^^ | - = note: this link partially resolves to the function `f` = note: `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:19:6 + --> $DIR/intra-link-errors.rs:17:6 | LL | /// [S::A] | ^^^^ | - = note: this link partially resolves to the struct `S` - = note: no `A` in `S` + = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:24:6 + --> $DIR/intra-link-errors.rs:21:6 | LL | /// [S::fmt] | ^^^^^^ | - = note: this link partially resolves to the struct `S` - = note: no `fmt` in `S` + = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:29:6 + --> $DIR/intra-link-errors.rs:25:6 | LL | /// [E::D] | ^^^^ | - = note: this link partially resolves to the enum `E` - = note: no `D` in `E` + = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:34:6 + --> $DIR/intra-link-errors.rs:29:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -57,7 +53,7 @@ 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 + --> $DIR/intra-link-errors.rs:33:6 | LL | /// [S!] | ^^ help: to link to the struct, use its disambiguator: `struct@S` @@ -65,7 +61,7 @@ LL | /// [S!] = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:56:6 + --> $DIR/intra-link-errors.rs:51:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` @@ -73,7 +69,7 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:61:6 + --> $DIR/intra-link-errors.rs:56:6 | LL | /// [T::h!] | ^^^^^ @@ -82,7 +78,7 @@ LL | /// [T::h!] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:48:6 + --> $DIR/intra-link-errors.rs:43:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index b51c8e89600b0..988a65c7a591c 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -5,8 +5,7 @@ LL | //! Test with [Foo::baz], [Bar::foo], ... | ^^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default - = note: this link partially resolves to the struct `Foo` - = note: no `baz` in `Foo` + = note: the struct `Foo` has no field or associated item named `baz` warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 diff --git a/src/test/rustdoc-ui/lint-group.stderr b/src/test/rustdoc-ui/lint-group.stderr index 04296d2e44a7c..10fcebb27fe0c 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] - | ^^^^^ unresolved link + | ^^^^^ | note: the lint level is defined here --> $DIR/lint-group.rs:7:9 @@ -40,6 +40,7 @@ note: the lint level is defined here LL | #![deny(rustdoc)] | ^^^^^^^ = note: `#[deny(broken_intra_doc_links)]` implied by `#[deny(rustdoc)]` + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to 3 previous errors From 19d100278d9609c389780e2692dddaeb45fba301 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 15:00:30 -0400 Subject: [PATCH 10/25] Fix failures to resolve primitives Previously, when looking for the associated items for primitives, rustdoc would look for primitives in the current namespace. But all primitives are in the type namespace. To fix this, rustdoc now always looks for primitives in the namespace when considering them as a stepping stone to the associated item. However, fixing that bug caused several duplicate errors because rustdoc now reports the same error in each namespace. To avoid this, rustdoc now ignores all duplicate errors when issuing them. --- .../passes/collect_intra_doc_links.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index c69850f9d9743..9a88c8eb42ad6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -331,7 +331,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string().into())) })?; - if let Some((path, prim)) = is_primitive(&path_root, ns) { + if let Some((path, prim)) = is_primitive(&path_root, TypeNS) { let impls = primitive_impl(cx, &path).ok_or_else(|| { ErrorKind::Resolve(ResolutionFailure::NoPrimitiveImpl(prim, path_root.into())) })?; @@ -355,6 +355,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { return Ok(link); } } + debug!( + "returning primitive error for {}::{} in {} namespace", + path, + item_name, + ns.descr() + ); return Err(ErrorKind::Resolve(ResolutionFailure::NoPrimitiveAssocItem { res: prim, prim_name: path, @@ -1404,7 +1410,6 @@ fn resolution_failure( &link_range, |diag, sp| { let in_scope = kinds.iter().any(|kind| kind.res().is_some()); - let mut reported_not_in_scope = false; let item = |res: Res| { format!("the {} `{}`", res.descr(), cx.tcx.item_name(res.def_id()).to_string()) }; @@ -1419,14 +1424,19 @@ fn resolution_failure( ); diag.note(¬e); }; + // ignore duplicates + let mut variants_seen = SmallVec::<[_; 3]>::new(); for failure in kinds { + let variant = std::mem::discriminant(&failure); + if variants_seen.contains(&variant) { + continue; + } + variants_seen.push(variant); match failure { - // already handled above ResolutionFailure::NotInScope(base) => { - if in_scope || reported_not_in_scope { + if 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 `\]`"#); } From ebc8cb470fd137214911fa317ff6b06c6683ad3c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 15:19:47 -0400 Subject: [PATCH 11/25] Turn NoParentItem from a panic into an ICE --- src/librustdoc/passes/collect_intra_doc_links.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 9a88c8eb42ad6..a859d1b2f5f59 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -541,7 +541,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } else { debug!("attempting to resolve item without parent module: {}", path_str); - // TODO: maybe this should just be an ICE? Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)) } } @@ -1462,7 +1461,8 @@ fn resolution_failure( } } ResolutionFailure::NoParentItem => { - panic!("all intra doc links should have a parent item") + diag.level = rustc_errors::Level::Bug; + diag.note("all intra doc links should have a parent item"); } ResolutionFailure::NoPrimitiveImpl(res, _) => { let note = format!( @@ -1694,7 +1694,6 @@ fn handle_variant( let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { parent } else { - // 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); From 6875220e1abea26c67885833c27365854cd7f73c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 16:12:39 -0400 Subject: [PATCH 12/25] Use rustc_resolve's descr() instead of rewriting it --- .../passes/collect_intra_doc_links.rs | 86 ++++++++----------- .../rustdoc-ui/intra-links-anchors.stderr | 6 +- 2 files changed, 38 insertions(+), 54 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a859d1b2f5f59..1c709cead7017 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -110,12 +110,7 @@ impl ResolutionFailure<'a> { enum AnchorFailure { MultipleAnchors, - Primitive, - Variant, - AssocConstant, - AssocType, - Field, - Method, + RustdocAnchorConflict(Res), } struct LinkCollector<'a, 'tcx> { @@ -288,7 +283,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Not a trait item; just return what we found. Res::PrimTy(..) => { if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); + return Err(ErrorKind::AnchorFailure( + AnchorFailure::RustdocAnchorConflict(res), + )); } return Ok((res, Some(path_str.to_owned()))); } @@ -305,7 +302,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } else if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); + return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + prim, + ))); } return Ok((prim, Some(path.to_owned()))); } @@ -444,11 +443,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Type => "associatedtype", }; Some(if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(if kind == ty::AssocKind::Fn { - AnchorFailure::Method - } else { - AnchorFailure::AssocConstant - })) + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + ty_res, + ))) } else { // HACK(jynelson): `clean` expects the type, not the associated item. // but the disambiguator logic expects the associated item. @@ -470,11 +467,17 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; field.map(|item| { if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(if def.is_enum() { - AnchorFailure::Variant - } else { - AnchorFailure::Field - })) + let res = Res::Def( + if def.is_enum() { + DefKind::Variant + } else { + DefKind::Field + }, + item.did, + ); + Err(ErrorKind::AnchorFailure( + AnchorFailure::RustdocAnchorConflict(res), + )) } else { Ok(( ty_res, @@ -518,13 +521,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(if item.kind == ty::AssocKind::Const { - AnchorFailure::AssocConstant - } else if item.kind == ty::AssocKind::Type { - AnchorFailure::AssocType - } else { - AnchorFailure::Method - })) + 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)))) @@ -889,8 +888,10 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Some(res.0) } Err(ErrorKind::Resolve(kind)) => kind.full_res(), - // TODO: add `Res` to AnchorFailure - Err(ErrorKind::AnchorFailure(_)) => None, + Err(ErrorKind::AnchorFailure( + AnchorFailure::RustdocAnchorConflict(res), + )) => Some(res), + Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, }; this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) }; @@ -1070,7 +1071,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { path_str, &dox, link_range, - AnchorFailure::Primitive, + AnchorFailure::RustdocAnchorConflict(prim), ); continue; } @@ -1555,28 +1556,11 @@ fn anchor_failure( ) { let msg = match failure { AnchorFailure::MultipleAnchors => format!("`{}` contains multiple anchors", path_str), - AnchorFailure::Primitive - | AnchorFailure::Variant - | AnchorFailure::AssocConstant - | AnchorFailure::AssocType - | AnchorFailure::Field - | AnchorFailure::Method => { - let kind = match failure { - AnchorFailure::Primitive => "primitive type", - AnchorFailure::Variant => "enum variant", - AnchorFailure::AssocConstant => "associated constant", - AnchorFailure::AssocType => "associated type", - AnchorFailure::Field => "struct field", - AnchorFailure::Method => "method", - AnchorFailure::MultipleAnchors => unreachable!("should be handled already"), - }; - - format!( - "`{}` contains an anchor, but links to {kind}s are already anchored", - path_str, - kind = kind - ) - } + AnchorFailure::RustdocAnchorConflict(res) => format!( + "`{}` contains an anchor, but links to {kind}s are already anchored", + path_str, + kind = res.descr(), + ), }; report_diagnostic(cx, &msg, item, dox, &link_range, |diag, sp| { @@ -1689,7 +1673,7 @@ fn handle_variant( use rustc_middle::ty::DefIdTree; if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure(AnchorFailure::Variant)); + return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))); } let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { parent diff --git a/src/test/rustdoc-ui/intra-links-anchors.stderr b/src/test/rustdoc-ui/intra-links-anchors.stderr index e737b84320d94..1825a4ad1fa6b 100644 --- a/src/test/rustdoc-ui/intra-links-anchors.stderr +++ b/src/test/rustdoc-ui/intra-links-anchors.stderr @@ -1,4 +1,4 @@ -error: `Foo::f#hola` contains an anchor, but links to struct fields are already anchored +error: `Foo::f#hola` contains an anchor, but links to fields are already anchored --> $DIR/intra-links-anchors.rs:25:15 | LL | /// Or maybe [Foo::f#hola]. @@ -16,13 +16,13 @@ error: `hello#people#!` contains multiple anchors LL | /// Another anchor error: [hello#people#!]. | ^^^^^^^^^^^^^^ contains invalid anchor -error: `Enum::A#whatever` contains an anchor, but links to enum variants are already anchored +error: `Enum::A#whatever` contains an anchor, but links to variants are already anchored --> $DIR/intra-links-anchors.rs:37:28 | LL | /// Damn enum's variants: [Enum::A#whatever]. | ^^^^^^^^^^^^^^^^ contains invalid anchor -error: `u32#hello` contains an anchor, but links to primitive types are already anchored +error: `u32#hello` contains an anchor, but links to builtin types are already anchored --> $DIR/intra-links-anchors.rs:43:6 | LL | /// [u32#hello] From 418f6089e93d7f784643d55e6782055c9bb479d5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 16:37:31 -0400 Subject: [PATCH 13/25] Give a better error message when linking to a macro with the wrong disambiguator Before: ``` warning: unresolved link to `m` --> m.rs:1:6 | 1 | /// [value@m] | ^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default = note: no item named `m` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` ``` After: ``` warning: unresolved link to `m` --> m.rs:1:6 | 1 | /// [value@m] | ^^^^^^^ help: to link to the macro, use its disambiguator: `m!` | = note: `#[warn(broken_intra_doc_links)]` on by default = note: this link resolves to the macro `m`, which is not in the value namespace ``` --- .../passes/collect_intra_doc_links.rs | 119 ++++++++++++------ src/test/rustdoc-ui/intra-link-errors.rs | 21 ++++ src/test/rustdoc-ui/intra-link-errors.stderr | 52 ++++++-- 3 files changed, 142 insertions(+), 50 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1c709cead7017..7bd4b8ca854b5 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -74,8 +74,6 @@ enum ResolutionFailure<'a> { NoAssocItem(Res, Symbol), /// 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, Symbol), @@ -91,7 +89,6 @@ impl ResolutionFailure<'a> { NoPrimitiveAssocItem { res, .. } | NoAssocItem(res, _) | NoPrimitiveImpl(res, _) - | NotAnEnum(res) | NotAVariant(res, _) | WrongNamespace(res, _) | CannotHaveAssociatedItems(res, _) => Some(*res), @@ -133,6 +130,7 @@ 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; @@ -202,7 +200,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { _ => unreachable!(), } } - _ => Err(ErrorKind::Resolve(ResolutionFailure::NotAnEnum(ty_res))), + // `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, + Some(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(ErrorKind::Resolve(kind)) + } } } @@ -376,7 +395,7 @@ 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) + self.variant_field(path_str, current_item, module_id, extra_fragment) } else { // See if it only broke because of the namespace. let kind = cx.enter_resolver(|resolver| { @@ -533,7 +552,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; res.unwrap_or_else(|| { if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id) + self.variant_field(path_str, current_item, module_id, extra_fragment) } else { Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(ty_res, item_name))) } @@ -543,6 +562,41 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)) } } + + // used for reporting better errors + fn check_full_res( + &self, + ns: Namespace, + path_str: &str, + base_node: Option, + current_item: &Option, + extra_fragment: &Option, + ) -> Option { + let check_full_res_inner = |this: &Self, result: Result>| { + let res = match result { + Ok(res) => Some(res), + Err(ErrorKind::Resolve(kind)) => kind.full_res(), + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => { + Some(res) + } + Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, + }; + this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) + }; + // cannot be used for macro namespace + let check_full_res = |this: &Self, ns| { + let result = this.resolve(path_str, ns, current_item, base_node, extra_fragment); + check_full_res_inner(this, result.map(|(res, _)| res)) + }; + let check_full_res_macro = |this: &Self| { + let result = this.macro_resolve(path_str, base_node); + check_full_res_inner(this, result.map_err(ErrorKind::Resolve)) + }; + match ns { + Namespace::MacroNS => check_full_res_macro(self), + Namespace::TypeNS | Namespace::ValueNS => check_full_res(self, ns), + } + } } fn resolve_associated_trait_item( @@ -652,7 +706,7 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); // Check if these are the same type. let impl_type = trait_ref.self_ty(); - debug!( + trace!( "comparing type {} with kind {:?} against type {:?}", impl_type, impl_type.kind(), @@ -875,27 +929,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } - // used for reporting better errors - let check_full_res = |this: &mut Self, ns| { - let res = - match this.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) - { - Ok(res) => { - debug!( - "check_full_res: saw res for {} in {:?} ns: {:?}", - path_str, ns, res.0 - ); - Some(res.0) - } - Err(ErrorKind::Resolve(kind)) => kind.full_res(), - Err(ErrorKind::AnchorFailure( - AnchorFailure::RustdocAnchorConflict(res), - )) => Some(res), - Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, - }; - this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) - }; - match disambiguator.map(Disambiguator::ns) { Some(ns @ (ValueNS | TypeNS)) => { match self.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) @@ -903,12 +936,19 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Ok(res) => res, Err(ErrorKind::Resolve(mut kind)) => { // We only looked in one namespace. Try to give a better error if possible. - // TODO: handle MacroNS too if kind.full_res().is_none() { let other_ns = if ns == ValueNS { TypeNS } else { ValueNS }; - if let Some(res) = check_full_res(self, other_ns) { - // recall that this stores the _expected_ namespace - kind = ResolutionFailure::WrongNamespace(res, ns); + for &ns in &[other_ns, MacroNS] { + if let Some(res) = self.check_full_res( + ns, + path_str, + base_node, + ¤t_item, + &extra_fragment, + ) { + kind = ResolutionFailure::WrongNamespace(res, ns); + break; + } } } resolution_failure( @@ -1033,7 +1073,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Err(mut kind) => { // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible. for &ns in &[TypeNS, ValueNS] { - if let Some(res) = check_full_res(self, ns) { + if let Some(res) = self.check_full_res( + ns, + path_str, + base_node, + ¤t_item, + &extra_fragment, + ) { kind = ResolutionFailure::WrongNamespace(res, MacroNS); break; } @@ -1525,13 +1571,6 @@ fn resolution_failure( ResolutionFailure::CannotHaveAssociatedItems(res, _) => { assoc_item_not_allowed(res, diag) } - // TODO: is there ever a case where this happens? - ResolutionFailure::NotAnEnum(res) => { - let note = - format!("this link resolves to {}, which is not an enum", item(res)); - 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 {}", diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index ecf2b05e2818b..33c8d1b3c4965 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -10,6 +10,18 @@ //~| NOTE no item named `path::to` is in scope //~| HELP to escape +/// [std::io::not::here] +//~^ ERROR unresolved link +//~| NOTE the module `io` has no inner item + +/// [std::io::Error::x] +//~^ ERROR unresolved link +//~| NOTE the struct `Error` has no field + +/// [std::io::ErrorKind::x] +//~^ ERROR unresolved link +//~| NOTE the enum `ErrorKind` has no variant + /// [f::A] //~^ ERROR unresolved link //~| NOTE `f` is a function, not a module @@ -60,3 +72,12 @@ impl S { pub trait T { fn g() {} } + +/// [m()] +//~^ ERROR unresolved link +//~| HELP to link to the macro +//~| NOTE not in the value namespace +#[macro_export] +macro_rules! m { + () => {}; +} diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index ba091ff854ce6..0b9149cd8ea45 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -12,16 +12,40 @@ LL | #![deny(broken_intra_doc_links)] = note: no item named `path::to` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -error: unresolved link to `f::A` +error: unresolved link to `std::io::not::here` --> $DIR/intra-link-errors.rs:13:6 | +LL | /// [std::io::not::here] + | ^^^^^^^^^^^^^^^^^^ + | + = note: the module `io` has no inner item named `not` + +error: unresolved link to `std::io::Error::x` + --> $DIR/intra-link-errors.rs:17:6 + | +LL | /// [std::io::Error::x] + | ^^^^^^^^^^^^^^^^^ + | + = note: the struct `Error` has no field or associated item named `x` + +error: unresolved link to `std::io::ErrorKind::x` + --> $DIR/intra-link-errors.rs:21:6 + | +LL | /// [std::io::ErrorKind::x] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: the enum `ErrorKind` has no variant or associated item named `x` + +error: unresolved link to `f::A` + --> $DIR/intra-link-errors.rs:25:6 + | LL | /// [f::A] | ^^^^ | = note: `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:17:6 + --> $DIR/intra-link-errors.rs:29:6 | LL | /// [S::A] | ^^^^ @@ -29,7 +53,7 @@ LL | /// [S::A] = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:21:6 + --> $DIR/intra-link-errors.rs:33:6 | LL | /// [S::fmt] | ^^^^^^ @@ -37,7 +61,7 @@ LL | /// [S::fmt] = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:25:6 + --> $DIR/intra-link-errors.rs:37:6 | LL | /// [E::D] | ^^^^ @@ -45,7 +69,7 @@ LL | /// [E::D] = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:29:6 + --> $DIR/intra-link-errors.rs:41:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -53,7 +77,7 @@ 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:33:6 + --> $DIR/intra-link-errors.rs:45:6 | LL | /// [S!] | ^^ help: to link to the struct, use its disambiguator: `struct@S` @@ -61,7 +85,7 @@ LL | /// [S!] = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:51:6 + --> $DIR/intra-link-errors.rs:63:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` @@ -69,7 +93,7 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:56:6 + --> $DIR/intra-link-errors.rs:68:6 | LL | /// [T::h!] | ^^^^^ @@ -78,12 +102,20 @@ LL | /// [T::h!] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:43:6 + --> $DIR/intra-link-errors.rs:55:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` | = note: this link resolves to the associated function `h`, which is not in the type namespace -error: aborting due to 10 previous errors +error: unresolved link to `m` + --> $DIR/intra-link-errors.rs:76:6 + | +LL | /// [m()] + | ^^^ help: to link to the macro, use its disambiguator: `m!` + | + = note: this link resolves to the macro `m`, which is not in the value namespace + +error: aborting due to 14 previous errors From 2ca6f11663709bcbc113d3dd223ae51442460e15 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 22 Aug 2020 00:50:26 -0400 Subject: [PATCH 14/25] Fix rebase conflicts --- src/librustdoc/passes/collect_intra_doc_links.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 7bd4b8ca854b5..43003c944918e 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -938,9 +938,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // 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 }; - for &ns in &[other_ns, MacroNS] { + for &new_ns in &[other_ns, MacroNS] { if let Some(res) = self.check_full_res( - ns, + new_ns, path_str, base_node, ¤t_item, From bb9d1576044020851ff2f528879a41e1ca0b7b5d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 25 Aug 2020 17:39:50 -0400 Subject: [PATCH 15/25] Address my own review comments - Remove unneeded lifetime parameter - Comment why some code doesn't use `check_full_res` --- src/librustdoc/passes/collect_intra_doc_links.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 43003c944918e..b35ec2e73cb27 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -399,6 +399,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } 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) @@ -563,9 +564,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } - // used for reporting better errors + /// Used for reporting better errors. + /// + /// Returns whether the link resolved 'fully' in another namespace. + /// 'fully' here means that all parts of the link resolved, not just some path segments. + /// This returns the `Res` even if it was erroneous for some reason + /// (such as having invalid URL fragments or being in the wrong namespace). fn check_full_res( &self, + // TODO: is this parameter actually needed, since we return results for the wrong namespace? ns: Namespace, path_str: &str, base_node: Option, @@ -1609,10 +1616,10 @@ fn anchor_failure( }); } -fn ambiguity_error<'a>( +fn ambiguity_error( cx: &DocContext<'_>, item: &Item, - path_str: &'a str, + path_str: &str, dox: &str, link_range: Option>, candidates: Vec, From f2826d9e9bcba3c161812fca893f98b3dc010f19 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 25 Aug 2020 18:45:57 -0400 Subject: [PATCH 16/25] Show the first path segment which failed to resolve. Before, it would arbitrarily pick the third-to-last if the last three or more did not resolve. --- .../passes/collect_intra_doc_links.rs | 30 +++++++++++++++---- src/test/rustdoc-ui/intra-link-errors.rs | 5 ++-- src/test/rustdoc-ui/intra-link-errors.stderr | 14 +++++---- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index b35ec2e73cb27..098ac13ffe238 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -156,15 +156,35 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope( variant_name.to_string().into(), )))?; - let (_, ty_res) = cx + let ty_res = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) }) - .map_err(|_| { - ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string().into())) - })?; + .map(|(_, res)| res) + .unwrap_or(Res::Err); + // This code only gets hit if three path segments in a row don't get resolved. + // It's a good time to 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 Res::Err = ty_res { - return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(path.to_string().into()))); + let mut current = path.as_str(); + while let Some(parent) = current.rsplitn(2, "::").nth(1) { + current = parent; + if let Some(res) = self.check_full_res( + TypeNS, + ¤t, + Some(module_id), + current_item, + extra_fragment, + ) { + return Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem( + res, + Symbol::intern(&path), + ))); + } + } + return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope( + current.to_string().into(), + ))); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 33c8d1b3c4965..d8d4ebeb60bdb 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -4,10 +4,9 @@ // 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` is in scope +//~| NOTE no item named `path` is in scope //~| HELP to escape /// [std::io::not::here] @@ -44,7 +43,7 @@ /// [S!] //~^ ERROR unresolved link -//~| HELP to link to the struct, use its disambiguator +//~| HELP to link to the struct, prefix with the item kind //~| NOTE this link resolves to the struct `S` pub fn f() {} #[derive(Debug)] diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 0b9149cd8ea45..0275fd692feb4 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: no item named `path::to` is in scope + = note: no item named `path` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `std::io::not::here` @@ -80,15 +80,19 @@ error: unresolved link to `S` --> $DIR/intra-link-errors.rs:45:6 | LL | /// [S!] - | ^^ help: to link to the struct, use its disambiguator: `struct@S` + | ^^ | = note: this link resolves to the struct `S`, which is not in the macro namespace +help: to link to the struct, prefix with the item kind + | +LL | /// [struct@S] + | ^^^^^^^^ error: unresolved link to `T::g` --> $DIR/intra-link-errors.rs:63:6 | LL | /// [type@T::g] - | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` + | ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()` | = note: this link resolves to the associated function `g`, which is not in the type namespace @@ -105,7 +109,7 @@ error: unresolved link to `S::h` --> $DIR/intra-link-errors.rs:55:6 | LL | /// [type@S::h] - | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` + | ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()` | = note: this link resolves to the associated function `h`, which is not in the type namespace @@ -113,7 +117,7 @@ error: unresolved link to `m` --> $DIR/intra-link-errors.rs:76:6 | LL | /// [m()] - | ^^^ help: to link to the macro, use its disambiguator: `m!` + | ^^^ help: to link to the macro, add an exclamation mark: `m!` | = note: this link resolves to the macro `m`, which is not in the value namespace From d67eb1f1487c04cdd2564741828cba0784ed8501 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 28 Aug 2020 00:23:24 -0400 Subject: [PATCH 17/25] Don't suggest \[ \] if there's a :: in the path --- .../passes/collect_intra_doc_links.rs | 9 ++++-- src/test/rustdoc-ui/intra-link-errors.rs | 2 -- src/test/rustdoc-ui/intra-link-errors.stderr | 28 +++++++++---------- .../rustdoc-ui/intra-links-warning.stderr | 5 ---- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 098ac13ffe238..0ae322ee11ae3 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -592,7 +592,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// (such as having invalid URL fragments or being in the wrong namespace). fn check_full_res( &self, - // TODO: is this parameter actually needed, since we return results for the wrong namespace? ns: Namespace, path_str: &str, base_node: Option, @@ -1511,7 +1510,13 @@ fn resolution_failure( continue; } diag.note(&format!("no item named `{}` is in scope", base)); - diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); + // If the link has `::` in the path, assume it's meant to be an intra-doc link + 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 `\]`"#); + } } ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace(res, expected_ns) => { diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index d8d4ebeb60bdb..f66bb8a453305 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -7,7 +7,6 @@ /// [path::to::nonexistent::module] //~^ ERROR unresolved link //~| NOTE no item named `path` is in scope -//~| HELP to escape /// [std::io::not::here] //~^ ERROR unresolved link @@ -67,7 +66,6 @@ impl S { /// [T::h!] //~^ ERROR unresolved link //~| NOTE no item named `T::h` -//~| HELP to escape pub trait T { fn g() {} } diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 0275fd692feb4..4dac892b82051 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -10,10 +10,9 @@ note: the lint level is defined here LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: no item named `path` is in scope - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `std::io::not::here` - --> $DIR/intra-link-errors.rs:13:6 + --> $DIR/intra-link-errors.rs:12:6 | LL | /// [std::io::not::here] | ^^^^^^^^^^^^^^^^^^ @@ -21,7 +20,7 @@ LL | /// [std::io::not::here] = note: the module `io` has no inner item named `not` error: unresolved link to `std::io::Error::x` - --> $DIR/intra-link-errors.rs:17:6 + --> $DIR/intra-link-errors.rs:16:6 | LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ @@ -29,7 +28,7 @@ LL | /// [std::io::Error::x] = note: the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` - --> $DIR/intra-link-errors.rs:21:6 + --> $DIR/intra-link-errors.rs:20:6 | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +36,7 @@ LL | /// [std::io::ErrorKind::x] = note: the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:25:6 + --> $DIR/intra-link-errors.rs:24:6 | LL | /// [f::A] | ^^^^ @@ -45,7 +44,7 @@ LL | /// [f::A] = note: `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:29:6 + --> $DIR/intra-link-errors.rs:28:6 | LL | /// [S::A] | ^^^^ @@ -53,7 +52,7 @@ LL | /// [S::A] = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:33:6 + --> $DIR/intra-link-errors.rs:32:6 | LL | /// [S::fmt] | ^^^^^^ @@ -61,7 +60,7 @@ LL | /// [S::fmt] = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:37:6 + --> $DIR/intra-link-errors.rs:36:6 | LL | /// [E::D] | ^^^^ @@ -69,7 +68,7 @@ LL | /// [E::D] = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:41:6 + --> $DIR/intra-link-errors.rs:40:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -77,7 +76,7 @@ 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:45:6 + --> $DIR/intra-link-errors.rs:44:6 | LL | /// [S!] | ^^ @@ -89,7 +88,7 @@ LL | /// [struct@S] | ^^^^^^^^ error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:63:6 + --> $DIR/intra-link-errors.rs:62:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()` @@ -97,16 +96,15 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:68:6 + --> $DIR/intra-link-errors.rs:67:6 | LL | /// [T::h!] | ^^^^^ | = note: no item named `T::h` is in scope - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:55:6 + --> $DIR/intra-link-errors.rs:54:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()` @@ -114,7 +112,7 @@ LL | /// [type@S::h] = note: this link resolves to the associated function `h`, which is not in the type namespace error: unresolved link to `m` - --> $DIR/intra-link-errors.rs:76:6 + --> $DIR/intra-link-errors.rs:74:6 | LL | /// [m()] | ^^^ help: to link to the macro, add an exclamation mark: `m!` diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 988a65c7a591c..25c6975d3a509 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -14,7 +14,6 @@ LL | //! Test with [Foo::baz], [Bar::foo], ... | ^^^^^^^^ | = 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 @@ -23,7 +22,6 @@ LL | //! , [Uniooon::X] and [Qux::Z]. | ^^^^^^^^^^ | = 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 @@ -32,7 +30,6 @@ LL | //! , [Uniooon::X] and [Qux::Z]. | ^^^^^^ | = 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 @@ -41,7 +38,6 @@ LL | //! , [Uniooon::X] and [Qux::Z]. | ^^^^^^^^^^ | = 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 @@ -50,7 +46,6 @@ LL | //! , [Uniooon::X] and [Qux::Z]. | ^^^^^^ | = 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 From ee683ef8532034a4bee01e9aa8fd92dbe38ac6f1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 28 Aug 2020 00:53:36 -0400 Subject: [PATCH 18/25] Say 'prefix with `kind@`' instead of 'prefix with the item kind' This is both more specific and easier to read. --- .../passes/collect_intra_doc_links.rs | 57 +++++++++++++------ src/test/rustdoc-ui/intra-link-errors.rs | 2 +- src/test/rustdoc-ui/intra-link-errors.stderr | 6 +- .../rustdoc-ui/intra-link-prim-conflict.rs | 4 +- .../intra-link-prim-conflict.stderr | 20 ++----- .../rustdoc-ui/intra-links-ambiguity.stderr | 12 ++-- .../intra-links-disambiguator-mismatch.rs | 18 +++--- .../intra-links-disambiguator-mismatch.stderr | 54 +++--------------- 8 files changed, 73 insertions(+), 100 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0ae322ee11ae3..a19387ad992fa 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1328,21 +1328,16 @@ impl Disambiguator { } } - /// Return (description of the change, suggestion) - fn suggestion_for(self, path_str: &str) -> (&'static str, String) { - const PREFIX: &str = "prefix with the item kind"; - const FUNCTION: &str = "add parentheses"; - const MACRO: &str = "add an exclamation mark"; - + fn suggestion(self) -> Suggestion { let kind = match self { - Disambiguator::Primitive => return (PREFIX, format!("prim@{}", path_str)), + Disambiguator::Primitive => return Suggestion::Prefix("prim"), Disambiguator::Kind(kind) => kind, Disambiguator::Namespace(_) => panic!("display_for cannot be used on namespaces"), }; if kind == DefKind::Macro(MacroKind::Bang) { - return (MACRO, format!("{}!", path_str)); + return Suggestion::Macro; } else if kind == DefKind::Fn || kind == DefKind::AssocFn { - return (FUNCTION, format!("{}()", path_str)); + return Suggestion::Function; } let prefix = match kind { @@ -1367,8 +1362,7 @@ impl Disambiguator { }, }; - // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` - (PREFIX, format!("{}@{}", prefix, path_str)) + Suggestion::Prefix(prefix) } fn ns(self) -> Namespace { @@ -1400,6 +1394,31 @@ impl Disambiguator { } } +enum Suggestion { + Prefix(&'static str), + Function, + Macro, +} + +impl Suggestion { + fn descr(&self) -> Cow<'static, str> { + match self { + Self::Prefix(x) => format!("prefix with `{}@`", x).into(), + Self::Function => "add parentheses".into(), + Self::Macro => "add an exclamation mark".into(), + } + } + + fn as_help(&self, path_str: &str) -> String { + // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` + match self { + Self::Prefix(prefix) => format!("{}@{}", prefix, path_str), + Self::Function => format!("{}()", path_str), + Self::Macro => format!("{}!", path_str), + } + } +} + /// Reports a diagnostic for an intra-doc link. /// /// If no link range is provided, or the source span of the link cannot be determined, the span of @@ -1695,18 +1714,20 @@ fn suggest_disambiguator( sp: Option, link_range: &Option>, ) { - let (action, mut suggestion) = disambiguator.suggestion_for(path_str); - let help = format!("to link to the {}, {}", disambiguator.descr(), action); + let suggestion = disambiguator.suggestion(); + let help = format!("to link to the {}, {}", disambiguator.descr(), suggestion.descr()); if let Some(sp) = sp { let link_range = link_range.as_ref().expect("must have a link range if we have a span"); - if dox.bytes().nth(link_range.start) == Some(b'`') { - suggestion = format!("`{}`", suggestion); - } + let msg = if dox.bytes().nth(link_range.start) == Some(b'`') { + format!("`{}`", suggestion.as_help(path_str)) + } else { + suggestion.as_help(path_str) + }; - diag.span_suggestion(sp, &help, suggestion, Applicability::MaybeIncorrect); + diag.span_suggestion(sp, &help, msg, Applicability::MaybeIncorrect); } else { - diag.help(&format!("{}: {}", help, suggestion)); + diag.help(&format!("{}: {}", help, suggestion.as_help(path_str))); } } diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index f66bb8a453305..919c46fe1dab2 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -42,7 +42,7 @@ /// [S!] //~^ ERROR unresolved link -//~| HELP to link to the struct, prefix with the item kind +//~| HELP to link to the struct, prefix with `struct@` //~| NOTE this link resolves to the struct `S` pub fn f() {} #[derive(Debug)] diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 4dac892b82051..7318193964f8d 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -79,13 +79,9 @@ error: unresolved link to `S` --> $DIR/intra-link-errors.rs:44:6 | LL | /// [S!] - | ^^ + | ^^ help: to link to the struct, prefix with `struct@`: `struct@S` | = note: this link resolves to the struct `S`, which is not in the macro namespace -help: to link to the struct, prefix with the item kind - | -LL | /// [struct@S] - | ^^^^^^^^ error: unresolved link to `T::g` --> $DIR/intra-link-errors.rs:62:6 diff --git a/src/test/rustdoc-ui/intra-link-prim-conflict.rs b/src/test/rustdoc-ui/intra-link-prim-conflict.rs index 548d3e2544a00..85738ceae8e61 100644 --- a/src/test/rustdoc-ui/intra-link-prim-conflict.rs +++ b/src/test/rustdoc-ui/intra-link-prim-conflict.rs @@ -18,13 +18,13 @@ /// [struct@char] //~^ ERROR incompatible link -//~| HELP prefix with the item kind +//~| HELP prefix with `mod@` //~| NOTE resolved to a module pub mod char {} pub mod inner { //! [struct@char] //~^ ERROR incompatible link - //~| HELP prefix with the item kind + //~| HELP prefix with `prim@` //~| NOTE resolved to a builtin type } diff --git a/src/test/rustdoc-ui/intra-link-prim-conflict.stderr b/src/test/rustdoc-ui/intra-link-prim-conflict.stderr index 53dccfbf1a2c4..43587a80021af 100644 --- a/src/test/rustdoc-ui/intra-link-prim-conflict.stderr +++ b/src/test/rustdoc-ui/intra-link-prim-conflict.stderr @@ -9,11 +9,11 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ -help: to link to the module, prefix with the item kind +help: to link to the module, prefix with `mod@` | LL | /// [mod@char] | ^^^^^^^^ -help: to link to the builtin type, prefix with the item kind +help: to link to the builtin type, prefix with `prim@` | LL | /// [prim@char] | ^^^^^^^^^ @@ -24,11 +24,11 @@ error: `char` is both a module and a builtin type LL | /// [type@char] | ^^^^^^^^^ ambiguous link | -help: to link to the module, prefix with the item kind +help: to link to the module, prefix with `mod@` | LL | /// [mod@char] | ^^^^^^^^ -help: to link to the builtin type, prefix with the item kind +help: to link to the builtin type, prefix with `prim@` | LL | /// [prim@char] | ^^^^^^^^^ @@ -37,25 +37,17 @@ error: incompatible link kind for `char` --> $DIR/intra-link-prim-conflict.rs:19:6 | LL | /// [struct@char] - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: to link to the module, prefix with `mod@`: `mod@char` | = note: this link resolved to a module, which is not a struct -help: to link to the module, prefix with the item kind - | -LL | /// [mod@char] - | ^^^^^^^^ error: incompatible link kind for `char` --> $DIR/intra-link-prim-conflict.rs:26:10 | LL | //! [struct@char] - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: to link to the builtin type, prefix with `prim@`: `prim@char` | = note: this link resolved to a builtin type, which is not a struct -help: to link to the builtin type, prefix with the item kind - | -LL | //! [prim@char] - | ^^^^^^^^^ error: aborting due to 4 previous errors diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.stderr b/src/test/rustdoc-ui/intra-links-ambiguity.stderr index 7912c046f1c78..17891ca05efa1 100644 --- a/src/test/rustdoc-ui/intra-links-ambiguity.stderr +++ b/src/test/rustdoc-ui/intra-links-ambiguity.stderr @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ -help: to link to the struct, prefix with the item kind +help: to link to the struct, prefix with `struct@` | LL | /// [`struct@ambiguous`] is ambiguous. | ^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ error: `ambiguous` is both a struct and a function LL | /// [ambiguous] is ambiguous. | ^^^^^^^^^ ambiguous link | -help: to link to the struct, prefix with the item kind +help: to link to the struct, prefix with `struct@` | LL | /// [struct@ambiguous] is ambiguous. | ^^^^^^^^^^^^^^^^ @@ -39,7 +39,7 @@ error: `multi_conflict` is a struct, a function, and a macro LL | /// [`multi_conflict`] is a three-way conflict. | ^^^^^^^^^^^^^^^^ ambiguous link | -help: to link to the struct, prefix with the item kind +help: to link to the struct, prefix with `struct@` | LL | /// [`struct@multi_conflict`] is a three-way conflict. | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -58,11 +58,11 @@ error: `type_and_value` is both a module and a constant LL | /// Ambiguous [type_and_value]. | ^^^^^^^^^^^^^^ ambiguous link | -help: to link to the module, prefix with the item kind +help: to link to the module, prefix with `mod@` | LL | /// Ambiguous [mod@type_and_value]. | ^^^^^^^^^^^^^^^^^^ -help: to link to the constant, prefix with the item kind +help: to link to the constant, prefix with `const@` | LL | /// Ambiguous [const@type_and_value]. | ^^^^^^^^^^^^^^^^^^^^ @@ -73,7 +73,7 @@ error: `foo::bar` is both an enum and a function LL | /// Ambiguous non-implied shortcut link [`foo::bar`]. | ^^^^^^^^^^ ambiguous link | -help: to link to the enum, prefix with the item kind +help: to link to the enum, prefix with `enum@` | LL | /// Ambiguous non-implied shortcut link [`enum@foo::bar`]. | ^^^^^^^^^^^^^^^ diff --git a/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.rs b/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.rs index 54e507adfe550..b9c8e033b1b21 100644 --- a/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.rs +++ b/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.rs @@ -14,27 +14,27 @@ trait T {} /// Link to [struct@S] //~^ ERROR incompatible link kind for `S` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `enum@` /// Link to [mod@S] //~^ ERROR incompatible link kind for `S` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `enum@` /// Link to [union@S] //~^ ERROR incompatible link kind for `S` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `enum@` /// Link to [trait@S] //~^ ERROR incompatible link kind for `S` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `enum@` /// Link to [struct@T] //~^ ERROR incompatible link kind for `T` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `trait@` /// Link to [derive@m] //~^ ERROR incompatible link kind for `m` @@ -44,22 +44,22 @@ trait T {} /// Link to [const@s] //~^ ERROR incompatible link kind for `s` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `static@` /// Link to [static@c] //~^ ERROR incompatible link kind for `c` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `const@` /// Link to [fn@c] //~^ ERROR incompatible link kind for `c` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `const@` /// Link to [c()] //~^ ERROR incompatible link kind for `c` //~| NOTE this link resolved -//~| HELP prefix with the item kind +//~| HELP prefix with `const@` /// Link to [const@f] //~^ ERROR incompatible link kind for `f` diff --git a/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.stderr b/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.stderr index 27b94af0378c2..2e732baf6e01e 100644 --- a/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.stderr +++ b/src/test/rustdoc-ui/intra-links-disambiguator-mismatch.stderr @@ -2,7 +2,7 @@ error: incompatible link kind for `S` --> $DIR/intra-links-disambiguator-mismatch.rs:14:14 | LL | /// Link to [struct@S] - | ^^^^^^^^ + | ^^^^^^^^ help: to link to the enum, prefix with `enum@`: `enum@S` | note: the lint level is defined here --> $DIR/intra-links-disambiguator-mismatch.rs:1:9 @@ -10,58 +10,38 @@ note: the lint level is defined here LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: this link resolved to an enum, which is not a struct -help: to link to the enum, prefix with the item kind - | -LL | /// Link to [enum@S] - | ^^^^^^ error: incompatible link kind for `S` --> $DIR/intra-links-disambiguator-mismatch.rs:19:14 | LL | /// Link to [mod@S] - | ^^^^^ + | ^^^^^ help: to link to the enum, prefix with `enum@`: `enum@S` | = note: this link resolved to an enum, which is not a module -help: to link to the enum, prefix with the item kind - | -LL | /// Link to [enum@S] - | ^^^^^^ error: incompatible link kind for `S` --> $DIR/intra-links-disambiguator-mismatch.rs:24:14 | LL | /// Link to [union@S] - | ^^^^^^^ + | ^^^^^^^ help: to link to the enum, prefix with `enum@`: `enum@S` | = note: this link resolved to an enum, which is not a union -help: to link to the enum, prefix with the item kind - | -LL | /// Link to [enum@S] - | ^^^^^^ error: incompatible link kind for `S` --> $DIR/intra-links-disambiguator-mismatch.rs:29:14 | LL | /// Link to [trait@S] - | ^^^^^^^ + | ^^^^^^^ help: to link to the enum, prefix with `enum@`: `enum@S` | = note: this link resolved to an enum, which is not a trait -help: to link to the enum, prefix with the item kind - | -LL | /// Link to [enum@S] - | ^^^^^^ error: incompatible link kind for `T` --> $DIR/intra-links-disambiguator-mismatch.rs:34:14 | LL | /// Link to [struct@T] - | ^^^^^^^^ + | ^^^^^^^^ help: to link to the trait, prefix with `trait@`: `trait@T` | = note: this link resolved to a trait, which is not a struct -help: to link to the trait, prefix with the item kind - | -LL | /// Link to [trait@T] - | ^^^^^^^ error: incompatible link kind for `m` --> $DIR/intra-links-disambiguator-mismatch.rs:39:14 @@ -75,49 +55,33 @@ error: incompatible link kind for `s` --> $DIR/intra-links-disambiguator-mismatch.rs:44:14 | LL | /// Link to [const@s] - | ^^^^^^^ + | ^^^^^^^ help: to link to the static, prefix with `static@`: `static@s` | = note: this link resolved to a static, which is not a constant -help: to link to the static, prefix with the item kind - | -LL | /// Link to [static@s] - | ^^^^^^^^ error: incompatible link kind for `c` --> $DIR/intra-links-disambiguator-mismatch.rs:49:14 | LL | /// Link to [static@c] - | ^^^^^^^^ + | ^^^^^^^^ help: to link to the constant, prefix with `const@`: `const@c` | = note: this link resolved to a constant, which is not a static -help: to link to the constant, prefix with the item kind - | -LL | /// Link to [const@c] - | ^^^^^^^ error: incompatible link kind for `c` --> $DIR/intra-links-disambiguator-mismatch.rs:54:14 | LL | /// Link to [fn@c] - | ^^^^ + | ^^^^ help: to link to the constant, prefix with `const@`: `const@c` | = note: this link resolved to a constant, which is not a function -help: to link to the constant, prefix with the item kind - | -LL | /// Link to [const@c] - | ^^^^^^^ error: incompatible link kind for `c` --> $DIR/intra-links-disambiguator-mismatch.rs:59:14 | LL | /// Link to [c()] - | ^^^ + | ^^^ help: to link to the constant, prefix with `const@`: `const@c` | = note: this link resolved to a constant, which is not a function -help: to link to the constant, prefix with the item kind - | -LL | /// Link to [const@c] - | ^^^^^^^ error: incompatible link kind for `f` --> $DIR/intra-links-disambiguator-mismatch.rs:64:14 From efdc3facdfe931108146587b2f9d22b7dfd217c5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 28 Aug 2020 01:31:33 -0400 Subject: [PATCH 19/25] Give a much better error message when an item has a macro disambiguator Previously, this called `check_full_res` for values and types, but not macros. This would result in not showing when there was a partial resolution for a parent of the item. This now calls `check_full_res`. Additionally, it checks if there was a disambiguator, and if so, says that specific kind wasn't found instead of saying generically 'associated item'. --- .../passes/collect_intra_doc_links.rs | 87 ++++++++++++------- src/test/rustdoc-ui/intra-link-errors.rs | 2 +- src/test/rustdoc-ui/intra-link-errors.stderr | 2 +- 3 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a19387ad992fa..df8ecf26a412e 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -262,11 +262,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { false, ) { if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind { - return Ok(res.map_id(|_| panic!("unexpected id"))); + return Some(Ok(res.map_id(|_| panic!("unexpected id")))); } } if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) { - return Ok(res.map_id(|_| panic!("unexpected id"))); + return Some(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); @@ -276,14 +276,32 @@ 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 Ok(res); + return Some(Ok(res)); } } } else { debug!("attempting to resolve item without parent module: {}", path_str); - return Err(ResolutionFailure::NoParentItem); + return Some(Err(ResolutionFailure::NoParentItem)); } - return Err(ResolutionFailure::NotInScope(path_str.into())); + 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, parent_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(path_str.into())) }) } /// Resolves a string as a path within a particular namespace. Also returns an optional @@ -981,6 +999,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { cx, &item, path_str, + disambiguator, &dox, link_range, smallvec![kind], @@ -1060,6 +1079,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { cx, &item, path_str, + disambiguator, &dox, link_range, candidates.into_iter().filter_map(|res| res.err()).collect(), @@ -1114,6 +1134,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { cx, &item, path_str, + disambiguator, &dox, link_range, smallvec![kind], @@ -1489,6 +1510,7 @@ fn resolution_failure( cx: &DocContext<'_>, item: &Item, path_str: &str, + disambiguator: Option, dox: &str, link_range: Option>, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, @@ -1581,34 +1603,39 @@ fn resolution_failure( let (kind, def_id) = match res { Res::Def(kind, def_id) => (kind, def_id), - _ => unreachable!( - "primitives are covered above and other `Res` variants aren't possible at module scope" + x => unreachable!( + "primitives are covered above and other `Res` variants aren't possible at module scope: {:?}", + x, ), }; let name = cx.tcx.item_name(def_id); - let path_description = 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 => return assoc_item_not_allowed(res, diag), - Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam - | Static => "associated item", - Impl | GlobalAsm => unreachable!("not a path"), + 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 => return assoc_item_not_allowed(res, diag), + Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam + | Static => "associated item", + Impl | GlobalAsm => unreachable!("not a path"), + } }; let note = format!( "the {} `{}` has no {} named `{}`", diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 919c46fe1dab2..ae89f418984d2 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -65,7 +65,7 @@ impl S { /// [T::h!] //~^ ERROR unresolved link -//~| NOTE no item named `T::h` +//~| NOTE `T` has no macro named `h` pub trait T { fn g() {} } diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 7318193964f8d..68941d64c7a2c 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -97,7 +97,7 @@ error: unresolved link to `T::h` LL | /// [T::h!] | ^^^^^ | - = note: no item named `T::h` is in scope + = note: the trait `T` has no macro named `h` error: unresolved link to `S::h` --> $DIR/intra-link-errors.rs:54:6 From 8318a185f301de617c64376fbd3a2d556296c8bb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 29 Aug 2020 09:59:57 -0400 Subject: [PATCH 20/25] x.py bless --- src/test/rustdoc-ui/intra-link-errors.stderr | 28 ++++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 68941d64c7a2c..73e16bef0aa9f 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -1,5 +1,5 @@ error: unresolved link to `path::to::nonexistent::module` - --> $DIR/intra-link-errors.rs:8:6 + --> $DIR/intra-link-errors.rs:7:6 | LL | /// [path::to::nonexistent::module] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -12,7 +12,7 @@ LL | #![deny(broken_intra_doc_links)] = note: no item named `path` is in scope error: unresolved link to `std::io::not::here` - --> $DIR/intra-link-errors.rs:12:6 + --> $DIR/intra-link-errors.rs:11:6 | LL | /// [std::io::not::here] | ^^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ LL | /// [std::io::not::here] = note: the module `io` has no inner item named `not` error: unresolved link to `std::io::Error::x` - --> $DIR/intra-link-errors.rs:16:6 + --> $DIR/intra-link-errors.rs:15:6 | LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ @@ -28,7 +28,7 @@ LL | /// [std::io::Error::x] = note: the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` - --> $DIR/intra-link-errors.rs:20:6 + --> $DIR/intra-link-errors.rs:19:6 | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +36,7 @@ LL | /// [std::io::ErrorKind::x] = note: the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:24:6 + --> $DIR/intra-link-errors.rs:23:6 | LL | /// [f::A] | ^^^^ @@ -44,7 +44,7 @@ LL | /// [f::A] = note: `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:28:6 + --> $DIR/intra-link-errors.rs:27:6 | LL | /// [S::A] | ^^^^ @@ -52,7 +52,7 @@ LL | /// [S::A] = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:32:6 + --> $DIR/intra-link-errors.rs:31:6 | LL | /// [S::fmt] | ^^^^^^ @@ -60,7 +60,7 @@ LL | /// [S::fmt] = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:36:6 + --> $DIR/intra-link-errors.rs:35:6 | LL | /// [E::D] | ^^^^ @@ -68,7 +68,7 @@ LL | /// [E::D] = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:40:6 + --> $DIR/intra-link-errors.rs:39:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -76,7 +76,7 @@ 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:44:6 + --> $DIR/intra-link-errors.rs:43:6 | LL | /// [S!] | ^^ help: to link to the struct, prefix with `struct@`: `struct@S` @@ -84,7 +84,7 @@ LL | /// [S!] = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:62:6 + --> $DIR/intra-link-errors.rs:61:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()` @@ -92,7 +92,7 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:67:6 + --> $DIR/intra-link-errors.rs:66:6 | LL | /// [T::h!] | ^^^^^ @@ -100,7 +100,7 @@ LL | /// [T::h!] = note: the trait `T` has no macro named `h` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:54:6 + --> $DIR/intra-link-errors.rs:53:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()` @@ -108,7 +108,7 @@ LL | /// [type@S::h] = note: this link resolves to the associated function `h`, which is not in the type namespace error: unresolved link to `m` - --> $DIR/intra-link-errors.rs:74:6 + --> $DIR/intra-link-errors.rs:73:6 | LL | /// [m()] | ^^^ help: to link to the macro, add an exclamation mark: `m!` From cd72d9029ff5b2368e5c539f9b326a2eea855127 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 31 Aug 2020 23:28:38 -0400 Subject: [PATCH 21/25] Find the first segment that failed to resolve for _any_ namespace Moves this detection into `resolution_failure` to avoid doing unnecessary work and make the control flow a little easier to work with. --- .../passes/collect_intra_doc_links.rs | 109 ++++++++++-------- src/test/rustdoc-ui/intra-link-errors.rs | 8 ++ src/test/rustdoc-ui/intra-link-errors.stderr | 44 ++++--- 3 files changed, 102 insertions(+), 59 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index df8ecf26a412e..d8abf411de7c2 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -60,8 +60,8 @@ enum ResolutionFailure<'a> { /// This has a partial resolution, but is not in the TypeNS and so cannot /// have associated items or fields. CannotHaveAssociatedItems(Res, Namespace), - /// `String` is the base name of the path (not necessarily the whole link) - NotInScope(Cow<'a, str>), + /// `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 @@ -92,7 +92,7 @@ impl ResolutionFailure<'a> { | NotAVariant(res, _) | WrongNamespace(res, _) | CannotHaveAssociatedItems(res, _) => Some(*res), - NotInScope(_) | NoParentItem | Dummy => None, + NotInScope { .. } | NoParentItem | Dummy => None, } } @@ -142,7 +142,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .expect("fold_item should ensure link is non-empty"); let 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(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.into())))?; + split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope{ + module_id, + name: path_str.into(), + }))?; let path = split .next() .map(|f| { @@ -153,38 +156,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope( - variant_name.to_string().into(), - )))?; + .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope { + module_id, + name: variant_name.to_string().into(), + }))?; let ty_res = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) }) .map(|(_, res)| res) .unwrap_or(Res::Err); - // This code only gets hit if three path segments in a row don't get resolved. - // It's a good time to 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 Res::Err = ty_res { - let mut current = path.as_str(); - while let Some(parent) = current.rsplitn(2, "::").nth(1) { - current = parent; - if let Some(res) = self.check_full_res( - TypeNS, - ¤t, - Some(module_id), - current_item, - extra_fragment, - ) { - return Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem( - res, - Symbol::intern(&path), - ))); - } - } - return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope( - current.to_string().into(), - ))); + return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope { + module_id, + name: path.into(), + })); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { @@ -301,7 +287,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }); } } - Err(ResolutionFailure::NotInScope(path_str.into())) + Err(ResolutionFailure::NotInScope { + module_id: parent_id.expect("already saw `Some` when resolving as a macro"), + name: path_str.into(), + }) }) } /// Resolves a string as a path within a particular namespace. Also returns an optional @@ -384,7 +373,10 @@ 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); - ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string().into())) + ErrorKind::Resolve(ResolutionFailure::NotInScope { + module_id, + name: item_name.to_string().into(), + }) })?; if let Some((path, prim)) = is_primitive(&path_root, TypeNS) { @@ -451,7 +443,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } } - ResolutionFailure::NotInScope(path_root.into()) + ResolutionFailure::NotInScope { module_id, name: path_root.into() } }); Err(ErrorKind::Resolve(kind)) }; @@ -996,7 +988,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } resolution_failure( - cx, + self, &item, path_str, disambiguator, @@ -1076,7 +1068,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { if len == 0 { drop(candidates_iter); resolution_failure( - cx, + self, &item, path_str, disambiguator, @@ -1096,8 +1088,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } else { drop(candidates_iter); if is_derive_trait_collision(&candidates) { - candidates.macro_ns = - Err(ResolutionFailure::NotInScope(path_str.into())); + candidates.macro_ns = Err(ResolutionFailure::Dummy); } // If we're reporting an ambiguity, don't mention the namespaces that failed let candidates = @@ -1131,7 +1122,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } resolution_failure( - cx, + self, &item, path_str, disambiguator, @@ -1507,7 +1498,7 @@ fn report_diagnostic( } fn resolution_failure( - cx: &DocContext<'_>, + collector: &LinkCollector<'_, '_>, item: &Item, path_str: &str, disambiguator: Option, @@ -1516,7 +1507,7 @@ fn resolution_failure( kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { report_diagnostic( - cx, + collector.cx, &format!("unresolved link to `{}`", path_str), item, dox, @@ -1524,11 +1515,15 @@ fn resolution_failure( |diag, sp| { let in_scope = kinds.iter().any(|kind| kind.res().is_some()); let item = |res: Res| { - format!("the {} `{}`", res.descr(), cx.tcx.item_name(res.def_id()).to_string()) + format!( + "the {} `{}`", + res.descr(), + collector.cx.tcx.item_name(res.def_id()).to_string() + ) }; let assoc_item_not_allowed = |res: Res, diag: &mut DiagnosticBuilder<'_>| { let def_id = res.def_id(); - let name = cx.tcx.item_name(def_id); + let name = collector.cx.tcx.item_name(def_id); let note = format!( "`{}` is {} {}, not a module or type, and cannot have associated items", name, @@ -1539,18 +1534,42 @@ fn resolution_failure( }; // ignore duplicates let mut variants_seen = SmallVec::<[_; 3]>::new(); - for failure in kinds { + 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, + Some(*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); match failure { - ResolutionFailure::NotInScope(base) => { + ResolutionFailure::NotInScope { name, .. } => { if in_scope { continue; } - diag.note(&format!("no item named `{}` is in scope", base)); + diag.note(&format!("no item named `{}` is in scope", name)); // If the link has `::` in the path, assume it's meant to be an intra-doc link if !path_str.contains("::") { // Otherwise, the `[]` might be unrelated. @@ -1608,7 +1627,7 @@ fn resolution_failure( x, ), }; - let name = cx.tcx.item_name(def_id); + let name = collector.cx.tcx.item_name(def_id); let path_description = if let Some(disambiguator) = disambiguator { disambiguator.descr() } else { diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index ae89f418984d2..477ad6464f887 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -8,6 +8,14 @@ //~^ ERROR unresolved link //~| NOTE no item named `path` is in scope +/// [path::to::nonexistent::macro!] +//~^ ERROR unresolved link +//~| NOTE no item named `path` is in scope + +/// [type@path::to::nonexistent::type] +//~^ ERROR unresolved link +//~| NOTE no item named `path` is in scope + /// [std::io::not::here] //~^ ERROR unresolved link //~| NOTE the module `io` has no inner item diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 73e16bef0aa9f..3b1a09f913e9f 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -11,16 +11,32 @@ LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: no item named `path` is in scope -error: unresolved link to `std::io::not::here` +error: unresolved link to `path::to::nonexistent::macro` --> $DIR/intra-link-errors.rs:11:6 | +LL | /// [path::to::nonexistent::macro!] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: no item named `path` is in scope + +error: unresolved link to `path::to::nonexistent::type` + --> $DIR/intra-link-errors.rs:15:6 + | +LL | /// [type@path::to::nonexistent::type] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: no item named `path` is in scope + +error: unresolved link to `std::io::not::here` + --> $DIR/intra-link-errors.rs:19:6 + | LL | /// [std::io::not::here] | ^^^^^^^^^^^^^^^^^^ | = note: the module `io` has no inner item named `not` error: unresolved link to `std::io::Error::x` - --> $DIR/intra-link-errors.rs:15:6 + --> $DIR/intra-link-errors.rs:23:6 | LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ @@ -28,7 +44,7 @@ LL | /// [std::io::Error::x] = note: the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` - --> $DIR/intra-link-errors.rs:19:6 + --> $DIR/intra-link-errors.rs:27:6 | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +52,7 @@ LL | /// [std::io::ErrorKind::x] = note: the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:23:6 + --> $DIR/intra-link-errors.rs:31:6 | LL | /// [f::A] | ^^^^ @@ -44,7 +60,7 @@ LL | /// [f::A] = note: `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:27:6 + --> $DIR/intra-link-errors.rs:35:6 | LL | /// [S::A] | ^^^^ @@ -52,7 +68,7 @@ LL | /// [S::A] = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:31:6 + --> $DIR/intra-link-errors.rs:39:6 | LL | /// [S::fmt] | ^^^^^^ @@ -60,7 +76,7 @@ LL | /// [S::fmt] = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:35:6 + --> $DIR/intra-link-errors.rs:43:6 | LL | /// [E::D] | ^^^^ @@ -68,7 +84,7 @@ LL | /// [E::D] = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:39:6 + --> $DIR/intra-link-errors.rs:47:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -76,7 +92,7 @@ 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:43:6 + --> $DIR/intra-link-errors.rs:51:6 | LL | /// [S!] | ^^ help: to link to the struct, prefix with `struct@`: `struct@S` @@ -84,7 +100,7 @@ LL | /// [S!] = note: this link resolves to the struct `S`, which is not in the macro namespace error: unresolved link to `T::g` - --> $DIR/intra-link-errors.rs:61:6 + --> $DIR/intra-link-errors.rs:69:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()` @@ -92,7 +108,7 @@ LL | /// [type@T::g] = note: this link resolves to the associated function `g`, which is not in the type namespace error: unresolved link to `T::h` - --> $DIR/intra-link-errors.rs:66:6 + --> $DIR/intra-link-errors.rs:74:6 | LL | /// [T::h!] | ^^^^^ @@ -100,7 +116,7 @@ LL | /// [T::h!] = note: the trait `T` has no macro named `h` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:53:6 + --> $DIR/intra-link-errors.rs:61:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()` @@ -108,12 +124,12 @@ LL | /// [type@S::h] = note: this link resolves to the associated function `h`, which is not in the type namespace error: unresolved link to `m` - --> $DIR/intra-link-errors.rs:73:6 + --> $DIR/intra-link-errors.rs:81:6 | LL | /// [m()] | ^^^ help: to link to the macro, add an exclamation mark: `m!` | = note: this link resolves to the macro `m`, which is not in the value namespace -error: aborting due to 14 previous errors +error: aborting due to 16 previous errors From c213c68500f5fff51f26997d27a346c896232df7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Sep 2020 13:29:55 -0400 Subject: [PATCH 22/25] box ResolutionFailures on the heap This decreases the size of the `Result`s being returned, improving performance in the common case. --- .../passes/collect_intra_doc_links.rs | 69 +++++++++---------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d8abf411de7c2..7fea70253b301 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -48,10 +48,16 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { } enum ErrorKind<'a> { - Resolve(ResolutionFailure<'a>), + Resolve(Box>), AnchorFailure(AnchorFailure), } +impl<'a> From> for ErrorKind<'a> { + fn from(err: ResolutionFailure<'a>) -> Self { + ErrorKind::Resolve(box err) + } +} + #[derive(Debug)] enum ResolutionFailure<'a> { /// This resolved, but with the wrong namespace. @@ -142,10 +148,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .expect("fold_item should ensure link is non-empty"); let 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(ErrorKind::Resolve(ResolutionFailure::NotInScope{ + split.next().map(|f| Symbol::intern(f)).ok_or_else(|| ResolutionFailure::NotInScope { module_id, name: path_str.into(), - }))?; + })?; let path = split .next() .map(|f| { @@ -156,10 +162,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } f.to_owned() }) - .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope { + .ok_or_else(|| ResolutionFailure::NotInScope { module_id, name: variant_name.to_string().into(), - }))?; + })?; let ty_res = cx .enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) @@ -167,10 +173,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .map(|(_, res)| res) .unwrap_or(Res::Err); if let Res::Err = ty_res { - return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope { - module_id, - name: path.into(), - })); + return Err(ResolutionFailure::NotInScope { module_id, name: path.into() }.into()); } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { @@ -184,7 +187,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { { // 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)); + return Err(ResolutionFailure::Dummy.into()); } match cx.tcx.type_of(did).kind() { ty::Adt(def, _) if def.is_enum() => { @@ -197,10 +200,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { )), )) } else { - Err(ErrorKind::Resolve(ResolutionFailure::NotAVariant( - ty_res, - variant_field_name, - ))) + Err(ResolutionFailure::NotAVariant(ty_res, variant_field_name).into()) } } _ => unreachable!(), @@ -226,7 +226,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Even with the shorter path, it didn't resolve, so say that. ResolutionFailure::NoAssocItem(ty_res, variant_name) }; - Err(ErrorKind::Resolve(kind)) + Err(kind.into()) } } } @@ -344,7 +344,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if value != (ns == ValueNS) { - return Err(ErrorKind::Resolve(ResolutionFailure::WrongNamespace(res, ns))); + return Err(ResolutionFailure::WrongNamespace(res, ns).into()); } } else if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { @@ -373,16 +373,12 @@ 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); - ErrorKind::Resolve(ResolutionFailure::NotInScope { - module_id, - name: item_name.to_string().into(), - }) + ResolutionFailure::NotInScope { module_id, name: item_name.to_string().into() } })?; if let Some((path, prim)) = is_primitive(&path_root, TypeNS) { - let impls = primitive_impl(cx, &path).ok_or_else(|| { - ErrorKind::Resolve(ResolutionFailure::NoPrimitiveImpl(prim, path_root.into())) - })?; + let impls = primitive_impl(cx, &path) + .ok_or_else(|| ResolutionFailure::NoPrimitiveImpl(prim, path_root.into()))?; for &impl_ in impls { let link = cx .tcx @@ -409,11 +405,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { item_name, ns.descr() ); - return Err(ErrorKind::Resolve(ResolutionFailure::NoPrimitiveAssocItem { + return Err(ResolutionFailure::NoPrimitiveAssocItem { res: prim, prim_name: path, assoc_item: item_name, - })); + } + .into()); } let ty_res = cx @@ -445,7 +442,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } ResolutionFailure::NotInScope { module_id, name: path_root.into() } }); - Err(ErrorKind::Resolve(kind)) + Err(kind.into()) }; } Ok(res) => res, @@ -548,9 +545,7 @@ 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::Resolve(ResolutionFailure::NoAssocItem( - ty_res, item_name, - ))); + return Err(ResolutionFailure::NoAssocItem(ty_res, item_name).into()); } } Res::Def(DefKind::Trait, did) => cx @@ -585,12 +580,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if ns == Namespace::ValueNS { self.variant_field(path_str, current_item, module_id, extra_fragment) } else { - Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(ty_res, item_name))) + Err(ResolutionFailure::NoAssocItem(ty_res, item_name).into()) } }) } else { debug!("attempting to resolve item without parent module: {}", path_str); - Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)) + Err(ResolutionFailure::NoParentItem.into()) } } @@ -611,7 +606,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let check_full_res_inner = |this: &Self, result: Result>| { let res = match result { Ok(res) => Some(res), - Err(ErrorKind::Resolve(kind)) => kind.full_res(), + Err(ErrorKind::Resolve(box kind)) => kind.full_res(), Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => { Some(res) } @@ -626,7 +621,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; let check_full_res_macro = |this: &Self| { let result = this.macro_resolve(path_str, base_node); - check_full_res_inner(this, result.map_err(ErrorKind::Resolve)) + check_full_res_inner(this, result.map_err(ErrorKind::from)) }; match ns { Namespace::MacroNS => check_full_res_macro(self), @@ -970,7 +965,7 @@ 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::Resolve(mut kind)) => { + Err(ErrorKind::Resolve(box mut kind)) => { // 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 }; @@ -1028,7 +1023,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { anchor_failure(cx, &item, &ori_link, &dox, link_range, msg); continue; } - Err(ErrorKind::Resolve(kind)) => Err(kind), + Err(ErrorKind::Resolve(box kind)) => Err(kind), }, value_ns: match self.resolve( path_str, @@ -1042,7 +1037,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { anchor_failure(cx, &item, &ori_link, &dox, link_range, msg); continue; } - Err(ErrorKind::Resolve(kind)) => Err(kind), + Err(ErrorKind::Resolve(box kind)) => Err(kind), } .and_then(|(res, fragment)| { // Constructors are picked up in the type namespace. @@ -1816,7 +1811,7 @@ fn handle_variant( let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { parent } else { - return Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem)); + return Err(ResolutionFailure::NoParentItem.into()); }; let parent_def = Res::Def(DefKind::Enum, parent); let variant = cx.tcx.expect_variant_res(res); From 57250eff55db7c034b9d3468b45b0df2d6ee4bb3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Sep 2020 13:42:56 -0400 Subject: [PATCH 23/25] Use `span_label` instead of `note` This puts the error message closer to the link, making it easier to see what went wrong. --- .../passes/collect_intra_doc_links.rs | 84 +++++++++++-------- .../intra-links-warning-crlf.stderr | 12 +-- .../rustdoc-ui/intra-links-warning.stderr | 47 ++++------- 3 files changed, 65 insertions(+), 78 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 7fea70253b301..0319096635211 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1516,16 +1516,15 @@ fn resolution_failure( collector.cx.tcx.item_name(res.def_id()).to_string() ) }; - let assoc_item_not_allowed = |res: Res, diag: &mut DiagnosticBuilder<'_>| { + let assoc_item_not_allowed = |res: Res| { let def_id = res.def_id(); let name = collector.cx.tcx.item_name(def_id); - let note = format!( + format!( "`{}` is {} {}, not a module or type, and cannot have associated items", name, res.article(), res.descr() - ); - diag.note(¬e); + ) }; // ignore duplicates let mut variants_seen = SmallVec::<[_; 3]>::new(); @@ -1559,12 +1558,18 @@ fn resolution_failure( continue; } variants_seen.push(variant); - match failure { + let note = match failure { ResolutionFailure::NotInScope { name, .. } => { if in_scope { continue; } - diag.note(&format!("no item named `{}` is in scope", name)); + // NOTE: uses an explicit `continue` so the `note:` will come before the `help:` + let note = format!("no item named `{}` is in scope", name); + 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 !path_str.contains("::") { // Otherwise, the `[]` might be unrelated. @@ -1572,16 +1577,10 @@ fn resolution_failure( // don't show this for autolinks (`<>`), `()` style links, or reference links diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); } + continue; } ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace(res, expected_ns) => { - let note = format!( - "this link resolves to {}, which is not in the {} namespace", - item(res), - expected_ns.descr() - ); - diag.note(¬e); - if let Res::Def(kind, _) = res { let disambiguator = Disambiguator::Kind(kind); suggest_disambiguator( @@ -1593,24 +1592,26 @@ fn resolution_failure( &link_range, ) } + + format!( + "this link resolves to {}, which is not in the {} namespace", + item(res), + expected_ns.descr() + ) } ResolutionFailure::NoParentItem => { diag.level = rustc_errors::Level::Bug; - diag.note("all intra doc links should have a parent item"); - } - ResolutionFailure::NoPrimitiveImpl(res, _) => { - let note = format!( - "this link partially resolves to {}, which does not have any associated items", - item(res), - ); - diag.note(¬e); + "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, .. } => { - let note = format!( + format!( "the builtin type `{}` does not have an associated item named `{}`", prim_name, assoc_item - ); - diag.note(¬e); + ) } ResolutionFailure::NoAssocItem(res, assoc_item) => { use DefKind::*; @@ -1645,32 +1646,41 @@ fn resolution_failure( | Use | LifetimeParam | Ctor(_, _) - | AnonConst => return assoc_item_not_allowed(res, diag), + | 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"), } }; - let note = format!( + format!( "the {} `{}` has no {} named `{}`", res.descr(), name, path_description, assoc_item - ); - diag.note(¬e); + ) } ResolutionFailure::CannotHaveAssociatedItems(res, _) => { - assoc_item_not_allowed(res, diag) - } - ResolutionFailure::NotAVariant(res, variant) => { - let note = format!( - "this link partially resolves to {}, but there is no variant named {}", - item(res), - variant - ); - diag.note(¬e); + 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); + } else { + diag.note(¬e); } } }, diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr index 1da27b786180f..d64c7e14ba61d 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr @@ -2,37 +2,33 @@ warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:7:6 | LL | /// [error] - | ^^^^^ + | ^^^^^ no item named `error` is in scope | = 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] - | ^^^^^^ + | ^^^^^^ no item named `error1` is in scope | - = 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] - | ^^^^^^ + | ^^^^^^ no item named `error2` is in scope | - = 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]. - | ^^^^^ + | ^^^^^ no item named `error` is in scope | - = 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 25c6975d3a509..0eaad25501a32 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -2,76 +2,62 @@ warning: unresolved link to `Foo::baz` --> $DIR/intra-links-warning.rs:3:23 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ + | ^^^^^^^^ the struct `Foo` has no field or associated item named `baz` | = note: `#[warn(broken_intra_doc_links)]` on by default - = note: the struct `Foo` has no field 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], ... - | ^^^^^^^^ - | - = note: no item named `Bar` is in scope + | ^^^^^^^^ no item named `Bar` is in scope warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:6:13 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ - | - = note: no item named `Uniooon` is in scope + | ^^^^^^^^^^ no item named `Uniooon` is in scope warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:6:30 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ - | - = note: no item named `Qux` is in scope + | ^^^^^^ no item named `Qux` is in scope warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:10:14 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ - | - = note: no item named `Uniooon` is in scope + | ^^^^^^^^^^ no item named `Uniooon` is in scope warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:10:31 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ - | - = note: no item named `Qux` is in scope + | ^^^^^^ no item named `Qux` is in scope warning: unresolved link to `Qux:Y` --> $DIR/intra-links-warning.rs:14:13 | LL | /// [Qux:Y] - | ^^^^^ + | ^^^^^ no item named `Qux:Y` is in scope | - = 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]*/ - | ^^^^^ + | ^^^^^ no item named `error` is in scope | - = 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] - | ^^^^^ + | ^^^^^ no item named `error` is in scope | - = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` @@ -119,45 +105,40 @@ warning: unresolved link to `error1` --> $DIR/intra-links-warning.rs:80:11 | LL | /// docs [error1] - | ^^^^^^ + | ^^^^^^ no item named `error1` is in scope | - = 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] - | ^^^^^^ + | ^^^^^^ no item named `error2` is in scope | - = 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 - | ^^^^ + | ^^^^ no item named `BarA` is in scope | - = 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 - | ^^^^ + | ^^^^ no item named `BarB` is in scope | - = 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 - | ^^^^ + | ^^^^ no item named `BarC` is in scope | - = note: no item named `BarC` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarD` From b2a5a7a8c21c9cc271f3ce44b4cffdaefaa7f6d1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Sep 2020 13:49:45 -0400 Subject: [PATCH 24/25] Remove unnecessary clone --- src/librustdoc/passes/collect_intra_doc_links.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0319096635211..3e3eebcb51827 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1056,12 +1056,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { }), }; - let mut candidates_iter = - candidates.iter().filter_map(|res| res.as_ref().ok()); - let len = candidates_iter.clone().count(); + let len = candidates.iter().filter(|res| res.is_ok()).count(); if len == 0 { - drop(candidates_iter); resolution_failure( self, &item, @@ -1076,12 +1073,10 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } if len == 1 { - candidates_iter.next().unwrap().clone() + candidates.into_iter().filter_map(|res| res.ok()).next().unwrap() } 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 = Err(ResolutionFailure::Dummy); } From 5ea3eaf237793c978cad9d48f7850619c7f49d69 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Sep 2020 14:00:40 -0400 Subject: [PATCH 25/25] Name the current module 'not in scope' -> 'not in `module`' --- .../passes/collect_intra_doc_links.rs | 5 +- .../rustdoc-ui/assoc-item-not-in-scope.stderr | 3 +- .../deny-intra-link-resolution-failure.stderr | 3 +- .../rustdoc-ui/intra-doc-alias-ice.stderr | 3 +- src/test/rustdoc-ui/intra-link-errors.rs | 6 +- src/test/rustdoc-ui/intra-link-errors.stderr | 75 +++++++------------ .../intra-link-span-ice-55723.stderr | 3 +- .../intra-links-warning-crlf.stderr | 8 +- .../rustdoc-ui/intra-links-warning.stderr | 36 ++++----- src/test/rustdoc-ui/lint-group.stderr | 3 +- 10 files changed, 61 insertions(+), 84 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 3e3eebcb51827..5780610c86210 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1554,12 +1554,13 @@ fn resolution_failure( } variants_seen.push(variant); let note = match failure { - ResolutionFailure::NotInScope { name, .. } => { + ResolutionFailure::NotInScope { module_id, name, .. } => { if in_scope { continue; } // NOTE: uses an explicit `continue` so the `note:` will come before the `help:` - let note = format!("no item named `{}` is in scope", name); + let module_name = collector.cx.tcx.item_name(module_id); + let note = format!("no item named `{}` in `{}`", name, module_name); if let Some(span) = sp { diag.span_label(span, ¬e); } else { diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr index 8acfcca2139e3..92d27179e8c3f 100644 --- a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr @@ -2,14 +2,13 @@ error: unresolved link to `S::fmt` --> $DIR/assoc-item-not-in-scope.rs:4:14 | LL | /// Link to [`S::fmt`] - | ^^^^^^^^ + | ^^^^^^^^ the struct `S` has no field or associated item named `fmt` | note: the lint level is defined here --> $DIR/assoc-item-not-in-scope.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: the struct `S` has no field or associated item named `fmt` error: aborting due to previous error 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 4ae53e83613e0..5020b97b2f201 100644 --- a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr +++ b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr @@ -2,14 +2,13 @@ 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` | 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 5379a91883753..771fc2204f5f8 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -2,14 +2,13 @@ error: unresolved link to `TypeAlias::hoge` --> $DIR/intra-doc-alias-ice.rs:5:30 | LL | /// [broken cross-reference](TypeAlias::hoge) - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ the type alias `TypeAlias` has no associated item named `hoge` | note: the lint level is defined here --> $DIR/intra-doc-alias-ice.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: the type alias `TypeAlias` has no 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 index 477ad6464f887..26b629b1313da 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -6,15 +6,15 @@ /// [path::to::nonexistent::module] //~^ ERROR unresolved link -//~| NOTE no item named `path` is in scope +//~| NOTE no item named `path` in `intra_link_errors` /// [path::to::nonexistent::macro!] //~^ ERROR unresolved link -//~| NOTE no item named `path` is in scope +//~| NOTE no item named `path` in `intra_link_errors` /// [type@path::to::nonexistent::type] //~^ ERROR unresolved link -//~| NOTE no item named `path` is in scope +//~| NOTE no item named `path` in `intra_link_errors` /// [std::io::not::here] //~^ ERROR unresolved link diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 3b1a09f913e9f..fbf3dcbbec29a 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -2,134 +2,115 @@ 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` | 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` is in scope error: unresolved link to `path::to::nonexistent::macro` --> $DIR/intra-link-errors.rs:11:6 | LL | /// [path::to::nonexistent::macro!] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: no item named `path` is in scope + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `path` in `intra_link_errors` error: unresolved link to `path::to::nonexistent::type` --> $DIR/intra-link-errors.rs:15:6 | LL | /// [type@path::to::nonexistent::type] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: no item named `path` is in scope + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `path` in `intra_link_errors` error: unresolved link to `std::io::not::here` --> $DIR/intra-link-errors.rs:19:6 | LL | /// [std::io::not::here] - | ^^^^^^^^^^^^^^^^^^ - | - = note: the module `io` has no inner item named `not` + | ^^^^^^^^^^^^^^^^^^ the module `io` has no inner item named `not` error: unresolved link to `std::io::Error::x` --> $DIR/intra-link-errors.rs:23:6 | LL | /// [std::io::Error::x] - | ^^^^^^^^^^^^^^^^^ - | - = note: the struct `Error` has no field or associated item named `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 | LL | /// [std::io::ErrorKind::x] - | ^^^^^^^^^^^^^^^^^^^^^ - | - = note: the enum `ErrorKind` has no variant or associated item named `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 | LL | /// [f::A] - | ^^^^ - | - = note: `f` is a function, not a module or type, and cannot have associated items + | ^^^^ `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 | LL | /// [S::A] - | ^^^^ - | - = note: the struct `S` has no field or associated item named `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 | LL | /// [S::fmt] - | ^^^^^^ - | - = note: the struct `S` has no field or associated item named `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 | LL | /// [E::D] - | ^^^^ - | - = note: the enum `E` has no variant or associated item named `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 | LL | /// [u8::not_found] - | ^^^^^^^^^^^^^ - | - = note: 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` error: unresolved link to `S` --> $DIR/intra-link-errors.rs:51:6 | LL | /// [S!] - | ^^ help: to link to the struct, prefix with `struct@`: `struct@S` - | - = note: this link resolves to the struct `S`, which is not in the macro namespace + | ^^ + | | + | this link resolves to the struct `S`, which is not in the macro namespace + | help: to link to the struct, prefix with `struct@`: `struct@S` error: unresolved link to `T::g` --> $DIR/intra-link-errors.rs:69:6 | LL | /// [type@T::g] - | ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()` - | - = note: this link resolves to the associated function `g`, which is not in the type namespace + | ^^^^^^^^^ + | | + | this link resolves to the associated function `g`, which is not in the type namespace + | help: to link to the associated function, add parentheses: `T::g()` error: unresolved link to `T::h` --> $DIR/intra-link-errors.rs:74:6 | LL | /// [T::h!] - | ^^^^^ - | - = note: the trait `T` has no macro named `h` + | ^^^^^ the trait `T` has no macro named `h` error: unresolved link to `S::h` --> $DIR/intra-link-errors.rs:61:6 | LL | /// [type@S::h] - | ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()` - | - = note: this link resolves to the associated function `h`, which is not in the type namespace + | ^^^^^^^^^ + | | + | this link resolves to the associated function `h`, which is not in the type namespace + | help: to link to the associated function, add parentheses: `S::h()` error: unresolved link to `m` --> $DIR/intra-link-errors.rs:81:6 | LL | /// [m()] - | ^^^ help: to link to the macro, add an exclamation mark: `m!` - | - = note: this link resolves to the macro `m`, which is not in the value namespace + | ^^^ + | | + | 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 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 47b6a08baf37d..3c13df20588d8 100644 --- a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr +++ b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr @@ -2,14 +2,13 @@ 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` | 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 d64c7e14ba61d..351f8fafa64d8 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` is in scope + | ^^^^^ no item named `error` in `intra_links_warning_crlf` | = 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` is in scope + | ^^^^^^ no item named `error1` in `intra_links_warning_crlf` | = 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` is in scope + | ^^^^^^ no item named `error2` in `intra_links_warning_crlf` | = 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` is in scope + | ^^^^^ no item named `error` in `intra_links_warning_crlf` | = 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 0eaad25501a32..0832e00d35a00 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` is in scope + | ^^^^^^^^ no item named `Bar` in `intra_links_warning` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:6:13 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ no item named `Uniooon` is in scope + | ^^^^^^^^^^ no item named `Uniooon` in `intra_links_warning` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:6:30 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ no item named `Qux` is in scope + | ^^^^^^ no item named `Qux` in `intra_links_warning` warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:10:14 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ no item named `Uniooon` is in scope + | ^^^^^^^^^^ no item named `Uniooon` in `intra_links_warning` warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:10:31 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ no item named `Qux` is in scope + | ^^^^^^ no item named `Qux` in `intra_links_warning` warning: unresolved link to `Qux:Y` --> $DIR/intra-links-warning.rs:14:13 | LL | /// [Qux:Y] - | ^^^^^ no item named `Qux:Y` is in scope + | ^^^^^ no item named `Qux:Y` in `intra_links_warning` | = 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` is in scope + | ^^^^^ no item named `error` in `intra_links_warning` | = 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` is in scope + | ^^^^^ no item named `error` in `intra_links_warning` | = 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` is in scope + = note: no item named `error` in `intra_links_warning` = 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` is in scope + = note: no item named `error` in `intra_links_warning` = 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` is in scope + = note: no item named `error` in `intra_links_warning` = 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` is in scope + | ^^^^^^ no item named `error1` in `intra_links_warning` | = 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` is in scope + | ^^^^^^ no item named `error2` in `intra_links_warning` | = 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` is in scope + | ^^^^ no item named `BarA` in `intra_links_warning` | = 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` is in scope + | ^^^^ no item named `BarB` in `intra_links_warning` | = 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` is in scope + | ^^^^ no item named `BarC` in `intra_links_warning` | = 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` is in scope + = note: no item named `BarD` in `intra_links_warning` = 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` is in scope + = note: no item named `BarF` in `intra_links_warning` = 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 10fcebb27fe0c..550b79f6e8928 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` | note: the lint level is defined here --> $DIR/lint-group.rs:7:9 @@ -40,7 +40,6 @@ note: the lint level is defined here LL | #![deny(rustdoc)] | ^^^^^^^ = note: `#[deny(broken_intra_doc_links)]` implied by `#[deny(rustdoc)]` - = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to 3 previous errors