Merge pull request #1646 from ehuss/test-summary

Add test linking

Author: ehuss

.github/workflows/main.yml

@@ -35,6 +35,11 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@master
+    - name: Checkout rust-lang/rust
+      uses: actions/checkout@master
+      with:
+        repository: rust-lang/rust
+        path: rust
     - name: Update rustup
       run: rustup self update
     - name: Install Rust
@@ -52,16 +57,17 @@ jobs:
         rustup --version
         rustc -Vv
         mdbook --version
-    - name: Verify the book builds
-      env:
-        SPEC_DENY_WARNINGS: 1
-      run: mdbook build
     - name: Style checks
       working-directory: style-check
       run: cargo run --locked -- ../src
     - name: Style fmt
       working-directory: style-check
       run: cargo fmt --check
+    - name: Verify the book builds
+      env:
+        SPEC_DENY_WARNINGS: 1
+        SPEC_RUST_ROOT: ${{ github.workspace }}/rust
+      run: mdbook build
     - name: Check for broken links
       run: |
         curl -sSLo linkcheck.sh \
@@ -103,6 +109,11 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@master
+    - name: Checkout rust-lang/rust
+      uses: actions/checkout@master
+      with:
+        repository: rust-lang/rust
+        path: rust
     - name: Update rustup
       run: rustup self update
     - name: Install Rust
@@ -117,7 +128,8 @@ jobs:
         echo "$(pwd)/bin" >> $GITHUB_PATH
     - name: Build the book
       env:
-          SPEC_RELATIVE: 0
+        SPEC_RELATIVE: 0
+        SPEC_RUST_ROOT: ${{ github.workspace }}/rust
       run: mdbook build --dest-dir dist/preview-${{ github.event.pull_request.number }}
     - name: Upload artifact
       uses: actions/upload-artifact@v4

README.md

@@ -66,10 +66,22 @@ SPEC_RELATIVE=0 mdbook build --open
 
 This will open a browser with a websocket live-link to automatically reload whenever the source is updated.
 
-The `SPEC_RELATIVE=0` environment variable makes links to the standard library go to <https://doc.rust-lang.org/> instead of being relative, which is useful when viewing locally since you normally don't have a copy of the standard library.
-
 You can also use mdbook's live webserver option, which will automatically rebuild the book and reload your web browser whenever a source file is modified:
 
 ```sh
 SPEC_RELATIVE=0 mdbook serve --open
 ```
+
+### `SPEC_RELATIVE`
+
+The `SPEC_RELATIVE=0` environment variable makes links to the standard library go to <https://doc.rust-lang.org/> instead of being relative, which is useful when viewing locally since you normally don't have a copy of the standard library.
+
+The published site at <https://doc.rust-lang.org/reference/> (or local docs using `rustup doc`) does not set this, which means it will use relative links which supports offline viewing and links to the correct version (for example, links in <https://doc.rust-lang.org/1.81.0/reference/> will stay within the 1.81.0 directory).
+
+### `SPEC_DENY_WARNINGS`
+
+The `SPEC_DENY_WARNINGS=1` environment variable will turn all warnings generated by `mdbook-spec` to errors. This is used in CI to ensure that there aren't any problems with the book content.
+
+### `SPEC_RUST_ROOT`
+
+The `SPEC_RUST_ROOT` can be used to point to the directory of a checkout of <https://github.com./rust-lang/rust>. This is used by the test-linking feature so that it can find tests linked to reference rules. If this is not set, then the tests won't be linked.

book.toml

@@ -5,6 +5,7 @@ author = "The Rust Project Developers"
 
 [output.html]
 additional-css = ["theme/reference.css"]
+additional-js = ["theme/reference.js"]
 git-repository-url = "https://github.com./rust-lang/reference/"
 edit-url-template = "https://github.com./rust-lang/reference/edit/master/{path}"
 smart-punctuation = true

docs/authoring.md

@@ -99,6 +99,22 @@ When assigning rules to new paragraphs, or when modifying rule names, use the fo
     * Target specific admonitions should typically be named by the least specific target property to which they apply (e.g. if a rule affects all x86 CPUs, the rule name should include `x86` rather than separately listing `i586`, `i686` and `x86_64`, and if a rule applies to all ELF platforms, it should be named `elf` rather than listing every ELF OS).
     * Use an appropriately descriptive, but short, name if the language does not provide one.
 
+#### Test rule annotations
+
+Tests in <https://github.com./rust-lang/rust> can be linked to rules in the reference. The rule will include a link to the tests, and there is also an [appendix] which tracks how the rules are currently linked.
+
+Tests in the `tests` directory can be annotated with the `//@ reference: x.y.z` header to link it to a rule. The header can be specified multiple times if a single file covers multiple rules.
+
+Compiler developers are not expected to add `reference` annotations to tests. However, if they do want to help, their cooperation is very welcome. Reference authors and editors are responsible for making sure every rule has a test associated with it.
+
+The tests are beneficial for reviewers to see the behavior of a rule. It is also a benefit to readers who may want to see examples of particular behaviors. When adding new rules, you should wait until the reference side is approved before submitting a PR to `rust-lang/rust` (to avoid churn if we decide on different names).
+
+Prefixed rule names should not be used in tests. That is, do not use something like `asm.rules` when there are specific rules like `asm.rules.reg-not-input`.
+
+We are not expecting 100% coverage at any time. Although it would be nice, it is unrealistic due to the sequence things are developed, and resources available.
+
+[appendix]: https://doc.rust-lang.org/nightly/reference/test-summary.html
+
 ### Standard library links
 
 You should link to the standard library without specifying a URL in a fashion similar to [rustdoc intra-doc links][intra]. Some examples:

mdbook-spec/Cargo.lock

@@ -19,3 +19,4 @@ regex = "1.9.4"
 semver = "1.0.21"
 serde_json = "1.0.113"
 tempfile = "3.10.1"
+walkdir = "2.5.0"

mdbook-spec/Cargo.toml

@@ -1,28 +1,29 @@
 #![deny(rust_2018_idioms, unused_lifetimes)]
 
+use crate::rules::Rules;
+use anyhow::{bail, Context, Result};
 use mdbook::book::{Book, Chapter};
 use mdbook::errors::Error;
 use mdbook::preprocess::{CmdPreprocessor, Preprocessor, PreprocessorContext};
 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;
-
-/// The Regex for rules like `r[foo]`.
-static RULE_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?m)^r\[([^]]+)]$").unwrap());
+mod test_links;
 
 /// The Regex for the syntax for blockquotes that have a specific CSS class,
 /// like `> [!WARNING]`.
 static ADMONITION_RE: Lazy<Regex> = Lazy::new(|| {
     Regex::new(r"(?m)^ *> \[!(?<admon>[^]]+)\]\n(?<blockquote>(?: *>.*\n)+)").unwrap()
 });
 
-pub fn handle_preprocessing(pre: &dyn Preprocessor) -> Result<(), Error> {
+pub fn handle_preprocessing() -> Result<(), Error> {
+    let pre = Spec::new()?;
     let (ctx, book) = CmdPreprocessor::parse_input(io::stdin())?;
 
     let book_version = Version::parse(&ctx.mdbook_version)?;
@@ -48,59 +49,45 @@ pub struct Spec {
     /// Whether or not warnings should be errors (set by SPEC_DENY_WARNINGS
     /// environment variable).
     deny_warnings: bool,
+    /// Path to the rust-lang/rust git repository (set by SPEC_RUST_ROOT
+    /// environment variable).
+    rust_root: Option<PathBuf>,
+    /// The git ref that can be used in a URL to the rust-lang/rust repository.
+    git_ref: String,
 }
 
 impl Spec {
-    pub fn new() -> Spec {
-        Spec {
-            deny_warnings: std::env::var("SPEC_DENY_WARNINGS").as_deref() == Ok("1"),
+    fn new() -> Result<Spec> {
+        let deny_warnings = std::env::var("SPEC_DENY_WARNINGS").as_deref() == Ok("1");
+        let rust_root = std::env::var_os("SPEC_RUST_ROOT").map(PathBuf::from);
+        if deny_warnings && rust_root.is_none() {
+            bail!("SPEC_RUST_ROOT environment variable must be set");
         }
-    }
-
-    /// Converts lines that start with `r[…]` into a "rule" which has special
-    /// styling and can be linked to.
-    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();
-        RULE_RE
-            .replace_all(&chapter.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}");
-                    }
+        let git_ref = match git_ref(&rust_root) {
+            Ok(s) => s,
+            Err(e) => {
+                if deny_warnings {
+                    eprintln!("error: {e:?}");
+                    std::process::exit(1);
+                } else {
+                    eprintln!("warning: {e:?}");
+                    "master".into()
                 }
-                format!(
-                    "<div class=\"rule\" id=\"r-{rule_id}\">\
-                     <a class=\"rule-link\" href=\"#r-{rule_id}\">[{rule_id}]</a>\
-                     </div>\n"
-                )
-            })
-            .to_string()
+            }
+        };
+        Ok(Spec {
+            deny_warnings,
+            rust_root,
+            git_ref,
+        })
     }
 
     /// 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();
@@ -155,34 +142,53 @@ fn to_initial_case(s: &str) -> String {
     format!("{first}{rest}")
 }
 
+/// Determines the git ref used for linking to a particular branch/tag in GitHub.
+fn git_ref(rust_root: &Option<PathBuf>) -> Result<String> {
+    let Some(rust_root) = rust_root else {
+        return Ok("master".into());
+    };
+    let channel = std::fs::read_to_string(rust_root.join("src/ci/channel"))
+        .context("failed to read src/ci/channel")?;
+    let git_ref = match channel.trim() {
+        // nightly/beta are branches, not stable references. Should be ok
+        // because we're not expecting those channels to be long-lived.
+        "nightly" => "master".into(),
+        "beta" => "beta".into(),
+        "stable" => {
+            let version = std::fs::read_to_string(rust_root.join("src/version"))
+                .context("|| failed to read src/version")?;
+            version.trim().into()
+        }
+        ch => bail!("unknown channel {ch}"),
+    };
+    Ok(git_ref)
+}
+
 impl Preprocessor for Spec {
     fn name(&self) -> &str {
         "spec"
     }
 
     fn run(&self, _ctx: &PreprocessorContext, mut book: Book) -> Result<Book, Error> {
-        let mut found_rules = BTreeMap::new();
+        let rules = self.collect_rules(&book);
+        let tests = self.collect_tests(&rules);
+        let summary_table = test_links::make_summary_table(&book, &tests, &rules);
+
         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);
-        });
-        // 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, &rules);
+            ch.content = self.render_rule_definitions(&ch.content, &tests);
+            if ch.name == "Test summary" {
+                ch.content = ch.content.replace("{{summary-table}}", &summary_table);
             }
-            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/lib.rs

@@ -12,9 +12,7 @@ fn main() {
         None => {}
     }
 
-    let preprocessor = mdbook_spec::Spec::new();
-
-    if let Err(e) = mdbook_spec::handle_preprocessing(&preprocessor) {
+    if let Err(e) = mdbook_spec::handle_preprocessing() {
         eprintln!("{}", e);
         std::process::exit(1);
     }