From 1384869f88c953c55634137a798ece2c502b1e22 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Thu, 6 Jun 2024 17:26:35 +0800 Subject: [PATCH 1/5] test: Add test for weak optional feature in edition2024 --- .../lints/unused_optional_dependencies.rs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/tests/testsuite/lints/unused_optional_dependencies.rs b/tests/testsuite/lints/unused_optional_dependencies.rs index 5f63796606a..cf1d6424712 100644 --- a/tests/testsuite/lints/unused_optional_dependencies.rs +++ b/tests/testsuite/lints/unused_optional_dependencies.rs @@ -201,3 +201,105 @@ warning: unused optional dependency ) .run(); } + +#[cargo_test(nightly, reason = "edition2024 is not stable")] +fn inactive_weak_optional_dep() { + Package::new("dep_name", "0.1.0") + .feature("dep_feature", &[]) + .publish(); + + // `dep_name`` is included as a weak optional dependency throught speficying the `dep_name?/dep_feature` in feature table. + // In edition2024, `dep_name` need to be add `dep:dep_name` to feature table to speficying activate it. + + // This test explain the conclusion mentioned above + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + [package] + name = "foo" + version = "0.1.0" + edition = "2024" + + [dependencies] + dep_name = { version = "0.1.0", optional = true } + + [features] + foo_feature = ["dep:dep_name", "dep_name?/dep_feature"] + "#, + ) + .file("src/lib.rs", "") + .build(); + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) + .run(); + + // This test proves no regression when dep_name isn't included + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + [package] + name = "foo" + version = "0.1.0" + edition = "2024" + + [dependencies] + + [features] + foo_feature = ["dep_name?/dep_feature"] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency +", + ) + .run(); + + // This test is that we need to improve in edition2024, we need to tell that a weak optioanl dependency needs specify + // the `dep:` syntax, like `dep:dep_name`. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + [package] + name = "foo" + version = "0.1.0" + edition = "2024" + + [dependencies] + dep_name = { version = "0.1.0", optional = true } + + [features] + foo_feature = ["dep_name?/dep_feature"] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency +", + ) + .run(); +} From 1306429464e8182d987cf4c4715a162b04e10df2 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Thu, 27 Jun 2024 16:24:31 +0800 Subject: [PATCH 2/5] fix: improve message for inactive weak optional feature with edition2024 --- src/cargo/core/summary.rs | 36 ++++++++- src/cargo/util/toml/mod.rs | 81 ++++++++++++++----- .../lints/unused_optional_dependencies.rs | 56 ++++++++++--- 3 files changed, 141 insertions(+), 32 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 18eaa16a0a7..c551b05b7c8 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -30,6 +30,35 @@ struct Inner { rust_version: Option, } +/// Indicates the dependency inferred from the `dep` syntax that should exist, +/// but missing on the resolved dependencies tables. +#[derive(Debug)] +pub struct MissingDependencyError { + pub dep_name: InternedString, + pub feature: InternedString, + pub feature_value: FeatureValue, + /// Indicates the dependency inferred from the `dep?` syntax that is weak optional + pub weak_optional: bool, +} + +impl std::error::Error for MissingDependencyError {} + +impl fmt::Display for MissingDependencyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { + dep_name, + feature, + feature_value: fv, + .. + } = self; + + write!( + f, + "feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency", + ) + } +} + impl Summary { #[tracing::instrument(skip_all)] pub fn new( @@ -274,7 +303,12 @@ fn build_feature_map( // Validation of the feature name will be performed in the resolver. if !is_any_dep { - bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency"); + bail!(MissingDependencyError { + feature: *feature, + feature_value: (*fv).clone(), + dep_name: *dep_name, + weak_optional: *weak, + }) } if *weak && !is_optional_dep { bail!( diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 001b1bc74e4..68fbed26bfa 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str::{self, FromStr}; +use crate::core::summary::MissingDependencyError; use crate::AlreadyPrintedError; use anyhow::{anyhow, bail, Context as _}; use cargo_platform::Platform; @@ -1435,24 +1436,35 @@ fn to_real_manifest( .unwrap_or_else(|| semver::Version::new(0, 0, 0)), source_id, ); - let summary = Summary::new( - pkgid, - deps, - &resolved_toml - .features - .as_ref() - .unwrap_or(&Default::default()) - .iter() - .map(|(k, v)| { - ( - InternedString::new(k), - v.iter().map(InternedString::from).collect(), - ) - }) - .collect(), - resolved_package.links.as_deref(), - rust_version.clone(), - )?; + let summary = { + let summary = Summary::new( + pkgid, + deps, + &resolved_toml + .features + .as_ref() + .unwrap_or(&Default::default()) + .iter() + .map(|(k, v)| { + ( + InternedString::new(k), + v.iter().map(InternedString::from).collect(), + ) + }) + .collect(), + resolved_package.links.as_deref(), + rust_version.clone(), + ); + // editon2024 stops exposing implicit features, which will strip weak optional dependencies from `dependencies`, + // need to check whether `dep_name` is stripped as unused dependency + if let Err(ref err) = summary { + if let Some(missing_dep) = err.downcast_ref::() { + check_weak_optional_dep_unused(&original_toml, missing_dep)?; + } + } + summary? + }; + if summary.features().contains_key("default-features") { warnings.push( "`default-features = [\"..\"]` was found in [features]. \ @@ -1558,6 +1570,39 @@ fn to_real_manifest( Ok(manifest) } +fn check_weak_optional_dep_unused( + original_toml: &TomlManifest, + missing_dep: &MissingDependencyError, +) -> CargoResult<()> { + if missing_dep.weak_optional { + // dev-dependencies are not allowed to be optional + let mut orig_deps = vec![ + original_toml.dependencies.as_ref(), + original_toml.build_dependencies.as_ref(), + ]; + for (_, platform) in original_toml.target.iter().flatten() { + orig_deps.extend(vec![ + platform.dependencies.as_ref(), + platform.build_dependencies.as_ref(), + ]); + } + for deps in orig_deps { + if let Some(deps) = deps { + if deps.keys().any(|p| *p.as_str() == *missing_dep.dep_name) { + bail!( + "feature `{feature}` includes `{feature_value}`, but missing `dep:{dep_name}` to activate it", + feature = missing_dep.feature, + feature_value = missing_dep.feature_value, + dep_name = missing_dep.dep_name, + ) + } + } + } + } + + Ok(()) +} + fn to_virtual_manifest( contents: String, document: toml_edit::ImDocument, diff --git a/tests/testsuite/lints/unused_optional_dependencies.rs b/tests/testsuite/lints/unused_optional_dependencies.rs index cf1d6424712..fa3d5364fdc 100644 --- a/tests/testsuite/lints/unused_optional_dependencies.rs +++ b/tests/testsuite/lints/unused_optional_dependencies.rs @@ -2,6 +2,7 @@ use cargo_test_support::project; use cargo_test_support::registry::Package; +use cargo_test_support::str; #[cargo_test(nightly, reason = "edition2024 is not stable")] fn default() { @@ -209,7 +210,7 @@ fn inactive_weak_optional_dep() { .publish(); // `dep_name`` is included as a weak optional dependency throught speficying the `dep_name?/dep_feature` in feature table. - // In edition2024, `dep_name` need to be add `dep:dep_name` to feature table to speficying activate it. + // In edition2024, `dep_name` need to be add `dep:dep_name` to feature table to activate it. // This test explain the conclusion mentioned above let p = project() @@ -258,18 +259,16 @@ fn inactive_weak_optional_dep() { p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) .with_status(101) - .with_stderr( - "\ -error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency -", - ) + +"#]]) .run(); - // This test is that we need to improve in edition2024, we need to tell that a weak optioanl dependency needs specify - // the `dep:` syntax, like `dep:dep_name`. + // Ensure that a weak dependency feature requires the existence of a `dep:` feature in edition 2024. let p = project() .file( "Cargo.toml", @@ -293,13 +292,44 @@ Caused by: p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) .with_status(101) - .with_stderr( - "\ -error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: - feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency -", + feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it + +"#]]) + .run(); + // Check target.'cfg(unix)'.dependencies can work + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + [package] + name = "foo" + version = "0.1.0" + edition = "2024" + + [target.'cfg(unix)'.dependencies] + dep_name = { version = "0.1.0", optional = true } + + [features] + foo_feature = ["dep_name?/dep_feature"] + "#, ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it + +"#]]) .run(); } From 1b86aaf9169aa9e4714012b6587f215eae8d2299 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Tue, 2 Jul 2024 10:33:51 +0800 Subject: [PATCH 3/5] chore: Rename `MissingField` to `MissingFieldError` --- src/cargo/util/context/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index b84c04ca58c..f275c23877a 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -2031,7 +2031,7 @@ impl ConfigError { } fn is_missing_field(&self) -> bool { - self.error.downcast_ref::().is_some() + self.error.downcast_ref::().is_some() } fn missing(key: &ConfigKey) -> ConfigError { @@ -2067,15 +2067,15 @@ impl fmt::Display for ConfigError { } #[derive(Debug)] -struct MissingField(String); +struct MissingFieldError(String); -impl fmt::Display for MissingField { +impl fmt::Display for MissingFieldError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "missing field `{}`", self.0) } } -impl std::error::Error for MissingField {} +impl std::error::Error for MissingFieldError {} impl serde::de::Error for ConfigError { fn custom(msg: T) -> Self { @@ -2087,7 +2087,7 @@ impl serde::de::Error for ConfigError { fn missing_field(field: &'static str) -> Self { ConfigError { - error: anyhow::Error::new(MissingField(field.to_string())), + error: anyhow::Error::new(MissingFieldError(field.to_string())), definition: None, } } From 3eb6bdf41e9998a0f29dc6a8353be62ce0c25a6c Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Tue, 2 Jul 2024 10:48:56 +0800 Subject: [PATCH 4/5] test: Migrate `unused_optional_dependencies` to snapshot --- .../lints/unused_optional_dependencies.rs | 96 +++++++++---------- 1 file changed, 45 insertions(+), 51 deletions(-) diff --git a/tests/testsuite/lints/unused_optional_dependencies.rs b/tests/testsuite/lints/unused_optional_dependencies.rs index fa3d5364fdc..10d8c2d1ee9 100644 --- a/tests/testsuite/lints/unused_optional_dependencies.rs +++ b/tests/testsuite/lints/unused_optional_dependencies.rs @@ -1,5 +1,3 @@ -#![allow(deprecated)] - use cargo_test_support::project; use cargo_test_support::registry::Package; use cargo_test_support::str; @@ -34,34 +32,33 @@ target-dep = { version = "0.1.0", optional = true } p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) - .with_stderr( - "\ -warning: unused optional dependency + .with_stderr_data(str![[r#" +[WARNING] unused optional dependency --> Cargo.toml:9:1 | -9 | bar = { version = \"0.1.0\", optional = true } +9 | bar = { version = "0.1.0", optional = true } | --- | - = note: `cargo::unused_optional_dependency` is set to `warn` by default - = help: remove the dependency or activate it in a feature with `dep:bar` -warning: unused optional dependency + = [NOTE] `cargo::unused_optional_dependency` is set to `warn` by default + = [HELP] remove the dependency or activate it in a feature with `dep:bar` +[WARNING] unused optional dependency --> Cargo.toml:12:1 | -12 | baz = { version = \"0.1.0\", optional = true } +12 | baz = { version = "0.1.0", optional = true } | --- | - = help: remove the dependency or activate it in a feature with `dep:baz` -warning: unused optional dependency + = [HELP] remove the dependency or activate it in a feature with `dep:baz` +[WARNING] unused optional dependency --> Cargo.toml:15:1 | -15 | target-dep = { version = \"0.1.0\", optional = true } +15 | target-dep = { version = "0.1.0", optional = true } | ---------- | - = help: remove the dependency or activate it in a feature with `dep:target-dep` -[CHECKING] foo v0.1.0 ([CWD]) -[FINISHED] [..] -", - ) + = [HELP] remove the dependency or activate it in a feature with `dep:target-dep` +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) .run(); } @@ -89,14 +86,13 @@ implicit_features = "allow" p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints"]) - .with_stderr( - "\ -[UPDATING] [..] + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index [LOCKING] 2 packages to latest compatible versions -[CHECKING] foo v0.1.0 ([CWD]) -[FINISHED] [..] -", - ) +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) .run(); } @@ -130,34 +126,33 @@ target-dep = { version = "0.1.0", optional = true } p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) - .with_stderr( - "\ -warning: unused optional dependency + .with_stderr_data(str![[r#" +[WARNING] unused optional dependency --> Cargo.toml:9:1 | -9 | bar = { version = \"0.1.0\", optional = true } +9 | bar = { version = "0.1.0", optional = true } | --- | - = note: `cargo::unused_optional_dependency` is set to `warn` by default - = help: remove the dependency or activate it in a feature with `dep:bar` -warning: unused optional dependency + = [NOTE] `cargo::unused_optional_dependency` is set to `warn` by default + = [HELP] remove the dependency or activate it in a feature with `dep:bar` +[WARNING] unused optional dependency --> Cargo.toml:12:1 | -12 | baz = { version = \"0.2.0\", package = \"bar\", optional = true } +12 | baz = { version = "0.2.0", package = "bar", optional = true } | --- | - = help: remove the dependency or activate it in a feature with `dep:baz` -warning: unused optional dependency + = [HELP] remove the dependency or activate it in a feature with `dep:baz` +[WARNING] unused optional dependency --> Cargo.toml:15:1 | -15 | target-dep = { version = \"0.1.0\", optional = true } +15 | target-dep = { version = "0.1.0", optional = true } | ---------- | - = help: remove the dependency or activate it in a feature with `dep:target-dep` -[CHECKING] foo v0.1.0 ([CWD]) -[FINISHED] [..] -", - ) + = [HELP] remove the dependency or activate it in a feature with `dep:target-dep` +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) .run(); } @@ -186,20 +181,19 @@ optional-dep = [] p.cargo("check -Zcargo-lints") .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) - .with_stderr( - "\ -warning: unused optional dependency + .with_stderr_data(str![[r#" +[WARNING] unused optional dependency --> Cargo.toml:9:1 | -9 | optional-dep = { version = \"0.1.0\", optional = true } +9 | optional-dep = { version = "0.1.0", optional = true } | ------------ | - = note: `cargo::unused_optional_dependency` is set to `warn` by default - = help: remove the dependency or activate it in a feature with `dep:optional-dep` -[CHECKING] foo v0.1.0 ([CWD]) -[FINISHED] [..] -", - ) + = [NOTE] `cargo::unused_optional_dependency` is set to `warn` by default + = [HELP] remove the dependency or activate it in a feature with `dep:optional-dep` +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) .run(); } From b28eef9651de5ef3574c97f08aa7b5d62c505e00 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Wed, 3 Jul 2024 11:43:50 +0800 Subject: [PATCH 5/5] feat: Add `missing_dep_diagnostic` thanks to @Muscraft --- src/cargo/util/lints.rs | 8 +- src/cargo/util/toml/mod.rs | 97 +++++++++++++++---- tests/testsuite/features.rs | 18 ++-- .../lints/unused_optional_dependencies.rs | 37 +++++-- 4 files changed, 122 insertions(+), 38 deletions(-) diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index 569176169fb..f443fdd6ec2 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -199,7 +199,11 @@ fn verify_feature_enabled( Ok(()) } -fn get_span(document: &ImDocument, path: &[&str], get_value: bool) -> Option> { +pub fn get_span( + document: &ImDocument, + path: &[&str], + get_value: bool, +) -> Option> { let mut table = document.as_item().as_table_like()?; let mut iter = path.into_iter().peekable(); while let Some(key) = iter.next() { @@ -240,7 +244,7 @@ fn get_span(document: &ImDocument, path: &[&str], get_value: bool) -> Op /// 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 { +pub fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String { diff_paths(path, gctx.cwd()) .unwrap_or_else(|| path.to_path_buf()) .display() diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 68fbed26bfa..1d2c23a92b6 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -15,6 +15,7 @@ use cargo_util_schemas::manifest::{RustVersion, StringOrBool}; use itertools::Itertools; use lazycell::LazyCell; use pathdiff::diff_paths; +use toml_edit::ImDocument; use url::Url; use crate::core::compiler::{CompileKind, CompileTarget}; @@ -29,6 +30,7 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; +use crate::util::lints::{get_span, rel_cwd_manifest_path}; use crate::util::{self, context::ConfigRelativePath, GlobalContext, IntoUrl, OptVersionReq}; mod embedded; @@ -1459,7 +1461,14 @@ fn to_real_manifest( // need to check whether `dep_name` is stripped as unused dependency if let Err(ref err) = summary { if let Some(missing_dep) = err.downcast_ref::() { - check_weak_optional_dep_unused(&original_toml, missing_dep)?; + missing_dep_diagnostic( + missing_dep, + &original_toml, + &document, + &contents, + manifest_file, + gctx, + )?; } } summary? @@ -1570,37 +1579,83 @@ fn to_real_manifest( Ok(manifest) } -fn check_weak_optional_dep_unused( - original_toml: &TomlManifest, +fn missing_dep_diagnostic( missing_dep: &MissingDependencyError, + orig_toml: &TomlManifest, + document: &ImDocument, + contents: &str, + manifest_file: &Path, + gctx: &GlobalContext, ) -> CargoResult<()> { - if missing_dep.weak_optional { - // dev-dependencies are not allowed to be optional + let dep_name = missing_dep.dep_name; + let manifest_path = rel_cwd_manifest_path(manifest_file, gctx); + let feature_value_span = + get_span(&document, &["features", missing_dep.feature.as_str()], true).unwrap(); + + let title = format!( + "feature `{}` includes `{}`, but `{}` is not a dependency", + missing_dep.feature, missing_dep.feature_value, &dep_name + ); + let help = format!("enable the dependency with `dep:{dep_name}`"); + let info_label = format!( + "`{}` is an unused optional dependency since no feature enables it", + &dep_name + ); + let message = Level::Error.title(&title); + let snippet = Snippet::source(&contents) + .origin(&manifest_path) + .fold(true) + .annotation(Level::Error.span(feature_value_span.start..feature_value_span.end)); + let message = if missing_dep.weak_optional { let mut orig_deps = vec![ - original_toml.dependencies.as_ref(), - original_toml.build_dependencies.as_ref(), + ( + orig_toml.dependencies.as_ref(), + vec![DepKind::Normal.kind_table()], + ), + ( + orig_toml.build_dependencies.as_ref(), + vec![DepKind::Build.kind_table()], + ), ]; - for (_, platform) in original_toml.target.iter().flatten() { - orig_deps.extend(vec![ + for (name, platform) in orig_toml.target.iter().flatten() { + orig_deps.push(( platform.dependencies.as_ref(), + vec!["target", name, DepKind::Normal.kind_table()], + )); + orig_deps.push(( platform.build_dependencies.as_ref(), - ]); + vec!["target", name, DepKind::Normal.kind_table()], + )); } - for deps in orig_deps { + + if let Some((_, toml_path)) = orig_deps.iter().find(|(deps, _)| { if let Some(deps) = deps { - if deps.keys().any(|p| *p.as_str() == *missing_dep.dep_name) { - bail!( - "feature `{feature}` includes `{feature_value}`, but missing `dep:{dep_name}` to activate it", - feature = missing_dep.feature, - feature_value = missing_dep.feature_value, - dep_name = missing_dep.dep_name, - ) - } + deps.keys().any(|p| *p.as_str() == *dep_name) + } else { + false } + }) { + let toml_path = toml_path + .iter() + .map(|s| *s) + .chain(std::iter::once(dep_name.as_str())) + .collect::>(); + let dep_span = get_span(&document, &toml_path, false).unwrap(); + + message + .snippet(snippet.annotation(Level::Warning.span(dep_span).label(&info_label))) + .footer(Level::Help.title(&help)) + } else { + message.snippet(snippet) } - } + } else { + message.snippet(snippet) + }; - Ok(()) + if let Err(err) = gctx.shell().print_message(message) { + return Err(err.into()); + } + Err(AlreadyPrintedError::new(anyhow!("").into()).into()) } fn to_virtual_manifest( diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index bbfce71e53c..2ffc950c51e 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -256,11 +256,14 @@ fn invalid6() { p.cargo("check --features foo") .with_status(101) .with_stderr_data(str![[r#" +[ERROR] feature `foo` includes `bar/baz`, but `bar` is not a dependency + --> Cargo.toml:9:23 + | +9 | foo = ["bar/baz"] + | ^^^^^^^^^^^ + | [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` -Caused by: - feature `foo` includes `bar/baz`, but `bar` is not a dependency - "#]]) .run(); } @@ -288,11 +291,14 @@ fn invalid7() { p.cargo("check --features foo") .with_status(101) .with_stderr_data(str![[r#" +[ERROR] feature `foo` includes `bar/baz`, but `bar` is not a dependency + --> Cargo.toml:9:23 + | +9 | foo = ["bar/baz"] + | ^^^^^^^^^^^ + | [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` -Caused by: - feature `foo` includes `bar/baz`, but `bar` is not a dependency - "#]]) .run(); } diff --git a/tests/testsuite/lints/unused_optional_dependencies.rs b/tests/testsuite/lints/unused_optional_dependencies.rs index 10d8c2d1ee9..3828211ff62 100644 --- a/tests/testsuite/lints/unused_optional_dependencies.rs +++ b/tests/testsuite/lints/unused_optional_dependencies.rs @@ -254,11 +254,14 @@ fn inactive_weak_optional_dep() { .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) .with_status(101) .with_stderr_data(str![[r#" +[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency + --> Cargo.toml:11:27 + | +11 | foo_feature = ["dep_name?/dep_feature"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` -Caused by: - feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency - "#]]) .run(); @@ -287,11 +290,19 @@ Caused by: .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) .with_status(101) .with_stderr_data(str![[r#" +[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency + --> Cargo.toml:12:31 + | + 9 | dep_name = { version = "0.1.0", optional = true } + | -------- `dep_name` is an unused optional dependency since no feature enables it +10 | +11 | [features] +12 | foo_feature = ["dep_name?/dep_feature"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [HELP] enable the dependency with `dep:dep_name` [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` -Caused by: - feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it - "#]]) .run(); // Check target.'cfg(unix)'.dependencies can work @@ -319,11 +330,19 @@ Caused by: .masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"]) .with_status(101) .with_stderr_data(str![[r#" +[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency + --> Cargo.toml:12:27 + | + 9 | dep_name = { version = "0.1.0", optional = true } + | -------- `dep_name` is an unused optional dependency since no feature enables it +10 | +11 | [features] +12 | foo_feature = ["dep_name?/dep_feature"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [HELP] enable the dependency with `dep:dep_name` [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` -Caused by: - feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it - "#]]) .run(); }