diff options
| -rw-r--r-- | compiler/rustc_lint_defs/src/lib.rs | 24 | ||||
| -rw-r--r-- | src/bootstrap/builder.rs | 1 | ||||
| -rw-r--r-- | src/bootstrap/doc.rs | 5 | ||||
| -rw-r--r-- | src/bootstrap/test.rs | 33 | ||||
| -rw-r--r-- | src/tools/lint-docs/src/groups.rs | 203 | ||||
| -rw-r--r-- | src/tools/lint-docs/src/lib.rs | 756 | ||||
| -rw-r--r-- | src/tools/lint-docs/src/main.rs | 33 |
7 files changed, 580 insertions, 475 deletions
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index af9926400ca..aec0fc253ca 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/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) => ( diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 508d785834f..9336d7165ee 100644 --- a/src/bootstrap/builder.rs +++ b/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, diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index af7f7eff894..bb0555c227d 100644 --- a/src/bootstrap/doc.rs +++ b/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. diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index e087e2b8ff1..611fecca054 100644 --- a/src/bootstrap/test.rs +++ b/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, + }); + } +} diff --git a/src/tools/lint-docs/src/groups.rs b/src/tools/lint-docs/src/groups.rs index 6b32ebdc284..0a69b18a332 100644 --- a/src/tools/lint-docs/src/groups.rs +++ b/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) } diff --git a/src/tools/lint-docs/src/lib.rs b/src/tools/lint-docs/src/lib.rs index 6ca71dcaf3c..326b7948098 100644 --- a/src/tools/lint-docs/src/lib.rs +++ b/src/tools/lint-docs/src/lib.rs @@ -7,6 +7,22 @@ use walkdir::WalkDir; mod groups; +pub struct LintExtractor<'a> { + /// Path to the `src` directory, where it will scan for `.rs` files to + /// find lint declarations. + pub src_path: &'a Path, + /// Path where to save the output. + pub out_path: &'a Path, + /// Path to the `rustc` executable. + pub rustc_path: &'a Path, + /// The target arch to build the docs for. + pub rustc_target: &'a str, + /// Verbose output. + pub verbose: bool, + /// Validate the style and the code example. + pub validate: bool, +} + struct Lint { name: String, doc: Vec<String>, @@ -26,6 +42,28 @@ impl Lint { .filter(|line| line.starts_with("```rust")) .all(|line| line.contains(",ignore")) } + + /// Checks the doc style of the lint. + fn check_style(&self) -> Result<(), Box<dyn Error>> { + for &expected in &["### Example", "### Explanation", "{{produces}}"] { + if expected == "{{produces}}" && self.is_ignored() { + continue; + } + if !self.doc_contains(expected) { + return Err(format!("lint docs should contain the line `{}`", expected).into()); + } + } + if let Some(first) = self.doc.first() { + if !first.starts_with(&format!("The `{}` lint", self.name)) { + return Err(format!( + "lint docs should start with the text \"The `{}` lint\" to introduce the lint", + self.name + ) + .into()); + } + } + Ok(()) + } } #[derive(Clone, Copy, PartialEq)] @@ -45,382 +83,396 @@ impl Level { } } -#[derive(Copy, Clone)] -pub struct Rustc<'a> { - pub path: &'a Path, - pub target: &'a str, -} - -/// Collects all lints, and writes the markdown documentation at the given directory. -pub fn extract_lint_docs( - src_path: &Path, - out_path: &Path, - rustc: Rustc<'_>, - verbose: bool, -) -> Result<(), Box<dyn Error>> { - let mut lints = gather_lints(src_path)?; - for lint in &mut lints { - generate_output_example(lint, rustc, verbose).map_err(|e| { - format!( - "failed to test example in lint docs for `{}` in {}:{}: {}", - lint.name, - lint.path.display(), - lint.lineno, - e - ) - })?; +impl<'a> LintExtractor<'a> { + /// Collects all lints, and writes the markdown documentation at the given directory. + pub fn extract_lint_docs(&self) -> Result<(), Box<dyn Error>> { + let mut lints = self.gather_lints()?; + for lint in &mut lints { + self.generate_output_example(lint).map_err(|e| { + format!( + "failed to test example in lint docs for `{}` in {}:{}: {}", + lint.name, + lint.path.display(), + lint.lineno, + e + ) + })?; + } + self.save_lints_markdown(&lints)?; + self.generate_group_docs(&lints)?; + Ok(()) } - save_lints_markdown(&lints, &out_path.join("listing"))?; - groups::generate_group_docs(&lints, rustc, out_path)?; - Ok(()) -} -/// Collects all lints from all files in the given directory. -fn gather_lints(src_path: &Path) -> Result<Vec<Lint>, Box<dyn Error>> { - let mut lints = Vec::new(); - for entry in WalkDir::new(src_path).into_iter().filter_map(|e| e.ok()) { - if !entry.path().extension().map_or(false, |ext| ext == "rs") { - continue; + /// Collects all lints from all files in the given directory. + fn gather_lints(&self) -> Result<Vec<Lint>, Box<dyn Error>> { + let mut lints = Vec::new(); + for entry in WalkDir::new(self.src_path).into_iter().filter_map(|e| e.ok()) { + if !entry.path().extension().map_or(false, |ext| ext == "rs") { + continue; + } + lints.extend(self.lints_from_file(entry.path())?); } - lints.extend(lints_from_file(entry.path())?); - } - if lints.is_empty() { - return Err("no lints were found!".into()); + if lints.is_empty() { + return Err("no lints were found!".into()); + } + Ok(lints) } - Ok(lints) -} -/// Collects all lints from the given file. -fn lints_from_file(path: &Path) -> Result<Vec<Lint>, Box<dyn Error>> { - let mut lints = Vec::new(); - let contents = fs::read_to_string(path) - .map_err(|e| format!("could not read {}: {}", path.display(), e))?; - let mut lines = contents.lines().enumerate(); - loop { - // Find a lint declaration. - let lint_start = loop { - match lines.next() { - Some((lineno, line)) => { - if line.trim().starts_with("declare_lint!") { - break lineno + 1; + /// Collects all lints from the given file. + fn lints_from_file(&self, path: &Path) -> Result<Vec<Lint>, Box<dyn Error>> { + let mut lints = Vec::new(); + let contents = fs::read_to_string(path) + .map_err(|e| format!("could not read {}: {}", path.display(), e))?; + let mut lines = contents.lines().enumerate(); + 'outer: loop { + // Find a lint declaration. + let lint_start = loop { + match lines.next() { + Some((lineno, line)) => { + if line.trim().starts_with("declare_lint!") { + break lineno + 1; + } + } + None => return Ok(lints), + } + }; + // Read the lint. + let mut doc_lines = Vec::new(); + let (doc, name) = loop { + match lines.next() { + Some((lineno, line)) => { + let line = line.trim(); + if let Some(text) = line.strip_prefix("/// ") { + doc_lines.push(text.trim().to_string()); + } else if line.starts_with("///") { + doc_lines.push("".to_string()); + } else if line.starts_with("// ") { + // Ignore comments. + continue; + } else { + let name = lint_name(line).map_err(|e| { + format!( + "could not determine lint name in {}:{}: {}, line was `{}`", + path.display(), + lineno, + e, + line + ) + })?; + if doc_lines.is_empty() { + if self.validate { + return Err(format!( + "did not find doc lines for lint `{}` in {}", + name, + path.display() + ) + .into()); + } else { + eprintln!( + "warning: lint `{}` in {} does not define any doc lines, \ + these are required for the lint documentation", + name, + path.display() + ); + continue 'outer; + } + } + break (doc_lines, name); + } + } + None => { + return Err(format!( + "unexpected EOF for lint definition at {}:{}", + path.display(), + lint_start + ) + .into()); } } - None => return Ok(lints), + }; + // These lints are specifically undocumented. This should be reserved + // for internal rustc-lints only. + if name == "deprecated_in_future" { + continue; } - }; - // Read the lint. - let mut doc_lines = Vec::new(); - let (doc, name) = loop { - match lines.next() { - Some((lineno, line)) => { - let line = line.trim(); - if line.starts_with("/// ") { - doc_lines.push(line.trim()[4..].to_string()); - } else if line.starts_with("///") { - doc_lines.push("".to_string()); - } else if line.starts_with("// ") { - // Ignore comments. - continue; - } else { - let name = lint_name(line).map_err(|e| { - format!( - "could not determine lint name in {}:{}: {}, line was `{}`", - path.display(), - lineno, - e, - line - ) - })?; - if doc_lines.is_empty() { + // Read the level. + let level = loop { + match lines.next() { + // Ignore comments. + Some((_, line)) if line.trim().starts_with("// ") => {} + Some((lineno, line)) => match line.trim() { + "Allow," => break Level::Allow, + "Warn," => break Level::Warn, + "Deny," => break Level::Deny, + _ => { return Err(format!( - "did not find doc lines for lint `{}` in {}", - name, - path.display() + "unexpected lint level `{}` in {}:{}", + line, + path.display(), + lineno ) .into()); } - break (doc_lines, name); - } - } - None => { - return Err(format!( - "unexpected EOF for lint definition at {}:{}", - path.display(), - lint_start - ) - .into()); - } - } - }; - // These lints are specifically undocumented. This should be reserved - // for internal rustc-lints only. - if name == "deprecated_in_future" { - continue; - } - // Read the level. - let level = loop { - match lines.next() { - // Ignore comments. - Some((_, line)) if line.trim().starts_with("// ") => {} - Some((lineno, line)) => match line.trim() { - "Allow," => break Level::Allow, - "Warn," => break Level::Warn, - "Deny," => break Level::Deny, - _ => { + }, + None => { return Err(format!( - "unexpected lint level `{}` in {}:{}", - line, + "expected lint level in {}:{}, got EOF", path.display(), - lineno + lint_start ) .into()); } - }, - None => { - return Err(format!( - "expected lint level in {}:{}, got EOF", - path.display(), - lint_start - ) - .into()); } - } - }; - // The rest of the lint definition is ignored. - assert!(!doc.is_empty()); - lints.push(Lint { name, doc, level, path: PathBuf::from(path), lineno: lint_start }); - } -} - -/// Extracts the lint name (removing the visibility modifier, and checking validity). -fn lint_name(line: &str) -> Result<String, &'static str> { - // Skip over any potential `pub` visibility. - match line.trim().split(' ').next_back() { - Some(name) => { - if !name.ends_with(',') { - return Err("lint name should end with comma"); - } - let name = &name[..name.len() - 1]; - if !name.chars().all(|ch| ch.is_uppercase() || ch == '_') || name.is_empty() { - return Err("lint name did not have expected format"); - } - Ok(name.to_lowercase().to_string()) + }; + // The rest of the lint definition is ignored. + assert!(!doc.is_empty()); + lints.push(Lint { name, doc, level, path: PathBuf::from(path), lineno: lint_start }); } - None => Err("could not find lint name"), - } -} - -/// Mutates the lint definition to replace the `{{produces}}` marker with the -/// actual output from the compiler. -fn generate_output_example( - lint: &mut Lint, - rustc: Rustc<'_>, - verbose: bool, -) -> Result<(), Box<dyn Error>> { - // Explicit list of lints that are allowed to not have an example. Please - // try to avoid adding to this list. - if matches!( - lint.name.as_str(), - "unused_features" // broken lint - | "unstable_features" // deprecated - ) { - return Ok(()); - } - if lint.doc_contains("[rustdoc book]") && !lint.doc_contains("{{produces}}") { - // Rustdoc lints are documented in the rustdoc book, don't check these. - return Ok(()); } - check_style(lint)?; - // Unfortunately some lints have extra requirements that this simple test - // setup can't handle (like extern crates). An alternative is to use a - // separate test suite, and use an include mechanism such as mdbook's - // `{{#rustdoc_include}}`. - if !lint.is_ignored() { - replace_produces(lint, rustc, verbose)?; - } - Ok(()) -} -/// Checks the doc style of the lint. -fn check_style(lint: &Lint) -> Result<(), Box<dyn Error>> { - for &expected in &["### Example", "### Explanation", "{{produces}}"] { - if expected == "{{produces}}" && lint.is_ignored() { - continue; + /// Mutates the lint definition to replace the `{{produces}}` marker with the + /// actual output from the compiler. + fn generate_output_example(&self, lint: &mut Lint) -> Result<(), Box<dyn Error>> { + // Explicit list of lints that are allowed to not have an example. Please + // try to avoid adding to this list. + if matches!( + lint.name.as_str(), + "unused_features" // broken lint + | "unstable_features" // deprecated + ) { + return Ok(()); } - if !lint.doc_contains(expected) { - return Err(format!("lint docs should contain the line `{}`", expected).into()); + if lint.doc_contains("[rustdoc book]") && !lint.doc_contains("{{produces}}") { + // Rustdoc lints are documented in the rustdoc book, don't check these. + return Ok(()); } - } - if let Some(first) = lint.doc.first() { - if !first.starts_with(&format!("The `{}` lint", lint.name)) { - return Err(format!( - "lint docs should start with the text \"The `{}` lint\" to introduce the lint", - lint.name - ) - .into()); + if self.validate { + lint.check_style()?; + } + // Unfortunately some lints have extra requirements that this simple test + // setup can't handle (like extern crates). An alternative is to use a + // separate test suite, and use an include mechanism such as mdbook's + // `{{#rustdoc_include}}`. + if !lint.is_ignored() { + if let Err(e) = self.replace_produces(lint) { + if self.validate { + return Err(e); + } + eprintln!( + "warning: the code example in lint `{}` in {} failed to \ + generate the expected output: {}", + lint.name, + lint.path.display(), + e + ); + } } + Ok(()) } - Ok(()) -} -/// Mutates the lint docs to replace the `{{produces}}` marker with the actual -/// output from the compiler. -fn replace_produces( - lint: &mut Lint, - rustc: Rustc<'_>, - verbose: bool, -) -> Result<(), Box<dyn Error>> { - let mut lines = lint.doc.iter_mut(); - loop { - // Find start of example. - let options = loop { - match lines.next() { - Some(line) if line.starts_with("```rust") => { - break line[7..].split(',').collect::<Vec<_>>(); + /// Mutates the lint docs to replace the `{{produces}}` marker with the actual + /// output from the compiler. + fn replace_produces(&self, lint: &mut Lint) -> Result<(), Box<dyn Error>> { + let mut lines = lint.doc.iter_mut(); + loop { + // Find start of example. + let options = loop { + match lines.next() { + Some(line) if line.starts_with("```rust") => { + break line[7..].split(',').collect::<Vec<_>>(); + } + Some(line) if line.contains("{{produces}}") => { + return Err("lint marker {{{{produces}}}} found, \ + but expected to immediately follow a rust code block" + .into()); + } + Some(_) => {} + None => return Ok(()), } - Some(line) if line.contains("{{produces}}") => { - return Err("lint marker {{{{produces}}}} found, \ - but expected to immediately follow a rust code block" + }; + // Find the end of example. + let mut example = Vec::new(); + loop { + match lines.next() { + Some(line) if line == "```" => break, + Some(line) => example.push(line), + None => { + return Err(format!( + "did not find end of example triple ticks ```, docs were:\n{:?}", + lint.doc + ) .into()); - } - Some(_) => {} - None => return Ok(()), - } - }; - // Find the end of example. - let mut example = Vec::new(); - loop { - match lines.next() { - Some(line) if line == "```" => break, - Some(line) => example.push(line), - None => { - return Err(format!( - "did not find end of example triple ticks ```, docs were:\n{:?}", - lint.doc - ) - .into()); + } } } - } - // Find the {{produces}} line. - loop { - match lines.next() { - Some(line) if line.is_empty() => {} - Some(line) if line == "{{produces}}" => { - let output = - generate_lint_output(&lint.name, &example, &options, rustc, verbose)?; - line.replace_range( - .., - &format!( - "This will produce:\n\ - \n\ - ```text\n\ - {}\ - ```", - output - ), - ); - break; + // Find the {{produces}} line. + loop { + match lines.next() { + Some(line) if line.is_empty() => {} + Some(line) if line == "{{produces}}" => { + let output = self.generate_lint_output(&lint.name, &example, &options)?; + line.replace_range( + .., + &format!( + "This will produce:\n\ + \n\ + ```text\n\ + {}\ + ```", + output + ), + ); + break; + } + // No {{produces}} after example, find next example. + Some(_line) => break, + None => return Ok(()), } - // No {{produces}} after example, find next example. - Some(_line) => break, - None => return Ok(()), } } } -} -/// Runs the compiler against the example, and extracts the output. -fn generate_lint_output( - name: &str, - example: &[&mut String], - options: &[&str], - rustc: Rustc<'_>, - verbose: bool, -) -> Result<String, Box<dyn Error>> { - if verbose { - eprintln!("compiling lint {}", name); - } - let tempdir = tempfile::TempDir::new()?; - let tempfile = tempdir.path().join("lint_example.rs"); - let mut source = String::new(); - let needs_main = !example.iter().any(|line| line.contains("fn main")); - // Remove `# ` prefix for hidden lines. - let unhidden = - example.iter().map(|line| if line.starts_with("# ") { &line[2..] } else { line }); - let mut lines = unhidden.peekable(); - while let Some(line) = lines.peek() { - if line.starts_with("#!") { + /// Runs the compiler against the example, and extracts the output. + fn generate_lint_output( + &self, + name: &str, + example: &[&mut String], + options: &[&str], + ) -> Result<String, Box<dyn Error>> { + if self.verbose { + eprintln!("compiling lint {}", name); + } + let tempdir = tempfile::TempDir::new()?; + let tempfile = tempdir.path().join("lint_example.rs"); + let mut source = String::new(); + let needs_main = !example.iter().any(|line| line.contains("fn main")); + // Remove `# ` prefix for hidden lines. + let unhidden = example.iter().map(|line| line.strip_prefix("# ").unwrap_or(line)); + let mut lines = unhidden.peekable(); + while let Some(line) = lines.peek() { + if line.starts_with("#!") { + source.push_str(line); + source.push('\n'); + lines.next(); + } else { + break; + } + } + if needs_main { + source.push_str("fn main() {\n"); + } + for line in lines { source.push_str(line); - source.push('\n'); - lines.next(); + source.push('\n') + } + if needs_main { + source.push_str("}\n"); + } + fs::write(&tempfile, source) + .map_err(|e| format!("failed to write {}: {}", tempfile.display(), e))?; + let mut cmd = Command::new(self.rustc_path); + if options.contains(&"edition2015") { + cmd.arg("--edition=2015"); } else { - break; + cmd.arg("--edition=2018"); + } + cmd.arg("--error-format=json"); + cmd.arg("--target").arg(self.rustc_target); + if options.contains(&"test") { + cmd.arg("--test"); + } + cmd.arg("lint_example.rs"); + cmd.current_dir(tempdir.path()); + let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?; + let stderr = std::str::from_utf8(&output.stderr).unwrap(); + let msgs = stderr + .lines() + .filter(|line| line.starts_with('{')) + .map(serde_json::from_str) + .collect::<Result<Vec<serde_json::Value>, _>>()?; + match msgs + .iter() + .find(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s==name)) + { + Some(msg) => { + let rendered = msg["rendered"].as_str().expect("rendered field should exist"); + Ok(rendered.to_string()) + } + None => { + match msgs.iter().find( + |msg| matches!(&msg["rendered"], serde_json::Value::String(s) if s.contains(name)), + ) { + Some(msg) => { + let rendered = msg["rendered"].as_str().expect("rendered field should exist"); + Ok(rendered.to_string()) + } + None => { + let rendered: Vec<&str> = + msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect(); + let non_json: Vec<&str> = + stderr.lines().filter(|line| !line.starts_with('{')).collect(); + Err(format!( + "did not find lint `{}` in output of example, got:\n{}\n{}", + name, + non_json.join("\n"), + rendered.join("\n") + ) + .into()) + } + } + } } } - if needs_main { - source.push_str("fn main() {\n"); - } - for line in lines { - source.push_str(line); - source.push('\n') - } - if needs_main { - source.push_str("}\n"); - } - fs::write(&tempfile, source) - .map_err(|e| format!("failed to write {}: {}", tempfile.display(), e))?; - let mut cmd = Command::new(rustc.path); - if options.contains(&"edition2015") { - cmd.arg("--edition=2015"); - } else { - cmd.arg("--edition=2018"); - } - cmd.arg("--error-format=json"); - cmd.arg("--target").arg(rustc.target); - if options.contains(&"test") { - cmd.arg("--test"); + + /// Saves the mdbook lint chapters at the given path. + fn save_lints_markdown(&self, lints: &[Lint]) -> Result<(), Box<dyn Error>> { + self.save_level(lints, Level::Allow, ALLOWED_MD)?; + self.save_level(lints, Level::Warn, WARN_MD)?; + self.save_level(lints, Level::Deny, DENY_MD)?; + Ok(()) } - cmd.arg("lint_example.rs"); - cmd.current_dir(tempdir.path()); - let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?; - let stderr = std::str::from_utf8(&output.stderr).unwrap(); - let msgs = stderr - .lines() - .filter(|line| line.starts_with('{')) - .map(serde_json::from_str) - .collect::<Result<Vec<serde_json::Value>, _>>()?; - match msgs - .iter() - .find(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s==name)) - { - Some(msg) => { - let rendered = msg["rendered"].as_str().expect("rendered field should exist"); - Ok(rendered.to_string()) + + fn save_level(&self, lints: &[Lint], level: Level, header: &str) -> Result<(), Box<dyn Error>> { + let mut result = String::new(); + result.push_str(header); + let mut these_lints: Vec<_> = lints.iter().filter(|lint| lint.level == level).collect(); + these_lints.sort_unstable_by_key(|lint| &lint.name); + for lint in &these_lints { + write!(result, "* [`{}`](#{})\n", lint.name, lint.name.replace("_", "-")).unwrap(); } - None => { - match msgs.iter().find( - |msg| matches!(&msg["rendered"], serde_json::Value::String(s) if s.contains(name)), - ) { - Some(msg) => { - let rendered = msg["rendered"].as_str().expect("rendered field should exist"); - Ok(rendered.to_string()) - } - None => { - let rendered: Vec<&str> = - msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect(); - let non_json: Vec<&str> = - stderr.lines().filter(|line| !line.starts_with('{')).collect(); - Err(format!( - "did not find lint `{}` in output of example, got:\n{}\n{}", - name, - non_json.join("\n"), - rendered.join("\n") - ) - .into()) - } + result.push('\n'); + for lint in &these_lints { + write!(result, "## {}\n\n", lint.name.replace("_", "-")).unwrap(); + for line in &lint.doc { + result.push_str(line); + result.push('\n'); + } + result.push('\n'); + } + let out_path = self.out_path.join("listing").join(level.doc_filename()); + // Delete the output because rustbuild uses hard links in its copies. + let _ = fs::remove_file(&out_path); + fs::write(&out_path, result) + .map_err(|e| format!("could not write to {}: {}", out_path.display(), e))?; + Ok(()) + } +} + +/// Extracts the lint name (removing the visibility modifier, and checking validity). +fn lint_name(line: &str) -> Result<String, &'static str> { + // Skip over any potential `pub` visibility. + match line.trim().split(' ').next_back() { + Some(name) => { + if !name.ends_with(',') { + return Err("lint name should end with comma"); } + let name = &name[..name.len() - 1]; + if !name.chars().all(|ch| ch.is_uppercase() || ch == '_') || name.is_empty() { + return Err("lint name did not have expected format"); + } + Ok(name.to_lowercase().to_string()) } + None => Err("could not find lint name"), } } @@ -442,41 +494,3 @@ static DENY_MD: &str = r#"# Deny-by-default lints These lints are all set to the 'deny' level by default. "#; - -/// Saves the mdbook lint chapters at the given path. -fn save_lints_markdown(lints: &[Lint], out_dir: &Path) -> Result<(), Box<dyn Error>> { - save_level(lints, Level::Allow, out_dir, ALLOWED_MD)?; - save_level(lints, Level::Warn, out_dir, WARN_MD)?; - save_level(lints, Level::Deny, out_dir, DENY_MD)?; - Ok(()) -} - -fn save_level( - lints: &[Lint], - level: Level, - out_dir: &Path, - header: &str, -) -> Result<(), Box<dyn Error>> { - let mut result = String::new(); - result.push_str(header); - let mut these_lints: Vec<_> = lints.iter().filter(|lint| lint.level == level).collect(); - these_lints.sort_unstable_by_key(|lint| &lint.name); - for lint in &these_lints { - write!(result, "* [`{}`](#{})\n", lint.name, lint.name.replace("_", "-")).unwrap(); - } - result.push('\n'); - for lint in &these_lints { - write!(result, "## {}\n\n", lint.name.replace("_", "-")).unwrap(); - for line in &lint.doc { - result.push_str(line); - result.push('\n'); - } - result.push('\n'); - } - let out_path = out_dir.join(level.doc_filename()); - // Delete the output because rustbuild uses hard links in its copies. - let _ = fs::remove_file(&out_path); - fs::write(&out_path, result) - .map_err(|e| format!("could not write to {}: {}", out_path.display(), e))?; - Ok(()) -} diff --git a/src/tools/lint-docs/src/main.rs b/src/tools/lint-docs/src/main.rs index 5db49007d37..2055fed2b48 100644 --- a/src/tools/lint-docs/src/main.rs +++ b/src/tools/lint-docs/src/main.rs @@ -3,7 +3,20 @@ use std::path::PathBuf; fn main() { if let Err(e) = doit() { - println!("error: {}", e); + eprintln!("error: {}", e); + eprintln!( + " +This error was generated by the lint-docs tool. +This tool extracts documentation for lints from the source code and places +them in the rustc book. See the declare_lint! documentation +https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/macro.declare_lint.html +for an example of the format of documentation this tool expects. + +To re-run these tests, run: ./x.py test --keep-stage=0 src/tools/lint-docs +The --keep-stage flag should be used if you have already built the compiler +and are only modifying the doc comments to avoid rebuilding the compiler. +" + ); std::process::exit(1); } } @@ -15,6 +28,7 @@ fn doit() -> Result<(), Box<dyn Error>> { let mut rustc_path = None; let mut rustc_target = None; let mut verbose = false; + let mut validate = false; while let Some(arg) = args.next() { match arg.as_str() { "--src" => { @@ -42,6 +56,7 @@ fn doit() -> Result<(), Box<dyn Error>> { }; } "-v" | "--verbose" => verbose = true, + "--validate" => validate = true, s => return Err(format!("unexpected argument `{}`", s).into()), } } @@ -57,13 +72,13 @@ fn doit() -> Result<(), Box<dyn Error>> { if rustc_target.is_none() { return Err("--rustc-target must be specified to the rustc target".into()); } - lint_docs::extract_lint_docs( - &src_path.unwrap(), - &out_path.unwrap(), - lint_docs::Rustc { - path: rustc_path.as_deref().unwrap(), - target: rustc_target.as_deref().unwrap(), - }, + let le = lint_docs::LintExtractor { + src_path: &src_path.unwrap(), + out_path: &out_path.unwrap(), + rustc_path: &rustc_path.unwrap(), + rustc_target: &rustc_target.unwrap(), verbose, - ) + validate, + }; + le.extract_lint_docs() } |