Skip to content

Commit 82630a6

Browse files
authored
Rollup merge of rust-lang#79522 - ehuss:lint-check-validate, r=Mark-Simulacrum
Validate lint docs separately. This addresses some concerns raised in rust-lang#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.
2 parents de32e4b + a90fdfc commit 82630a6

File tree

7 files changed

+580
-475
lines changed

7 files changed

+580
-475
lines changed

compiler/rustc_lint_defs/src/lib.rs

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

src/bootstrap/builder.rs

+1
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ impl<'a> Builder<'a> {
413413
test::TheBook,
414414
test::UnstableBook,
415415
test::RustcBook,
416+
test::LintDocs,
416417
test::RustcGuide,
417418
test::EmbeddedBook,
418419
test::EditionGuide,

src/bootstrap/doc.rs

+5
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,7 @@ fn symlink_dir_force(config: &Config, src: &Path, dst: &Path) -> io::Result<()>
726726
pub struct RustcBook {
727727
pub compiler: Compiler,
728728
pub target: TargetSelection,
729+
pub validate: bool,
729730
}
730731

731732
impl Step for RustcBook {
@@ -742,6 +743,7 @@ impl Step for RustcBook {
742743
run.builder.ensure(RustcBook {
743744
compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
744745
target: run.target,
746+
validate: false,
745747
});
746748
}
747749

@@ -772,6 +774,9 @@ impl Step for RustcBook {
772774
if builder.config.verbose() {
773775
cmd.arg("--verbose");
774776
}
777+
if self.validate {
778+
cmd.arg("--validate");
779+
}
775780
// If the lib directories are in an unusual location (changed in
776781
// config.toml), then this needs to explicitly update the dylib search
777782
// path.

src/bootstrap/test.rs

+33
Original file line numberDiff line numberDiff line change
@@ -2115,3 +2115,36 @@ impl Step for TierCheck {
21152115
try_run(builder, &mut cargo.into());
21162116
}
21172117
}
2118+
2119+
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
2120+
pub struct LintDocs {
2121+
pub compiler: Compiler,
2122+
pub target: TargetSelection,
2123+
}
2124+
2125+
impl Step for LintDocs {
2126+
type Output = ();
2127+
const DEFAULT: bool = true;
2128+
const ONLY_HOSTS: bool = true;
2129+
2130+
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
2131+
run.path("src/tools/lint-docs")
2132+
}
2133+
2134+
fn make_run(run: RunConfig<'_>) {
2135+
run.builder.ensure(LintDocs {
2136+
compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
2137+
target: run.target,
2138+
});
2139+
}
2140+
2141+
/// Tests that the lint examples in the rustc book generate the correct
2142+
/// lints and have the expected format.
2143+
fn run(self, builder: &Builder<'_>) {
2144+
builder.ensure(crate::doc::RustcBook {
2145+
compiler: self.compiler,
2146+
target: self.target,
2147+
validate: true,
2148+
});
2149+
}
2150+
}

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

+113-90
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
use crate::Lint;
1+
use crate::{Lint, LintExtractor};
22
use std::collections::{BTreeMap, BTreeSet};
33
use std::error::Error;
44
use std::fmt::Write;
55
use std::fs;
6-
use std::path::Path;
76
use std::process::Command;
87

8+
/// Descriptions of rustc lint groups.
99
static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
1010
("unused", "Lints that detect things being declared but not used, or excess syntax"),
1111
("rustdoc", "Rustdoc-specific lints"),
@@ -15,100 +15,123 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
1515
("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),
1616
];
1717

18-
/// Updates the documentation of lint groups.
19-
pub(crate) fn generate_group_docs(
20-
lints: &[Lint],
21-
rustc: crate::Rustc<'_>,
22-
out_path: &Path,
23-
) -> Result<(), Box<dyn Error>> {
24-
let groups = collect_groups(rustc)?;
25-
let groups_path = out_path.join("groups.md");
26-
let contents = fs::read_to_string(&groups_path)
27-
.map_err(|e| format!("could not read {}: {}", groups_path.display(), e))?;
28-
let new_contents = contents.replace("{{groups-table}}", &make_groups_table(lints, &groups)?);
29-
// Delete the output because rustbuild uses hard links in its copies.
30-
let _ = fs::remove_file(&groups_path);
31-
fs::write(&groups_path, new_contents)
32-
.map_err(|e| format!("could not write to {}: {}", groups_path.display(), e))?;
33-
Ok(())
34-
}
35-
3618
type LintGroups = BTreeMap<String, BTreeSet<String>>;
3719

38-
/// Collects the group names from rustc.
39-
fn collect_groups(rustc: crate::Rustc<'_>) -> Result<LintGroups, Box<dyn Error>> {
40-
let mut result = BTreeMap::new();
41-
let mut cmd = Command::new(rustc.path);
42-
cmd.arg("-Whelp");
43-
let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?;
44-
if !output.status.success() {
45-
return Err(format!(
46-
"failed to collect lint info: {:?}\n--- stderr\n{}--- stdout\n{}\n",
47-
output.status,
48-
std::str::from_utf8(&output.stderr).unwrap(),
49-
std::str::from_utf8(&output.stdout).unwrap(),
50-
)
51-
.into());
20+
impl<'a> LintExtractor<'a> {
21+
/// Updates the documentation of lint groups.
22+
pub(crate) fn generate_group_docs(&self, lints: &[Lint]) -> Result<(), Box<dyn Error>> {
23+
let groups = self.collect_groups()?;
24+
let groups_path = self.out_path.join("groups.md");
25+
let contents = fs::read_to_string(&groups_path)
26+
.map_err(|e| format!("could not read {}: {}", groups_path.display(), e))?;
27+
let new_contents =
28+
contents.replace("{{groups-table}}", &self.make_groups_table(lints, &groups)?);
29+
// Delete the output because rustbuild uses hard links in its copies.
30+
let _ = fs::remove_file(&groups_path);
31+
fs::write(&groups_path, new_contents)
32+
.map_err(|e| format!("could not write to {}: {}", groups_path.display(), e))?;
33+
Ok(())
5234
}
53-
let stdout = std::str::from_utf8(&output.stdout).unwrap();
54-
let lines = stdout.lines();
55-
let group_start = lines.skip_while(|line| !line.contains("groups provided")).skip(1);
56-
let table_start = group_start.skip_while(|line| !line.contains("----")).skip(1);
57-
for line in table_start {
58-
if line.is_empty() {
59-
break;
35+
36+
/// Collects the group names from rustc.
37+
fn collect_groups(&self) -> Result<LintGroups, Box<dyn Error>> {
38+
let mut result = BTreeMap::new();
39+
let mut cmd = Command::new(self.rustc_path);
40+
cmd.arg("-Whelp");
41+
let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?;
42+
if !output.status.success() {
43+
return Err(format!(
44+
"failed to collect lint info: {:?}\n--- stderr\n{}--- stdout\n{}\n",
45+
output.status,
46+
std::str::from_utf8(&output.stderr).unwrap(),
47+
std::str::from_utf8(&output.stdout).unwrap(),
48+
)
49+
.into());
6050
}
61-
let mut parts = line.trim().splitn(2, ' ');
62-
let name = parts.next().expect("name in group");
63-
if name == "warnings" {
64-
// This is special.
65-
continue;
51+
let stdout = std::str::from_utf8(&output.stdout).unwrap();
52+
let lines = stdout.lines();
53+
let group_start = lines.skip_while(|line| !line.contains("groups provided")).skip(1);
54+
let table_start = group_start.skip_while(|line| !line.contains("----")).skip(1);
55+
for line in table_start {
56+
if line.is_empty() {
57+
break;
58+
}
59+
let mut parts = line.trim().splitn(2, ' ');
60+
let name = parts.next().expect("name in group");
61+
if name == "warnings" {
62+
// This is special.
63+
continue;
64+
}
65+
let lints = parts
66+
.next()
67+
.ok_or_else(|| format!("expected lints following name, got `{}`", line))?;
68+
let lints = lints.split(',').map(|l| l.trim().to_string()).collect();
69+
assert!(result.insert(name.to_string(), lints).is_none());
6670
}
67-
let lints =
68-
parts.next().ok_or_else(|| format!("expected lints following name, got `{}`", line))?;
69-
let lints = lints.split(',').map(|l| l.trim().to_string()).collect();
70-
assert!(result.insert(name.to_string(), lints).is_none());
71-
}
72-
if result.is_empty() {
73-
return Err(
74-
format!("expected at least one group in -Whelp output, got:\n{}", stdout).into()
75-
);
71+
if result.is_empty() {
72+
return Err(
73+
format!("expected at least one group in -Whelp output, got:\n{}", stdout).into()
74+
);
75+
}
76+
Ok(result)
7677
}
77-
Ok(result)
78-
}
7978

80-
fn make_groups_table(lints: &[Lint], groups: &LintGroups) -> Result<String, Box<dyn Error>> {
81-
let mut result = String::new();
82-
let mut to_link = Vec::new();
83-
result.push_str("| Group | Description | Lints |\n");
84-
result.push_str("|-------|-------------|-------|\n");
85-
result.push_str("| warnings | All lints that are set to issue warnings | See [warn-by-default] for the default set of warnings |\n");
86-
for (group_name, group_lints) in groups {
87-
let description = GROUP_DESCRIPTIONS.iter().find(|(n, _)| n == group_name)
88-
.ok_or_else(|| format!("lint group `{}` does not have a description, please update the GROUP_DESCRIPTIONS list", group_name))?
89-
.1;
90-
to_link.extend(group_lints);
91-
let brackets: Vec<_> = group_lints.iter().map(|l| format!("[{}]", l)).collect();
92-
write!(result, "| {} | {} | {} |\n", group_name, description, brackets.join(", ")).unwrap();
93-
}
94-
result.push('\n');
95-
result.push_str("[warn-by-default]: listing/warn-by-default.md\n");
96-
for lint_name in to_link {
97-
let lint_def =
98-
lints.iter().find(|l| l.name == lint_name.replace("-", "_")).ok_or_else(|| {
99-
format!(
100-
"`rustc -W help` defined lint `{}` but that lint does not appear to exist",
101-
lint_name
102-
)
103-
})?;
104-
write!(
105-
result,
106-
"[{}]: listing/{}#{}\n",
107-
lint_name,
108-
lint_def.level.doc_filename(),
109-
lint_name
110-
)
111-
.unwrap();
79+
fn make_groups_table(
80+
&self,
81+
lints: &[Lint],
82+
groups: &LintGroups,
83+
) -> Result<String, Box<dyn Error>> {
84+
let mut result = String::new();
85+
let mut to_link = Vec::new();
86+
result.push_str("| Group | Description | Lints |\n");
87+
result.push_str("|-------|-------------|-------|\n");
88+
result.push_str("| warnings | All lints that are set to issue warnings | See [warn-by-default] for the default set of warnings |\n");
89+
for (group_name, group_lints) in groups {
90+
let description = match GROUP_DESCRIPTIONS.iter().find(|(n, _)| n == group_name) {
91+
Some((_, desc)) => desc,
92+
None if self.validate => {
93+
return Err(format!(
94+
"lint group `{}` does not have a description, \
95+
please update the GROUP_DESCRIPTIONS list in \
96+
src/tools/lint-docs/src/groups.rs",
97+
group_name
98+
)
99+
.into());
100+
}
101+
None => {
102+
eprintln!(
103+
"warning: lint group `{}` is missing from the GROUP_DESCRIPTIONS list\n\
104+
If this is a new lint group, please update the GROUP_DESCRIPTIONS in \
105+
src/tools/lint-docs/src/groups.rs",
106+
group_name
107+
);
108+
continue;
109+
}
110+
};
111+
to_link.extend(group_lints);
112+
let brackets: Vec<_> = group_lints.iter().map(|l| format!("[{}]", l)).collect();
113+
write!(result, "| {} | {} | {} |\n", group_name, description, brackets.join(", "))
114+
.unwrap();
115+
}
116+
result.push('\n');
117+
result.push_str("[warn-by-default]: listing/warn-by-default.md\n");
118+
for lint_name in to_link {
119+
let lint_def =
120+
lints.iter().find(|l| l.name == lint_name.replace("-", "_")).ok_or_else(|| {
121+
format!(
122+
"`rustc -W help` defined lint `{}` but that lint does not appear to exist",
123+
lint_name
124+
)
125+
})?;
126+
write!(
127+
result,
128+
"[{}]: listing/{}#{}\n",
129+
lint_name,
130+
lint_def.level.doc_filename(),
131+
lint_name
132+
)
133+
.unwrap();
134+
}
135+
Ok(result)
112136
}
113-
Ok(result)
114137
}

0 commit comments

Comments
 (0)