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

Issue:
Ability to group multiple features for the powerset
With an increasing number of features, the powerset inherently grows exponentially. As such, even the ability to group two features (dependencies in my case) together would provide a significant improvement in CI time. The use-case behind this is for features that are wholly unrelated and do not interact in any way. In my case, I have `serde` and `rand` implementations, but enabling one would never cause the other to fail. Because of this, having an option like `--group-features serde,rand` would be great. The flag could be passed multiple times to create multiple groups.

Pull request description:
Add --group-features option
Closes #80

```text
--group-features <FEATURES>...
    Space-separated list of features to group.

    To specify multiple groups, use this option multiple times: `--group-features a,b --group-features c,d`

    This flag can only be used together with --feature-powerset flag.
```

```console
$ cargo hack check --feature-powerset --group-features a,b
info: running `cargo check --no-default-features` on crate1 (1/5)
...
info: running `cargo check --no-default-features --features c` on crate1 (2/5)
...
info: running `cargo check --no-default-features --features a,b` on crate1 (3/5)
...
info: running `cargo check --no-default-features --features c,a,b` on crate1 (4/5)
...
info: running `cargo check --no-default-features --all-features` on crate1 (5/5)
...
```

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7791189..a959413 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ This project adheres to [Semantic Versioning](https://semver.org).
 
 ## [Unreleased]
 
+* [Add `--group-features` option.][82]
+
+[82]: https://github.com/taiki-e/cargo-hack/pull/82
+
 ## [0.4.0] - 2020-10-21
 
 * [Remove `--ignore-non-exist-features` flag.][62] Use `--ignore-unknown-features` flag instead.
diff --git a/README.md b/README.md
index 4e84e36..7992ec5 100644
--- a/README.md
+++ b/README.md
@@ -108,6 +108,12 @@ The following flags can be used with `--each-feature` and `--feature-powerset`.
 
   If the number is set to 1, `--feature-powerset` is equivalent to `--each-feature`.
 
+* **`--group-features`**
+
+  Space-separated list of features to group.
+
+  To specify multiple groups, use this option multiple times: `--group-features a,b --group-features c,d`
+
 `cargo-hack` changes the behavior of the following existing flags.
 
 * **`--features`**, **`--no-default-features`**
diff --git a/src/cli.rs b/src/cli.rs
index 4700968..8b96154 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -49,6 +49,10 @@ const HELP: &[(&str, &str, &str, &[&str])] = &[
             "This flag can only be used together with --feature-powerset flag.",
         ],
     ),
+    ("", "--group-features <FEATURES>...", "Space-separated list of features to group", &[
+        "To specify multiple groups, use this option multiple times: `--group-features a,b --group-features c,d`",
+        "This flag can only be used together with --feature-powerset flag.",
+    ]),
     (
         "",
         "--include-features <FEATURES>...",
@@ -202,22 +206,17 @@ pub(crate) struct Args<'a> {
     pub(crate) ignore_private: bool,
     /// --ignore-unknown-features
     pub(crate) ignore_unknown_features: bool,
-    /// --optional-deps [DEPS]...
-    pub(crate) optional_deps: Option<Vec<&'a str>>,
     /// --clean-per-run
     pub(crate) clean_per_run: bool,
-    /// --depth <NUM>
-    pub(crate) depth: Option<usize>,
+
+    // options for --each-feature and --feature-powerset
+    /// --optional-deps [DEPS]...
+    pub(crate) optional_deps: Option<Vec<&'a str>>,
     /// --include-features
     pub(crate) include_features: Vec<&'a str>,
     /// --include-deps-features
     pub(crate) include_deps_features: bool,
 
-    /// --no-default-features
-    pub(crate) no_default_features: bool,
-    /// -v, --verbose, -vv
-    pub(crate) verbose: bool,
-
     // Note: These values are not always exactly the same as the input.
     // Error messages should not assume that these options have been specified.
     /// --exclude-features <FEATURES>..., --skip <FEATURES>...
@@ -227,7 +226,18 @@ pub(crate) struct Args<'a> {
     /// --exclude-all-features
     pub(crate) exclude_all_features: bool,
 
-    // flags that will be propagated to cargo
+    // options for --feature-powerset
+    /// --depth <NUM>
+    pub(crate) depth: Option<usize>,
+    /// --group-features <FEATURES>...
+    pub(crate) group_features: Vec<Vec<&'a str>>,
+
+    /// --no-default-features
+    pub(crate) no_default_features: bool,
+    /// -v, --verbose, -vv
+    pub(crate) verbose: bool,
+
+    // options that will be propagated to cargo
     /// --features <FEATURES>...
     pub(crate) features: Vec<&'a str>,
 }
@@ -267,8 +277,6 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
     let mut package = Vec::new();
     let mut exclude = Vec::new();
     let mut features = Vec::new();
-    let mut optional_deps = None;
-    let mut include_features = Vec::new();
 
     let mut workspace = None;
     let mut no_dev_deps = false;
@@ -278,7 +286,9 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
     let mut ignore_private = false;
     let mut ignore_unknown_features = false;
     let mut clean_per_run = false;
-    let mut depth = None;
+
+    let mut optional_deps = None;
+    let mut include_features = Vec::new();
     let mut include_deps_features = false;
 
     let mut exclude_features = Vec::new();
@@ -286,6 +296,9 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
     let mut exclude_all_features = false;
     let mut skip_no_default_features = false;
 
+    let mut group_features = Vec::new();
+    let mut depth = None;
+
     let mut verbose = false;
     let mut no_default_features = false;
     let mut all_features = false;
@@ -310,7 +323,7 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
             }
 
             macro_rules! parse_opt {
-                ($opt:ident, $propagate:expr, $pat:expr, $help:expr) => {
+                ($opt:ident, $propagate:expr, $pat:expr, $help:expr $(,)?) => {
                     if arg == $pat {
                         if $opt.is_some() {
                             return Err(multi_arg($help, subcommand));
@@ -338,7 +351,7 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
             }
 
             macro_rules! parse_multi_opt {
-                ($v:ident, $allow_split:expr, $require_value:expr, $pat:expr, $help:expr) => {
+                ($v:ident, $allow_split:expr, $require_value:expr, $pat:expr, $help:expr $(,)?) => {
                     if arg == $pat {
                         if !$require_value && args.peek().map_or(true, |s| s.starts_with('-')) {
                             continue;
@@ -400,14 +413,21 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
                 true,
                 true,
                 "--exclude-features",
-                "--exclude-features <FEATURES>..."
+                "--exclude-features <FEATURES>...",
             );
             parse_multi_opt!(
                 include_features,
                 true,
                 true,
                 "--include-features",
-                "--include-features <FEATURES>..."
+                "--include-features <FEATURES>...",
+            );
+            parse_multi_opt!(
+                group_features,
+                false,
+                true,
+                "--group-features",
+                "--group-features <FEATURES>...",
             );
 
             if arg.starts_with("--optional-deps") {
@@ -499,10 +519,24 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
         // in the root of a virtual workspace as well?
         bail!("--exclude can only be used together with --workspace");
     }
-    if ignore_unknown_features && features.is_empty() && include_features.is_empty() {
-        bail!(
-            "--ignore-unknown-features can only be used together with either --features or --include-features"
-        );
+    if ignore_unknown_features {
+        if features.is_empty() && include_features.is_empty() && group_features.is_empty() {
+            bail!(
+                "--ignore-unknown-features can only be used together with --features, --include-features, or --group-features"
+            );
+        }
+        if !include_features.is_empty() {
+            // TODO
+            warn!(
+                "--ignore-unknown-features for --include-features is not fully implemented and may not work as intended"
+            )
+        }
+        if !group_features.is_empty() {
+            // TODO
+            warn!(
+                "--ignore-unknown-features for --group-features is not fully implemented and may not work as intended"
+            )
+        }
     }
     if !each_feature && !feature_powerset {
         if optional_deps.is_some() {
@@ -531,10 +565,25 @@ pub(crate) fn parse_args<'a>(raw: &'a RawArgs, cargo: &OsStr, version: u32) -> R
             );
         }
     }
-    if depth.is_some() && !feature_powerset {
-        bail!("--depth can only be used together with --feature-powerset");
+    if !feature_powerset {
+        if depth.is_some() {
+            bail!("--depth can only be used together with --feature-powerset");
+        } else if !group_features.is_empty() {
+            bail!("--group-features can only be used together with --feature-powerset");
+        }
     }
     let depth = depth.map(str::parse::<usize>).transpose()?;
+    let group_features =
+        group_features.iter().try_fold(Vec::with_capacity(group_features.len()), |mut v, g| {
+            if g.contains(',') {
+                v.push(g.split(',').collect::<Vec<_>>());
+            } else if g.contains(' ') {
+                v.push(g.split(' ').collect());
+            } else {
+                bail!("--group-features requires a list of two or more features separated by space or comma");
+            }
+            Ok(v)
+        })?;
 
     if let Some(subcommand) = subcommand {
         if subcommand == "test" || subcommand == "bench" {
@@ -637,10 +686,12 @@ For more information try --help
         ignore_unknown_features,
         optional_deps,
         clean_per_run,
-        depth,
         include_features,
         include_deps_features,
 
+        depth,
+        group_features,
+
         no_default_features,
         verbose,
 
diff --git a/src/context.rs b/src/context.rs
index 90c9203..2570f3e 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -87,6 +87,28 @@ impl<'a> Context<'a> {
         }
         command
     }
+
+    pub(crate) fn deps_features(&self, id: &PackageId) -> Vec<String> {
+        let node = self.nodes(id);
+        let package = self.packages(id);
+        let mut features = Vec::new();
+        // TODO: Unpublished dependencies are not included in `node.deps`.
+        for dep in node.deps.iter().filter(|dep| {
+            // ignore if `dep_kinds` is empty (i.e., not Rust 1.41+), target specific or not a normal dependency.
+            dep.dep_kinds.iter().any(|kind| kind.kind.is_none() && kind.target.is_none())
+        }) {
+            let dep_package = self.packages(&dep.pkg);
+            // TODO: `dep.name` (`resolve.nodes[].deps[].name`) is a valid rust identifier, not a valid feature flag.
+            // And `packages[].dependencies` doesn't have package identifier,
+            // so I'm not sure if there is a way to find the actual feature name exactly.
+            if let Some(d) = package.dependencies.iter().find(|d| d.name == dep_package.name) {
+                let name = d.rename.as_ref().unwrap_or(&d.name);
+                features.extend(dep_package.features().map(|f| format!("{}/{}", name, f)));
+            }
+            // TODO: Optional deps of `dep_package`.
+        }
+        features
+    }
 }
 
 impl<'a> Deref for Context<'a> {
diff --git a/src/main.rs b/src/main.rs
index af3d607..dae33a8 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -23,12 +23,7 @@ mod version;
 use anyhow::{bail, Context as _};
 use std::{borrow::Cow, fmt::Write, fs};
 
-use crate::{
-    context::Context,
-    metadata::{Dependency, PackageId},
-    process::ProcessBuilder,
-    restore::Restore,
-};
+use crate::{context::Context, metadata::PackageId, process::ProcessBuilder, restore::Restore};
 
 type Result<T, E = anyhow::Error> = std::result::Result<T, E>;
 
@@ -92,18 +87,15 @@ fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progre
     }
 
     let package = cx.packages(id);
+    let filter = |f: &&_| {
+        !cx.exclude_features.contains(f) && !cx.group_features.iter().any(|g| g.contains(f))
+    };
     let features = if cx.include_features.is_empty() {
-        let mut features: Vec<_> = package
-            .features
-            .iter()
-            .map(String::as_str)
-            .filter(|f| !cx.exclude_features.contains(f))
-            .map(Cow::Borrowed)
-            .collect();
+        let mut features: Vec<_> = package.features().filter(filter).map(Cow::Borrowed).collect();
 
         if let Some(opt_deps) = &cx.optional_deps {
             opt_deps.iter().for_each(|&d| {
-                if !package.dependencies.iter().filter_map(Dependency::as_feature).any(|f| f == d) {
+                if !package.optional_deps().any(|f| f == d) {
                     warn!(
                         "specified optional dependency `{}` not found in package `{}`",
                         d, package.name
@@ -113,51 +105,25 @@ fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progre
 
             features.extend(
                 package
-                    .dependencies
-                    .iter()
-                    .filter_map(Dependency::as_feature)
-                    .filter(|f| {
-                        !cx.exclude_features.contains(f)
-                            && (opt_deps.is_empty() || opt_deps.contains(f))
-                    })
+                    .optional_deps()
+                    .filter(|f| filter(f) && (opt_deps.is_empty() || opt_deps.contains(f)))
                     .map(Cow::Borrowed),
             );
         }
 
+        let deps_features =
+            if cx.include_deps_features { cx.deps_features(id) } else { Vec::new() };
         if cx.include_deps_features {
-            let node = cx.nodes(id);
-            let package = cx.packages(id);
-            // TODO: Unpublished dependencies are not included in `node.deps`.
-            for dep in node.deps.iter().filter(|dep| {
-                // ignore if `dep_kinds` is empty (i.e., not Rust 1.41+), target specific or not a normal dependency.
-                dep.dep_kinds.iter().any(|kind| kind.kind.is_none() && kind.target.is_none())
-            }) {
-                let dep_package = cx.packages(&dep.pkg);
-                // TODO: `dep.name` (`resolve.nodes[].deps[].name`) is a valid rust identifier, not a valid feature flag.
-                // And `packages[].dependencies` doesn't have package identifier,
-                // so I'm not sure if there is a way to find the actual feature name exactly.
-                if let Some(d) = package.dependencies.iter().find(|d| d.name == dep_package.name) {
-                    let name = d.rename.as_ref().unwrap_or(&d.name);
-                    features.extend(
-                        dep_package
-                            .features
-                            .iter()
-                            .filter(|&f| !cx.exclude_features.contains(&&**f))
-                            .map(|f| Cow::Owned(format!("{}/{}", name, f))),
-                    );
-                }
-                // TODO: Optional deps of `dep_package`.
-            }
+            features.extend(deps_features.into_iter().map(Cow::Owned).filter(|f| filter(&&**f)));
+        }
+
+        if !cx.group_features.is_empty() {
+            features.extend(cx.group_features.iter().map(|g| Cow::Owned(g.join(","))));
         }
 
         features
     } else {
-        cx.include_features
-            .iter()
-            .filter(|f| !cx.exclude_features.contains(f))
-            .copied()
-            .map(Cow::Borrowed)
-            .collect()
+        cx.include_features.iter().copied().filter(filter).map(Cow::Borrowed).collect()
     };
 
     if cx.each_feature {
diff --git a/src/metadata.rs b/src/metadata.rs
index 5603a59..ca25bd7 100644
--- a/src/metadata.rs
+++ b/src/metadata.rs
@@ -248,6 +248,14 @@ impl Package {
             Cow::Borrowed(&self.name)
         }
     }
+
+    pub(crate) fn features(&self) -> impl Iterator<Item = &str> {
+        self.features.iter().map(String::as_str)
+    }
+
+    pub(crate) fn optional_deps(&self) -> impl Iterator<Item = &str> {
+        self.dependencies.iter().filter_map(Dependency::as_feature)
+    }
 }
 
 /// A dependency of the main crate


Test patch:
diff --git a/tests/long-help.txt b/tests/long-help.txt
index 661c0be..29289bf 100644
--- a/tests/long-help.txt
+++ b/tests/long-help.txt
@@ -75,6 +75,13 @@ OPTIONS:
 
             This flag can only be used together with --feature-powerset flag.
 
+        --group-features <FEATURES>...
+            Space-separated list of features to group.
+
+            To specify multiple groups, use this option multiple times: `--group-features a,b --group-features c,d`
+
+            This flag can only be used together with --feature-powerset flag.
+
         --include-features <FEATURES>...
             Include only the specified features in the feature combinations instead of package features.
 
diff --git a/tests/short-help.txt b/tests/short-help.txt
index b5b57e1..7213547 100644
--- a/tests/short-help.txt
+++ b/tests/short-help.txt
@@ -21,6 +21,7 @@ OPTIONS:
         --exclude-no-default-features    Exclude run of just --no-default-features flag
         --exclude-all-features           Exclude run of just --all-features flag
         --depth <NUM>                    Specify a max number of simultaneous feature flags of --feature-powerset
+        --group-features <FEATURES>...   Space-separated list of features to group
         --include-features <FEATURES>... Include only the specified features in the feature combinations instead of package features
         --no-dev-deps                    Perform without dev-dependencies
         --remove-dev-deps                Equivalent to --no-dev-deps flag except for does not restore the original `Cargo.toml` after performed
diff --git a/tests/test.rs b/tests/test.rs
index 617442b..679dc3f 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -336,8 +336,43 @@ fn ignore_unknown_features_failure() {
         .test_dir("tests/fixtures/virtual")
         .assert_failure()
         .assert_stderr_contains(
-            "--ignore-unknown-features can only be used together with either --features or --include-features",
+            "
+            --ignore-unknown-features can only be used together with --features, --include-features, \
+            or --group-features
+            ",
         );
+
+    cargo_hack([
+        "check",
+        "--ignore-unknown-features",
+        "--feature-powerset",
+        "--include-features",
+        "a",
+    ])
+    .test_dir("tests/fixtures/real")
+    .assert_success()
+    .assert_stderr_contains(
+        "
+        --ignore-unknown-features for --include-features is not fully implemented and may not \
+        work as intended
+        ",
+    );
+
+    cargo_hack([
+        "check",
+        "--ignore-unknown-features",
+        "--feature-powerset",
+        "--group-features",
+        "a,b",
+    ])
+    .test_dir("tests/fixtures/real")
+    .assert_success()
+    .assert_stderr_contains(
+        "
+        --ignore-unknown-features for --group-features is not fully implemented and may not \
+        work as intended
+        ",
+    );
 }
 
 #[test]
@@ -451,6 +486,102 @@ fn depth_failure() {
         .assert_stderr_contains("--depth can only be used together with --feature-powerset");
 }
 
+#[test]
+fn powerset_group_features() {
+    cargo_hack(["check", "--feature-powerset", "--group-features", "a,b"])
+        .test_dir("tests/fixtures/real")
+        .assert_success()
+        .assert_stderr_contains(
+            "
+            running `cargo check --no-default-features` on real (1/9)
+            running `cargo check --no-default-features --features c` on real (2/9)
+            running `cargo check --no-default-features --features default` on real (3/9)
+            running `cargo check --no-default-features --features c,default` on real (4/9)
+            running `cargo check --no-default-features --features a,b` on real (5/9)
+            running `cargo check --no-default-features --features c,a,b` on real (6/9)
+            running `cargo check --no-default-features --features default,a,b` on real (7/9)
+            running `cargo check --no-default-features --features c,default,a,b` on real (8/9)
+            running `cargo check --no-default-features --all-features` on real (9/9)
+            ",
+        )
+        .assert_stderr_not_contains(
+            "
+            --features a`
+            --features b`
+            ",
+        );
+
+    cargo_hack(["check", "--feature-powerset", "--group-features", "a,b,c"])
+        .test_dir("tests/fixtures/real")
+        .assert_success()
+        .assert_stderr_contains(
+            "
+            running `cargo check --no-default-features` on real (1/5)
+            running `cargo check --no-default-features --features default` on real (2/5)
+            running `cargo check --no-default-features --features a,b,c` on real (3/5)
+            running `cargo check --no-default-features --features default,a,b,c` on real (4/5)
+            running `cargo check --no-default-features --all-features` on real (5/5)
+            ",
+        )
+        .assert_stderr_not_contains(
+            "
+            --features a`
+            --features b`
+            --features c`
+            ",
+        );
+
+    // overlapping
+    // TODO: Maybe we should warn this, but allow it for now.
+    cargo_hack([
+        "check",
+        "--feature-powerset",
+        "--group-features",
+        "a,b",
+        "--group-features",
+        "a,c",
+    ])
+    .test_dir("tests/fixtures/real")
+    .assert_success()
+    .assert_stderr_contains(
+        "
+        running `cargo check --no-default-features` on real (1/9)
+        running `cargo check --no-default-features --features default` on real (2/9)
+        running `cargo check --no-default-features --features a,b` on real (3/9)
+        running `cargo check --no-default-features --features default,a,b` on real (4/9)
+        running `cargo check --no-default-features --features a,c` on real (5/9)
+        running `cargo check --no-default-features --features default,a,c` on real (6/9)
+        running `cargo check --no-default-features --features a,b,a,c` on real (7/9)
+        running `cargo check --no-default-features --features default,a,b,a,c` on real (8/9)
+        running `cargo check --no-default-features --all-features` on real (9/9)
+        ",
+    )
+    .assert_stderr_not_contains(
+        "
+        --features a`
+        --features b`
+        --features c`
+        ",
+    );
+}
+
+#[test]
+fn group_features_failure() {
+    cargo_hack(["check", "--each-feature", "--group-features", "a,b"])
+        .test_dir("tests/fixtures/real")
+        .assert_failure()
+        .assert_stderr_contains(
+            "--group-features can only be used together with --feature-powerset",
+        );
+
+    cargo_hack(["check", "--feature-powerset", "--group-features", "a"])
+        .test_dir("tests/fixtures/real")
+        .assert_failure()
+        .assert_stderr_contains(
+            "--group-features requires a list of two or more features separated by space or comma",
+        );
+}
+
 #[test]
 fn include_features() {
     cargo_hack(["check", "--each-feature", "--include-features", "a,b"])