Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test linking #1646

Merged
merged 10 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 \
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 14 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 1 addition & 0 deletions book.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions docs/authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
31 changes: 30 additions & 1 deletion mdbook-spec/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions mdbook-spec/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
126 changes: 66 additions & 60 deletions mdbook-spec/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)?;
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions mdbook-spec/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading