Extract function to build feature map for readability

diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
index 34cdc72b5f72ebc119c504fbdc8c17289b2aef64..1c8d95b36e4d1a8d4b11123bd5e028c68c522248 100644 (file)
--- a/src/cargo/core/summary.rs
+++ b/src/cargo/core/summary.rs
@@ -48,60 +48,12 @@ impl Summary {
                 )
             }
         }
-        let mut features_new = BTreeMap::new();
-        for (feature, list) in features.iter() {
-            let mut values = vec![];
-            for dep in list {
-                use self::FeatureValue::*;
-                let val = FeatureValue::build(dep, |fs| (&features).get(fs).is_some());
-                if let &Feature(_) = &val {
-                    // Return early to avoid doing unnecessary work
-                    values.push(val);
-                    continue;
-                }
-                // Find data for the referenced dependency...
-                let dep_data = {
-                    let dep_name = match &val {
-                        &Feature(_) => "",
-                        &Crate(ref dep_name) | &CrateFeature(ref dep_name, _) => dep_name,
-                    };
-                    dependencies.iter().find(|d| *d.name() == *dep_name)
-                };
-                match (&val, dep_data) {
-                    (&Crate(ref dep), Some(d)) => {
-                        if !d.is_optional() {
-                            bail!(
-                                "Feature `{}` depends on `{}` which is not an \
-                                 optional dependency.\nConsider adding \
-                                 `optional = true` to the dependency",
-                                feature,
-                                dep
-                            )
-                        }
-                    }
-                    (&CrateFeature(ref dep_name, _), None) => bail!(
-                        "Feature `{}` requires a feature of `{}` which is not a \
-                         dependency",
-                        feature,
-                        dep_name
-                    ),
-                    (&Crate(ref dep), None) => bail!(
-                        "Feature `{}` includes `{}` which is neither \
-                         a dependency nor another feature",
-                        feature,
-                        dep
-                    ),
-                    (&CrateFeature(_, _), Some(_)) | (&Feature(_), _) => {}
-                }
-                values.push(val);
-            }
-            features_new.insert(feature.clone(), values);
-        }
+        let feature_map = build_feature_map(features, &dependencies)?;
         Ok(Summary {
             inner: Rc::new(Inner {
                 package_id: pkg_id,
                 dependencies,
-                features: features_new,
+                features: feature_map,
                 checksum: None,
                 links: links.map(|l| InternedString::new(&l)),
             }),
@@ -172,6 +124,65 @@ impl PartialEq for Summary {
     }
 }
 
+// Checks features for errors, bailing out a CargoResult:Err if invalid,
+// and creates FeatureValues for each feature.
+fn build_feature_map(
+    features: BTreeMap<String, Vec<String>>,
+    dependencies: &Vec<Dependency>,
+) -> CargoResult<FeatureMap> {
+    use self::FeatureValue::*;
+    let mut map = BTreeMap::new();
+    for (feature, list) in features.iter() {
+        let mut values = vec![];
+        for dep in list {
+            let val = FeatureValue::build(dep, |fs| (&features).get(fs).is_some());
+            if let &Feature(_) = &val {
+                values.push(val);
+                continue;
+            }
+
+            // Find data for the referenced dependency...
+            let dep_data = {
+                let dep_name = match &val {
+                    &Feature(_) => "",
+                    &Crate(ref dep_name) | &CrateFeature(ref dep_name, _) => dep_name,
+                };
+                dependencies.iter().find(|d| *d.name() == *dep_name)
+            };
+
+            match (&val, dep_data) {
+                (&Crate(ref dep), Some(d)) => {
+                    if !d.is_optional() {
+                        bail!(
+                            "Feature `{}` depends on `{}` which is not an \
+                             optional dependency.\nConsider adding \
+                             `optional = true` to the dependency",
+                            feature,
+                            dep
+                        )
+                    }
+                }
+                (&CrateFeature(ref dep_name, _), None) => bail!(
+                    "Feature `{}` requires a feature of `{}` which is not a \
+                     dependency",
+                    feature,
+                    dep_name
+                ),
+                (&Crate(ref dep), None) => bail!(
+                    "Feature `{}` includes `{}` which is neither \
+                     a dependency nor another feature",
+                    feature,
+                    dep
+                ),
+                (&CrateFeature(_, _), Some(_)) | (&Feature(_), _) => {}
+            }
+            values.push(val);
+        }
+        map.insert(feature.clone(), values);
+    }
+    Ok(map)
+}
+
 /// FeatureValue represents the types of dependencies a feature can have:
 ///
 /// * Another feature