Skip to content

Commit 20f9989

Browse files
committed
fix: improve message for inactive weak optional feature with edition2024
1 parent 1384869 commit 20f9989

File tree

3 files changed

+139
-29
lines changed

3 files changed

+139
-29
lines changed

src/cargo/core/summary.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,49 @@ struct Inner {
3030
rust_version: Option<RustVersion>,
3131
}
3232

33+
// Indicates the dependency inferred from the `dep` syntax that should exist, but missing on the resolved dependencies
34+
pub struct MissingDependency {
35+
dep_name: InternedString,
36+
feature: InternedString,
37+
feature_value: FeatureValue,
38+
weak_optional: bool, // Indicates the dependency inferred from the `dep?` syntax that is weak optional
39+
unused_dependency: bool, // Indicates the dependency is unused but not absent in the manifest
40+
}
41+
42+
impl MissingDependency {
43+
pub fn set_unused_dependency(&mut self, flag: bool) {
44+
self.unused_dependency = flag
45+
}
46+
pub fn weak_optional(&self) -> bool {
47+
self.weak_optional
48+
}
49+
pub fn dep_name(&self) -> String {
50+
self.dep_name.to_string()
51+
}
52+
}
53+
54+
impl fmt::Display for MissingDependency {
55+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
56+
if self.unused_dependency && self.weak_optional {
57+
write!(
58+
f,
59+
"feature `{feature}` includes `{fv}`, but missing `dep:{dep_name}` to activate it",
60+
feature = &self.feature,
61+
fv = &self.feature_value,
62+
dep_name = &self.dep_name
63+
)
64+
} else {
65+
write!(
66+
f,
67+
"feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency",
68+
feature = &self.feature,
69+
fv = &self.feature_value,
70+
dep_name = &self.dep_name
71+
)
72+
}
73+
}
74+
}
75+
3376
impl Summary {
3477
#[tracing::instrument(skip_all)]
3578
pub fn new(
@@ -274,7 +317,13 @@ fn build_feature_map(
274317

275318
// Validation of the feature name will be performed in the resolver.
276319
if !is_any_dep {
277-
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency");
320+
bail!(MissingDependency {
321+
feature: *feature,
322+
feature_value: (*fv).clone(),
323+
dep_name: *dep_name,
324+
weak_optional: *weak,
325+
unused_dependency: false,
326+
})
278327
}
279328
if *weak && !is_optional_dep {
280329
bail!(

src/cargo/util/toml/mod.rs

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
55
use std::rc::Rc;
66
use std::str::{self, FromStr};
77

8+
use crate::core::summary::MissingDependency;
89
use crate::AlreadyPrintedError;
910
use anyhow::{anyhow, bail, Context as _};
1011
use cargo_platform::Platform;
@@ -1435,24 +1436,53 @@ fn to_real_manifest(
14351436
.unwrap_or_else(|| semver::Version::new(0, 0, 0)),
14361437
source_id,
14371438
);
1438-
let summary = Summary::new(
1439-
pkgid,
1440-
deps,
1441-
&resolved_toml
1442-
.features
1443-
.as_ref()
1444-
.unwrap_or(&Default::default())
1445-
.iter()
1446-
.map(|(k, v)| {
1447-
(
1448-
InternedString::new(k),
1449-
v.iter().map(InternedString::from).collect(),
1450-
)
1451-
})
1452-
.collect(),
1453-
resolved_package.links.as_deref(),
1454-
rust_version.clone(),
1455-
)?;
1439+
let summary = {
1440+
let mut summary = Summary::new(
1441+
pkgid,
1442+
deps,
1443+
&resolved_toml
1444+
.features
1445+
.as_ref()
1446+
.unwrap_or(&Default::default())
1447+
.iter()
1448+
.map(|(k, v)| {
1449+
(
1450+
InternedString::new(k),
1451+
v.iter().map(InternedString::from).collect(),
1452+
)
1453+
})
1454+
.collect(),
1455+
resolved_package.links.as_deref(),
1456+
rust_version.clone(),
1457+
);
1458+
// editon2024 stops expose implicit features, which will strip weak optional dependencies from `dependencies`,
1459+
// need to check whether `dep_name` is stripped as unused dependency
1460+
if let Err(ref mut err) = summary {
1461+
if let Some(missing_dep) = err.downcast_mut::<MissingDependency>() {
1462+
if missing_dep.weak_optional() {
1463+
// dev-dependencies are not allowed to be optional
1464+
let mut orig_deps = vec![
1465+
original_toml.dependencies.as_ref(),
1466+
original_toml.build_dependencies.as_ref(),
1467+
];
1468+
for (_, platform) in original_toml.target.iter().flatten() {
1469+
orig_deps.extend(vec![
1470+
platform.dependencies.as_ref(),
1471+
platform.build_dependencies.as_ref(),
1472+
]);
1473+
}
1474+
for deps in orig_deps {
1475+
if let Some(deps) = deps {
1476+
if deps.keys().any(|p| *p.as_str() == missing_dep.dep_name()) {
1477+
missing_dep.set_unused_dependency(true);
1478+
}
1479+
}
1480+
}
1481+
}
1482+
}
1483+
}
1484+
summary?
1485+
};
14561486
if summary.features().contains_key("default-features") {
14571487
warnings.push(
14581488
"`default-features = [\"..\"]` was found in [features]. \

tests/testsuite/lints/unused_optional_dependencies.rs

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use cargo_test_support::project;
44
use cargo_test_support::registry::Package;
5+
use cargo_test_support::str;
56

67
#[cargo_test(nightly, reason = "edition2024 is not stable")]
78
fn default() {
@@ -258,14 +259,13 @@ fn inactive_weak_optional_dep() {
258259
p.cargo("check -Zcargo-lints")
259260
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
260261
.with_status(101)
261-
.with_stderr(
262-
"\
263-
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
262+
.with_stderr_data(str![[r#"
263+
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
264264
265265
Caused by:
266266
feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
267-
",
268-
)
267+
268+
"#]])
269269
.run();
270270

271271
// This test is that we need to improve in edition2024, we need to tell that a weak optioanl dependency needs specify
@@ -293,13 +293,44 @@ Caused by:
293293
p.cargo("check -Zcargo-lints")
294294
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
295295
.with_status(101)
296-
.with_stderr(
297-
"\
298-
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
296+
.with_stderr_data(str![[r#"
297+
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
299298
300299
Caused by:
301-
feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
302-
",
300+
feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it
301+
302+
"#]])
303+
.run();
304+
// Check target.'cfg(unix)'.dependencies can work
305+
let p = project()
306+
.file(
307+
"Cargo.toml",
308+
r#"
309+
cargo-features = ["edition2024"]
310+
[package]
311+
name = "foo"
312+
version = "0.1.0"
313+
edition = "2024"
314+
315+
[target.'cfg(unix)'.dependencies]
316+
dep_name = { version = "0.1.0", optional = true }
317+
318+
[features]
319+
foo_feature = ["dep_name?/dep_feature"]
320+
"#,
303321
)
322+
.file("src/lib.rs", "")
323+
.build();
324+
325+
p.cargo("check -Zcargo-lints")
326+
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
327+
.with_status(101)
328+
.with_stderr_data(str![[r#"
329+
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
330+
331+
Caused by:
332+
feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it
333+
334+
"#]])
304335
.run();
305336
}

0 commit comments

Comments
 (0)