diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index a458cdab30303..223fda84871e9 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -118,7 +118,7 @@ impl Item { self.attrs.collapsed_doc_value() } - pub fn links(&self) -> Vec<(String, String)> { + pub fn links(&self) -> Vec { self.attrs.links(&self.def_id.krate) } @@ -425,10 +425,38 @@ pub struct Attributes { pub cfg: Option>, pub span: Option, /// map from Rust paths to resolved defs and potential URL fragments - pub links: Vec<(String, Option, Option)>, + pub links: Vec, pub inner_docs: bool, } +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] +/// A link that has not yet been rendered. +/// +/// This link will be turned into a rendered link by [`Attributes::links`] +pub struct ItemLink { + /// The original link written in the markdown + pub(crate) link: String, + /// The link text displayed in the HTML. + /// + /// This may not be the same as `link` if there was a disambiguator + /// in an intra-doc link (e.g. \[`fn@f`\]) + pub(crate) link_text: String, + pub(crate) did: Option, + /// The url fragment to append to the link + pub(crate) fragment: Option, +} + +pub struct RenderedLink { + /// The text the link was original written as. + /// + /// This could potentially include disambiguators and backticks. + pub(crate) original_text: String, + /// The text to display in the HTML + pub(crate) new_text: String, + /// The URL to put in the `href` + pub(crate) href: String, +} + impl Attributes { /// Extracts the content from an attribute `#[doc(cfg(content))]`. pub fn extract_cfg(mi: &ast::MetaItem) -> Option<&ast::MetaItem> { @@ -605,21 +633,25 @@ impl Attributes { /// Gets links as a vector /// /// Cache must be populated before call - pub fn links(&self, krate: &CrateNum) -> Vec<(String, String)> { + pub fn links(&self, krate: &CrateNum) -> Vec { use crate::html::format::href; use crate::html::render::CURRENT_DEPTH; self.links .iter() - .filter_map(|&(ref s, did, ref fragment)| { - match did { + .filter_map(|ItemLink { link: s, link_text, did, fragment }| { + match *did { Some(did) => { if let Some((mut href, ..)) = href(did) { if let Some(ref fragment) = *fragment { href.push_str("#"); href.push_str(fragment); } - Some((s.clone(), href)) + Some(RenderedLink { + original_text: s.clone(), + new_text: link_text.clone(), + href, + }) } else { None } @@ -639,16 +671,17 @@ impl Attributes { }; // This is a primitive so the url is done "by hand". let tail = fragment.find('#').unwrap_or_else(|| fragment.len()); - Some(( - s.clone(), - format!( + Some(RenderedLink { + original_text: s.clone(), + new_text: link_text.clone(), + href: format!( "{}{}std/primitive.{}.html{}", url, if !url.ends_with('/') { "/" } else { "" }, &fragment[..tail], &fragment[tail..] ), - )) + }) } else { panic!("This isn't a primitive?!"); } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 098ece9a1d50e..a8c60e4a76df4 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -34,6 +34,7 @@ use std::fmt::Write; use std::ops::Range; use std::str; +use crate::clean::RenderedLink; use crate::doctest; use crate::html::highlight; use crate::html::toc::TocBuilder; @@ -52,7 +53,7 @@ fn opts() -> Options { pub struct Markdown<'a>( pub &'a str, /// A list of link replacements. - pub &'a [(String, String)], + pub &'a [RenderedLink], /// The current list of used header IDs. pub &'a mut IdMap, /// Whether to allow the use of explicit error codes in doctest lang strings. @@ -78,7 +79,7 @@ pub struct MarkdownHtml<'a>( pub &'a Option, ); /// A tuple struct like `Markdown` that renders only the first paragraph. -pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); +pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [RenderedLink]); #[derive(Copy, Clone, PartialEq, Debug)] pub enum ErrorCodes { @@ -337,31 +338,107 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { } /// Make headings links with anchor IDs and build up TOC. -struct LinkReplacer<'a, 'b, I: Iterator>> { +struct LinkReplacer<'a, I: Iterator>> { inner: I, - links: &'b [(String, String)], + links: &'a [RenderedLink], + shortcut_link: Option<&'a RenderedLink>, } -impl<'a, 'b, I: Iterator>> LinkReplacer<'a, 'b, I> { - fn new(iter: I, links: &'b [(String, String)]) -> Self { - LinkReplacer { inner: iter, links } +impl<'a, I: Iterator>> LinkReplacer<'a, I> { + fn new(iter: I, links: &'a [RenderedLink]) -> Self { + LinkReplacer { inner: iter, links, shortcut_link: None } } } -impl<'a, 'b, I: Iterator>> Iterator for LinkReplacer<'a, 'b, I> { +impl<'a, I: Iterator>> Iterator for LinkReplacer<'a, I> { type Item = Event<'a>; fn next(&mut self) -> Option { - let event = self.inner.next(); - if let Some(Event::Start(Tag::Link(kind, dest, text))) = event { - if let Some(&(_, ref replace)) = self.links.iter().find(|link| link.0 == *dest) { - Some(Event::Start(Tag::Link(kind, replace.to_owned().into(), text))) - } else { - Some(Event::Start(Tag::Link(kind, dest, text))) + use pulldown_cmark::LinkType; + + let mut event = self.inner.next(); + + // Replace intra-doc links and remove disambiguators from shortcut links (`[fn@f]`). + match &mut event { + // This is a shortcut link that was resolved by the broken_link_callback: `[fn@f]` + // Remove any disambiguator. + Some(Event::Start(Tag::Link( + // [fn@f] or [fn@f][] + LinkType::ShortcutUnknown | LinkType::CollapsedUnknown, + dest, + title, + ))) => { + debug!("saw start of shortcut link to {} with title {}", dest, title); + // If this is a shortcut link, it was resolved by the broken_link_callback. + // So the URL will already be updated properly. + let link = self.links.iter().find(|&link| *link.href == **dest); + // Since this is an external iterator, we can't replace the inner text just yet. + // Store that we saw a link so we know to replace it later. + if let Some(link) = link { + trace!("it matched"); + assert!(self.shortcut_link.is_none(), "shortcut links cannot be nested"); + self.shortcut_link = Some(link); + } } - } else { - event + // Now that we're done with the shortcut link, don't replace any more text. + Some(Event::End(Tag::Link( + LinkType::ShortcutUnknown | LinkType::CollapsedUnknown, + dest, + _, + ))) => { + debug!("saw end of shortcut link to {}", dest); + if self.links.iter().find(|&link| *link.href == **dest).is_some() { + assert!(self.shortcut_link.is_some(), "saw closing link without opening tag"); + self.shortcut_link = None; + } + } + // Handle backticks in inline code blocks, but only if we're in the middle of a shortcut link. + // [`fn@f`] + Some(Event::Code(text)) => { + trace!("saw code {}", text); + if let Some(link) = self.shortcut_link { + trace!("original text was {}", link.original_text); + // NOTE: this only replaces if the code block is the *entire* text. + // If only part of the link has code highlighting, the disambiguator will not be removed. + // e.g. [fn@`f`] + // This is a limitation from `collect_intra_doc_links`: it passes a full link, + // and does not distinguish at all between code blocks. + // So we could never be sure we weren't replacing too much: + // [fn@my_`f`unc] is treated the same as [my_func()] in that pass. + // + // NOTE: &[1..len() - 1] is to strip the backticks + if **text == link.original_text[1..link.original_text.len() - 1] { + debug!("replacing {} with {}", text, link.new_text); + *text = CowStr::Borrowed(&link.new_text); + } + } + } + // Replace plain text in links, but only in the middle of a shortcut link. + // [fn@f] + Some(Event::Text(text)) => { + trace!("saw text {}", text); + if let Some(link) = self.shortcut_link { + trace!("original text was {}", link.original_text); + // NOTE: same limitations as `Event::Code` + if **text == *link.original_text { + debug!("replacing {} with {}", text, link.new_text); + *text = CowStr::Borrowed(&link.new_text); + } + } + } + // If this is a link, but not a shortcut link, + // replace the URL, since the broken_link_callback was not called. + Some(Event::Start(Tag::Link(_, dest, _))) => { + if let Some(link) = self.links.iter().find(|&link| *link.original_text == **dest) { + *dest = CowStr::Borrowed(link.href.as_ref()); + } + } + // Anything else couldn't have been a valid Rust path, so no need to replace the text. + _ => {} } + + // Yield the modified event + event } } @@ -855,8 +932,8 @@ impl Markdown<'_> { return String::new(); } let replacer = |_: &str, s: &str| { - if let Some(&(_, ref replace)) = links.iter().find(|link| &*link.0 == s) { - Some((replace.clone(), s.to_owned())) + if let Some(link) = links.iter().find(|link| &*link.original_text == s) { + Some((link.href.clone(), link.new_text.clone())) } else { None } @@ -933,8 +1010,8 @@ impl MarkdownSummaryLine<'_> { } let replacer = |_: &str, s: &str| { - if let Some(&(_, ref replace)) = links.iter().find(|link| &*link.0 == s) { - Some((replace.clone(), s.to_owned())) + if let Some(link) = links.iter().find(|link| &*link.original_text == s) { + Some((link.href.clone(), link.new_text.clone())) } else { None } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 57b3b9502a37f..f095f67b54c63 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -63,9 +63,8 @@ use rustc_span::symbol::{sym, Symbol}; use serde::ser::SerializeSeq; use serde::{Serialize, Serializer}; -use crate::clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy, TypeKind}; -use crate::config::RenderInfo; -use crate::config::RenderOptions; +use crate::clean::{self, AttributesExt, Deprecation, GetDefId, RenderedLink, SelfTy, TypeKind}; +use crate::config::{RenderInfo, RenderOptions}; use crate::docfs::{DocFS, PathError}; use crate::doctree; use crate::error::Error; @@ -1774,7 +1773,7 @@ fn render_markdown( w: &mut Buffer, cx: &Context, md_text: &str, - links: Vec<(String, String)>, + links: Vec, prefix: &str, is_hidden: bool, ) { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a4f5530815ce1..13bfca4c5cdbf 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -697,11 +697,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // This is an anchor to an element of the current page, nothing to do in here! continue; } - (parts[0].to_owned(), Some(parts[1].to_owned())) + (parts[0], Some(parts[1].to_owned())) } else { - (parts[0].to_owned(), None) + (parts[0], None) }; let resolved_self; + let link_text; let mut path_str; let disambiguator; let (mut res, mut fragment) = { @@ -718,6 +719,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } + // We stripped `()` and `!` when parsing the disambiguator. + // Add them back to be displayed, but not prefix disambiguators. + link_text = disambiguator + .map(|d| d.display_for(path_str)) + .unwrap_or_else(|| path_str.to_owned()); + // In order to correctly resolve intra-doc-links we need to // pick a base AST node to work from. If the documentation for // this module came from an inner comment (//!) then we anchor @@ -906,7 +913,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { if let Res::PrimTy(_) = res { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { - item.attrs.links.push((ori_link, None, fragment)) + item.attrs.links.push(ItemLink { + link: ori_link, + link_text: path_str.to_owned(), + did: None, + fragment, + }); } Some(other) => { report_mismatch(other, Disambiguator::Primitive); @@ -957,7 +969,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } let id = register_res(cx, res); - item.attrs.links.push((ori_link, Some(id), fragment)); + item.attrs.links.push(ItemLink { + link: ori_link, + link_text, + did: Some(id), + fragment, + }); } } @@ -985,6 +1002,18 @@ enum Disambiguator { } impl Disambiguator { + /// The text that should be displayed when the path is rendered as HTML. + /// + /// NOTE: `path` is not the original link given by the user, but a name suitable for passing to `resolve`. + fn display_for(&self, path: &str) -> String { + match self { + // FIXME: this will have different output if the user had `m!()` originally. + Self::Kind(DefKind::Macro(MacroKind::Bang)) => format!("{}!", path), + Self::Kind(DefKind::Fn) => format!("{}()", path), + _ => path.to_owned(), + } + } + /// (disambiguator, path_str) fn from_str(link: &str) -> Result<(Self, &str), ()> { use Disambiguator::{Kind, Namespace as NS, Primitive}; @@ -1037,7 +1066,7 @@ impl Disambiguator { } /// Return (description of the change, suggestion) - fn display_for(self, path_str: &str) -> (&'static str, String) { + 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"; @@ -1292,7 +1321,7 @@ fn suggest_disambiguator( sp: Option, link_range: &Option>, ) { - let (action, mut suggestion) = disambiguator.display_for(path_str); + let (action, mut suggestion) = disambiguator.suggestion_for(path_str); let help = format!("to link to the {}, {}", disambiguator.descr(), action); if let Some(sp) = sp { diff --git a/src/test/rustdoc/intra-link-disambiguators-removed.rs b/src/test/rustdoc/intra-link-disambiguators-removed.rs new file mode 100644 index 0000000000000..26d05b484b919 --- /dev/null +++ b/src/test/rustdoc/intra-link-disambiguators-removed.rs @@ -0,0 +1,51 @@ +// ignore-tidy-linelength +#![deny(intra_doc_link_resolution_failure)] +// first try backticks +/// Trait: [`trait@Name`], fn: [`fn@Name`], [`Name`][`macro@Name`] +// @has intra_link_disambiguators_removed/struct.AtDisambiguator.html +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"][code]' "Name" +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name" +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"][code]' "Name" +pub struct AtDisambiguator; + +/// fn: [`Name()`], macro: [`Name!`] +// @has intra_link_disambiguators_removed/struct.SymbolDisambiguator.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name()" +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"][code]' "Name!" +pub struct SymbolDisambiguator; + +// Now make sure that backticks aren't added if they weren't already there +/// [fn@Name] +// @has intra_link_disambiguators_removed/trait.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"]' "Name" +// @!has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name" + +// FIXME: this will turn !() into ! alone +/// [Name!()] +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"]' "Name!" +pub trait Name {} + +#[allow(non_snake_case)] + +// Try collapsed reference links +/// [macro@Name][] +// @has intra_link_disambiguators_removed/fn.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"]' "Name" + +// Try links that have the same text as a generated URL +/// Weird URL aligned [../intra_link_disambiguators_removed/macro.Name.html][trait@Name] +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"]' "../intra_link_disambiguators_removed/macro.Name.html" +pub fn Name() {} + +#[macro_export] +// Rustdoc doesn't currently handle links that have weird interspersing of inline code blocks. +/// [fn@Na`m`e] +// @has intra_link_disambiguators_removed/macro.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"]' "fn@Name" + +// It also doesn't handle any case where the code block isn't the whole link text: +/// [trait@`Name`] +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"]' "trait@Name" +macro_rules! Name { + () => () +}