diff --git a/Cargo.lock b/Cargo.lock index e9da353a21e..1d9d53e77eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3428,9 +3428,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.7" +version = "0.22.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18769cd1cec395d70860ceb4d932812a0b4d06b1a4bb336745a4d21b9496e992" +checksum = "8e40bb779c5187258fd7aad0eb68cb8706a0a81fa712fbea808ab43c4b8374c4" dependencies = [ "indexmap", "serde", diff --git a/Cargo.toml b/Cargo.toml index 26e4e81256d..c4092348631 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -97,7 +97,7 @@ tempfile = "3.10.1" thiserror = "1.0.57" time = { version = "0.3", features = ["parsing", "formatting", "serde"] } toml = "0.8.10" -toml_edit = { version = "0.22.7", features = ["serde"] } +toml_edit = { version = "0.22.9", features = ["serde"] } tracing = "0.1.40" # be compatible with rustc_log: https://github.com/rust-lang/rust/blob/e51e98dde6a/compiler/rustc_log/Cargo.toml#L9 tracing-chrome = "0.7.1" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 24e208fc913..a78598e82e0 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -750,6 +750,7 @@ unstable_cli_options!( #[serde(deserialize_with = "deserialize_build_std")] build_std: Option> = ("Enable Cargo to compile the standard library itself as part of a crate graph compilation"), build_std_features: Option> = ("Configure features enabled for the standard library itself when building the standard library"), + cargo_lints: bool = ("Enable the `[lints.cargo]` table"), check_cfg: bool = ("Enable compile-time checking of `cfg` names/values/features"), codegen_backend: bool = ("Enable the `codegen-backend` option in profiles in .cargo/config.toml file"), config_include: bool = ("Enable the `include` key in config files"), @@ -1117,6 +1118,7 @@ impl CliUnstable { self.build_std = Some(crate::core::compiler::standard_lib::parse_unstable_flag(v)) } "build-std-features" => self.build_std_features = Some(parse_features(v)), + "cargo-lints" => self.cargo_lints = parse_empty(k, v)?, "check-cfg" => { self.check_cfg = parse_empty(k, v)?; } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index e689933492a..0b0e6d74961 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -24,10 +24,12 @@ 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_implicit_features; use crate::util::toml::{read_manifest, InheritableFields}; use crate::util::{context::ConfigRelativePath, Filesystem, GlobalContext, IntoUrl}; use cargo_util::paths; use cargo_util::paths::normalize_path; +use cargo_util_schemas::manifest; use cargo_util_schemas::manifest::RustVersion; use cargo_util_schemas::manifest::{TomlDependency, TomlProfiles}; use pathdiff::diff_paths; @@ -1095,11 +1097,14 @@ impl<'gctx> Workspace<'gctx> { pub fn emit_warnings(&self) -> CargoResult<()> { for (path, maybe_pkg) in &self.packages.packages { + let path = path.join("Cargo.toml"); + if let MaybePackage::Package(pkg) = maybe_pkg { + self.emit_lints(pkg, &path)? + } let warnings = match maybe_pkg { MaybePackage::Package(pkg) => pkg.manifest().warnings().warnings(), MaybePackage::Virtual(vm) => vm.warnings().warnings(), }; - let path = path.join("Cargo.toml"); for warning in warnings { if warning.is_critical { let err = anyhow::format_err!("{}", warning.message); @@ -1121,6 +1126,30 @@ impl<'gctx> Workspace<'gctx> { Ok(()) } + pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> { + let mut error_count = 0; + let lints = pkg + .manifest() + .resolved_toml() + .lints + .clone() + .map(|lints| lints.lints) + .unwrap_or(manifest::TomlLints::default()) + .get("cargo") + .cloned() + .unwrap_or(manifest::TomlToolLints::default()); + + check_implicit_features(pkg, &path, &lints, &mut error_count, self.gctx)?; + if error_count > 0 { + Err(crate::util::errors::AlreadyPrintedError::new(anyhow!( + "encountered {error_count} errors(s) while running lints" + )) + .into()) + } else { + Ok(()) + } + } + pub fn set_target_dir(&mut self, target_dir: Filesystem) { self.target_dir = Some(target_dir); } diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs new file mode 100644 index 00000000000..7c6ecb58ea3 --- /dev/null +++ b/src/cargo/util/lints.rs @@ -0,0 +1,226 @@ +use crate::core::FeatureValue::Dep; +use crate::core::{Edition, FeatureValue, Package}; +use crate::util::interning::InternedString; +use crate::{CargoResult, GlobalContext}; +use annotate_snippets::{Level, Renderer, Snippet}; +use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints}; +use pathdiff::diff_paths; +use std::collections::HashSet; +use std::ops::Range; +use std::path::Path; +use toml_edit::ImDocument; + +fn get_span(document: &ImDocument, path: &[&str], get_value: bool) -> Option> { + let mut table = document.as_item().as_table_like().unwrap(); + let mut iter = path.into_iter().peekable(); + while let Some(key) = iter.next() { + let (key, item) = table.get_key_value(key).unwrap(); + if iter.peek().is_none() { + return if get_value { + item.span() + } else { + let leaf_decor = key.dotted_decor(); + let leaf_prefix_span = leaf_decor.prefix().and_then(|p| p.span()); + let leaf_suffix_span = leaf_decor.suffix().and_then(|s| s.span()); + if let (Some(leaf_prefix_span), Some(leaf_suffix_span)) = + (leaf_prefix_span, leaf_suffix_span) + { + Some(leaf_prefix_span.start..leaf_suffix_span.end) + } else { + key.span() + } + }; + } + if item.is_table_like() { + table = item.as_table_like().unwrap(); + } + if item.is_array() && iter.peek().is_some() { + let array = item.as_array().unwrap(); + let next = iter.next().unwrap(); + return array.iter().find_map(|item| { + if next == &item.to_string() { + item.span() + } else { + None + } + }); + } + } + None +} + +/// Gets the relative path to a manifest from the current working directory, or +/// the absolute path of the manifest if a relative path cannot be constructed +fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String { + diff_paths(path, gctx.cwd()) + .unwrap_or_else(|| path.to_path_buf()) + .display() + .to_string() +} + +#[derive(Copy, Clone, Debug)] +pub struct LintGroup { + pub name: &'static str, + pub default_level: LintLevel, + pub desc: &'static str, + pub edition_lint_opts: Option<(Edition, LintLevel)>, +} + +const RUST_2024_COMPATIBILITY: LintGroup = LintGroup { + name: "rust-2024-compatibility", + default_level: LintLevel::Allow, + desc: "warn about compatibility with Rust 2024", + edition_lint_opts: Some((Edition::Edition2024, LintLevel::Deny)), +}; + +#[derive(Copy, Clone, Debug)] +pub struct Lint { + pub name: &'static str, + pub desc: &'static str, + pub groups: &'static [LintGroup], + pub default_level: LintLevel, + pub edition_lint_opts: Option<(Edition, LintLevel)>, +} + +impl Lint { + pub fn level(&self, lints: &TomlToolLints, edition: Edition) -> LintLevel { + let level = self + .groups + .iter() + .map(|g| g.name) + .chain(std::iter::once(self.name)) + .filter_map(|n| lints.get(n).map(|l| (n, l))) + .max_by_key(|(n, l)| (l.priority(), std::cmp::Reverse(*n))); + + match level { + Some((_, toml_lint)) => toml_lint.level().into(), + None => { + if let Some((lint_edition, lint_level)) = self.edition_lint_opts { + if edition >= lint_edition { + return lint_level; + } + } + self.default_level + } + } + } +} + +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum LintLevel { + Allow, + Warn, + Deny, + Forbid, +} + +impl LintLevel { + pub fn to_diagnostic_level(self) -> Level { + match self { + LintLevel::Allow => Level::Note, + LintLevel::Warn => Level::Warning, + LintLevel::Deny => Level::Error, + LintLevel::Forbid => Level::Error, + } + } +} + +impl From for LintLevel { + fn from(toml_lint_level: TomlLintLevel) -> LintLevel { + match toml_lint_level { + TomlLintLevel::Allow => LintLevel::Allow, + TomlLintLevel::Warn => LintLevel::Warn, + TomlLintLevel::Deny => LintLevel::Deny, + TomlLintLevel::Forbid => LintLevel::Forbid, + } + } +} + +/// 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: "warn about the use of unstable features", + groups: &[RUST_2024_COMPATIBILITY], + default_level: LintLevel::Allow, + edition_lint_opts: Some((Edition::Edition2024, LintLevel::Deny)), +}; + +pub fn check_implicit_features( + pkg: &Package, + path: &Path, + lints: &TomlToolLints, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + let lint_level = IMPLICIT_FEATURES.level(lints, pkg.manifest().edition()); + if lint_level == LintLevel::Allow { + return Ok(()); + } + + let manifest = pkg.manifest(); + let user_defined_features = manifest.resolved_toml().features(); + let features = user_defined_features.map_or(HashSet::new(), |f| { + f.keys().map(|k| InternedString::new(&k)).collect() + }); + // Add implicit features for optional dependencies if they weren't + // explicitly listed anywhere. + let explicitly_listed = user_defined_features.map_or(HashSet::new(), |f| { + f.values() + .flatten() + .filter_map(|v| match FeatureValue::new(v.into()) { + Dep { dep_name } => Some(dep_name), + _ => None, + }) + .collect() + }); + + for dep in manifest.dependencies() { + let dep_name_in_toml = dep.name_in_toml(); + if !dep.is_optional() + || features.contains(&dep_name_in_toml) + || explicitly_listed.contains(&dep_name_in_toml) + { + continue; + } + if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny { + *error_count += 1; + } + let level = lint_level.to_diagnostic_level(); + let manifest_path = rel_cwd_manifest_path(path, gctx); + let message = level.title("unused optional dependency").snippet( + Snippet::source(manifest.contents()) + .origin(&manifest_path) + .annotation( + level.span( + get_span( + manifest.document(), + &["dependencies", &dep_name_in_toml], + false, + ) + .unwrap(), + ), + ) + .fold(true), + ); + let renderer = Renderer::styled().term_width( + gctx.shell() + .err_width() + .diagnostic_terminal_width() + .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH), + ); + writeln!(gctx.shell().err(), "{}", renderer.render(message))?; + } + Ok(()) +} diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index cacc88fcb28..7b32f108fa7 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -51,6 +51,7 @@ pub mod into_url; mod into_url_with_base; mod io; pub mod job; +pub mod lints; mod lockserver; pub mod machine_message; pub mod network; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 3898695fbe7..3a4fd6847ae 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -494,7 +494,7 @@ pub fn to_real_manifest( let workspace_config = match (original_toml.workspace.as_ref(), package.workspace.as_ref()) { (Some(toml_config), None) => { - verify_lints(toml_config.lints.as_ref())?; + verify_lints(toml_config.lints.as_ref(), gctx, &mut warnings)?; if let Some(ws_deps) = &toml_config.dependencies { for (name, dep) in ws_deps { unused_dep_keys( @@ -828,7 +828,7 @@ pub fn to_real_manifest( .clone() .map(|mw| lints_inherit_with(mw, || inherit()?.lints())) .transpose()?; - verify_lints(lints.as_ref())?; + verify_lints(lints.as_ref(), gctx, manifest_ctx.warnings)?; let default = manifest::TomlLints::default(); let rustflags = lints_to_rustflags(lints.as_ref().unwrap_or(&default)); @@ -1322,7 +1322,7 @@ fn to_virtual_manifest( .transpose()?; let workspace_config = match original_toml.workspace { Some(ref toml_config) => { - verify_lints(toml_config.lints.as_ref())?; + verify_lints(toml_config.lints.as_ref(), gctx, &mut warnings)?; let ws_root_config = to_workspace_config(toml_config, root); gctx.ws_roots .borrow_mut() @@ -1448,17 +1448,24 @@ struct ManifestContext<'a, 'b> { features: &'a Features, } -fn verify_lints(lints: Option<&manifest::TomlLints>) -> CargoResult<()> { +fn verify_lints( + lints: Option<&manifest::TomlLints>, + gctx: &GlobalContext, + warnings: &mut Vec, +) -> CargoResult<()> { let Some(lints) = lints else { return Ok(()); }; for (tool, lints) in lints { - let supported = ["rust", "clippy", "rustdoc"]; + let supported = ["cargo", "clippy", "rust", "rustdoc"]; if !supported.contains(&tool.as_str()) { let supported = supported.join(", "); anyhow::bail!("unsupported `{tool}` in `[lints]`, must be one of {supported}") } + if tool == "cargo" && !gctx.cli_unstable().cargo_lints { + warn_for_cargo_lint_feature(gctx, warnings); + } for name in lints.keys() { if let Some((prefix, suffix)) = name.split_once("::") { if tool == prefix { @@ -1479,9 +1486,43 @@ fn verify_lints(lints: Option<&manifest::TomlLints>) -> CargoResult<()> { Ok(()) } +fn warn_for_cargo_lint_feature(gctx: &GlobalContext, warnings: &mut Vec) { + use std::fmt::Write as _; + + let key_name = "lints.cargo"; + let feature_name = "cargo-lints"; + + let mut message = String::new(); + + let _ = write!( + message, + "unused manifest key `{key_name}` (may be supported in a future version)" + ); + if gctx.nightly_features_allowed { + let _ = write!( + message, + " + +consider passing `-Z{feature_name}` to enable this feature." + ); + } else { + let _ = write!( + message, + " + +this Cargo does not support nightly features, but if you +switch to nightly channel you can pass +`-Z{feature_name}` to enable this feature.", + ); + } + warnings.push(message); +} + fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec { let mut rustflags = lints .iter() + // We don't want to pass any of the `cargo` lints to `rustc` + .filter(|(tool, _)| tool != &"cargo") .flat_map(|(tool, lints)| { lints.iter().map(move |(name, config)| { let flag = match config.level() { diff --git a/tests/testsuite/cargo/z_help/stdout.term.svg b/tests/testsuite/cargo/z_help/stdout.term.svg index 5848c09f543..f500049ae58 100644 --- a/tests/testsuite/cargo/z_help/stdout.term.svg +++ b/tests/testsuite/cargo/z_help/stdout.term.svg @@ -1,4 +1,4 @@ - +