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

Add tooling to document lints #14025

Merged
merged 2 commits into from
Jun 7, 2024
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
1 change: 1 addition & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
build-man = "run --package xtask-build-man --"
stale-label = "run --package xtask-stale-label --"
bump-check = "run --package xtask-bump-check --"
lint-docs = "run --package xtask-lint-docs --"

[env]
# HACK: Until this is stabilized, `snapbox`s polyfill could get confused
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ jobs:
- run: rustup update stable && rustup default stable
- run: cargo stale-label

lint-docs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: rustup update stable && rustup default stable
- run: cargo lint-docs --check

# Ensure Cargo.lock is up-to-date
lockfile:
runs-on: ubuntu-latest
Expand Down
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions crates/xtask-lint-docs/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "xtask-lint-docs"
version = "0.1.0"
edition.workspace = true
publish = false

[dependencies]
anyhow.workspace = true
cargo.workspace = true
clap.workspace = true
itertools.workspace = true

[lints]
workspace = true
108 changes: 108 additions & 0 deletions crates/xtask-lint-docs/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
use cargo::util::command_prelude::{flag, ArgMatchesExt};
use cargo::util::lints::{Lint, LintLevel};
use itertools::Itertools;
use std::fmt::Write;
use std::path::PathBuf;

fn cli() -> clap::Command {
clap::Command::new("xtask-lint-docs").arg(flag("check", "Check that the docs are up-to-date"))
}

fn main() -> anyhow::Result<()> {
let args = cli().get_matches();
let check = args.flag("check");

let mut allow = Vec::new();
let mut warn = Vec::new();
let mut deny = Vec::new();
let mut forbid = Vec::new();

let mut lint_docs = String::new();
for lint in cargo::util::lints::LINTS
.iter()
.sorted_by_key(|lint| lint.name)
{
if lint.docs.is_some() {
let sectipn = match lint.default_level {
LintLevel::Allow => &mut allow,
LintLevel::Warn => &mut warn,
LintLevel::Deny => &mut deny,
LintLevel::Forbid => &mut forbid,
};
sectipn.push(lint.name);
add_lint(lint, &mut lint_docs)?;
}
}

let mut buf = String::new();
writeln!(buf, "# Lints\n")?;
writeln!(
buf,
"Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains"
)?;
writeln!(buf)?;

if !allow.is_empty() {
add_level_section(LintLevel::Allow, &allow, &mut buf)?;
}
if !warn.is_empty() {
add_level_section(LintLevel::Warn, &warn, &mut buf)?;
}
if !deny.is_empty() {
add_level_section(LintLevel::Deny, &deny, &mut buf)?;
}
if !forbid.is_empty() {
add_level_section(LintLevel::Forbid, &forbid, &mut buf)?;
}

buf.push_str(&lint_docs);

if check {
let old = std::fs::read_to_string(lint_docs_path())?;
if old != buf {
anyhow::bail!(
"The lints documentation is out-of-date. Run `cargo lint-docs` to update it."
);
}
} else {
std::fs::write(lint_docs_path(), buf)?;
}
Ok(())
}

fn add_lint(lint: &Lint, buf: &mut String) -> std::fmt::Result {
writeln!(buf, "## `{}`", lint.name)?;
writeln!(buf, "Set to `{}` by default", lint.default_level)?;
writeln!(buf, "{}\n", lint.docs.as_ref().unwrap())
}

fn add_level_section(level: LintLevel, lint_names: &[&str], buf: &mut String) -> std::fmt::Result {
let title = match level {
LintLevel::Allow => "Allowed-by-default",
LintLevel::Warn => "Warn-by-default",
LintLevel::Deny => "Deny-by-default",
LintLevel::Forbid => "Forbid-by-default",
};
writeln!(buf, "## {title}\n")?;
writeln!(
buf,
"These lints are all set to the '{}' level by default.",
level
)?;

for name in lint_names {
writeln!(buf, "- [`{}`](#{})", name, name)?;
}
writeln!(buf)?;
Ok(())
}

fn lint_docs_path() -> PathBuf {
let pkg_root = env!("CARGO_MANIFEST_DIR");
let ws_root = PathBuf::from(format!("{pkg_root}/../.."));
let path = {
let path = ws_root.join("src/doc/src/reference/lints.md");
path.canonicalize().unwrap_or(path)
};
path
}
119 changes: 105 additions & 14 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::path::Path;
use toml_edit::ImDocument;

const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
const LINTS: &[Lint] = &[
pub const LINTS: &[Lint] = &[
IM_A_TEAPOT,
IMPLICIT_FEATURES,
UNKNOWN_LINTS,
Expand Down Expand Up @@ -256,6 +256,7 @@ pub struct LintGroup {
pub feature_gate: Option<&'static Feature>,
}

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

impl Lint {
Expand Down Expand Up @@ -420,13 +425,15 @@ fn level_priority(
}
}

/// This lint is only to be used for testing purposes
const IM_A_TEAPOT: Lint = Lint {
name: "im_a_teapot",
desc: "`im_a_teapot` is specified",
groups: &[TEST_DUMMY_UNSTABLE],
default_level: LintLevel::Allow,
edition_lint_opts: None,
feature_gate: Some(Feature::test_dummy_unstable()),
docs: None,
};

pub fn check_im_a_teapot(
Expand Down Expand Up @@ -476,26 +483,55 @@ pub fn check_im_a_teapot(
Ok(())
}

/// By default, cargo will treat any optional dependency as a [feature]. As of
/// cargo 1.60, these can be disabled by declaring a feature that activates the
/// optional dependency as `dep:<name>` (see [RFC #3143]).
///
/// In the 2024 edition, `cargo` will stop exposing optional dependencies as
/// features implicitly, requiring users to add `foo = ["dep:foo"]` if they
/// still want it exposed.
///
/// For more information, see [RFC #3491]
///
/// [feature]: https://doc.rust-lang.org/cargo/reference/features.html
/// [RFC #3143]: https://rust-lang.github.io/rfcs/3143-cargo-weak-namespaced-features.html
/// [RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
const IMPLICIT_FEATURES: Lint = Lint {
name: "implicit_features",
desc: "implicit features for optional dependencies is deprecated and will be unavailable in the 2024 edition",
groups: &[],
default_level: LintLevel::Allow,
edition_lint_opts: None,
feature_gate: None,
docs: Some(r#"
### What it does
Checks for implicit features for optional dependencies

### Why it is bad
By default, cargo will treat any optional dependency as a [feature]. As of
cargo 1.60, these can be disabled by declaring a feature that activates the
optional dependency as `dep:<name>` (see [RFC #3143]).

In the 2024 edition, `cargo` will stop exposing optional dependencies as
features implicitly, requiring users to add `foo = ["dep:foo"]` if they
still want it exposed.

For more information, see [RFC #3491]

### Example
```toml
edition = "2021"

[dependencies]
bar = { version = "0.1.0", optional = true }

[features]
# No explicit feature activation for `bar`
```

Instead, the dependency should have an explicit feature:
```toml
edition = "2021"

[dependencies]
bar = { version = "0.1.0", optional = true }

[features]
bar = ["dep:bar"]
```

[feature]: https://doc.rust-lang.org/cargo/reference/features.html
[RFC #3143]: https://rust-lang.github.io/rfcs/3143-cargo-weak-namespaced-features.html
[RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
"#
),
};

pub fn check_implicit_features(
Expand Down Expand Up @@ -575,6 +611,24 @@ const UNKNOWN_LINTS: Lint = Lint {
default_level: LintLevel::Warn,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
r#"
### What it does
Checks for unknown lints in the `[lints.cargo]` table

### Why it is bad
- The lint name could be misspelled, leading to confusion as to why it is
not working as expected
- The unknown lint could end up causing an error if `cargo` decides to make
a lint with the same name in the future

### Example
```toml
[lints.cargo]
this-lint-does-not-exist = "warn"
```
"#,
),
};

fn output_unknown_lints(
Expand Down Expand Up @@ -684,6 +738,43 @@ const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint {
default_level: LintLevel::Warn,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
r#"
### What it does
Checks for optional dependencies that are not activated by any feature

### Why it is bad
Starting in the 2024 edition, `cargo` no longer implicitly creates features
for optional dependencies (see [RFC #3491]). This means that any optional
dependency not specified with `"dep:<name>"` in some feature is now unused.
This change may be surprising to users who have been using the implicit
features `cargo` has been creating for optional dependencies.

### Example
```toml
edition = "2024"

[dependencies]
bar = { version = "0.1.0", optional = true }

[features]
# No explicit feature activation for `bar`
```

Instead, the dependency should be removed or activated in a feature:
```toml
edition = "2024"

[dependencies]
bar = { version = "0.1.0", optional = true }

[features]
bar = ["dep:bar"]
```

[RFC #3491]: https://rust-lang.github.io/rfcs/3491-remove-implicit-features.html
"#,
),
};

pub fn unused_dependencies(
Expand Down
1 change: 1 addition & 0 deletions src/doc/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
* [SemVer Compatibility](reference/semver.md)
* [Future incompat report](reference/future-incompat-report.md)
* [Reporting build timings](reference/timings.md)
* [Lints](reference/lints.md)
* [Unstable Features](reference/unstable.md)

* [Cargo Commands](commands/index.md)
Expand Down
1 change: 1 addition & 0 deletions src/doc/src/reference/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ The reference covers the details of various areas of Cargo.
* [SemVer Compatibility](semver.md)
* [Future incompat report](future-incompat-report.md)
* [Reporting build timings](timings.md)
* [Lints](lints.md)
* [Unstable Features](unstable.md)
Loading