Skip to content

Commit

Permalink
Rollup merge of #108459 - benediktwerner:rustdoc-fix-link-match, r=Gu…
Browse files Browse the repository at this point in the history
…illaumeGomez

rustdoc: Fix LinkReplacer link matching

It currently just uses the first link with the same href which might not necessarily be the matching one.

This fixes replacements when there are several links to the same item but with different text (e.g. `[X] and [struct@X]`). It also fixes replacements in summaries since those use a links list with empty hrefs, so currently all links would always match the first link by href but then not match its text. This could also lead to a panic in the `original_lext[1..len() - 1]` part when the first link only has a single character, which is why the new code uses `.get(..)` instead.
  • Loading branch information
Dylan-DPC committed Jun 1, 2023
2 parents ba1690b + 9968f3c commit 0baa301
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 6 deletions.
15 changes: 10 additions & 5 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
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`]
Expand All @@ -390,8 +389,11 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
// 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] {
// NOTE: .get(1..len() - 1) is to strip the backticks
if let Some(link) = self.links.iter().find(|l| {
l.href == link.href
&& Some(&**text) == l.original_text.get(1..l.original_text.len() - 1)
}) {
debug!("replacing {} with {}", text, link.new_text);
*text = CowStr::Borrowed(&link.new_text);
}
Expand All @@ -402,9 +404,12 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
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 {
if let Some(link) = self
.links
.iter()
.find(|l| l.href == link.href && **text == *l.original_text)
{
debug!("replacing {} with {}", text, link.new_text);
*text = CowStr::Borrowed(&link.new_text);
}
Expand Down
37 changes: 37 additions & 0 deletions tests/rustdoc/intra-doc/issue-108459.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#![deny(rustdoc::broken_intra_doc_links)]

pub struct S;
pub mod char {}

// Ensure this doesn't ICE due to trying to slice off non-existent backticks from "S"

/// See [S] and [`S`]
pub struct MyStruct1;

// Ensure that link texts are replaced correctly even if there are multiple links with
// the same target but different text

/// See also [crate::char] and [mod@char] and [prim@char]
// @has issue_108459/struct.MyStruct2.html '//*[@href="char/index.html"]' 'crate::char'
// @has - '//*[@href="char/index.html"]' 'char'
// @has - '//*[@href="{{channel}}/std/primitive.char.html"]' 'char'
pub struct MyStruct2;

/// See also [mod@char] and [prim@char] and [crate::char]
// @has issue_108459/struct.MyStruct3.html '//*[@href="char/index.html"]' 'crate::char'
// @has - '//*[@href="char/index.html"]' 'char'
// @has - '//*[@href="{{channel}}/std/primitive.char.html"]' 'char'
pub struct MyStruct3;

// Ensure that links are correct even if there are multiple links with the same text but
// different targets

/// See also [char][mod@char] and [char][prim@char]
// @has issue_108459/struct.MyStruct4.html '//*[@href="char/index.html"]' 'char'
// @has - '//*[@href="{{channel}}/std/primitive.char.html"]' 'char'
pub struct MyStruct4;

/// See also [char][prim@char] and [char][crate::char]
// @has issue_108459/struct.MyStruct5.html '//*[@href="char/index.html"]' 'char'
// @has - '//*[@href="{{channel}}/std/primitive.char.html"]' 'char'
pub struct MyStruct5;
2 changes: 1 addition & 1 deletion tests/rustdoc/intra-doc/prim-precedence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ pub struct MyString;

/// See also [crate::char] and [mod@char]
// @has prim_precedence/struct.MyString2.html '//*[@href="char/index.html"]' 'crate::char'
// @has - '//*[@href="char/index.html"]' 'mod@char'
// @has - '//*[@href="char/index.html"]' 'char'
pub struct MyString2;

0 comments on commit 0baa301

Please sign in to comment.