From d9aac8cfcefcd4063fc39ff19e9ace373471bbbc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 13 Nov 2024 16:56:18 +0100 Subject: [PATCH 1/2] Fix duplicated footnote IDs --- src/librustdoc/html/markdown.rs | 46 +++++++++++++++-------- src/librustdoc/html/markdown/footnotes.rs | 33 +++++++++------- src/librustdoc/html/render/context.rs | 4 +- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 7826a5d8394a7..1d42ec8eec7cb 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1333,12 +1333,14 @@ impl Markdown<'_> { let mut s = String::with_capacity(md.len() * 3 / 2); - let p = HeadingLinks::new(p, None, ids, heading_offset); - let p = footnotes::Footnotes::new(p); - let p = LinkReplacer::new(p.map(|(ev, _)| ev), links); - let p = TableWrapper::new(p); - let p = CodeBlocks::new(p, codes, edition, playground); - html::push_html(&mut s, p); + ids.handle_footnotes(|ids, existing_footnotes| { + let p = HeadingLinks::new(p, None, ids, heading_offset); + let p = footnotes::Footnotes::new(p, existing_footnotes); + let p = LinkReplacer::new(p.map(|(ev, _)| ev), links); + let p = TableWrapper::new(p); + let p = CodeBlocks::new(p, codes, edition, playground); + html::push_html(&mut s, p); + }); s } @@ -1367,13 +1369,13 @@ impl MarkdownWithToc<'_> { let mut toc = TocBuilder::new(); - { + ids.handle_footnotes(|ids, existing_footnotes| { let p = HeadingLinks::new(p, Some(&mut toc), ids, HeadingOffset::H1); - let p = footnotes::Footnotes::new(p); + let p = footnotes::Footnotes::new(p, existing_footnotes); let p = TableWrapper::new(p.map(|(ev, _)| ev)); let p = CodeBlocks::new(p, codes, edition, playground); html::push_html(&mut s, p); - } + }); (toc.into_toc(), s) } @@ -1401,13 +1403,15 @@ impl MarkdownItemInfo<'_> { let mut s = String::with_capacity(md.len() * 3 / 2); - let p = HeadingLinks::new(p, None, ids, HeadingOffset::H1); - let p = footnotes::Footnotes::new(p); - let p = TableWrapper::new(p.map(|(ev, _)| ev)); - let p = p.filter(|event| { - !matches!(event, Event::Start(Tag::Paragraph) | Event::End(TagEnd::Paragraph)) + ids.handle_footnotes(|ids, existing_footnotes| { + let p = HeadingLinks::new(p, None, ids, HeadingOffset::H1); + let p = footnotes::Footnotes::new(p, existing_footnotes); + let p = TableWrapper::new(p.map(|(ev, _)| ev)); + let p = p.filter(|event| { + !matches!(event, Event::Start(Tag::Paragraph) | Event::End(TagEnd::Paragraph)) + }); + html::push_html(&mut s, p); }); - html::push_html(&mut s, p); s } @@ -1882,6 +1886,7 @@ pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec, usize>, + existing_footnotes: usize, } // The map is pre-initialized and cloned each time to avoid reinitializing it repeatedly. @@ -1943,7 +1948,7 @@ fn init_id_map() -> FxHashMap, usize> { impl IdMap { pub fn new() -> Self { - IdMap { map: DEFAULT_ID_MAP.get_or_init(init_id_map).clone() } + IdMap { map: DEFAULT_ID_MAP.get_or_init(init_id_map).clone(), existing_footnotes: 0 } } pub(crate) fn derive + ToString>(&mut self, candidate: S) -> String { @@ -1959,4 +1964,13 @@ impl IdMap { self.map.insert(id.clone().into(), 1); id } + + /// Method to handle `existing_footnotes` increment automatically (to prevent forgetting + /// about it). + pub(crate) fn handle_footnotes(&mut self, closure: F) { + let mut existing_footnotes = self.existing_footnotes; + + closure(self, &mut existing_footnotes); + self.existing_footnotes = existing_footnotes; + } } diff --git a/src/librustdoc/html/markdown/footnotes.rs b/src/librustdoc/html/markdown/footnotes.rs index 3f0e586b8e372..0958d446c2d6c 100644 --- a/src/librustdoc/html/markdown/footnotes.rs +++ b/src/librustdoc/html/markdown/footnotes.rs @@ -8,36 +8,35 @@ use super::SpannedEvent; /// Moves all footnote definitions to the end and add back links to the /// references. -pub(super) struct Footnotes<'a, I> { +pub(super) struct Footnotes<'a, 'b, I> { inner: I, footnotes: FxIndexMap>, + existing_footnotes: &'b mut usize, } /// The definition of a single footnote. struct FootnoteDef<'a> { content: Vec>, /// The number that appears in the footnote reference and list. - id: u16, + id: usize, } -impl<'a, I> Footnotes<'a, I> { - pub(super) fn new(iter: I) -> Self { - Footnotes { inner: iter, footnotes: FxIndexMap::default() } +impl<'a, 'b, I> Footnotes<'a, 'b, I> { + pub(super) fn new(iter: I, existing_footnotes: &'b mut usize) -> Self { + Footnotes { inner: iter, footnotes: FxIndexMap::default(), existing_footnotes } } - fn get_entry(&mut self, key: &str) -> (&mut Vec>, u16) { - let new_id = self.footnotes.len() + 1; + fn get_entry(&mut self, key: &str) -> (&mut Vec>, usize) { + let new_id = self.footnotes.len() + 1 + *self.existing_footnotes; let key = key.to_owned(); - let FootnoteDef { content, id } = self - .footnotes - .entry(key) - .or_insert(FootnoteDef { content: Vec::new(), id: new_id as u16 }); + let FootnoteDef { content, id } = + self.footnotes.entry(key).or_insert(FootnoteDef { content: Vec::new(), id: new_id }); // Don't allow changing the ID of existing entrys, but allow changing the contents. (content, *id) } } -impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { +impl<'a, 'b, I: Iterator>> Iterator for Footnotes<'a, 'b, I> { type Item = SpannedEvent<'a>; fn next(&mut self) -> Option { @@ -47,8 +46,13 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { // When we see a reference (to a footnote we may not know) the definition of, // reserve a number for it, and emit a link to that number. let (_, id) = self.get_entry(reference); - let reference = - format!("{0}", id); + let reference = format!( + "{1}", + id, + // Although the ID count is for the whole page, the footnote reference + // are local to the item so we make this ID "local" when displayed. + id - *self.existing_footnotes + ); return Some((Event::Html(reference.into()), range)); } Some((Event::Start(Tag::FootnoteDefinition(def)), _)) => { @@ -64,6 +68,7 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { // After all the markdown is emmited, emit an
then all the footnotes // in a list. let defs: Vec<_> = self.footnotes.drain(..).map(|(_, x)| x).collect(); + *self.existing_footnotes += defs.len(); let defs_html = render_footnotes_defs(defs); return Some((Event::Html(defs_html.into()), 0..0)); } else { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index dc4d45e592eb7..5f255c18ec6aa 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -76,9 +76,9 @@ pub(crate) struct Context<'tcx> { // `Context` is cloned a lot, so we don't want the size to grow unexpectedly. #[cfg(all(not(windows), target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Context<'_>, 184); -#[cfg(all(windows, target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Context<'_>, 192); +#[cfg(all(windows, target_pointer_width = "64"))] +rustc_data_structures::static_assert_size!(Context<'_>, 200); /// Shared mutable state used in [`Context`] and elsewhere. pub(crate) struct SharedContext<'tcx> { From 7a8257dc471762343457c2963b1ccf1a868d06e8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 13 Nov 2024 16:56:40 +0100 Subject: [PATCH 2/2] Add regression test for #131901 --- tests/rustdoc/footnote-ids.rs | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/rustdoc/footnote-ids.rs diff --git a/tests/rustdoc/footnote-ids.rs b/tests/rustdoc/footnote-ids.rs new file mode 100644 index 0000000000000..d3a8435bb4701 --- /dev/null +++ b/tests/rustdoc/footnote-ids.rs @@ -0,0 +1,41 @@ +// This test ensures that footnotes ID are not duplicated across an item page. +// This is a regression test for . + +#![crate_name = "foo"] + +//@ has 'foo/struct.Foo.html' + +pub struct Foo; + +impl Foo { + //@ has - '//a[@href="#fn1"]' '1' + //@ has - '//li[@id="fn1"]' 'Hiya' + //@ has - '//a[@href="#fn2"]' '2' + //@ has - '//li[@id="fn2"]' 'Tiya' + /// Link 1 [^1] + /// Link 1.1 [^2] + /// + /// [^1]: Hiya + /// [^2]: Tiya + pub fn l1(){} + + //@ has - '//a[@href="#fn3"]' '1' + //@ has - '//li[@id="fn3"]' 'Yiya' + //@ has - '//a[@href="#fn4"]' '2' + //@ has - '//li[@id="fn4"]' 'Biya' + /// Link 2 [^1] + /// Link 3 [^2] + /// + /// [^1]: Yiya + /// [^2]: Biya + pub fn l2() {} +} + +impl Foo { + //@ has - '//a[@href="#fn5"]' '1' + //@ has - '//li[@id="fn5"]' 'Ciya' + /// Link 3 [^1] + /// + /// [^1]: Ciya + pub fn l3(){} +}