diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 0774413960501..cc6ce3115b640 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -37,7 +37,9 @@ use crate::doctest; use crate::html::highlight; use crate::html::toc::TocBuilder; -use pulldown_cmark::{html, BrokenLink, CodeBlockKind, CowStr, Event, Options, Parser, Tag}; +use pulldown_cmark::{ + html, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, +}; #[cfg(test)] mod tests; @@ -327,8 +329,6 @@ impl<'a, I: Iterator>> Iterator for LinkReplacer<'a, I> { type Item = Event<'a>; fn next(&mut self) -> Option { - use pulldown_cmark::LinkType; - let mut event = self.inner.next(); // Replace intra-doc links and remove disambiguators from shortcut links (`[fn@f]`). @@ -1123,7 +1123,13 @@ crate fn plain_text_summary(md: &str) -> String { s } -crate fn markdown_links(md: &str) -> Vec<(String, Range)> { +crate struct MarkdownLink { + pub kind: LinkType, + pub link: String, + pub range: Range, +} + +crate fn markdown_links(md: &str) -> Vec { if md.is_empty() { return vec![]; } @@ -1163,7 +1169,11 @@ crate fn markdown_links(md: &str) -> Vec<(String, Range)> { let mut push = |link: BrokenLink<'_>| { let span = span_for_link(&CowStr::Borrowed(link.reference), link.span); - links.borrow_mut().push((link.reference.to_owned(), span)); + links.borrow_mut().push(MarkdownLink { + kind: LinkType::ShortcutUnknown, + link: link.reference.to_owned(), + range: span, + }); None }; let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push)).into_offset_iter(); @@ -1174,10 +1184,10 @@ crate fn markdown_links(md: &str) -> Vec<(String, Range)> { let iter = Footnotes::new(HeadingLinks::new(p, None, &mut ids)); for ev in iter { - if let Event::Start(Tag::Link(_, dest, _)) = ev.0 { + if let Event::Start(Tag::Link(kind, dest, _)) = ev.0 { debug!("found link: {}", dest); let span = span_for_link(&dest, ev.1); - links.borrow_mut().push((dest.into_string(), span)); + links.borrow_mut().push(MarkdownLink { kind, link: dest.into_string(), range: span }); } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 6fcec6946ae79..0cefbb34791b2 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -25,6 +25,8 @@ use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; use smallvec::{smallvec, SmallVec}; +use pulldown_cmark::LinkType; + use std::borrow::Cow; use std::cell::Cell; use std::convert::{TryFrom, TryInto}; @@ -34,7 +36,7 @@ use std::ops::Range; use crate::clean::{self, utils::find_nearest_parent_module, Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::fold::DocFolder; -use crate::html::markdown::markdown_links; +use crate::html::markdown::{markdown_links, MarkdownLink}; use crate::passes::Pass; use super::span_of_attrs; @@ -265,8 +267,9 @@ struct LinkCollector<'a, 'tcx> { /// 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>, - /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link - visited_links: FxHashMap, + /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link. + /// The link will be `None` if it could not be resolved (i.e. the error was cached). + visited_links: FxHashMap>, } impl<'a, 'tcx> LinkCollector<'a, 'tcx> { @@ -901,16 +904,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { }; // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - for (ori_link, link_range) in markdown_links(&doc) { - let link = self.resolve_link( - &item, - &doc, - &self_name, - parent_node, - krate, - ori_link, - link_range, - ); + for md_link in markdown_links(&doc) { + let link = self.resolve_link(&item, &doc, &self_name, parent_node, krate, md_link); if let Some(link) = link { item.attrs.links.push(link); } @@ -942,27 +937,26 @@ impl LinkCollector<'_, '_> { self_name: &Option, parent_node: Option, krate: CrateNum, - ori_link: String, - link_range: Range, + ori_link: MarkdownLink, ) -> Option { - trace!("considering link '{}'", ori_link); + trace!("considering link '{}'", ori_link.link); // Bail early for real links. - if ori_link.contains('/') { + if ori_link.link.contains('/') { return None; } // [] is mostly likely not supposed to be a link - if ori_link.is_empty() { + if ori_link.link.is_empty() { return None; } let cx = self.cx; - let link = ori_link.replace("`", ""); + let link = ori_link.link.replace("`", ""); let parts = link.split('#').collect::>(); let (link, extra_fragment) = if parts.len() > 2 { // A valid link can't have multiple #'s - anchor_failure(cx, &item, &link, dox, link_range, AnchorFailure::MultipleAnchors); + anchor_failure(cx, &item, &link, dox, ori_link.range, AnchorFailure::MultipleAnchors); return None; } else if parts.len() == 2 { if parts[0].trim().is_empty() { @@ -1018,7 +1012,7 @@ impl LinkCollector<'_, '_> { path_str, disambiguator, dox, - link_range, + ori_link.range, smallvec![ResolutionFailure::NoParentItem], ); return None; @@ -1058,7 +1052,7 @@ impl LinkCollector<'_, '_> { path_str, disambiguator, dox, - link_range, + ori_link.range, smallvec![err_kind], ); return None; @@ -1074,15 +1068,22 @@ impl LinkCollector<'_, '_> { return None; } - let key = ResolutionInfo { - module_id, - dis: disambiguator, - path_str: path_str.to_owned(), - extra_fragment, + let diag_info = DiagnosticInfo { + item, + dox, + ori_link: &ori_link.link, + link_range: ori_link.range.clone(), }; - let diag = - DiagnosticInfo { item, dox, ori_link: &ori_link, link_range: link_range.clone() }; - let (mut res, mut fragment) = self.resolve_with_disambiguator_cached(key, diag)?; + let (mut res, mut fragment) = self.resolve_with_disambiguator_cached( + ResolutionInfo { + module_id, + dis: disambiguator, + path_str: path_str.to_owned(), + extra_fragment, + }, + diag_info, + matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), + )?; // Check for a primitive which might conflict with a module // Report the ambiguity and require that the user specify which one they meant. @@ -1101,7 +1102,7 @@ impl LinkCollector<'_, '_> { &item, path_str, dox, - link_range, + ori_link.range, AnchorFailure::RustdocAnchorConflict(prim), ); return None; @@ -1111,7 +1112,7 @@ impl LinkCollector<'_, '_> { } else { // `[char]` when a `char` module is in scope let candidates = vec![res, prim]; - ambiguity_error(cx, &item, path_str, dox, link_range, candidates); + ambiguity_error(cx, &item, path_str, dox, ori_link.range, candidates); return None; } } @@ -1129,14 +1130,22 @@ impl LinkCollector<'_, '_> { specified.descr() ); diag.note(¬e); - suggest_disambiguator(resolved, diag, path_str, dox, sp, &link_range); + suggest_disambiguator(resolved, diag, path_str, dox, sp, &ori_link.range); }; - report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, &item, dox, &link_range, callback); + report_diagnostic( + cx, + BROKEN_INTRA_DOC_LINKS, + &msg, + &item, + dox, + &ori_link.range, + callback, + ); }; match res { Res::Primitive(_) => match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { - Some(ItemLink { link: ori_link, link_text, did: None, fragment }) + Some(ItemLink { link: ori_link.link, link_text, did: None, fragment }) } Some(other) => { report_mismatch(other, Disambiguator::Primitive); @@ -1179,11 +1188,11 @@ impl LinkCollector<'_, '_> { if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src) && !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst) { - privacy_error(cx, &item, &path_str, dox, link_range); + privacy_error(cx, &item, &path_str, dox, &ori_link); } } let id = clean::register_res(cx, rustc_hir::def::Res::Def(kind, id)); - Some(ItemLink { link: ori_link, link_text, did: Some(id), fragment }) + Some(ItemLink { link: ori_link.link, link_text, did: Some(id), fragment }) } } } @@ -1192,28 +1201,47 @@ impl LinkCollector<'_, '_> { &mut self, key: ResolutionInfo, diag: DiagnosticInfo<'_>, + cache_resolution_failure: bool, ) -> Option<(Res, Option)> { // Try to look up both the result and the corresponding side channel value if let Some(ref cached) = self.visited_links.get(&key) { - self.kind_side_channel.set(cached.side_channel); - return Some(cached.res.clone()); + match cached { + Some(cached) => { + self.kind_side_channel.set(cached.side_channel.clone()); + return Some(cached.res.clone()); + } + None if cache_resolution_failure => return None, + None => { + // Although we hit the cache and found a resolution error, this link isn't + // supposed to cache those. Run link resolution again to emit the expected + // resolution error. + } + } } let res = self.resolve_with_disambiguator(&key, diag); // Cache only if resolved successfully - don't silence duplicate errors - if let Some(res) = &res { + if let Some(res) = res { // Store result for the actual namespace self.visited_links.insert( key, - CachedLink { + Some(CachedLink { res: res.clone(), side_channel: self.kind_side_channel.clone().into_inner(), - }, + }), ); - } - res + Some(res) + } else { + if cache_resolution_failure { + // For reference-style links we only want to report one resolution error + // so let's cache them as well. + self.visited_links.insert(key, None); + } + + None + } } /// After parsing the disambiguator, resolve the main part of the link. @@ -1964,13 +1992,7 @@ fn suggest_disambiguator( } /// Report a link from a public item to a private one. -fn privacy_error( - cx: &DocContext<'_>, - item: &Item, - path_str: &str, - dox: &str, - link_range: Range, -) { +fn privacy_error(cx: &DocContext<'_>, item: &Item, path_str: &str, dox: &str, link: &MarkdownLink) { let sym; let item_name = match item.name { Some(name) => { @@ -1982,7 +2004,7 @@ fn privacy_error( let msg = format!("public documentation for `{}` links to private item `{}`", item_name, path_str); - report_diagnostic(cx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { + report_diagnostic(cx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link.range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "this item is private"); } diff --git a/src/test/rustdoc-ui/reference-link-reports-error-once.rs b/src/test/rustdoc-ui/reference-link-reports-error-once.rs new file mode 100644 index 0000000000000..7957ee373c49e --- /dev/null +++ b/src/test/rustdoc-ui/reference-link-reports-error-once.rs @@ -0,0 +1,20 @@ +#![deny(broken_intra_doc_links)] + +/// Links to [a] [link][a] +/// And also a [third link][a] +/// And also a [reference link][b] +/// +/// Other links to the same target should still emit error: [ref] //~ERROR unresolved link to `ref` +/// Duplicate [ref] //~ERROR unresolved link to `ref` +/// +/// Other links to other targets should still emit error: [ref2] //~ERROR unresolved link to `ref2` +/// Duplicate [ref2] //~ERROR unresolved link to `ref2` +/// +/// [a]: ref +//~^ ERROR unresolved link to `ref` +/// [b]: ref2 +//~^ ERROR unresolved link to + +/// [ref][] +//~^ ERROR unresolved link +pub fn f() {} diff --git a/src/test/rustdoc-ui/reference-link-reports-error-once.stderr b/src/test/rustdoc-ui/reference-link-reports-error-once.stderr new file mode 100644 index 0000000000000..218eb334a6fc3 --- /dev/null +++ b/src/test/rustdoc-ui/reference-link-reports-error-once.stderr @@ -0,0 +1,63 @@ +error: unresolved link to `ref` + --> $DIR/reference-link-reports-error-once.rs:13:10 + | +LL | /// [a]: ref + | ^^^ no item named `ref` in scope + | +note: the lint level is defined here + --> $DIR/reference-link-reports-error-once.rs:1:9 + | +LL | #![deny(broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref2` + --> $DIR/reference-link-reports-error-once.rs:15:10 + | +LL | /// [b]: ref2 + | ^^^^ no item named `ref2` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref` + --> $DIR/reference-link-reports-error-once.rs:7:62 + | +LL | /// Other links to the same target should still emit error: [ref] + | ^^^ no item named `ref` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref` + --> $DIR/reference-link-reports-error-once.rs:8:16 + | +LL | /// Duplicate [ref] + | ^^^ no item named `ref` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref2` + --> $DIR/reference-link-reports-error-once.rs:10:60 + | +LL | /// Other links to other targets should still emit error: [ref2] + | ^^^^ no item named `ref2` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref2` + --> $DIR/reference-link-reports-error-once.rs:11:16 + | +LL | /// Duplicate [ref2] + | ^^^^ no item named `ref2` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: unresolved link to `ref` + --> $DIR/reference-link-reports-error-once.rs:18:6 + | +LL | /// [ref][] + | ^^^ no item named `ref` in scope + | + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: aborting due to 7 previous errors + diff --git a/src/test/rustdoc-ui/reference-links.rs b/src/test/rustdoc-ui/reference-links.rs index 7c1a79722c993..6e00b9f0fa1a9 100644 --- a/src/test/rustdoc-ui/reference-links.rs +++ b/src/test/rustdoc-ui/reference-links.rs @@ -4,4 +4,3 @@ //! //! [a]: std::process::Comman //~^ ERROR unresolved -//~| ERROR unresolved diff --git a/src/test/rustdoc-ui/reference-links.stderr b/src/test/rustdoc-ui/reference-links.stderr index 6ba73fbdb006d..3df89df21b4c6 100644 --- a/src/test/rustdoc-ui/reference-links.stderr +++ b/src/test/rustdoc-ui/reference-links.stderr @@ -10,11 +10,5 @@ note: the lint level is defined here LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ -error: unresolved link to `std::process::Comman` - --> $DIR/reference-links.rs:5:10 - | -LL | //! [a]: std::process::Comman - | ^^^^^^^^^^^^^^^^^^^^ no item named `Comman` in module `process` - -error: aborting due to 2 previous errors +error: aborting due to previous error