diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 7013cc2a458..cdcb76956a9 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -343,7 +343,7 @@ impl FromStr for Edition { } } -#[derive(PartialEq)] +#[derive(Debug, PartialEq)] enum Status { Stable, Unstable, @@ -387,11 +387,11 @@ macro_rules! features { $( $(#[$attr])* #[doc = concat!("\n\n\nSee .")] - pub fn $feature() -> &'static Feature { + pub const fn $feature() -> &'static Feature { fn get(features: &Features) -> bool { stab!($stab) == Status::Stable || features.$feature } - static FEAT: Feature = Feature { + const FEAT: Feature = Feature { name: stringify!($feature), stability: stab!($stab), version: $version, @@ -406,6 +406,10 @@ macro_rules! features { fn is_enabled(&self, features: &Features) -> bool { (self.get)(features) } + + pub(crate) fn name(&self) -> &str { + self.name + } } impl Features { @@ -512,6 +516,7 @@ features! { } /// Status and metadata for a single unstable feature. +#[derive(Debug)] pub struct Feature { /// Feature name. This is valid Rust identifier so no dash only underscore. name: &'static str, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7fb0b1ca5c0..bc49b74d687 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -24,7 +24,9 @@ use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::edit_distance; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; -use crate::util::lints::{check_im_a_teapot, check_implicit_features, unused_dependencies}; +use crate::util::lints::{ + analyze_cargo_lints_table, check_im_a_teapot, check_implicit_features, unused_dependencies, +}; use crate::util::toml::{read_manifest, InheritableFields}; use crate::util::{ context::CargoResolverConfig, context::CargoResolverPrecedence, context::ConfigRelativePath, @@ -1227,6 +1229,26 @@ impl<'gctx> Workspace<'gctx> { .is_some_and(|l| l.workspace) .then(|| ws_cargo_lints); + let ws_contents = match self.root_maybe() { + MaybePackage::Package(pkg) => pkg.manifest().contents(), + MaybePackage::Virtual(v) => v.contents(), + }; + + let ws_document = match self.root_maybe() { + MaybePackage::Package(pkg) => pkg.manifest().document(), + MaybePackage::Virtual(v) => v.document(), + }; + + analyze_cargo_lints_table( + pkg, + &path, + &normalized_lints, + ws_cargo_lints, + ws_contents, + ws_document, + self.root_manifest(), + self.gctx, + )?; check_im_a_teapot( pkg, &path, diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index 839d29b0180..5f09249342a 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -1,6 +1,6 @@ use crate::core::dependency::DepKind; use crate::core::FeatureValue::Dep; -use crate::core::{Edition, FeatureValue, Package}; +use crate::core::{Edition, Feature, FeatureValue, Features, Manifest, Package}; use crate::util::interning::InternedString; use crate::{CargoResult, GlobalContext}; use annotate_snippets::{Level, Snippet}; @@ -12,11 +12,199 @@ use std::ops::Range; use std::path::Path; use toml_edit::ImDocument; +const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE]; +const LINTS: &[Lint] = &[IM_A_TEAPOT, IMPLICIT_FEATURES, UNUSED_OPTIONAL_DEPENDENCY]; + +pub fn analyze_cargo_lints_table( + pkg: &Package, + path: &Path, + pkg_lints: &TomlToolLints, + ws_lints: Option<&TomlToolLints>, + ws_contents: &str, + ws_document: &ImDocument, + ws_path: &Path, + gctx: &GlobalContext, +) -> CargoResult<()> { + let mut error_count = 0; + let manifest = pkg.manifest(); + let manifest_path = rel_cwd_manifest_path(path, gctx); + let ws_path = rel_cwd_manifest_path(ws_path, gctx); + + for lint_name in pkg_lints + .keys() + .chain(ws_lints.map(|l| l.keys()).unwrap_or_default()) + { + if let Some((name, default_level, edition_lint_opts, feature_gate)) = + find_lint_or_group(lint_name) + { + let (_, reason, _) = level_priority( + name, + *default_level, + *edition_lint_opts, + pkg_lints, + ws_lints, + manifest.edition(), + ); + + // Only run analysis on user-specified lints + if !reason.is_user_specified() { + continue; + } + + // Only run this on lints that are gated by a feature + if let Some(feature_gate) = feature_gate { + verify_feature_enabled( + name, + feature_gate, + reason, + manifest, + &manifest_path, + ws_contents, + ws_document, + &ws_path, + &mut error_count, + gctx, + )?; + } + } + } + if error_count > 0 { + Err(anyhow::anyhow!( + "encountered {error_count} errors(s) while verifying lints", + )) + } else { + Ok(()) + } +} + +fn find_lint_or_group<'a>( + name: &str, +) -> Option<( + &'static str, + &LintLevel, + &Option<(Edition, LintLevel)>, + &Option<&'static Feature>, +)> { + if let Some(lint) = LINTS.iter().find(|l| l.name == name) { + Some(( + lint.name, + &lint.default_level, + &lint.edition_lint_opts, + &lint.feature_gate, + )) + } else if let Some(group) = LINT_GROUPS.iter().find(|g| g.name == name) { + Some(( + group.name, + &group.default_level, + &group.edition_lint_opts, + &group.feature_gate, + )) + } else { + None + } +} + +fn verify_feature_enabled( + lint_name: &str, + feature_gate: &Feature, + reason: LintLevelReason, + manifest: &Manifest, + manifest_path: &str, + ws_contents: &str, + ws_document: &ImDocument, + ws_path: &str, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + if !manifest.unstable_features().is_enabled(feature_gate) { + let dash_name = lint_name.replace("_", "-"); + let dash_feature_name = feature_gate.name().replace("_", "-"); + let title = format!("use of unstable lint `{}`", dash_name); + let label = format!( + "this is behind `{}`, which is not enabled", + dash_feature_name + ); + let second_title = format!("`cargo::{}` was inherited", dash_name); + let help = format!( + "consider adding `cargo-features = [\"{}\"]` to the top of the manifest", + dash_feature_name + ); + let message = match reason { + LintLevelReason::Package => { + let span = get_span( + manifest.document(), + &["lints", "cargo", dash_name.as_str()], + false, + ) + .or(get_span( + manifest.document(), + &["lints", "cargo", lint_name], + false, + )) + .unwrap(); + + Level::Error + .title(&title) + .snippet( + Snippet::source(manifest.contents()) + .origin(&manifest_path) + .annotation(Level::Error.span(span).label(&label)) + .fold(true), + ) + .footer(Level::Help.title(&help)) + } + LintLevelReason::Workspace => { + let lint_span = get_span( + ws_document, + &["workspace", "lints", "cargo", dash_name.as_str()], + false, + ) + .or(get_span( + ws_document, + &["workspace", "lints", "cargo", lint_name], + false, + )) + .unwrap(); + let inherit_span_key = + get_span(manifest.document(), &["lints", "workspace"], false).unwrap(); + let inherit_span_value = + get_span(manifest.document(), &["lints", "workspace"], true).unwrap(); + + Level::Error + .title(&title) + .snippet( + Snippet::source(ws_contents) + .origin(&ws_path) + .annotation(Level::Error.span(lint_span).label(&label)) + .fold(true), + ) + .footer( + Level::Note.title(&second_title).snippet( + Snippet::source(manifest.contents()) + .origin(&manifest_path) + .annotation( + Level::Note + .span(inherit_span_key.start..inherit_span_value.end), + ) + .fold(true), + ), + ) + .footer(Level::Help.title(&help)) + } + _ => unreachable!("LintLevelReason should be one that is user specified"), + }; + + *error_count += 1; + gctx.shell().print_message(message)?; + } + Ok(()) +} + fn get_span(document: &ImDocument, path: &[&str], get_value: bool) -> Option> { - let mut table = document.as_item().as_table_like().unwrap(); + let mut table = document.as_item().as_table_like()?; let mut iter = path.into_iter().peekable(); while let Some(key) = iter.next() { - let (key, item) = table.get_key_value(key).unwrap(); + let (key, item) = table.get_key_value(key)?; if iter.peek().is_none() { return if get_value { item.span() @@ -66,6 +254,7 @@ pub struct LintGroup { pub default_level: LintLevel, pub desc: &'static str, pub edition_lint_opts: Option<(Edition, LintLevel)>, + pub feature_gate: Option<&'static Feature>, } const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup { @@ -73,6 +262,7 @@ const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup { desc: "test_dummy_unstable is meant to only be used in tests", default_level: LintLevel::Allow, edition_lint_opts: None, + feature_gate: Some(Feature::test_dummy_unstable()), }; #[derive(Copy, Clone, Debug)] @@ -82,6 +272,7 @@ pub struct Lint { pub groups: &'static [LintGroup], pub default_level: LintLevel, pub edition_lint_opts: Option<(Edition, LintLevel)>, + pub feature_gate: Option<&'static Feature>, } impl Lint { @@ -90,7 +281,17 @@ impl Lint { pkg_lints: &TomlToolLints, ws_lints: Option<&TomlToolLints>, edition: Edition, + unstable_features: &Features, ) -> (LintLevel, LintLevelReason) { + // We should return `Allow` if a lint is behind a feature, but it is + // not enabled, that way the lint does not run. + if self + .feature_gate + .is_some_and(|f| !unstable_features.is_enabled(f)) + { + return (LintLevel::Allow, LintLevelReason::Default); + } + self.groups .iter() .map(|g| { @@ -164,7 +365,7 @@ impl From for LintLevel { } } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum LintLevelReason { Default, Edition(Edition), @@ -183,6 +384,17 @@ impl Display for LintLevelReason { } } +impl LintLevelReason { + fn is_user_specified(&self) -> bool { + match self { + LintLevelReason::Default => false, + LintLevelReason::Edition(_) => false, + LintLevelReason::Package => true, + LintLevelReason::Workspace => true, + } + } +} + fn level_priority( name: &str, default_level: LintLevel, @@ -228,6 +440,7 @@ const IM_A_TEAPOT: Lint = Lint { groups: &[TEST_DUMMY_UNSTABLE], default_level: LintLevel::Allow, edition_lint_opts: None, + feature_gate: Some(Feature::test_dummy_unstable()), }; pub fn check_im_a_teapot( @@ -239,7 +452,13 @@ pub fn check_im_a_teapot( gctx: &GlobalContext, ) -> CargoResult<()> { let manifest = pkg.manifest(); - let (lint_level, reason) = IM_A_TEAPOT.level(pkg_lints, ws_lints, manifest.edition()); + let (lint_level, reason) = IM_A_TEAPOT.level( + pkg_lints, + ws_lints, + manifest.edition(), + manifest.unstable_features(), + ); + if lint_level == LintLevel::Allow { return Ok(()); } @@ -295,6 +514,7 @@ const IMPLICIT_FEATURES: Lint = Lint { groups: &[], default_level: LintLevel::Allow, edition_lint_opts: None, + feature_gate: None, }; pub fn check_implicit_features( @@ -305,19 +525,20 @@ pub fn check_implicit_features( error_count: &mut usize, gctx: &GlobalContext, ) -> CargoResult<()> { - let edition = pkg.manifest().edition(); + let manifest = pkg.manifest(); + let edition = manifest.edition(); // In Edition 2024+, instead of creating optional features, the dependencies are unused. // See `UNUSED_OPTIONAL_DEPENDENCY` if edition >= Edition::Edition2024 { return Ok(()); } - let (lint_level, reason) = IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition); + let (lint_level, reason) = + IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition, manifest.unstable_features()); if lint_level == LintLevel::Allow { return Ok(()); } - let manifest = pkg.manifest(); let activated_opt_deps = manifest .resolved_toml() .features() @@ -373,6 +594,7 @@ const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint { groups: &[], default_level: LintLevel::Warn, edition_lint_opts: None, + feature_gate: None, }; pub fn unused_dependencies( @@ -383,18 +605,23 @@ pub fn unused_dependencies( error_count: &mut usize, gctx: &GlobalContext, ) -> CargoResult<()> { - let edition = pkg.manifest().edition(); + let manifest = pkg.manifest(); + let edition = manifest.edition(); // Unused optional dependencies can only exist on edition 2024+ if edition < Edition::Edition2024 { return Ok(()); } - let (lint_level, reason) = UNUSED_OPTIONAL_DEPENDENCY.level(pkg_lints, ws_lints, edition); + let (lint_level, reason) = UNUSED_OPTIONAL_DEPENDENCY.level( + pkg_lints, + ws_lints, + edition, + manifest.unstable_features(), + ); if lint_level == LintLevel::Allow { return Ok(()); } let mut emitted_source = None; - let manifest = pkg.manifest(); let original_toml = manifest.original_toml(); // Unused dependencies were stripped from the manifest, leaving only the used ones let used_dependencies = manifest diff --git a/tests/testsuite/lints_table.rs b/tests/testsuite/lints_table.rs index 6b756903130..6faa7094424 100644 --- a/tests/testsuite/lints_table.rs +++ b/tests/testsuite/lints_table.rs @@ -1078,3 +1078,107 @@ implicit-features = "warn" ) .run(); } + +#[cargo_test] +fn check_feature_gated() { + let p = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[lints.cargo] +im-a-teapot = "warn" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_status(101) + .with_stderr( + "\ +error: use of unstable lint `im-a-teapot` + --> Cargo.toml:9:1 + | +9 | im-a-teapot = \"warn\" + | ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled + | + = help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest +error: encountered 1 errors(s) while verifying lints +", + ) + .run(); +} + +#[cargo_test] +fn check_feature_gated_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["foo"] + +[workspace.lints.cargo] +im-a-teapot = { level = "warn", priority = 10 } +test-dummy-unstable = { level = "forbid", priority = -1 } + "#, + ) + .file( + "foo/Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[lints] +workspace = true + "#, + ) + .file("foo/src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_status(101) + .with_stderr( + "\ +error: use of unstable lint `im-a-teapot` + --> Cargo.toml:6:1 + | +6 | im-a-teapot = { level = \"warn\", priority = 10 } + | ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled + | +note: `cargo::im-a-teapot` was inherited + --> foo/Cargo.toml:9:1 + | +9 | workspace = true + | ---------------- + | + = help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest +error: use of unstable lint `test-dummy-unstable` + --> Cargo.toml:7:1 + | +7 | test-dummy-unstable = { level = \"forbid\", priority = -1 } + | ^^^^^^^^^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled + | +note: `cargo::test-dummy-unstable` was inherited + --> foo/Cargo.toml:9:1 + | +9 | workspace = true + | ---------------- + | + = help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest +error: encountered 2 errors(s) while verifying lints +", + ) + .run(); +}