Skip to content

Commit

Permalink
fix: improve message for inactive weak optional feature with edition2024
Browse files Browse the repository at this point in the history
  • Loading branch information
linyihai committed Jun 28, 2024
1 parent 1384869 commit 44fd447
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 29 deletions.
52 changes: 51 additions & 1 deletion src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,50 @@ struct Inner {
rust_version: Option<RustVersion>,
}

// Indicates the dependency inferred from the `dep` syntax that should exist, but missing on the resolved dependencies
#[derive(Debug)]
pub struct MissingDependency {
dep_name: InternedString,
feature: InternedString,
feature_value: FeatureValue,
weak_optional: bool, // Indicates the dependency inferred from the `dep?` syntax that is weak optional
unused_dependency: bool, // Indicates the dependency is unused but not absent in the manifest
}

impl MissingDependency {
pub fn set_unused_dependency(&mut self, flag: bool) {
self.unused_dependency = flag
}
pub fn weak_optional(&self) -> bool {
self.weak_optional
}
pub fn dep_name(&self) -> String {
self.dep_name.to_string()
}
}

impl fmt::Display for MissingDependency {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.unused_dependency && self.weak_optional {
write!(
f,
"feature `{feature}` includes `{fv}`, but missing `dep:{dep_name}` to activate it",
feature = &self.feature,
fv = &self.feature_value,
dep_name = &self.dep_name
)
} else {
write!(
f,
"feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency",
feature = &self.feature,
fv = &self.feature_value,
dep_name = &self.dep_name
)
}
}
}

impl Summary {
#[tracing::instrument(skip_all)]
pub fn new(
Expand Down Expand Up @@ -274,7 +318,13 @@ 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!(MissingDependency {
feature: *feature,
feature_value: (*fv).clone(),
dep_name: *dep_name,
weak_optional: *weak,
unused_dependency: false,
})
}
if *weak && !is_optional_dep {
bail!(
Expand Down
66 changes: 48 additions & 18 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::str::{self, FromStr};

use crate::core::summary::MissingDependency;
use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
Expand Down Expand Up @@ -1435,24 +1436,53 @@ 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 mut 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 expose 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 mut err) = summary {
if let Some(missing_dep) = err.downcast_mut::<MissingDependency>() {
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()) {
missing_dep.set_unused_dependency(true);
}
}
}
}
}
}
summary?
};
if summary.features().contains_key("default-features") {
warnings.push(
"`default-features = [\"..\"]` was found in [features]. \
Expand Down
51 changes: 41 additions & 10 deletions tests/testsuite/lints/unused_optional_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -258,14 +259,13 @@ 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
Expand Down Expand Up @@ -293,13 +293,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();
}

0 comments on commit 44fd447

Please sign in to comment.