Move rule collection into a separate pass with a new structure

This adds a "Rules" struct which encapsulates the rules in the document.
This also removes some of the mutable variables.
This is paving the way for future updates which extends information
and processing about rules.

Author: ehuss

mdbook-spec/src/lib.rs

@@ -1,5 +1,6 @@
 #![deny(rust_2018_idioms, unused_lifetimes)]
 
+use crate::rules::Rules;
 use anyhow::Result;
 use mdbook::book::{Book, Chapter};
 use mdbook::errors::Error;
@@ -8,9 +9,7 @@ use mdbook::BookItem;
 use once_cell::sync::Lazy;
 use regex::{Captures, Regex};
 use semver::{Version, VersionReq};
-use std::collections::BTreeMap;
 use std::io;
-use std::path::PathBuf;
 
 mod rules;
 mod std_links;
@@ -58,13 +57,10 @@ impl Spec {
 
     /// Generates link references to all rules on all pages, so you can easily
     /// refer to rules anywhere in the book.
-    fn auto_link_references(
-        &self,
-        chapter: &Chapter,
-        found_rules: &BTreeMap<String, (PathBuf, PathBuf)>,
-    ) -> String {
+    fn auto_link_references(&self, chapter: &Chapter, rules: &Rules) -> String {
         let current_path = chapter.path.as_ref().unwrap().parent().unwrap();
-        let definitions: String = found_rules
+        let definitions: String = rules
+            .def_paths
             .iter()
             .map(|(rule_id, (_, path))| {
                 let relative = pathdiff::diff_paths(path, current_path).unwrap();
@@ -125,28 +121,20 @@ impl Preprocessor for Spec {
     }
 
     fn run(&self, _ctx: &PreprocessorContext, mut book: Book) -> Result<Book, Error> {
-        let mut found_rules = BTreeMap::new();
+        let rules = self.collect_rules(&book);
+
         book.for_each_mut(|item| {
             let BookItem::Chapter(ch) = item else {
                 return;
             };
             if ch.is_draft_chapter() {
                 return;
             }
-            ch.content = self.rule_definitions(&ch, &mut found_rules);
             ch.content = self.admonitions(&ch);
+            ch.content = self.auto_link_references(&ch, &rules);
+            ch.content = self.render_rule_definitions(&ch.content);
         });
-        // This is a separate pass because it relies on the modifications of
-        // the previous passes.
-        book.for_each_mut(|item| {
-            let BookItem::Chapter(ch) = item else {
-                return;
-            };
-            if ch.is_draft_chapter() {
-                return;
-            }
-            ch.content = self.auto_link_references(&ch, &found_rules);
-        });
+
         // Final pass will resolve everything as a std link (or error if the
         // link is unknown).
         std_links::std_links(&mut book);

mdbook-spec/src/rules.rs

@@ -1,7 +1,8 @@
 //! Handling for rule identifiers.
 
 use crate::Spec;
-use mdbook::book::Chapter;
+use mdbook::book::Book;
+use mdbook::BookItem;
 use once_cell::sync::Lazy;
 use regex::{Captures, Regex};
 use std::collections::BTreeMap;
@@ -10,33 +11,65 @@ use std::path::PathBuf;
 /// The Regex for rules like `r[foo]`.
 static RULE_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?m)^r\[([^]]+)]$").unwrap());
 
+/// The set of rules defined in the reference.
+#[derive(Default)]
+pub struct Rules {
+    /// A mapping from a rule identifier to a tuple of `(source_path, path)`.
+    ///
+    /// `source_path` is the path to the markdown source file relative to the
+    /// `SUMMARY.md`.
+    ///
+    /// `path` is the same as `source_path`, except filenames like `README.md`
+    /// are translated to `index.md`. Which to use depends on if you are
+    /// trying to access the source files (`source_path`), or creating links
+    /// in the output (`path`).
+    pub def_paths: BTreeMap<String, (PathBuf, PathBuf)>,
+}
+
 impl Spec {
+    /// Collects all rule definitions in the book.
+    pub fn collect_rules(&self, book: &Book) -> Rules {
+        let mut rules = Rules::default();
+        for item in book.iter() {
+            let BookItem::Chapter(ch) = item else {
+                continue;
+            };
+            if ch.is_draft_chapter() {
+                continue;
+            }
+            RULE_RE
+                .captures_iter(&ch.content)
+                .for_each(|caps: Captures<'_>| {
+                    let rule_id = &caps[1];
+                    let source_path = ch.source_path.clone().unwrap_or_default();
+                    let path = ch.path.clone().unwrap_or_default();
+                    if let Some((old, _)) = rules
+                        .def_paths
+                        .insert(rule_id.to_string(), (source_path.clone(), path.clone()))
+                    {
+                        let message = format!(
+                            "rule `{rule_id}` defined multiple times\n\
+                             First location: {old:?}\n\
+                             Second location: {source_path:?}"
+                        );
+                        if self.deny_warnings {
+                            panic!("error: {message}");
+                        } else {
+                            eprintln!("warning: {message}");
+                        }
+                    }
+                });
+        }
+
+        rules
+    }
+
     /// Converts lines that start with `r[…]` into a "rule" which has special
     /// styling and can be linked to.
-    pub fn rule_definitions(
-        &self,
-        chapter: &Chapter,
-        found_rules: &mut BTreeMap<String, (PathBuf, PathBuf)>,
-    ) -> String {
-        let source_path = chapter.source_path.clone().unwrap_or_default();
-        let path = chapter.path.clone().unwrap_or_default();
+    pub fn render_rule_definitions(&self, content: &str) -> String {
         RULE_RE
-            .replace_all(&chapter.content, |caps: &Captures<'_>| {
+            .replace_all(content, |caps: &Captures<'_>| {
                 let rule_id = &caps[1];
-                if let Some((old, _)) =
-                    found_rules.insert(rule_id.to_string(), (source_path.clone(), path.clone()))
-                {
-                    let message = format!(
-                        "rule `{rule_id}` defined multiple times\n\
-                        First location: {old:?}\n\
-                        Second location: {source_path:?}"
-                    );
-                    if self.deny_warnings {
-                        panic!("error: {message}");
-                    } else {
-                        eprintln!("warning: {message}");
-                    }
-                }
                 format!(
                     "<div class=\"rule\" id=\"r-{rule_id}\">\
                      <a class=\"rule-link\" href=\"#r-{rule_id}\">[{rule_id}]</a>\