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

Validate lint docs separately. #79522

Merged
merged 4 commits into from
Dec 1, 2020
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
24 changes: 19 additions & 5 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => (
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ impl<'a> Builder<'a> {
test::TheBook,
test::UnstableBook,
test::RustcBook,
test::LintDocs,
test::RustcGuide,
test::EmbeddedBook,
test::EditionGuide,
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
});
}

Expand Down Expand Up @@ -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.
Expand Down
33 changes: 33 additions & 0 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
}
203 changes: 113 additions & 90 deletions src/tools/lint-docs/src/groups.rs
Original file line number Diff line number Diff line change
@@ -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"),
Expand All @@ -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)
}
Loading