diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d7d0170e11c..84975beee5e 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -25,6 +25,7 @@ use crate::lints::analyze_cargo_lints_table; use crate::lints::rules::blanket_hint_mostly_unused; use crate::lints::rules::check_im_a_teapot; use crate::lints::rules::implicit_minimum_version_req; +use crate::lints::rules::missing_lints_inheritance; use crate::lints::rules::non_kebab_case_bins; use crate::lints::rules::non_kebab_case_features; use crate::lints::rules::non_kebab_case_packages; @@ -1413,6 +1414,14 @@ impl<'gctx> Workspace<'gctx> { &mut run_error_count, self.gctx, )?; + missing_lints_inheritance( + self, + pkg, + &path, + &cargo_lints, + &mut run_error_count, + self.gctx, + )?; if run_error_count > 0 { let plural = if run_error_count == 1 { "" } else { "s" }; diff --git a/src/cargo/lints/rules/missing_lints_inheritance.rs b/src/cargo/lints/rules/missing_lints_inheritance.rs new file mode 100644 index 00000000000..cde3341108c --- /dev/null +++ b/src/cargo/lints/rules/missing_lints_inheritance.rs @@ -0,0 +1,126 @@ +use std::path::Path; + +use annotate_snippets::Group; +use annotate_snippets::Level; +use annotate_snippets::Origin; +use annotate_snippets::Patch; +use annotate_snippets::Snippet; +use cargo_util_schemas::manifest::TomlToolLints; + +use crate::CargoResult; +use crate::GlobalContext; +use crate::core::Package; +use crate::core::Workspace; +use crate::lints::Lint; +use crate::lints::LintLevel; +use crate::lints::SUSPICIOUS; +use crate::lints::rel_cwd_manifest_path; + +pub const LINT: Lint = Lint { + name: "missing_lints_inheritance", + desc: "missing `[lints]` to inherit `[workspace.lints]`", + primary_group: &SUSPICIOUS, + edition_lint_opts: None, + feature_gate: None, + docs: Some( + r#" +### What it does + +Checks for packages without a `lints` table while `workspace.lints` is present. + +### Why it is bad + +Many people mistakenly think that `workspace.lints` is implicitly inherited when it is not. + +### Drawbacks + +### Example + +```toml +[workspace.lints.cargo] +``` + +Should be written as: + +```toml +[workspace.lints.cargo] + +[lints] +workspace = true +``` +"#, + ), +}; + +pub fn missing_lints_inheritance( + ws: &Workspace<'_>, + pkg: &Package, + manifest_path: &Path, + cargo_lints: &TomlToolLints, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + let (lint_level, reason) = LINT.level( + cargo_lints, + pkg.manifest().edition(), + pkg.manifest().unstable_features(), + ); + + if lint_level == LintLevel::Allow { + return Ok(()); + } + + let root = ws.root_maybe(); + // `normalized_toml` normally isn't guaranteed to include inheritance information except + // `workspace.lints` is used outside of inheritance for workspace-level lints. + let ws_lints = root + .normalized_toml() + .workspace + .as_ref() + .map(|ws| ws.lints.is_some()) + .unwrap_or(false); + if !ws_lints { + return Ok(()); + } + if pkg.manifest().normalized_toml().lints.is_some() { + return Ok(()); + } + + let manifest = pkg.manifest(); + let contents = manifest.contents(); + let level = lint_level.to_diagnostic_level(); + let emitted_source = LINT.emitted_source(lint_level, reason); + let manifest_path = rel_cwd_manifest_path(manifest_path, gctx); + + let mut primary = Group::with_title(level.primary_title(LINT.desc)); + primary = primary.element(Origin::path(&manifest_path)); + primary = primary.element(Level::NOTE.message(emitted_source)); + let mut report = vec![primary]; + if let Some(contents) = contents { + let span = contents.len()..contents.len(); + let mut help = + Group::with_title(Level::HELP.secondary_title("to inherit `workspace.lints, add:")); + help = help.element( + Snippet::source(contents) + .path(&manifest_path) + .patch(Patch::new(span.clone(), "\n[lints]\nworkspace = true")), + ); + report.push(help); + let mut help = Group::with_title( + Level::HELP.secondary_title("to clarify your intent to not inherit, add:"), + ); + help = help.element( + Snippet::source(contents) + .path(&manifest_path) + .patch(Patch::new(span, "\n[lints]")), + ); + report.push(help); + } + + if lint_level.is_error() { + *error_count += 1; + } + gctx.shell().print_report(&report, lint_level.force())?; + + Ok(()) +} diff --git a/src/cargo/lints/rules/mod.rs b/src/cargo/lints/rules/mod.rs index 7313545249e..610a535c6ed 100644 --- a/src/cargo/lints/rules/mod.rs +++ b/src/cargo/lints/rules/mod.rs @@ -1,6 +1,7 @@ mod blanket_hint_mostly_unused; mod im_a_teapot; mod implicit_minimum_version_req; +mod missing_lints_inheritance; mod non_kebab_case_bins; mod non_kebab_case_features; mod non_kebab_case_packages; @@ -15,6 +16,7 @@ mod unused_workspace_package_fields; pub use blanket_hint_mostly_unused::blanket_hint_mostly_unused; pub use im_a_teapot::check_im_a_teapot; pub use implicit_minimum_version_req::implicit_minimum_version_req; +pub use missing_lints_inheritance::missing_lints_inheritance; pub use non_kebab_case_bins::non_kebab_case_bins; pub use non_kebab_case_features::non_kebab_case_features; pub use non_kebab_case_packages::non_kebab_case_packages; @@ -30,6 +32,7 @@ pub const LINTS: &[crate::lints::Lint] = &[ blanket_hint_mostly_unused::LINT, implicit_minimum_version_req::LINT, im_a_teapot::LINT, + missing_lints_inheritance::LINT, non_kebab_case_bins::LINT, non_kebab_case_features::LINT, non_kebab_case_packages::LINT, diff --git a/src/doc/src/reference/lints.md b/src/doc/src/reference/lints.md index fff50e3798c..7d52c7c529c 100644 --- a/src/doc/src/reference/lints.md +++ b/src/doc/src/reference/lints.md @@ -29,6 +29,7 @@ These lints are all set to the 'allow' level by default. These lints are all set to the 'warn' level by default. - [`blanket_hint_mostly_unused`](#blanket_hint_mostly_unused) +- [`missing_lints_inheritance`](#missing_lints_inheritance) - [`non_kebab_case_bins`](#non_kebab_case_bins) - [`redundant_homepage`](#redundant_homepage) - [`redundant_readme`](#redundant_readme) @@ -117,6 +118,37 @@ serde = "1.0.219" ``` +## `missing_lints_inheritance` +Group: `suspicious` + +Level: `warn` + +### What it does + +Checks for packages without a `lints` table while `workspace.lints` is present. + +### Why it is bad + +Many people mistakenly think that `workspace.lints` is implicitly inherited when it is not. + +### Drawbacks + +### Example + +```toml +[workspace.lints.cargo] +``` + +Should be written as: + +```toml +[workspace.lints.cargo] + +[lints] +workspace = true +``` + + ## `non_kebab_case_bins` Group: `style` diff --git a/tests/testsuite/lints/implicit_minimum_version_req.rs b/tests/testsuite/lints/implicit_minimum_version_req.rs index 7735d82b6a2..d074978b6b7 100644 --- a/tests/testsuite/lints/implicit_minimum_version_req.rs +++ b/tests/testsuite/lints/implicit_minimum_version_req.rs @@ -1065,6 +1065,8 @@ implicit_minimum_version_req = "warn" [package] name = "member" edition = "2021" + +[lints] "#, ) .file("member/src/lib.rs", "") diff --git a/tests/testsuite/lints/missing_lints_inheritance.rs b/tests/testsuite/lints/missing_lints_inheritance.rs new file mode 100644 index 00000000000..c84a0e2113b --- /dev/null +++ b/tests/testsuite/lints/missing_lints_inheritance.rs @@ -0,0 +1,163 @@ +use crate::prelude::*; +use cargo_test_support::project; +use cargo_test_support::str; + +#[cargo_test] +fn no_lints() { + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["bar"] +"#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" +[package] +name = "bar" +version = "0.0.1" +edition = "2015" +"#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[CHECKING] bar v0.0.1 ([ROOT]/foo/bar) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn ws_lints() { + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["bar"] + +[workspace.lints.cargo] +missing_lints_inheritance = "warn" +"#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" +[package] +name = "bar" +version = "0.0.1" +edition = "2015" +"#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[WARNING] missing `[lints]` to inherit `[workspace.lints]` + --> bar/Cargo.toml + = [NOTE] `cargo::missing_lints_inheritance` is set to `warn` by default +[HELP] to inherit `workspace.lints, add: + | +5 ~ edition = "2015" +6 + [lints] +7 + workspace = true + | +[HELP] to clarify your intent to not inherit, add: + | +5 ~ edition = "2015" +6 + [lints] + | +[CHECKING] bar v0.0.1 ([ROOT]/foo/bar) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn empty_pkg_lints() { + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["bar"] + +[workspace.lints.cargo] +missing_lints_inheritance = "warn" +"#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" +[package] +name = "bar" +version = "0.0.1" +edition = "2015" + +[lints] +"#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[CHECKING] bar v0.0.1 ([ROOT]/foo/bar) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn inherit_lints() { + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["bar"] + +[workspace.lints.cargo] +missing_lints_inheritance = "warn" +"#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" +[package] +name = "bar" +version = "0.0.1" +edition = "2015" + +[lints] +workspace = true +"#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[CHECKING] bar v0.0.1 ([ROOT]/foo/bar) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} diff --git a/tests/testsuite/lints/mod.rs b/tests/testsuite/lints/mod.rs index 25b657960b5..31698704a75 100644 --- a/tests/testsuite/lints/mod.rs +++ b/tests/testsuite/lints/mod.rs @@ -7,6 +7,7 @@ mod blanket_hint_mostly_unused; mod error; mod implicit_minimum_version_req; mod inherited; +mod missing_lints_inheritance; mod non_kebab_case_bins; mod non_kebab_case_features; mod non_kebab_case_packages; @@ -176,6 +177,20 @@ im-a-teapot = true p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints"]) .with_stderr_data(str![[r#" +[WARNING] missing `[lints]` to inherit `[workspace.lints]` + --> foo/Cargo.toml + = [NOTE] `cargo::missing_lints_inheritance` is set to `warn` by default +[HELP] to inherit `workspace.lints, add: + | + 9 ~ im-a-teapot = true +10 + [lints] +11 + workspace = true + | +[HELP] to clarify your intent to not inherit, add: + | + 9 ~ im-a-teapot = true +10 + [lints] + | [CHECKING] foo v0.0.1 ([ROOT]/foo/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s @@ -373,6 +388,20 @@ authors = [] | ^^^^^^^^^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled | = [HELP] consider adding `cargo-features = ["test-dummy-unstable"]` to the top of the manifest +[WARNING] missing `[lints]` to inherit `[workspace.lints]` + --> foo/Cargo.toml + = [NOTE] `cargo::missing_lints_inheritance` is set to `warn` by default +[HELP] to inherit `workspace.lints, add: + | +7 ~ +8 + [lints] +9 + workspace = true + | +[HELP] to clarify your intent to not inherit, add: + | +7 ~ +8 + [lints] + | [ERROR] encountered 2 errors while verifying lints "#]]) diff --git a/tests/testsuite/lints/unknown_lints.rs b/tests/testsuite/lints/unknown_lints.rs index c2a57cbebf6..f6b272557dc 100644 --- a/tests/testsuite/lints/unknown_lints.rs +++ b/tests/testsuite/lints/unknown_lints.rs @@ -120,6 +120,20 @@ authors = [] | ^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `cargo::unknown_lints` is set to `warn` by default +[WARNING] missing `[lints]` to inherit `[workspace.lints]` + --> foo/Cargo.toml + = [NOTE] `cargo::missing_lints_inheritance` is set to `warn` by default +[HELP] to inherit `workspace.lints, add: + | +7 ~ +8 + [lints] +9 + workspace = true + | +[HELP] to clarify your intent to not inherit, add: + | +7 ~ +8 + [lints] + | [CHECKING] foo v0.0.1 ([ROOT]/foo/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s