Auto merge of #11098 - Emilgardis:early-fail, r=epage

make unknown features on `cargo add` more discoverable

When adding an unknown feature via `cargo add -F krate/feat`, it can be easy to miss the fact that the change failed.

This fixes that by showing the following output on fail

<img width="474" alt="image" src="https://user-images.githubusercontent.com/1502855/191100141-3603cc9a-d4b6-4d6a-bbc6-41b34144b3f0.png">

Author: bors

src/cargo/ops/cargo_add/mod.rs

@@ -5,11 +5,13 @@ mod crate_spec;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::VecDeque;
+use std::fmt::Write;
 use std::path::Path;
 
 use anyhow::Context as _;
 use cargo_util::paths;
 use indexmap::IndexSet;
+use itertools::Itertools;
 use termcolor::Color::Green;
 use termcolor::Color::Red;
 use termcolor::ColorSpec;
@@ -99,7 +101,7 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
             table_option.map_or(true, |table| is_sorted(table.iter().map(|(name, _)| name)))
         });
     for dep in deps {
-        print_msg(&mut options.config.shell(), &dep, &dep_table)?;
+        print_action_msg(&mut options.config.shell(), &dep, &dep_table)?;
         if let Some(Source::Path(src)) = dep.source() {
             if src.path == manifest.path.parent().unwrap_or_else(|| Path::new("")) {
                 anyhow::bail!(
@@ -124,11 +126,63 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
                 inherited_features.iter().map(|s| s.as_str()).collect();
             unknown_features.extend(inherited_features.difference(&available_features).copied());
         }
+
         unknown_features.sort();
+
         if !unknown_features.is_empty() {
-            anyhow::bail!("unrecognized features: {unknown_features:?}");
+            let (mut activated, mut deactivated) = dep.features();
+            // Since the unknown features have been added to the DependencyUI we need to remove
+            // them to present the "correct" features that can be specified for the crate.
+            deactivated.retain(|f| !unknown_features.contains(f));
+            activated.retain(|f| !unknown_features.contains(f));
+
+            let mut message = format!(
+                "unrecognized feature{} for crate {}: {}\n",
+                if unknown_features.len() == 1 { "" } else { "s" },
+                dep.name,
+                unknown_features.iter().format(", "),
+            );
+            if activated.is_empty() && deactivated.is_empty() {
+                write!(message, "no features available for crate {}", dep.name)?;
+            } else {
+                if !deactivated.is_empty() {
+                    writeln!(
+                        message,
+                        "disabled features:\n    {}",
+                        deactivated
+                            .iter()
+                            .map(|s| s.to_string())
+                            .coalesce(|x, y| if x.len() + y.len() < 78 {
+                                Ok(format!("{x}, {y}"))
+                            } else {
+                                Err((x, y))
+                            })
+                            .into_iter()
+                            .format("\n    ")
+                    )?
+                }
+                if !activated.is_empty() {
+                    writeln!(
+                        message,
+                        "enabled features:\n    {}",
+                        activated
+                            .iter()
+                            .map(|s| s.to_string())
+                            .coalesce(|x, y| if x.len() + y.len() < 78 {
+                                Ok(format!("{x}, {y}"))
+                            } else {
+                                Err((x, y))
+                            })
+                            .into_iter()
+                            .format("\n    ")
+                    )?
+                }
+            }
+            anyhow::bail!(message.trim().to_owned());
         }
 
+        print_dep_table_msg(&mut options.config.shell(), &dep)?;
+
         manifest.insert_into_table(&dep_table, &dep)?;
         manifest.gc_dep(dep.toml_key());
     }
@@ -634,6 +688,42 @@ impl DependencyUI {
             })
             .collect();
     }
+
+    fn features(&self) -> (IndexSet<&str>, IndexSet<&str>) {
+        let mut activated: IndexSet<_> =
+            self.features.iter().flatten().map(|s| s.as_str()).collect();
+        if self.default_features().unwrap_or(true) {
+            activated.insert("default");
+        }
+        activated.extend(self.inherited_features.iter().flatten().map(|s| s.as_str()));
+        let mut walk: VecDeque<_> = activated.iter().cloned().collect();
+        while let Some(next) = walk.pop_front() {
+            walk.extend(
+                self.available_features
+                    .get(next)
+                    .into_iter()
+                    .flatten()
+                    .map(|s| s.as_str()),
+            );
+            activated.extend(
+                self.available_features
+                    .get(next)
+                    .into_iter()
+                    .flatten()
+                    .map(|s| s.as_str()),
+            );
+        }
+        activated.remove("default");
+        activated.sort();
+        let mut deactivated = self
+            .available_features
+            .keys()
+            .filter(|f| !activated.contains(f.as_str()) && *f != "default")
+            .map(|f| f.as_str())
+            .collect::<IndexSet<_>>();
+        deactivated.sort();
+        (activated, deactivated)
+    }
 }
 
 impl<'s> From<&'s Summary> for DependencyUI {
@@ -697,9 +787,7 @@ fn populate_available_features(
     Ok(dependency)
 }
 
-fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> CargoResult<()> {
-    use std::fmt::Write;
-
+fn print_action_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> CargoResult<()> {
     if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) {
         return Ok(());
     }
@@ -736,38 +824,14 @@ fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> Cargo
     };
     write!(message, " {section}")?;
     write!(message, ".")?;
-    shell.status("Adding", message)?;
-
-    let mut activated: IndexSet<_> = dep.features.iter().flatten().map(|s| s.as_str()).collect();
-    if dep.default_features().unwrap_or(true) {
-        activated.insert("default");
-    }
-    activated.extend(dep.inherited_features.iter().flatten().map(|s| s.as_str()));
-    let mut walk: VecDeque<_> = activated.iter().cloned().collect();
-    while let Some(next) = walk.pop_front() {
-        walk.extend(
-            dep.available_features
-                .get(next)
-                .into_iter()
-                .flatten()
-                .map(|s| s.as_str()),
-        );
-        activated.extend(
-            dep.available_features
-                .get(next)
-                .into_iter()
-                .flatten()
-                .map(|s| s.as_str()),
-        );
+    shell.status("Adding", message)
+}
+
+fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()> {
+    if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) {
+        return Ok(());
     }
-    activated.remove("default");
-    activated.sort();
-    let mut deactivated = dep
-        .available_features
-        .keys()
-        .filter(|f| !activated.contains(f.as_str()) && *f != "default")
-        .collect::<Vec<_>>();
-    deactivated.sort();
+    let (activated, deactivated) = dep.features();
     if !activated.is_empty() || !deactivated.is_empty() {
         let prefix = format!("{:>13}", " ");
         let suffix = if let Some(version) = &dep.available_version {

tests/testsuite/cargo_add/features_unknown/stderr.log

@@ -1,9 +1,5 @@
     Updating `dummy-registry` index
       Adding your-face v99999.0.0 to dependencies.
-             Features:
-             + noze
-             - ears
-             - eyes
-             - mouth
-             - nose
-error: unrecognized features: ["noze"]
+error: unrecognized feature for crate your-face: noze
+disabled features:
+    ears, eyes, mouth, nose

tests/testsuite/cargo_add/features_unknown_no_features/in

@@ -0,0 +1 @@
+../add-basic.in

tests/testsuite/cargo_add/features_unknown_no_features/mod.rs

@@ -0,0 +1,25 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::prelude::*;
+use cargo_test_support::Project;
+
+use crate::cargo_add::init_registry;
+use cargo_test_support::curr_dir;
+
+#[cargo_test]
+fn features_unknown_no_features() {
+    init_registry();
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line("my-package --features noze")
+        .current_dir(cwd)
+        .assert()
+        .code(101)
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_add/features_unknown_no_features/out/Cargo.toml

@@ -0,0 +1,5 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"

tests/testsuite/cargo_add/features_unknown_no_features/stderr.log

@@ -0,0 +1,4 @@
+    Updating `dummy-registry` index
+      Adding my-package v99999.0.0 to dependencies.
+error: unrecognized feature for crate my-package: noze
+no features available for crate my-package

tests/testsuite/cargo_add/features_unknown_no_features/stdout.log

@@ -20,6 +20,7 @@ mod features_multiple_occurrences;
 mod features_preserve;
 mod features_spaced_values;
 mod features_unknown;
+mod features_unknown_no_features;
 mod git;
 mod git_branch;
 mod git_conflicts_namever;

tests/testsuite/cargo_add/mod.rs

@@ -6,9 +6,15 @@ version = "0.0.0"
 default-base = []
 default-test-base = []
 default-merge-base = []
-default = ["default-base", "default-test-base", "default-merge-base"]
+long-feature-name-because-of-formatting-reasons = []
+default = [
+    "default-base",
+    "default-test-base",
+    "default-merge-base",
+    "long-feature-name-because-of-formatting-reasons",
+]
 test-base = []
 test = ["test-base", "default-test-base"]
 merge-base = []
 merge = ["merge-base", "default-merge-base"]
-unrelated = []
+unrelated = []

tests/testsuite/cargo_add/unknown_inherited_feature/in/dependency/Cargo.toml

@@ -6,9 +6,15 @@ version = "0.0.0"
 default-base = []
 default-test-base = []
 default-merge-base = []
-default = ["default-base", "default-test-base", "default-merge-base"]
+long-feature-name-because-of-formatting-reasons = []
+default = [
+    "default-base",
+    "default-test-base",
+    "default-merge-base",
+    "long-feature-name-because-of-formatting-reasons",
+]
 test-base = []
 test = ["test-base", "default-test-base"]
 merge-base = []
 merge = ["merge-base", "default-merge-base"]
-unrelated = []
+unrelated = []

tests/testsuite/cargo_add/unknown_inherited_feature/out/dependency/Cargo.toml

@@ -1,12 +1,7 @@
       Adding foo (workspace) to dependencies.
-             Features as of v0.0.0:
-             + default-base
-             + default-merge-base
-             + default-test-base
-             + not_recognized
-             + test
-             + test-base
-             - merge
-             - merge-base
-             - unrelated
-error: unrecognized features: ["not_recognized"]
+error: unrecognized feature for crate foo: not_recognized
+disabled features:
+    merge, merge-base, unrelated
+enabled features:
+    default-base, default-merge-base, default-test-base
+    long-feature-name-because-of-formatting-reasons, test, test-base