From 30a7eb55b927a2ebb5505313dddb8671c74ee99c Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Thu, 6 Jun 2024 20:17:54 -0600 Subject: [PATCH 1/2] chore(lints): Add documentation to public lints --- src/cargo/util/lints.rs | 117 +++++++++++++++++++++++++++++++++++----- 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index 85c485326ff..ed11bb328d0 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -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", @@ -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 { @@ -420,6 +425,7 @@ 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", @@ -427,6 +433,7 @@ const IM_A_TEAPOT: Lint = Lint { default_level: LintLevel::Allow, edition_lint_opts: None, feature_gate: Some(Feature::test_dummy_unstable()), + docs: None, }; pub fn check_im_a_teapot( @@ -476,19 +483,6 @@ 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:` (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", @@ -496,6 +490,48 @@ const IMPLICIT_FEATURES: Lint = Lint { 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:` (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( @@ -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( @@ -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:"` 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( From 7d7b7c2c8bca49b3671eb9ad00c3c9565ed19d18 Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Thu, 6 Jun 2024 23:03:53 -0600 Subject: [PATCH 2/2] feat: Add an xtask to generate lint documentation --- .cargo/config.toml | 1 + .github/workflows/main.yml | 7 ++ Cargo.lock | 10 +++ crates/xtask-lint-docs/Cargo.toml | 14 ++++ crates/xtask-lint-docs/src/main.rs | 108 +++++++++++++++++++++++++++ src/cargo/util/lints.rs | 2 +- src/doc/src/SUMMARY.md | 1 + src/doc/src/reference/index.md | 1 + src/doc/src/reference/lints.md | 116 +++++++++++++++++++++++++++++ 9 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 crates/xtask-lint-docs/Cargo.toml create mode 100644 crates/xtask-lint-docs/src/main.rs create mode 100644 src/doc/src/reference/lints.md diff --git a/.cargo/config.toml b/.cargo/config.toml index c4cd35e56bc..f1a26708418 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -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 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e680a32d61f..de81970f67d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -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 diff --git a/Cargo.lock b/Cargo.lock index e2a42023b9c..4be55d18d15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4021,6 +4021,16 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "xtask-lint-docs" +version = "0.1.0" +dependencies = [ + "anyhow", + "cargo", + "clap", + "itertools 0.12.1", +] + [[package]] name = "xtask-stale-label" version = "0.0.0" diff --git a/crates/xtask-lint-docs/Cargo.toml b/crates/xtask-lint-docs/Cargo.toml new file mode 100644 index 00000000000..16f15d2efe7 --- /dev/null +++ b/crates/xtask-lint-docs/Cargo.toml @@ -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 diff --git a/crates/xtask-lint-docs/src/main.rs b/crates/xtask-lint-docs/src/main.rs new file mode 100644 index 00000000000..79862d6c400 --- /dev/null +++ b/crates/xtask-lint-docs/src/main.rs @@ -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 +} diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index ed11bb328d0..1d9573f8980 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -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, diff --git a/src/doc/src/SUMMARY.md b/src/doc/src/SUMMARY.md index 9ebd7915bb9..b18e69111e5 100644 --- a/src/doc/src/SUMMARY.md +++ b/src/doc/src/SUMMARY.md @@ -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) diff --git a/src/doc/src/reference/index.md b/src/doc/src/reference/index.md index afff4fa89a4..d20efb8c2ba 100644 --- a/src/doc/src/reference/index.md +++ b/src/doc/src/reference/index.md @@ -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) diff --git a/src/doc/src/reference/lints.md b/src/doc/src/reference/lints.md new file mode 100644 index 00000000000..108d6f0504e --- /dev/null +++ b/src/doc/src/reference/lints.md @@ -0,0 +1,116 @@ +# Lints + +Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains + +## Allowed-by-default + +These lints are all set to the 'allow' level by default. +- [`implicit_features`](#implicit_features) + +## Warn-by-default + +These lints are all set to the 'warn' level by default. +- [`unknown_lints`](#unknown_lints) +- [`unused_optional_dependency`](#unused_optional_dependency) + +## `implicit_features` +Set to `allow` by default + +### 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:` (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 + + +## `unknown_lints` +Set to `warn` by default + +### 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" +``` + + +## `unused_optional_dependency` +Set to `warn` by default + +### 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:"` 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 + +