Fix ignoring optional deps when namespaced deps are used (#241)

CHANGELOG.md

@@ -10,6 +10,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Fix ignoring optional dependencies when namespaced dependencies are used. ([#241](https://github.com/taiki-e/cargo-hack/pull/241), thanks @xStrom)
+
 ## [0.6.22] - 2024-03-10
 
 - Pin `ctrlc` to fix [build error on macOS](https://github.com/Detegr/rust-ctrlc/pull/116).

src/features.rs

@@ -24,26 +24,25 @@ impl Features {
         let package = &metadata.packages[id];
 
         let mut features: Vec<_> = manifest.features.keys().map(Feature::from).collect();
-        let mut has_namespaced_features = false; // features with `dep:` prefix
+        let mut referenced_deps = BTreeSet::new(); // referenced in features with `dep:` prefix
 
         // package.features.values() does not provide a way to determine the `dep:` specified by the user.
         for names in manifest.features.values() {
             for name in names {
-                if name.starts_with("dep:") {
-                    has_namespaced_features = true;
-                    break;
+                if let Some(dep) = name.strip_prefix("dep:") {
+                    referenced_deps.insert(dep);
                 }
             }
         }
         let optional_deps_start = features.len();
-        // When namespace dependency is used, other optional dependencies are also not
-        // treated as implicit features.
-        if !has_namespaced_features {
-            for name in package.optional_deps() {
-                let feature = Feature::from(name);
-                if !features.contains(&feature) {
-                    features.push(feature);
-                }
+        for name in package.optional_deps() {
+            // Dependencies explicitly referenced with dep: are no longer implicit features.
+            if referenced_deps.contains(name) {
+                continue;
+            }
+            let feature = Feature::from(name);
+            if !features.contains(&feature) {
+                features.push(feature);
             }
         }
         let deps_features_start = features.len();

tests/fixtures/namespaced_features/Cargo.toml

@@ -5,16 +5,24 @@ edition = "2021"
 publish = false
 
 [features]
-easytime = ["dep:easytime"]
+explicit = ["dep:explicit"]
+combo = ["dep:member2", "dep:member3"]
 
 [dependencies]
-# easytime 0.2.6 requires Rust 1.58
-easytime = { version = "=0.2.5", optional = true, default-features = false }
-# When namespace dependency is used, other optional dependencies are also not
-# treated as implicit features.
-portable-atomic = { version = "1", optional = true }
-
-[dev-dependencies]
+explicit = { path = "explicit", optional = true } # Explicitly defined as a feature with dep:
+implicit = { path = "implicit", optional = true } # Implicit feature as an optional dependency
+member1 = { path = "member1" } # Regular dependency to be ignored
+member2 = { path = "member2", optional = true } # Available only through the combo feature
+member3 = { path = "member3", optional = true } # Available only through the combo feature
+renamed = { path = "member4", package = "member4", optional = true } # Renamed implicit feature
 
 [workspace]
 resolver = "2"
+members = [
+    "explicit",
+    "implicit",
+    "member1",
+    "member2",
+    "member3",
+    "member4",
+]

tests/fixtures/namespaced_features/explicit/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "explicit"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/explicit/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/implicit/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "implicit"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/implicit/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/member1/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "member1"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/member1/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/member2/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "member2"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/member2/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/member3/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "member3"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/member3/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/member4/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "member4"
+version = "0.0.0"
+publish = false
+
+[dependencies]
+
+[dev-dependencies]

tests/fixtures/namespaced_features/member4/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/namespaced_features/src/main.rs

@@ -0,0 +1,19 @@
+fn main() {
+    // Actual features
+    #[cfg(feature = "explicit")]
+    println!("explicit");
+    #[cfg(feature = "implicit")]
+    println!("implicit");
+    #[cfg(feature = "combo")]
+    println!("combo");
+    #[cfg(feature = "renamed")]
+    println!("renamed");
+
+    // Non-existent features
+    #[cfg(feature = "member1")]
+    println!("member1");
+    #[cfg(feature = "member2")]
+    println!("member2");
+    #[cfg(feature = "member3")]
+    println!("member3");
+}

tests/test.rs

@@ -1685,12 +1685,111 @@ fn namespaced_features() {
     // Namespaced features requires Rust 1.60.
     let require = Some(60);
 
+    cargo_hack([
+        "run",
+        "--features=explicit,implicit,combo,renamed,member1,member2,member3",
+        "--ignore-unknown-features",
+    ])
+    .assert_success2("namespaced_features", require)
+    .stderr_contains(
+        "
+            skipped applying unknown `member1` feature to namespaced_features
+            skipped applying unknown `member2` feature to namespaced_features
+            skipped applying unknown `member3` feature to namespaced_features
+            running `cargo run --features explicit,implicit,combo,renamed` on namespaced_features
+            ",
+    )
+    .stdout_contains(
+        "
+            explicit
+            implicit
+            combo
+            renamed
+            ",
+    )
+    .stdout_not_contains(
+        "
+            member1
+            member2
+            member3
+            ",
+    );
+
+    cargo_hack(["check", "--each-feature"])
+        .assert_success2("namespaced_features", require)
+        .stderr_contains(
+            "
+            running `cargo check --no-default-features` on namespaced_features (1/4)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/4)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/4)
+            running `cargo check --no-default-features --all-features` on namespaced_features (4/4)
+            ",
+        )
+        .stderr_not_contains(
+            "
+            --features implicit
+            --features renamed
+            ",
+        );
+
+    cargo_hack(["check", "--each-feature", "--optional-deps"])
+        .assert_success2("namespaced_features", require)
+        .stderr_contains(
+            "
+            running `cargo check --no-default-features` on namespaced_features (1/6)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/6)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/6)
+            running `cargo check --no-default-features --features implicit` on namespaced_features (4/6)
+            running `cargo check --no-default-features --features renamed` on namespaced_features (5/6)
+            running `cargo check --no-default-features --all-features` on namespaced_features (6/6)
+            ",
+        );
+
+    cargo_hack(["check", "--each-feature", "--optional-deps", "implicit"])
+        .assert_success2("namespaced_features", require)
+        .stderr_contains(
+            "
+            running `cargo check --no-default-features` on namespaced_features (1/5)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/5)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/5)
+            running `cargo check --no-default-features --features implicit` on namespaced_features (4/5)
+            running `cargo check --no-default-features --all-features` on namespaced_features (5/5)
+            ",
+        )
+        .stderr_not_contains("--features renamed");
+
+    cargo_hack(["check", "--each-feature", "--optional-deps=renamed"])
+        .assert_success2("namespaced_features", require)
+        .stderr_contains(
+            "
+            running `cargo check --no-default-features` on namespaced_features (1/5)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/5)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/5)
+            running `cargo check --no-default-features --features renamed` on namespaced_features (4/5)
+            running `cargo check --no-default-features --all-features` on namespaced_features (5/5)
+            ",
+        )
+        .stderr_not_contains("--features implicit");
+
+    cargo_hack(["check", "--each-feature", "--optional-deps="])
+        .assert_success2("namespaced_features", require)
+        .stderr_contains(
+            "
+            running `cargo check --no-default-features` on namespaced_features (1/4)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/4)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/4)
+            running `cargo check --no-default-features --all-features` on namespaced_features (4/4)
+            ",
+        );
+
     cargo_hack(["check", "--feature-powerset"])
         .assert_success2("namespaced_features", require)
         .stderr_contains(
             "
-            running `cargo check --no-default-features` on namespaced_features (1/2)
-            running `cargo check --no-default-features --features easytime` on namespaced_features (2/2)
+            running `cargo check --no-default-features` on namespaced_features (1/4)
+            running `cargo check --no-default-features --features combo` on namespaced_features (2/4)
+            running `cargo check --no-default-features --features explicit` on namespaced_features (3/4)
+            running `cargo check --no-default-features --features combo,explicit` on namespaced_features (4/4)
             ",
         );
 }