Rollup merge of #79522 - ehuss:lint-check-validate, r=Mark-Simulacrum

Validate lint docs separately.

This addresses some concerns raised in #76549 (comment) about errors with the lint docs being confusing and cumbersome. Errors from validating the lint documentation were being generated during `x.py doc` (and `x.py dist`), since extraction and validation are being done in a single step. This changes it so that extraction and validation are separated, so that `x.py doc` will not error if there is a validation problem, and tests are moved to `x.py test src/tools/lint-docs`.

This includes the following changes:

* Separate validation to `x.py test`.
* Added some more documentation on how to more easily modify and test the docs.
* Added more help to the error messages to hopefully provide more information on how to fix things.

The first commit just moves the code around, so you may consider looking at the other commits for a smaller diff.

compiler/rustc_lint_defs/src/lib.rs

@@ -366,11 +366,25 @@ impl LintBuffer {
 /// ```
 ///
 /// The `{{produces}}` tag will be automatically replaced with the output from
-/// the example by the build system. You can build and view the rustc book
-/// with `x.py doc --stage=1 src/doc/rustc --open`. If the lint example is too
-/// complex to run as a simple example (for example, it needs an extern
-/// crate), mark it with `ignore` and manually paste the expected output below
-/// the example.
+/// the example by the build system. If the lint example is too complex to run
+/// as a simple example (for example, it needs an extern crate), mark the code
+/// block with `ignore` and manually replace the `{{produces}}` line with the
+/// expected output in a `text` code block.
+///
+/// If this is a rustdoc-only lint, then only include a brief introduction
+/// with a link with the text `[rustdoc book]` so that the validator knows
+/// that this is for rustdoc only (see BROKEN_INTRA_DOC_LINKS as an example).
+///
+/// Commands to view and test the documentation:
+///
+/// * `./x.py doc --stage=1 src/doc/rustc --open`: Builds the rustc book and opens it.
+/// * `./x.py test src/tools/lint-docs`: Validates that the lint docs have the
+///   correct style, and that the code example actually emits the expected
+///   lint.
+///
+/// If you have already built the compiler, and you want to make changes to
+/// just the doc comments, then use the `--keep-stage=0` flag with the above
+/// commands to avoid rebuilding the compiler.
 #[macro_export]
 macro_rules! declare_lint {
     ($(#[$attr:meta])* $vis: vis $NAME: ident, $Level: ident, $desc: expr) => (

src/bootstrap/builder.rs

@@ -413,6 +413,7 @@ impl<'a> Builder<'a> {
                 test::TheBook,
                 test::UnstableBook,
                 test::RustcBook,
+                test::LintDocs,
                 test::RustcGuide,
                 test::EmbeddedBook,
                 test::EditionGuide,

src/bootstrap/doc.rs

@@ -726,6 +726,7 @@ fn symlink_dir_force(config: &Config, src: &Path, dst: &Path) -> io::Result<()>
 pub struct RustcBook {
     pub compiler: Compiler,
     pub target: TargetSelection,
+    pub validate: bool,
 }
 
 impl Step for RustcBook {
@@ -742,6 +743,7 @@ impl Step for RustcBook {
         run.builder.ensure(RustcBook {
             compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
             target: run.target,
+            validate: false,
         });
     }
 
@@ -772,6 +774,9 @@ impl Step for RustcBook {
         if builder.config.verbose() {
             cmd.arg("--verbose");
         }
+        if self.validate {
+            cmd.arg("--validate");
+        }
         // If the lib directories are in an unusual location (changed in
         // config.toml), then this needs to explicitly update the dylib search
         // path.

src/bootstrap/test.rs

@@ -2115,3 +2115,36 @@ impl Step for TierCheck {
         try_run(builder, &mut cargo.into());
     }
 }
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub struct LintDocs {
+    pub compiler: Compiler,
+    pub target: TargetSelection,
+}
+
+impl Step for LintDocs {
+    type Output = ();
+    const DEFAULT: bool = true;
+    const ONLY_HOSTS: bool = true;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.path("src/tools/lint-docs")
+    }
+
+    fn make_run(run: RunConfig<'_>) {
+        run.builder.ensure(LintDocs {
+            compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
+            target: run.target,
+        });
+    }
+
+    /// Tests that the lint examples in the rustc book generate the correct
+    /// lints and have the expected format.
+    fn run(self, builder: &Builder<'_>) {
+        builder.ensure(crate::doc::RustcBook {
+            compiler: self.compiler,
+            target: self.target,
+            validate: true,
+        });
+    }
+}

src/tools/lint-docs/src/groups.rs

@@ -1,11 +1,11 @@
-use crate::Lint;
+use crate::{Lint, LintExtractor};
 use std::collections::{BTreeMap, BTreeSet};
 use std::error::Error;
 use std::fmt::Write;
 use std::fs;
-use std::path::Path;
 use std::process::Command;
 
+/// Descriptions of rustc lint groups.
 static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
     ("unused", "Lints that detect things being declared but not used, or excess syntax"),
     ("rustdoc", "Rustdoc-specific lints"),
@@ -15,100 +15,123 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
     ("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),
 ];
 
-/// Updates the documentation of lint groups.
-pub(crate) fn generate_group_docs(
-    lints: &[Lint],
-    rustc: crate::Rustc<'_>,
-    out_path: &Path,
-) -> Result<(), Box<dyn Error>> {
-    let groups = collect_groups(rustc)?;
-    let groups_path = out_path.join("groups.md");
-    let contents = fs::read_to_string(&groups_path)
-        .map_err(|e| format!("could not read {}: {}", groups_path.display(), e))?;
-    let new_contents = contents.replace("{{groups-table}}", &make_groups_table(lints, &groups)?);
-    // Delete the output because rustbuild uses hard links in its copies.
-    let _ = fs::remove_file(&groups_path);
-    fs::write(&groups_path, new_contents)
-        .map_err(|e| format!("could not write to {}: {}", groups_path.display(), e))?;
-    Ok(())
-}
-
 type LintGroups = BTreeMap<String, BTreeSet<String>>;
 
-/// Collects the group names from rustc.
-fn collect_groups(rustc: crate::Rustc<'_>) -> Result<LintGroups, Box<dyn Error>> {
-    let mut result = BTreeMap::new();
-    let mut cmd = Command::new(rustc.path);
-    cmd.arg("-Whelp");
-    let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?;
-    if !output.status.success() {
-        return Err(format!(
-            "failed to collect lint info: {:?}\n--- stderr\n{}--- stdout\n{}\n",
-            output.status,
-            std::str::from_utf8(&output.stderr).unwrap(),
-            std::str::from_utf8(&output.stdout).unwrap(),
-        )
-        .into());
+impl<'a> LintExtractor<'a> {
+    /// Updates the documentation of lint groups.
+    pub(crate) fn generate_group_docs(&self, lints: &[Lint]) -> Result<(), Box<dyn Error>> {
+        let groups = self.collect_groups()?;
+        let groups_path = self.out_path.join("groups.md");
+        let contents = fs::read_to_string(&groups_path)
+            .map_err(|e| format!("could not read {}: {}", groups_path.display(), e))?;
+        let new_contents =
+            contents.replace("{{groups-table}}", &self.make_groups_table(lints, &groups)?);
+        // Delete the output because rustbuild uses hard links in its copies.
+        let _ = fs::remove_file(&groups_path);
+        fs::write(&groups_path, new_contents)
+            .map_err(|e| format!("could not write to {}: {}", groups_path.display(), e))?;
+        Ok(())
     }
-    let stdout = std::str::from_utf8(&output.stdout).unwrap();
-    let lines = stdout.lines();
-    let group_start = lines.skip_while(|line| !line.contains("groups provided")).skip(1);
-    let table_start = group_start.skip_while(|line| !line.contains("----")).skip(1);
-    for line in table_start {
-        if line.is_empty() {
-            break;
+
+    /// Collects the group names from rustc.
+    fn collect_groups(&self) -> Result<LintGroups, Box<dyn Error>> {
+        let mut result = BTreeMap::new();
+        let mut cmd = Command::new(self.rustc_path);
+        cmd.arg("-Whelp");
+        let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?;
+        if !output.status.success() {
+            return Err(format!(
+                "failed to collect lint info: {:?}\n--- stderr\n{}--- stdout\n{}\n",
+                output.status,
+                std::str::from_utf8(&output.stderr).unwrap(),
+                std::str::from_utf8(&output.stdout).unwrap(),
+            )
+            .into());
         }
-        let mut parts = line.trim().splitn(2, ' ');
-        let name = parts.next().expect("name in group");
-        if name == "warnings" {
-            // This is special.
-            continue;
+        let stdout = std::str::from_utf8(&output.stdout).unwrap();
+        let lines = stdout.lines();
+        let group_start = lines.skip_while(|line| !line.contains("groups provided")).skip(1);
+        let table_start = group_start.skip_while(|line| !line.contains("----")).skip(1);
+        for line in table_start {
+            if line.is_empty() {
+                break;
+            }
+            let mut parts = line.trim().splitn(2, ' ');
+            let name = parts.next().expect("name in group");
+            if name == "warnings" {
+                // This is special.
+                continue;
+            }
+            let lints = parts
+                .next()
+                .ok_or_else(|| format!("expected lints following name, got `{}`", line))?;
+            let lints = lints.split(',').map(|l| l.trim().to_string()).collect();
+            assert!(result.insert(name.to_string(), lints).is_none());
         }
-        let lints =
-            parts.next().ok_or_else(|| format!("expected lints following name, got `{}`", line))?;
-        let lints = lints.split(',').map(|l| l.trim().to_string()).collect();
-        assert!(result.insert(name.to_string(), lints).is_none());
-    }
-    if result.is_empty() {
-        return Err(
-            format!("expected at least one group in -Whelp output, got:\n{}", stdout).into()
-        );
+        if result.is_empty() {
+            return Err(
+                format!("expected at least one group in -Whelp output, got:\n{}", stdout).into()
+            );
+        }
+        Ok(result)
     }
-    Ok(result)
-}
 
-fn make_groups_table(lints: &[Lint], groups: &LintGroups) -> Result<String, Box<dyn Error>> {
-    let mut result = String::new();
-    let mut to_link = Vec::new();
-    result.push_str("| Group | Description | Lints |\n");
-    result.push_str("|-------|-------------|-------|\n");
-    result.push_str("| warnings | All lints that are set to issue warnings | See [warn-by-default] for the default set of warnings |\n");
-    for (group_name, group_lints) in groups {
-        let description = GROUP_DESCRIPTIONS.iter().find(|(n, _)| n == group_name)
-            .ok_or_else(|| format!("lint group `{}` does not have a description, please update the GROUP_DESCRIPTIONS list", group_name))?
-            .1;
-        to_link.extend(group_lints);
-        let brackets: Vec<_> = group_lints.iter().map(|l| format!("[{}]", l)).collect();
-        write!(result, "| {} | {} | {} |\n", group_name, description, brackets.join(", ")).unwrap();
-    }
-    result.push('\n');
-    result.push_str("[warn-by-default]: listing/warn-by-default.md\n");
-    for lint_name in to_link {
-        let lint_def =
-            lints.iter().find(|l| l.name == lint_name.replace("-", "_")).ok_or_else(|| {
-                format!(
-                    "`rustc -W help` defined lint `{}` but that lint does not appear to exist",
-                    lint_name
-                )
-            })?;
-        write!(
-            result,
-            "[{}]: listing/{}#{}\n",
-            lint_name,
-            lint_def.level.doc_filename(),
-            lint_name
-        )
-        .unwrap();
+    fn make_groups_table(
+        &self,
+        lints: &[Lint],
+        groups: &LintGroups,
+    ) -> Result<String, Box<dyn Error>> {
+        let mut result = String::new();
+        let mut to_link = Vec::new();
+        result.push_str("| Group | Description | Lints |\n");
+        result.push_str("|-------|-------------|-------|\n");
+        result.push_str("| warnings | All lints that are set to issue warnings | See [warn-by-default] for the default set of warnings |\n");
+        for (group_name, group_lints) in groups {
+            let description = match GROUP_DESCRIPTIONS.iter().find(|(n, _)| n == group_name) {
+                Some((_, desc)) => desc,
+                None if self.validate => {
+                    return Err(format!(
+                        "lint group `{}` does not have a description, \
+                         please update the GROUP_DESCRIPTIONS list in \
+                         src/tools/lint-docs/src/groups.rs",
+                        group_name
+                    )
+                    .into());
+                }
+                None => {
+                    eprintln!(
+                        "warning: lint group `{}` is missing from the GROUP_DESCRIPTIONS list\n\
+                         If this is a new lint group, please update the GROUP_DESCRIPTIONS in \
+                         src/tools/lint-docs/src/groups.rs",
+                        group_name
+                    );
+                    continue;
+                }
+            };
+            to_link.extend(group_lints);
+            let brackets: Vec<_> = group_lints.iter().map(|l| format!("[{}]", l)).collect();
+            write!(result, "| {} | {} | {} |\n", group_name, description, brackets.join(", "))
+                .unwrap();
+        }
+        result.push('\n');
+        result.push_str("[warn-by-default]: listing/warn-by-default.md\n");
+        for lint_name in to_link {
+            let lint_def =
+                lints.iter().find(|l| l.name == lint_name.replace("-", "_")).ok_or_else(|| {
+                    format!(
+                        "`rustc -W help` defined lint `{}` but that lint does not appear to exist",
+                        lint_name
+                    )
+                })?;
+            write!(
+                result,
+                "[{}]: listing/{}#{}\n",
+                lint_name,
+                lint_def.level.doc_filename(),
+                lint_name
+            )
+            .unwrap();
+        }
+        Ok(result)
     }
-    Ok(result)
 }