diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index 569176169..f443fdd6e 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 68fbed26b..1d2c23a92 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 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, - ) - } - } - } - } + 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(); - Ok(()) + 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![ + ( + orig_toml.dependencies.as_ref(), + vec![DepKind::Normal.kind_table()], + ), + ( + orig_toml.build_dependencies.as_ref(), + vec![DepKind::Build.kind_table()], + ), + ]; + 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()], + )); + } + + if let Some((_, toml_path)) = orig_deps.iter().find(|(deps, _)| { + if let Some(deps) = deps { + 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) + }; + + 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 bbfce71e5..2ffc950c5 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 10d8c2d1e..3828211ff 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(); }