Merge #146

146: Adjust optional dependencies handling to fix breakage on nightly-2022-01-20+ r=taiki-e a=taiki-e

Fixes #145

Co-authored-by: Taiki Endo <te316e89@gmail.com>

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 breakage on nightly-2022-01-20 or later. ([#146](https://github.com/taiki-e/cargo-hack/pull/146))
+
 ## [0.5.10] - 2022-01-05
 
 - Fix handling of combined short flags. ([#143](https://github.com/taiki-e/cargo-hack/pull/143))

src/cargo.rs

@@ -8,11 +8,8 @@ pub(crate) fn minor_version(mut cmd: ProcessBuilder<'_>) -> Result<u32> {
     let output = cmd.read()?;
 
     // Find the release line in the verbose version output.
-    let release = output
-        .lines()
-        .find(|line| line.starts_with("release: "))
-        .map(|line| &line["release: ".len()..])
-        .ok_or_else(|| {
+    let release =
+        output.lines().find_map(|line| line.strip_prefix("release: ")).ok_or_else(|| {
             format_err!("could not find rustc release from output of {}: {}", cmd, output)
         })?;
 

src/cli.rs

@@ -118,15 +118,15 @@ impl Args {
         }
         let rest = raw_args.collect::<Result<Vec<_>>>()?;
 
-        let mut cargo_args = Vec::new();
+        let mut cargo_args = vec![];
         let mut subcommand: Option<String> = None;
 
         let mut manifest_path = None;
         let mut color = None;
 
-        let mut package = Vec::new();
-        let mut exclude = Vec::new();
-        let mut features = Vec::new();
+        let mut package = vec![];
+        let mut exclude = vec![];
+        let mut features = vec![];
 
         let mut workspace = false;
         let mut no_dev_deps = false;
@@ -142,14 +142,14 @@ impl Args {
         let mut version_step = None;
 
         let mut optional_deps = None;
-        let mut include_features = Vec::new();
+        let mut include_features = vec![];
         let mut include_deps_features = false;
 
-        let mut exclude_features = Vec::new();
+        let mut exclude_features = vec![];
         let mut exclude_no_default_features = false;
         let mut exclude_all_features = false;
 
-        let mut group_features: Vec<String> = Vec::new();
+        let mut group_features: Vec<String> = vec![];
         let mut depth = None;
 
         let mut verbose = 0;
@@ -211,12 +211,13 @@ impl Args {
             }
 
             match arg {
+                Long("color") => parse_opt!(color, true),
+                Long("target") => parse_opt!(target, true),
+
                 Long("manifest-path") => parse_opt!(manifest_path, false),
                 Long("depth") => parse_opt!(depth, false),
-                Long("color") => parse_opt!(color, true),
                 Long("version-range") => parse_opt!(version_range, false),
                 Long("version-step") => parse_opt!(version_step, false),
-                Long("target") => parse_opt!(target, true),
 
                 Short('p') | Long("package") => package.push(parser.value()?.parse()?),
                 Long("exclude") => exclude.push(parser.value()?.parse()?),
@@ -272,10 +273,10 @@ impl Args {
 
                 // detect similar arg
                 Long("each-features") => {
-                    similar_arg(&arg, subcommand.as_deref(), "--each-feature", None)?
+                    similar_arg(&arg, subcommand.as_deref(), "--each-feature", None)?;
                 }
                 Long("features-powerset") => {
-                    similar_arg(&arg, subcommand.as_deref(), "--feature-powerset", None)?
+                    similar_arg(&arg, subcommand.as_deref(), "--feature-powerset", None)?;
                 }
 
                 // propagated
@@ -532,10 +533,10 @@ impl Args {
         term::verbose::set(verbose != 0);
         // If `-vv` is passed, propagate `-v` to cargo.
         if verbose > 1 {
-            cargo_args.push(format!("-{}", "v".repeat(verbose - 1)))
+            cargo_args.push(format!("-{}", "v".repeat(verbose - 1)));
         }
 
-        Ok(Args {
+        Ok(Self {
             leading_args: cargo_args,
             trailing_args: rest,
 
@@ -581,7 +582,7 @@ fn has_z_flag(args: &[String], name: &str) -> bool {
         if arg == "-Z" {
             arg = iter.next().unwrap();
         } else if let Some(a) = arg.strip_prefix("-Z") {
-            arg = a
+            arg = a;
         } else {
             continue;
         }

src/features.rs

@@ -5,10 +5,11 @@ use std::{
 
 use crate::{metadata::Metadata, PackageId};
 
+#[derive(Debug)]
 pub(crate) struct Features {
     features: Vec<Feature>,
-    /// [package features len, package features + optional deps len]
-    len: [usize; 2],
+    optional_deps_start: usize,
+    deps_features_start: usize,
 }
 
 impl Features {
@@ -17,14 +18,19 @@ impl Features {
         let node = &metadata.resolve.nodes[id];
 
         let mut features = Vec::with_capacity(package.features.len());
+        let mut optional_deps = vec![];
 
-        for name in package.features.keys() {
-            features.push(name.into());
-        }
         for name in package.optional_deps() {
-            features.push(name.into());
+            optional_deps.push(name);
+        }
+        for name in package.features.keys() {
+            if !optional_deps.contains(&&**name) {
+                features.push(name.into());
+            }
         }
-        let len = [package.features.len(), features.len()];
+        let optional_deps_start = features.len();
+        features.extend(optional_deps.into_iter().map(Into::into));
+        let deps_features_start = features.len();
 
         // TODO: Unpublished dependencies are not included in `node.deps`.
         for dep in node.deps.iter().filter(|dep| {
@@ -42,19 +48,19 @@ impl Features {
             // TODO: Optional deps of `dep_package`.
         }
 
-        Self { features, len }
+        Self { features, optional_deps_start, deps_features_start }
     }
 
     pub(crate) fn normal(&self) -> &[Feature] {
-        &self.features[..self.len[0]]
+        &self.features[..self.optional_deps_start]
     }
 
     pub(crate) fn optional_deps(&self) -> &[Feature] {
-        &self.features[self.len[0]..self.len[1]]
+        &self.features[self.optional_deps_start..self.deps_features_start]
     }
 
     pub(crate) fn deps_features(&self) -> &[Feature] {
-        &self.features[self.len[1]..]
+        &self.features[self.deps_features_start..]
     }
 
     pub(crate) fn contains(&self, name: &str) -> bool {
@@ -66,26 +72,23 @@ impl Features {
 #[derive(Debug)]
 pub(crate) enum Feature {
     /// A feature of the current crate.
-    #[allow(dead_code)] // false positive that fixed in Rust 1.44
     Normal {
         /// Feature name. It is considered indivisible.
         name: String,
     },
     /// Grouped features.
-    #[allow(dead_code)] // false positive that fixed in Rust 1.44
     Group {
         /// Feature name concatenated with `,`.
         name: String,
         /// Original feature list.
         list: Vec<String>,
     },
     /// A feature of a dependency.
-    #[allow(dead_code)] // false positive that fixed in Rust 1.44
     Path {
         /// Feature path separated with `/`.
         name: String,
         /// Index of `/`.
-        slash: usize,
+        _slash: usize,
     },
 }
 
@@ -96,7 +99,7 @@ impl Feature {
     }
 
     pub(crate) fn path(parent: &str, name: &str) -> Self {
-        Self::Path { name: format!("{}/{}", parent, name), slash: parent.len() }
+        Self::Path { name: format!("{}/{}", parent, name), _slash: parent.len() }
     }
 
     pub(crate) fn name(&self) -> &str {

src/main.rs

@@ -1,11 +1,6 @@
 #![forbid(unsafe_code)]
 #![warn(rust_2018_idioms, single_use_lifetimes, unreachable_pub)]
-#![warn(
-    clippy::default_trait_access,
-    clippy::disallowed_methods,
-    clippy::disallowed_types,
-    clippy::wildcard_imports
-)]
+#![warn(clippy::default_trait_access, clippy::wildcard_imports)]
 
 #[macro_use]
 mod term;
@@ -150,27 +145,26 @@ fn determine_kind<'a>(cx: &'a Context, id: &PackageId, progress: &mut Progress)
     }
 
     let package = cx.packages(id);
+    let pkg_features = cx.pkg_features(id);
     let filter = |&f: &&Feature| {
         !cx.exclude_features.iter().any(|s| f == s)
             && !cx.group_features.iter().any(|g| g.matches(f.name()))
     };
     let features = if cx.include_features.is_empty() {
-        let feature_list = cx.pkg_features(id);
-
         cx.exclude_features.iter().for_each(|d| {
-            if !feature_list.contains(d) {
+            if !pkg_features.contains(d) {
                 warn!("specified feature `{}` not found in package `{}`", d, package.name);
             }
         });
 
-        let mut features: Vec<_> = feature_list.normal().iter().filter(filter).collect();
+        let mut features: Vec<_> = pkg_features.normal().iter().filter(filter).collect();
 
         if let Some(opt_deps) = &cx.optional_deps {
             if opt_deps.len() == 1 && opt_deps[0].is_empty() {
                 // --optional-deps=
             } else {
                 for d in opt_deps {
-                    if !feature_list.optional_deps().iter().any(|f| f == d) {
+                    if !pkg_features.optional_deps().iter().any(|f| f == d) {
                         warn!(
                             "specified optional dependency `{}` not found in package `{}`",
                             d, package.name
@@ -179,13 +173,13 @@ fn determine_kind<'a>(cx: &'a Context, id: &PackageId, progress: &mut Progress)
                 }
             }
 
-            features.extend(feature_list.optional_deps().iter().filter(|f| {
+            features.extend(pkg_features.optional_deps().iter().filter(|f| {
                 filter(f) && (opt_deps.is_empty() || opt_deps.iter().any(|x| *f == x))
             }));
         }
 
         if cx.include_deps_features {
-            features.extend(feature_list.deps_features().iter().filter(filter));
+            features.extend(pkg_features.deps_features().iter().filter(filter));
         }
 
         if !cx.group_features.is_empty() {
@@ -198,29 +192,35 @@ fn determine_kind<'a>(cx: &'a Context, id: &PackageId, progress: &mut Progress)
     };
 
     if cx.each_feature {
-        if (package.features.is_empty() || !cx.include_features.is_empty()) && features.is_empty() {
+        if (pkg_features.normal().is_empty() && pkg_features.optional_deps().is_empty()
+            || !cx.include_features.is_empty())
+            && features.is_empty()
+        {
             progress.total += 1;
             Kind::Normal
         } else {
             progress.total += features.len()
                 + !cx.exclude_no_default_features as usize
                 + (!cx.exclude_all_features
-                    && package.features.len() + package.optional_deps().count() > 1)
+                    && pkg_features.normal().len() + pkg_features.optional_deps().len() > 1)
                     as usize;
             Kind::Each { features }
         }
     } else if cx.feature_powerset {
         let features = features::feature_powerset(features, cx.depth, &package.features);
 
-        if (package.features.is_empty() || !cx.include_features.is_empty()) && features.is_empty() {
+        if (pkg_features.normal().is_empty() && pkg_features.optional_deps().is_empty()
+            || !cx.include_features.is_empty())
+            && features.is_empty()
+        {
             progress.total += 1;
             Kind::Normal
         } else {
             // -1: the first element of a powerset is `[]`
             progress.total += features.len() - 1
                 + !cx.exclude_no_default_features as usize
                 + (!cx.exclude_all_features
-                    && package.features.len() + package.optional_deps().count() > 1)
+                    && pkg_features.normal().len() + pkg_features.optional_deps().len() > 1)
                     as usize;
             Kind::Powerset { features }
         }
@@ -355,8 +355,10 @@ fn exec_actual(
         _ => unreachable!(),
     }
 
-    let pkg = cx.packages(id);
-    if !cx.exclude_all_features && pkg.features.len() + pkg.optional_deps().count() > 1 {
+    let pkg_features = cx.pkg_features(id);
+    if !cx.exclude_all_features
+        && pkg_features.normal().len() + pkg_features.optional_deps().len() > 1
+    {
         // run with all features
         // https://github.com/taiki-e/cargo-hack/issues/42
         line.arg("--all-features");

src/term.rs

@@ -17,9 +17,9 @@ enum Coloring {
 }
 
 impl Coloring {
-    const AUTO: u8 = Coloring::Auto as _;
-    const ALWAYS: u8 = Coloring::Always as _;
-    const NEVER: u8 = Coloring::Never as _;
+    const AUTO: u8 = Self::Auto as _;
+    const ALWAYS: u8 = Self::Always as _;
+    const NEVER: u8 = Self::Never as _;
 }
 
 impl FromStr for Coloring {