Repository: taiki-e/cargo-hack (rust)

Issue:
Intelligently exclude redundant items in powerset
Consider the following case (the actual crate doesn't matter; I just tested it on an empty crate).

```toml
[features]
std = ["alloc"]
alloc = []

```

I then perform `cargo hack check --feature-powerset --exclude-all-features` (I've inquired about true necessity of the latter flag elsewhere). This is the output:

```none
info: running `cargo check --no-default-features` on tmp (1/4)
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
info: running `cargo check --no-default-features --features alloc` on tmp (2/4)
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
info: running `cargo check --no-default-features --features std` on tmp (3/4)
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
info: running `cargo check --no-default-features --features alloc,std` on tmp (4/4)
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
```

Presumably it would be possible to determine from `Cargo.toml` that `std` relies on `alloc`, and as such `alloc,std` is redundant (because `std` is already in the powerset).

Pull request description:
Exclude feature combinations by detecting dependencies of features
Closes #81

Patch:
diff --git a/src/features.rs b/src/features.rs
index 3bfc3a6..0bef4b3 100644
--- a/src/features.rs
+++ b/src/features.rs
@@ -1,3 +1,5 @@
+use std::collections::{BTreeMap, BTreeSet};
+
 use crate::{
     metadata::{Dependency, Metadata},
     PackageId,
@@ -60,10 +62,46 @@ impl Features {
     }
 }
 
-pub(crate) fn powerset<T: Clone>(
-    iter: impl IntoIterator<Item = T>,
+pub(crate) fn feature_powerset<'a>(
+    features: impl IntoIterator<Item = &'a str>,
     depth: Option<usize>,
-) -> Vec<Vec<T>> {
+    map: &BTreeMap<String, Vec<String>>,
+) -> Vec<Vec<&'a str>> {
+    let feature_deps = feature_deps(map);
+    let powerset = powerset(features, depth);
+    powerset
+        .into_iter()
+        .filter(|a| {
+            !a.iter().filter_map(|b| feature_deps.get(b)).any(|c| a.iter().any(|d| c.contains(d)))
+        })
+        .collect()
+}
+
+fn feature_deps(map: &BTreeMap<String, Vec<String>>) -> BTreeMap<&str, BTreeSet<&str>> {
+    let mut feat_deps = BTreeMap::new();
+    for feat in map.keys() {
+        let mut set = BTreeSet::new();
+        fn f<'a>(
+            map: &'a BTreeMap<String, Vec<String>>,
+            set: &mut BTreeSet<&'a str>,
+            curr: &str,
+            root: &str,
+        ) {
+            if let Some(v) = map.get(curr) {
+                for x in v {
+                    if x != root && set.insert(x) {
+                        f(map, set, x, root);
+                    }
+                }
+            }
+        }
+        f(map, &mut set, feat, feat);
+        feat_deps.insert(&**feat, set);
+    }
+    feat_deps
+}
+
+fn powerset<T: Clone>(iter: impl IntoIterator<Item = T>, depth: Option<usize>) -> Vec<Vec<T>> {
     iter.into_iter().fold(vec![vec![]], |mut acc, elem| {
         let ext = acc.clone().into_iter().map(|mut curr| {
             curr.push(elem.clone());
@@ -80,7 +118,66 @@ pub(crate) fn powerset<T: Clone>(
 
 #[cfg(test)]
 mod tests {
-    use super::powerset;
+    use super::{feature_deps, feature_powerset, powerset};
+    use std::{
+        collections::{BTreeMap, BTreeSet},
+        iter::FromIterator,
+    };
+
+    macro_rules! svec {
+        ($($expr:expr),* $(,)?) => {
+            vec![$($expr.into()),*]
+        };
+    }
+
+    macro_rules! map {
+        ($(($key:expr, $value:expr)),* $(,)?) => {
+            BTreeMap::from_iter(vec![$(($key.into(), $value)),*])
+        };
+    }
+
+    macro_rules! set {
+        ($($expr:expr),* $(,)?) => {
+            BTreeSet::from_iter(vec![$($expr),*])
+        };
+    }
+
+    #[test]
+    fn feature_deps1() {
+        let map =
+            map![("a", svec![]), ("b", svec!["a"]), ("c", svec!["b"]), ("d", svec!["a", "b"])];
+        let fd = feature_deps(&map);
+        assert_eq!(fd, map![
+            ("a", set![]),
+            ("b", set!["a"]),
+            ("c", set!["a", "b"]),
+            ("d", set!["a", "b"])
+        ]);
+        let list = vec!["a", "b", "c", "d"];
+        let ps = powerset(list.clone(), None);
+        assert_eq!(ps, vec![
+            vec![],
+            vec!["a"],
+            vec!["b"],
+            vec!["a", "b"],
+            vec!["c"],
+            vec!["a", "c"],
+            vec!["b", "c"],
+            vec!["a", "b", "c"],
+            vec!["d"],
+            vec!["a", "d"],
+            vec!["b", "d"],
+            vec!["a", "b", "d"],
+            vec!["c", "d"],
+            vec!["a", "c", "d"],
+            vec!["b", "c", "d"],
+            vec!["a", "b", "c", "d"],
+        ]);
+        let filtered = feature_powerset(list, None, &map);
+        assert_eq!(filtered, vec![vec![], vec!["a"], vec!["b"], vec!["c"], vec!["d"], vec![
+            "c", "d"
+        ]]);
+    }
 
     #[test]
     fn powerset_full() {
diff --git a/src/main.rs b/src/main.rs
index 1316ad0..e463bab 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -151,7 +151,7 @@ fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progre
             Kind::Each { features }
         }
     } else if cx.feature_powerset {
-        let features = features::powerset(features, cx.depth);
+        let features = features::feature_powerset(features, cx.depth, &package.features);
 
         if (package.features.is_empty() || !cx.include_features.is_empty()) && features.is_empty() {
             progress.total += 1;


Test patch:
diff --git a/tests/fixtures/default_feature_behavior/Cargo.lock b/tests/fixtures/default_feature_behavior/Cargo.lock
index 862c65c..b3247e9 100644
--- a/tests/fixtures/default_feature_behavior/Cargo.lock
+++ b/tests/fixtures/default_feature_behavior/Cargo.lock
@@ -7,3 +7,4 @@ version = "0.1.0"
 [[package]]
 name = "no_default"
 version = "0.1.0"
+
diff --git a/tests/fixtures/powerset_deduplication/.cargo/config b/tests/fixtures/powerset_deduplication/.cargo/config
new file mode 100644
index 0000000..88403f3
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/.cargo/config
@@ -0,0 +1,2 @@
+[build]
+target-dir = "../../../target"
diff --git a/tests/fixtures/powerset_deduplication/Cargo.lock b/tests/fixtures/powerset_deduplication/Cargo.lock
new file mode 100644
index 0000000..86a0732
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/Cargo.lock
@@ -0,0 +1,21 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+[[package]]
+name = "deduplication"
+version = "0.1.0"
+dependencies = [
+ "easytime 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "member1 0.1.0",
+]
+
+[[package]]
+name = "easytime"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
+[[package]]
+name = "member1"
+version = "0.1.0"
+
+[metadata]
+"checksum easytime 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "33419a13337acaad02f83e749e4a16e5b6988877add627963f3a84032a43f2db"
diff --git a/tests/fixtures/powerset_deduplication/Cargo.toml b/tests/fixtures/powerset_deduplication/Cargo.toml
new file mode 100644
index 0000000..a61802e
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/Cargo.toml
@@ -0,0 +1,24 @@
+[package]
+name = "deduplication"
+version = "0.1.0"
+authors = ["Taiki Endo <te316e89@gmail.com>"]
+publish = false
+
+[workspace]
+members = [
+    "member1",
+    ".",
+]
+
+[features]
+a = []
+b = ["a"]
+c = ["b"]
+d = ["member1"]
+e = ["b", "d"]
+
+[dependencies]
+member1 = { path = "member1", optional = true }
+easytime = { version = "0.1", default-features = false }
+
+[dev-dependencies]
diff --git a/tests/fixtures/powerset_deduplication/member1/Cargo.toml b/tests/fixtures/powerset_deduplication/member1/Cargo.toml
new file mode 100644
index 0000000..5376950
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/member1/Cargo.toml
@@ -0,0 +1,15 @@
+[package]
+name = "member1"
+version = "0.1.0"
+authors = ["Taiki Endo <te316e89@gmail.com>"]
+
+[features]
+a = []
+b = ["a"]
+c = ["b"]
+d = []
+e = ["b", "d"]
+
+[dependencies]
+
+[dev-dependencies]
diff --git a/tests/fixtures/powerset_deduplication/member1/src/main.rs b/tests/fixtures/powerset_deduplication/member1/src/main.rs
new file mode 100644
index 0000000..f328e4d
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/member1/src/main.rs
@@ -0,0 +1,1 @@
+fn main() {}
diff --git a/tests/fixtures/powerset_deduplication/src/main.rs b/tests/fixtures/powerset_deduplication/src/main.rs
new file mode 100644
index 0000000..f328e4d
--- /dev/null
+++ b/tests/fixtures/powerset_deduplication/src/main.rs
@@ -0,0 +1,1 @@
+fn main() {}
diff --git a/tests/test.rs b/tests/test.rs
index be4c062..a4f82c4 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -514,6 +514,126 @@ fn feature_powerset_failure() {
         );
 }
 
+#[test]
+fn powerset_deduplication() {
+    cargo_hack(["check", "--feature-powerset"])
+        .test_dir("tests/fixtures/powerset_deduplication")
+        .assert_success()
+        .assert_stderr_contains(
+            "
+            running `cargo check --no-default-features` on deduplication (1/11)
+            running `cargo check --no-default-features --features a` on deduplication (2/11)
+            running `cargo check --no-default-features --features b` on deduplication (3/11)
+            running `cargo check --no-default-features --features c` on deduplication (4/11)
+            running `cargo check --no-default-features --features d` on deduplication (5/11)
+            running `cargo check --no-default-features --features a,d` on deduplication (6/11)
+            running `cargo check --no-default-features --features b,d` on deduplication (7/11)
+            running `cargo check --no-default-features --features c,d` on deduplication (8/11)
+            running `cargo check --no-default-features --features e` on deduplication (9/11)
+            running `cargo check --no-default-features --features c,e` on deduplication (10/11)
+            running `cargo check --no-default-features --all-features` on deduplication (11/11)
+            ",
+        )
+        .assert_stderr_not_contains(
+            "
+            a,b
+            b,c
+            a,c
+            a,e
+            b,e
+            d,e
+            ",
+        );
+
+    cargo_hack(["check", "--feature-powerset", "--optional-deps"])
+        .test_dir("tests/fixtures/powerset_deduplication")
+        .assert_success()
+        .assert_stderr_contains(
+            "
+            running `cargo check --no-default-features` on deduplication (1/15)
+            running `cargo check --no-default-features --features a` on deduplication (2/15)
+            running `cargo check --no-default-features --features b` on deduplication (3/15)
+            running `cargo check --no-default-features --features c` on deduplication (4/15)
+            running `cargo check --no-default-features --features d` on deduplication (5/15)
+            running `cargo check --no-default-features --features a,d` on deduplication (6/15)
+            running `cargo check --no-default-features --features b,d` on deduplication (7/15)
+            running `cargo check --no-default-features --features c,d` on deduplication (8/15)
+            running `cargo check --no-default-features --features e` on deduplication (9/15)
+            running `cargo check --no-default-features --features c,e` on deduplication (10/15)
+            running `cargo check --no-default-features --features member1` on deduplication (11/15)
+            running `cargo check --no-default-features --features a,member1` on deduplication (12/15)
+            running `cargo check --no-default-features --features b,member1` on deduplication (13/15)
+            running `cargo check --no-default-features --features c,member1` on deduplication (14/15)
+            running `cargo check --no-default-features --all-features` on deduplication (15/15)
+            ",
+        )
+        .assert_stderr_not_contains(
+            "
+            a,b
+            b,c
+            a,c
+            a,e
+            b,e
+            d,e
+            ",
+        );
+}
+
+#[rustversion::attr(not(since(1.41)), ignore)]
+#[test]
+fn powerset_deduplication_include_deps_features() {
+    // TODO: Since easytime/default depends on easytime/std, their combination should be excluded,
+    // but it's not working yet because include-deps-features itself isn't fully implemented.
+    cargo_hack(["check", "--feature-powerset", "--include-deps-features"])
+        .test_dir("tests/fixtures/powerset_deduplication")
+        .assert_success()
+        .assert_stderr_contains(
+            "
+            running `cargo check --no-default-features` on deduplication (1/41)
+            running `cargo check --no-default-features --features a` on deduplication (2/41)
+            running `cargo check --no-default-features --features b` on deduplication (3/41)
+            running `cargo check --no-default-features --features c` on deduplication (4/41)
+            running `cargo check --no-default-features --features d` on deduplication (5/41)
+            running `cargo check --no-default-features --features a,d` on deduplication (6/41)
+            running `cargo check --no-default-features --features b,d` on deduplication (7/41)
+            running `cargo check --no-default-features --features c,d` on deduplication (8/41)
+            running `cargo check --no-default-features --features e` on deduplication (9/41)
+            running `cargo check --no-default-features --features c,e` on deduplication (10/41)
+            running `cargo check --no-default-features --features easytime/default` on deduplication (11/41)
+            running `cargo check --no-default-features --features a,easytime/default` on deduplication (12/41)
+            running `cargo check --no-default-features --features b,easytime/default` on deduplication (13/41)
+            running `cargo check --no-default-features --features c,easytime/default` on deduplication (14/41)
+            running `cargo check --no-default-features --features d,easytime/default` on deduplication (15/41)
+            running `cargo check --no-default-features --features a,d,easytime/default` on deduplication (16/41)
+            running `cargo check --no-default-features --features b,d,easytime/default` on deduplication (17/41)
+            running `cargo check --no-default-features --features c,d,easytime/default` on deduplication (18/41)
+            running `cargo check --no-default-features --features e,easytime/default` on deduplication (19/41)
+            running `cargo check --no-default-features --features c,e,easytime/default` on deduplication (20/41)
+            running `cargo check --no-default-features --features easytime/std` on deduplication (21/41)
+            running `cargo check --no-default-features --features a,easytime/std` on deduplication (22/41)
+            running `cargo check --no-default-features --features b,easytime/std` on deduplication (23/41)
+            running `cargo check --no-default-features --features c,easytime/std` on deduplication (24/41)
+            running `cargo check --no-default-features --features d,easytime/std` on deduplication (25/41)
+            running `cargo check --no-default-features --features a,d,easytime/std` on deduplication (26/41)
+            running `cargo check --no-default-features --features b,d,easytime/std` on deduplication (27/41)
+            running `cargo check --no-default-features --features c,d,easytime/std` on deduplication (28/41)
+            running `cargo check --no-default-features --features e,easytime/std` on deduplication (29/41)
+            running `cargo check --no-default-features --features c,e,easytime/std` on deduplication (30/41)
+            running `cargo check --no-default-features --features easytime/default,easytime/std` on deduplication (31/41)
+            running `cargo check --no-default-features --features a,easytime/default,easytime/std` on deduplication (32/41)
+            running `cargo check --no-default-features --features b,easytime/default,easytime/std` on deduplication (33/41)
+            running `cargo check --no-default-features --features c,easytime/default,easytime/std` on deduplication (34/41)
+            running `cargo check --no-default-features --features d,easytime/default,easytime/std` on deduplication (35/41)
+            running `cargo check --no-default-features --features a,d,easytime/default,easytime/std` on deduplication (36/41)
+            running `cargo check --no-default-features --features b,d,easytime/default,easytime/std` on deduplication (37/41)
+            running `cargo check --no-default-features --features c,d,easytime/default,easytime/std` on deduplication (38/41)
+            running `cargo check --no-default-features --features e,easytime/default,easytime/std` on deduplication (39/41)
+            running `cargo check --no-default-features --features c,e,easytime/default,easytime/std` on deduplication (40/41)
+            running `cargo check --no-default-features --all-features` on deduplication (41/41)
+            ",
+        );
+}
+
 #[test]
 fn feature_powerset_depth() {
     cargo_hack(["check", "--feature-powerset", "--depth", "2"])