Fix ID collisions when the numeric suffix gets used

This fixes a collision with the ID generation where it a previous entry
could generate a unique ID like "foo-1", but then a header with the text
"Foo 1" would collide with it. This fixes it so that when generating the
ID for "Foo 1", it will loop unit it finds an ID that doesn't collide
(in this case, `foo-1-1`).

Author: ehuss

crates/mdbook-html/src/html/print.rs

@@ -8,7 +8,7 @@ use super::Node;
 use crate::html::{ChapterTree, Element, serialize};
 use crate::utils::{ToUrlPath, id_from_content, normalize_path, unique_id};
 use mdbook_core::static_regex;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::path::{Component, PathBuf};
 
 /// Takes all the chapter trees, modifies them to be suitable to render for
@@ -42,12 +42,9 @@ pub(crate) fn render_print_page(mut chapter_trees: Vec<ChapterTree<'_>>) -> Stri
 /// been seen. This is used to generate unique IDs.
 fn make_ids_unique(
     chapter_trees: &mut [ChapterTree<'_>],
-) -> (
-    HashMap<PathBuf, HashMap<String, String>>,
-    HashMap<String, u32>,
-) {
+) -> (HashMap<PathBuf, HashMap<String, String>>, HashSet<String>) {
     let mut id_remap = HashMap::new();
-    let mut id_counter = HashMap::new();
+    let mut id_counter = HashSet::new();
     for ChapterTree {
         html_path, tree, ..
     } in chapter_trees
@@ -76,7 +73,7 @@ fn make_ids_unique(
 /// print output has something to link to.
 fn make_root_id_map(
     chapter_trees: &mut [ChapterTree<'_>],
-    id_counter: &mut HashMap<String, u32>,
+    id_counter: &mut HashSet<String>,
 ) -> HashMap<PathBuf, String> {
     let mut path_to_root_id = HashMap::new();
     for ChapterTree {

crates/mdbook-html/src/html/tree.rs

@@ -820,7 +820,7 @@ where
     /// to all header elements, and to also add an `<a>` tag so that clicking
     /// the header will set the current URL to that header's fragment.
     fn add_header_links(&mut self) {
-        let mut id_counter = HashMap::new();
+        let mut id_counter = HashSet::new();
         let headings =
             self.node_ids_for_tag(&|name| matches!(name, "h1" | "h2" | "h3" | "h4" | "h5" | "h6"));
         for heading in headings {

crates/mdbook-html/src/utils.rs

@@ -1,7 +1,6 @@
 //! Utilities for processing HTML.
 
-use std::collections::HashMap;
-use std::fmt::Write;
+use std::collections::HashSet;
 use std::path::{Component, Path, PathBuf};
 
 /// Utility function to normalize path elements like `..`.
@@ -54,18 +53,23 @@ impl ToUrlPath for Path {
 
 /// Make sure an HTML id is unique.
 ///
-/// The `id_counter` map is used to ensure the ID is globally unique. If the
-/// same id appears more than once, then it will have a number added to make
-/// it unique.
-pub(crate) fn unique_id(id: &str, id_counter: &mut HashMap<String, u32>) -> String {
-    let mut id = id.to_string();
-    let id_count = id_counter.entry(id.to_string()).or_insert(0);
-    if *id_count != 0 {
-        // FIXME: This should be a loop to ensure that the new ID is also unique.
-        write!(id, "-{id_count}").unwrap();
+/// Keeps a set of all previously returned IDs; if the requested id is already
+/// used, numeric suffixes (-1, -2, ...) are tried until an unused one is found.
+pub(crate) fn unique_id(id: &str, used: &mut HashSet<String>) -> String {
+    if used.insert(id.to_string()) {
+        return id.to_string();
+    }
+
+    // This ID is already in use. Generate one that is not by appending a
+    // numeric suffix.
+    let mut counter: u32 = 1;
+    loop {
+        let candidate = format!("{id}-{counter}");
+        if used.insert(candidate.clone()) {
+            return candidate;
+        }
+        counter += 1;
     }
-    *id_count += 1;
-    id
 }
 
 /// Generates an HTML id from the given text.

tests/testsuite/rendering/header_links/expected/header_links.html

@@ -6,5 +6,5 @@ <h2 id="hï"><a class="header" href="#hï">Hï</a></h2>
 <h2 id="repeat"><a class="header" href="#repeat">Repeat</a></h2>
 <h2 id="repeat-1"><a class="header" href="#repeat-1">Repeat</a></h2>
 <h2 id="repeat-2"><a class="header" href="#repeat-2">Repeat</a></h2>
-<h2 id="repeat-1"><a class="header" href="#repeat-1">Repeat 1</a></h2>
+<h2 id="repeat-1-1"><a class="header" href="#repeat-1-1">Repeat 1</a></h2>
 <h2 id="with-emphasis-bold-bold_emphasis-code-escaped-html-link-httpsexamplecom"><a class="header" href="#with-emphasis-bold-bold_emphasis-code-escaped-html-link-httpsexamplecom"><!--comment--> With <em>emphasis</em> <strong>bold</strong> <strong><em>bold_emphasis</em></strong> <code>code</code> &lt;escaped&gt; <span>html</span> <a href="https://example.com/link">link</a> <a href="https://example.com/">https://example.com/</a></a></h2>