Skip to content

Commit e8e50d0

Browse files
committed
Auto merge of #14025 - Muscraft:document-lints, r=epage
Add tooling to document lints One thing the linting system has been missing is documentation. There is no list of all `cargo`'s lints and their associated documentation, like `clippy` or `rustc` has. To resolve this problem, I wrote a very basic tool that gathers the doc comments for a `Lint` and copies them to a markdown file in the Cargo book. This solution is not perfect but it is a way to work towards a better overall experience.
2 parents 4189f50 + 7d7b7c2 commit e8e50d0

File tree

9 files changed

+363
-14
lines changed

9 files changed

+363
-14
lines changed

.cargo/config.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
build-man = "run --package xtask-build-man --"
33
stale-label = "run --package xtask-stale-label --"
44
bump-check = "run --package xtask-bump-check --"
5+
lint-docs = "run --package xtask-lint-docs --"
56

67
[env]
78
# HACK: Until this is stabilized, `snapbox`s polyfill could get confused

.github/workflows/main.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ jobs:
8383
- run: rustup update stable && rustup default stable
8484
- run: cargo stale-label
8585

86+
lint-docs:
87+
runs-on: ubuntu-latest
88+
steps:
89+
- uses: actions/checkout@v4
90+
- run: rustup update stable && rustup default stable
91+
- run: cargo lint-docs --check
92+
8693
# Ensure Cargo.lock is up-to-date
8794
lockfile:
8895
runs-on: ubuntu-latest

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/xtask-lint-docs/Cargo.toml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
[package]
2+
name = "xtask-lint-docs"
3+
version = "0.1.0"
4+
edition.workspace = true
5+
publish = false
6+
7+
[dependencies]
8+
anyhow.workspace = true
9+
cargo.workspace = true
10+
clap.workspace = true
11+
itertools.workspace = true
12+
13+
[lints]
14+
workspace = true

crates/xtask-lint-docs/src/main.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
use cargo::util::command_prelude::{flag, ArgMatchesExt};
2+
use cargo::util::lints::{Lint, LintLevel};
3+
use itertools::Itertools;
4+
use std::fmt::Write;
5+
use std::path::PathBuf;
6+
7+
fn cli() -> clap::Command {
8+
clap::Command::new("xtask-lint-docs").arg(flag("check", "Check that the docs are up-to-date"))
9+
}
10+
11+
fn main() -> anyhow::Result<()> {
12+
let args = cli().get_matches();
13+
let check = args.flag("check");
14+
15+
let mut allow = Vec::new();
16+
let mut warn = Vec::new();
17+
let mut deny = Vec::new();
18+
let mut forbid = Vec::new();
19+
20+
let mut lint_docs = String::new();
21+
for lint in cargo::util::lints::LINTS
22+
.iter()
23+
.sorted_by_key(|lint| lint.name)
24+
{
25+
if lint.docs.is_some() {
26+
let sectipn = match lint.default_level {
27+
LintLevel::Allow => &mut allow,
28+
LintLevel::Warn => &mut warn,
29+
LintLevel::Deny => &mut deny,
30+
LintLevel::Forbid => &mut forbid,
31+
};
32+
sectipn.push(lint.name);
33+
add_lint(lint, &mut lint_docs)?;
34+
}
35+
}
36+
37+
let mut buf = String::new();
38+
writeln!(buf, "# Lints\n")?;
39+
writeln!(
40+
buf,
41+
"Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains"
42+
)?;
43+
writeln!(buf)?;
44+
45+
if !allow.is_empty() {
46+
add_level_section(LintLevel::Allow, &allow, &mut buf)?;
47+
}
48+
if !warn.is_empty() {
49+
add_level_section(LintLevel::Warn, &warn, &mut buf)?;
50+
}
51+
if !deny.is_empty() {
52+
add_level_section(LintLevel::Deny, &deny, &mut buf)?;
53+
}
54+
if !forbid.is_empty() {
55+
add_level_section(LintLevel::Forbid, &forbid, &mut buf)?;
56+
}
57+
58+
buf.push_str(&lint_docs);
59+
60+
if check {
61+
let old = std::fs::read_to_string(lint_docs_path())?;
62+
if old != buf {
63+
anyhow::bail!(
64+
"The lints documentation is out-of-date. Run `cargo lint-docs` to update it."
65+
);
66+
}
67+
} else {
68+
std::fs::write(lint_docs_path(), buf)?;
69+
}
70+
Ok(())
71+
}
72+
73+
fn add_lint(lint: &Lint, buf: &mut String) -> std::fmt::Result {
74+
writeln!(buf, "## `{}`", lint.name)?;
75+
writeln!(buf, "Set to `{}` by default", lint.default_level)?;
76+
writeln!(buf, "{}\n", lint.docs.as_ref().unwrap())
77+
}
78+
79+
fn add_level_section(level: LintLevel, lint_names: &[&str], buf: &mut String) -> std::fmt::Result {
80+
let title = match level {
81+
LintLevel::Allow => "Allowed-by-default",
82+
LintLevel::Warn => "Warn-by-default",
83+
LintLevel::Deny => "Deny-by-default",
84+
LintLevel::Forbid => "Forbid-by-default",
85+
};
86+
writeln!(buf, "## {title}\n")?;
87+
writeln!(
88+
buf,
89+
"These lints are all set to the '{}' level by default.",
90+
level
91+
)?;
92+
93+
for name in lint_names {
94+
writeln!(buf, "- [`{}`](#{})", name, name)?;
95+
}
96+
writeln!(buf)?;
97+
Ok(())
98+
}
99+
100+
fn lint_docs_path() -> PathBuf {
101+
let pkg_root = env!("CARGO_MANIFEST_DIR");
102+
let ws_root = PathBuf::from(format!("{pkg_root}/../.."));
103+
let path = {
104+
let path = ws_root.join("src/doc/src/reference/lints.md");
105+
path.canonicalize().unwrap_or(path)
106+
};
107+
path
108+
}

src/cargo/util/lints.rs

Lines changed: 105 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::path::Path;
1313
use toml_edit::ImDocument;
1414

1515
const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
16-
const LINTS: &[Lint] = &[
16+
pub const LINTS: &[Lint] = &[
1717
IM_A_TEAPOT,
1818
IMPLICIT_FEATURES,
1919
UNKNOWN_LINTS,
@@ -256,6 +256,7 @@ pub struct LintGroup {
256256
pub feature_gate: Option<&'static Feature>,
257257
}
258258

259+
/// This lint group is only to be used for testing purposes
259260
const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup {
260261
name: "test_dummy_unstable",
261262
desc: "test_dummy_unstable is meant to only be used in tests",
@@ -272,6 +273,10 @@ pub struct Lint {
272273
pub default_level: LintLevel,
273274
pub edition_lint_opts: Option<(Edition, LintLevel)>,
274275
pub feature_gate: Option<&'static Feature>,
276+
/// This is a markdown formatted string that will be used when generating
277+
/// the lint documentation. If docs is `None`, the lint will not be
278+
/// documented.
279+
pub docs: Option<&'static str>,
275280
}
276281

277282
impl Lint {
@@ -420,13 +425,15 @@ fn level_priority(
420425
}
421426
}
422427

428+
/// This lint is only to be used for testing purposes
423429
const IM_A_TEAPOT: Lint = Lint {
424430
name: "im_a_teapot",
425431
desc: "`im_a_teapot` is specified",
426432
groups: &[TEST_DUMMY_UNSTABLE],
427433
default_level: LintLevel::Allow,
428434
edition_lint_opts: None,
429435
feature_gate: Some(Feature::test_dummy_unstable()),
436+
docs: None,
430437
};
431438

432439
pub fn check_im_a_teapot(
@@ -476,26 +483,55 @@ pub fn check_im_a_teapot(
476483
Ok(())
477484
}
478485

479-
/// By default, cargo will treat any optional dependency as a [feature]. As of
480-
/// cargo 1.60, these can be disabled by declaring a feature that activates the
481-
/// optional dependency as `dep:<name>` (see [RFC #3143]).
482-
///
483-
/// In the 2024 edition, `cargo` will stop exposing optional dependencies as
484-
/// features implicitly, requiring users to add `foo = ["dep:foo"]` if they
485-
/// still want it exposed.
486-
///
487-
/// For more information, see [RFC #3491]
488-
///
489-
/// [feature]: https://doc.rust-lang.org/cargo/reference/features.html
490-
/// [RFC #3143]: https://rust-lang.github.io/rfcs/3143-cargo-weak-namespaced-features.html
491-
/// [RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
492486
const IMPLICIT_FEATURES: Lint = Lint {
493487
name: "implicit_features",
494488
desc: "implicit features for optional dependencies is deprecated and will be unavailable in the 2024 edition",
495489
groups: &[],
496490
default_level: LintLevel::Allow,
497491
edition_lint_opts: None,
498492
feature_gate: None,
493+
docs: Some(r#"
494+
### What it does
495+
Checks for implicit features for optional dependencies
496+
497+
### Why it is bad
498+
By default, cargo will treat any optional dependency as a [feature]. As of
499+
cargo 1.60, these can be disabled by declaring a feature that activates the
500+
optional dependency as `dep:<name>` (see [RFC #3143]).
501+
502+
In the 2024 edition, `cargo` will stop exposing optional dependencies as
503+
features implicitly, requiring users to add `foo = ["dep:foo"]` if they
504+
still want it exposed.
505+
506+
For more information, see [RFC #3491]
507+
508+
### Example
509+
```toml
510+
edition = "2021"
511+
512+
[dependencies]
513+
bar = { version = "0.1.0", optional = true }
514+
515+
[features]
516+
# No explicit feature activation for `bar`
517+
```
518+
519+
Instead, the dependency should have an explicit feature:
520+
```toml
521+
edition = "2021"
522+
523+
[dependencies]
524+
bar = { version = "0.1.0", optional = true }
525+
526+
[features]
527+
bar = ["dep:bar"]
528+
```
529+
530+
[feature]: https://doc.rust-lang.org/cargo/reference/features.html
531+
[RFC #3143]: https://rust-lang.github.io/rfcs/3143-cargo-weak-namespaced-features.html
532+
[RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
533+
"#
534+
),
499535
};
500536

501537
pub fn check_implicit_features(
@@ -575,6 +611,24 @@ const UNKNOWN_LINTS: Lint = Lint {
575611
default_level: LintLevel::Warn,
576612
edition_lint_opts: None,
577613
feature_gate: None,
614+
docs: Some(
615+
r#"
616+
### What it does
617+
Checks for unknown lints in the `[lints.cargo]` table
618+
619+
### Why it is bad
620+
- The lint name could be misspelled, leading to confusion as to why it is
621+
not working as expected
622+
- The unknown lint could end up causing an error if `cargo` decides to make
623+
a lint with the same name in the future
624+
625+
### Example
626+
```toml
627+
[lints.cargo]
628+
this-lint-does-not-exist = "warn"
629+
```
630+
"#,
631+
),
578632
};
579633

580634
fn output_unknown_lints(
@@ -684,6 +738,43 @@ const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint {
684738
default_level: LintLevel::Warn,
685739
edition_lint_opts: None,
686740
feature_gate: None,
741+
docs: Some(
742+
r#"
743+
### What it does
744+
Checks for optional dependencies that are not activated by any feature
745+
746+
### Why it is bad
747+
Starting in the 2024 edition, `cargo` no longer implicitly creates features
748+
for optional dependencies (see [RFC #3491]). This means that any optional
749+
dependency not specified with `"dep:<name>"` in some feature is now unused.
750+
This change may be surprising to users who have been using the implicit
751+
features `cargo` has been creating for optional dependencies.
752+
753+
### Example
754+
```toml
755+
edition = "2024"
756+
757+
[dependencies]
758+
bar = { version = "0.1.0", optional = true }
759+
760+
[features]
761+
# No explicit feature activation for `bar`
762+
```
763+
764+
Instead, the dependency should be removed or activated in a feature:
765+
```toml
766+
edition = "2024"
767+
768+
[dependencies]
769+
bar = { version = "0.1.0", optional = true }
770+
771+
[features]
772+
bar = ["dep:bar"]
773+
```
774+
775+
[RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
776+
"#,
777+
),
687778
};
688779

689780
pub fn unused_dependencies(

src/doc/src/SUMMARY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* [SemVer Compatibility](reference/semver.md)
4646
* [Future incompat report](reference/future-incompat-report.md)
4747
* [Reporting build timings](reference/timings.md)
48+
* [Lints](reference/lints.md)
4849
* [Unstable Features](reference/unstable.md)
4950

5051
* [Cargo Commands](commands/index.md)

src/doc/src/reference/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ The reference covers the details of various areas of Cargo.
2323
* [SemVer Compatibility](semver.md)
2424
* [Future incompat report](future-incompat-report.md)
2525
* [Reporting build timings](timings.md)
26+
* [Lints](lints.md)
2627
* [Unstable Features](unstable.md)

0 commit comments

Comments
 (0)