Auto merge of #14647 - elchukc:refactor_pkg_activated_features, r=wei…

…hanglo

improve error reporting when feature not found in `activated_features`

Pulls the error message refactor out of #14593 (originally #13207) to improve error reporting when we fail to get the list of activated features enabled for the given package. It now fully lists the activated_features hashmap keys too.

From the [original author](#13207 (comment)):

> I moved `activated_features_int` into `activated_features` and `activated_features_unverified` as I was concerned of the performance cost of generating the full error when its not a fatal error and may occur many times.

Old vs new error message:
```diff
- activated_features for invalid package: features did not find PackageId { name: "bindep", version: "0.0.0", source: "[ROOT]/foo/bindep" } NormalOrDev
+ did not find features for (PackageId { name: "bindep", version: "0.0.0", source: "[ROOT]/foo/bindep" }, NormalOrDev) within activated_features:
+ [
+     (
+         PackageId {
+             name: "bindep",
+             version: "0.0.0",
+             source: "[ROOT]/foo/bindep",
+         },
+         ArtifactDep(
+             CompileTarget {
+                 name: "[ALT_TARGET]",
+             },
+         ),
+     ),
+     (
+         PackageId {
+             name: "foo",
+             version: "0.0.0",
+             source: "[ROOT]/foo",
+         },
+         NormalOrDev,
+     ),
+ ]
```
r? weihanglo

src/cargo/core/resolver/features.rs

@@ -319,8 +319,30 @@ impl ResolvedFeatures {
         pkg_id: PackageId,
         features_for: FeaturesFor,
     ) -> Vec<InternedString> {
-        self.activated_features_int(pkg_id, features_for)
-            .expect("activated_features for invalid package")
+        if let Some(res) = self.activated_features_unverified(pkg_id, features_for) {
+            res
+        } else {
+            panic!(
+                "did not find features for ({pkg_id:?}, {features_for:?}) within activated_features:\n{:#?}",
+                self.activated_features.keys()
+            )
+        }
+    }
+
+    /// Variant of `activated_features` that returns `None` if this is
+    /// not a valid pkg_id/is_build combination. Used in places which do
+    /// not know which packages are activated (like `cargo clean`).
+    pub fn activated_features_unverified(
+        &self,
+        pkg_id: PackageId,
+        features_for: FeaturesFor,
+    ) -> Option<Vec<InternedString>> {
+        let fk = features_for.apply_opts(&self.opts);
+        if let Some(fs) = self.activated_features.get(&(pkg_id, fk)) {
+            Some(fs.iter().cloned().collect())
+        } else {
+            None
+        }
     }
 
     /// Returns if the given dependency should be included.
@@ -340,30 +362,6 @@ impl ResolvedFeatures {
             .unwrap_or(false)
     }
 
-    /// Variant of `activated_features` that returns `None` if this is
-    /// not a valid pkg_id/is_build combination. Used in places which do
-    /// not know which packages are activated (like `cargo clean`).
-    pub fn activated_features_unverified(
-        &self,
-        pkg_id: PackageId,
-        features_for: FeaturesFor,
-    ) -> Option<Vec<InternedString>> {
-        self.activated_features_int(pkg_id, features_for).ok()
-    }
-
-    fn activated_features_int(
-        &self,
-        pkg_id: PackageId,
-        features_for: FeaturesFor,
-    ) -> CargoResult<Vec<InternedString>> {
-        let fk = features_for.apply_opts(&self.opts);
-        if let Some(fs) = self.activated_features.get(&(pkg_id, fk)) {
-            Ok(fs.iter().cloned().collect())
-        } else {
-            bail!("features did not find {:?} {:?}", pkg_id, fk)
-        }
-    }
-
     /// Compares the result against the original resolver behavior.
     ///
     /// Used by `cargo fix --edition` to display any differences.

tests/testsuite/artifact_dep.rs

@@ -1573,7 +1573,7 @@ fn artifact_dep_target_specified() {
         .masquerade_as_nightly_cargo(&["bindeps"])
         .with_stdout_data("")
         .with_stderr_data(r#"...
-[..]activated_features for invalid package: features did not find PackageId { name: "bindep", version: "0.0.0", source: "[..]" } NormalOrDev[..]
+[..]did not find features for (PackageId { name: "bindep", version: "0.0.0", source: "[..]" }, NormalOrDev) within activated_features:[..]
 ...
 "#)
         .with_status(101)