fix(add): Clarify which version the features are added for

src/cargo/ops/cargo_add/dependency.rs

@@ -1,13 +1,10 @@
-use std::collections::BTreeMap;
 use std::fmt::{Display, Formatter};
 use std::path::{Path, PathBuf};
 
 use indexmap::IndexSet;
 use toml_edit::KeyMut;
 
 use super::manifest::str_or_1_len_table;
-use crate::core::FeatureMap;
-use crate::core::FeatureValue;
 use crate::core::GitReference;
 use crate::core::SourceId;
 use crate::core::Summary;
@@ -40,9 +37,6 @@ pub struct Dependency {
     /// If the dependency is renamed, this is the new name for the dependency
     /// as a string.  None if it is not renamed.
     pub rename: Option<String>,
-
-    /// Features that are exposed by the dependency
-    pub available_features: BTreeMap<String, Vec<String>>,
 }
 
 impl Dependency {
@@ -57,7 +51,6 @@ impl Dependency {
             source: None,
             registry: None,
             rename: None,
-            available_features: Default::default(),
         }
     }
 
@@ -85,37 +78,6 @@ impl Dependency {
         self
     }
 
-    /// Set the available features of the dependency to a given vec
-    pub fn set_available_features(
-        mut self,
-        available_features: BTreeMap<String, Vec<String>>,
-    ) -> Self {
-        self.available_features = available_features;
-        self
-    }
-
-    /// Populate from cargo
-    pub fn set_available_features_from_cargo(
-        mut self,
-        available_features: &FeatureMap,
-    ) -> Dependency {
-        self.available_features = available_features
-            .iter()
-            .map(|(k, v)| {
-                (
-                    k.as_str().to_owned(),
-                    v.iter()
-                        .filter_map(|v| match v {
-                            FeatureValue::Feature(f) => Some(f.as_str().to_owned()),
-                            FeatureValue::Dep { .. } | FeatureValue::DepFeature { .. } => None,
-                        })
-                        .collect::<Vec<_>>(),
-                )
-            })
-            .collect();
-        self
-    }
-
     /// Set whether the dependency is optional
     #[allow(dead_code)]
     pub fn set_optional(mut self, opt: bool) -> Self {
@@ -347,8 +309,6 @@ impl Dependency {
                 None
             };
 
-            let available_features = BTreeMap::default();
-
             let optional = table.get("optional").and_then(|v| v.as_bool());
 
             let dep = Self {
@@ -358,7 +318,6 @@ impl Dependency {
                 registry,
                 default_features,
                 features,
-                available_features,
                 optional,
                 inherited_features: None,
             };
@@ -646,9 +605,7 @@ impl<'s> From<&'s Summary> for Dependency {
         } else {
             RegistrySource::new(other.version().to_string()).into()
         };
-        Dependency::new(other.name().as_str())
-            .set_source(source)
-            .set_available_features_from_cargo(other.features())
+        Dependency::new(other.name().as_str()).set_source(source)
     }
 }
 

src/cargo/ops/cargo_add/mod.rs

@@ -4,11 +4,12 @@ mod crate_spec;
 mod dependency;
 mod manifest;
 
-use anyhow::Context;
+use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::VecDeque;
 use std::path::Path;
 
+use anyhow::Context as _;
 use cargo_util::paths;
 use indexmap::IndexSet;
 use termcolor::Color::Green;
@@ -18,10 +19,12 @@ use toml_edit::Item as TomlItem;
 
 use crate::core::dependency::DepKind;
 use crate::core::registry::PackageRegistry;
+use crate::core::FeatureValue;
 use crate::core::Package;
 use crate::core::QueryKind;
 use crate::core::Registry;
 use crate::core::Shell;
+use crate::core::Summary;
 use crate::core::Workspace;
 use crate::CargoResult;
 use crate::Config;
@@ -200,7 +203,7 @@ fn resolve_dependency(
     section: &DepTable,
     config: &Config,
     registry: &mut PackageRegistry<'_>,
-) -> CargoResult<Dependency> {
+) -> CargoResult<DependencyUI> {
     let crate_spec = arg
         .crate_spec
         .as_deref()
@@ -284,9 +287,7 @@ fn resolve_dependency(
                 // Overwrite with `crate_spec`
                 old_dep.source = selected_dep.source;
             }
-            old_dep = populate_dependency(old_dep, arg);
-            old_dep.available_features = selected_dep.available_features;
-            old_dep
+            populate_dependency(old_dep, arg)
         }
     } else {
         selected_dep
@@ -318,9 +319,7 @@ fn resolve_dependency(
                 ))?;
                 dependency.name = latest.name; // Normalize the name
             }
-            dependency = dependency
-                .set_source(latest.source.expect("latest always has a source"))
-                .set_available_features(latest.available_features);
+            dependency = dependency.set_source(latest.source.expect("latest always has a source"));
         }
     }
 
@@ -339,7 +338,25 @@ fn resolve_dependency(
         dependency = dependency.clear_version();
     }
 
-    dependency = populate_available_features(dependency, config, registry, ws)?;
+    let query = dependency.query(config)?;
+    let query = match query {
+        MaybeWorkspace::Workspace(_workspace) => {
+            let dep = find_workspace_dep(dependency.toml_key(), ws.root_manifest())?;
+            if let Some(features) = dep.features.clone() {
+                dependency = dependency.set_inherited_features(features);
+            }
+            let query = dep.query(config)?;
+            match query {
+                MaybeWorkspace::Workspace(_) => {
+                    unreachable!("This should have been caught when parsing a workspace root")
+                }
+                MaybeWorkspace::Other(query) => query,
+            }
+        }
+        MaybeWorkspace::Other(query) => query,
+    };
+
+    let dependency = populate_available_features(dependency, &query, registry)?;
 
     Ok(dependency)
 }
@@ -582,34 +599,81 @@ fn populate_dependency(mut dependency: Dependency, arg: &DepOp) -> Dependency {
     dependency
 }
 
+/// Track presentation-layer information with the editable representation of a `[dependencies]`
+/// entry (Dependency)
+pub struct DependencyUI {
+    /// Editable representation of a `[depednencies]` entry
+    dep: Dependency,
+    /// The version of the crate that we pulled `available_features` from
+    available_version: Option<semver::Version>,
+    /// The widest set of features compatible with `Dependency`s version requirement
+    available_features: BTreeMap<String, Vec<String>>,
+}
+
+impl DependencyUI {
+    fn new(dep: Dependency) -> Self {
+        Self {
+            dep,
+            available_version: None,
+            available_features: Default::default(),
+        }
+    }
+
+    fn apply_summary(&mut self, summary: &Summary) {
+        self.available_version = Some(summary.version().clone());
+        self.available_features = summary
+            .features()
+            .iter()
+            .map(|(k, v)| {
+                (
+                    k.as_str().to_owned(),
+                    v.iter()
+                        .filter_map(|v| match v {
+                            FeatureValue::Feature(f) => Some(f.as_str().to_owned()),
+                            FeatureValue::Dep { .. } | FeatureValue::DepFeature { .. } => None,
+                        })
+                        .collect::<Vec<_>>(),
+                )
+            })
+            .collect();
+    }
+}
+
+impl<'s> From<&'s Summary> for DependencyUI {
+    fn from(other: &'s Summary) -> Self {
+        let dep = Dependency::from(other);
+        let mut dep = Self::new(dep);
+        dep.apply_summary(other);
+        dep
+    }
+}
+
+impl std::fmt::Display for DependencyUI {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.dep.fmt(f)
+    }
+}
+
+impl std::ops::Deref for DependencyUI {
+    type Target = Dependency;
+
+    fn deref(&self) -> &Self::Target {
+        &self.dep
+    }
+}
+
 /// Lookup available features
 fn populate_available_features(
-    mut dependency: Dependency,
-    config: &Config,
+    dependency: Dependency,
+    query: &crate::core::dependency::Dependency,
     registry: &mut PackageRegistry<'_>,
-    ws: &Workspace<'_>,
-) -> CargoResult<Dependency> {
+) -> CargoResult<DependencyUI> {
+    let mut dependency = DependencyUI::new(dependency);
+
     if !dependency.available_features.is_empty() {
         return Ok(dependency);
     }
 
-    let query = dependency.query(config)?;
-    let query = match query {
-        MaybeWorkspace::Workspace(_workspace) => {
-            let dep = find_workspace_dep(dependency.toml_key(), ws.root_manifest())?;
-            if let Some(features) = dep.features.clone() {
-                dependency = dependency.set_inherited_features(features);
-            }
-            let query = dep.query(config)?;
-            match query {
-                MaybeWorkspace::Workspace(_) => {
-                    unreachable!("This should have been caught when parsing a workspace root")
-                }
-                MaybeWorkspace::Other(query) => query,
-            }
-        }
-        MaybeWorkspace::Other(query) => query,
-    };
     let possibilities = loop {
         match registry.query_vec(&query, QueryKind::Fuzzy) {
             std::task::Poll::Ready(res) => {
@@ -631,12 +695,12 @@ fn populate_available_features(
         .ok_or_else(|| {
             anyhow::format_err!("the crate `{dependency}` could not be found in registry index.")
         })?;
-    dependency = dependency.set_available_features_from_cargo(lowest_common_denominator.features());
+    dependency.apply_summary(&lowest_common_denominator);
 
     Ok(dependency)
 }
 
-fn print_msg(shell: &mut Shell, dep: &Dependency, section: &[String]) -> CargoResult<()> {
+fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> CargoResult<()> {
     use std::fmt::Write;
 
     if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) {
@@ -709,7 +773,28 @@ fn print_msg(shell: &mut Shell, dep: &Dependency, section: &[String]) -> CargoRe
     deactivated.sort();
     if !activated.is_empty() || !deactivated.is_empty() {
         let prefix = format!("{:>13}", " ");
-        shell.write_stderr(format_args!("{}Features:\n", prefix), &ColorSpec::new())?;
+        let suffix = if let Some(version) = &dep.available_version {
+            let mut version = version.clone();
+            version.build = Default::default();
+            let version = version.to_string();
+            // Avoid displaying the version if it will visually look like the version req that we
+            // showed earlier
+            let version_req = dep
+                .version()
+                .and_then(|v| semver::VersionReq::parse(v).ok())
+                .and_then(|v| precise_version(&v));
+            if version_req.as_deref() != Some(version.as_str()) {
+                format!(" as of v{version}")
+            } else {
+                "".to_owned()
+            }
+        } else {
+            "".to_owned()
+        };
+        shell.write_stderr(
+            format_args!("{}Features{}:\n", prefix, suffix),
+            &ColorSpec::new(),
+        )?;
         for feat in activated {
             shell.write_stderr(&prefix, &ColorSpec::new())?;
             shell.write_stderr('+', &ColorSpec::new().set_bold(true).set_fg(Some(Green)))?;
@@ -765,3 +850,37 @@ fn find_workspace_dep(toml_key: &str, root_manifest: &Path) -> CargoResult<Depen
     ))?;
     Dependency::from_toml(root_manifest.parent().unwrap(), toml_key, dep_item)
 }
+
+/// Convert a `semver::VersionReq` into a rendered `semver::Version` if all fields are fully
+/// specified.
+fn precise_version(version_req: &semver::VersionReq) -> Option<String> {
+    version_req
+        .comparators
+        .iter()
+        .filter(|c| {
+            matches!(
+                c.op,
+                // Only ops we can determine a precise version from
+                semver::Op::Exact
+                    | semver::Op::GreaterEq
+                    | semver::Op::LessEq
+                    | semver::Op::Tilde
+                    | semver::Op::Caret
+                    | semver::Op::Wildcard
+            )
+        })
+        .filter_map(|c| {
+            // Only do it when full precision is specified
+            c.minor.and_then(|minor| {
+                c.patch.map(|patch| semver::Version {
+                    major: c.major,
+                    minor,
+                    patch,
+                    pre: c.pre.clone(),
+                    build: Default::default(),
+                })
+            })
+        })
+        .max()
+        .map(|v| v.to_string())
+}

tests/testsuite/cargo_add/detect_workspace_inherit_features/stderr.log

@@ -1,5 +1,5 @@
       Adding foo (workspace) to dependencies.
-             Features:
+             Features as of v0.0.0:
              + default-base
              + default-merge-base
              + default-test-base

tests/testsuite/cargo_add/dev_prefer_existing_version/stderr.log

@@ -1,4 +1,4 @@
       Adding cargo-list-test-fixture-dependency (local) to dev-dependencies.
-             Features:
+             Features as of v0.0.0:
              - one
              - two

tests/testsuite/cargo_add/merge_activated_features/stderr.log

@@ -1,5 +1,5 @@
       Adding foo (workspace) to dependencies.
-             Features:
+             Features as of v0.0.0:
              + default-base
              + default-merge-base
              + default-test-base

tests/testsuite/cargo_add/overwrite_inherit_features_noop/stderr.log

@@ -1,3 +1,3 @@
       Adding foo (workspace) to dependencies.
-             Features:
+             Features as of v0.0.0:
              + test