Consider public dependencies when choosing a version in cargo add (#1…

src/cargo/ops/cargo_add/mod.rs

@@ -20,6 +20,7 @@ use toml_edit::Item as TomlItem;
 
 use crate::CargoResult;
 use crate::GlobalContext;
+use crate::core::Feature;
 use crate::core::FeatureValue;
 use crate::core::Features;
 use crate::core::Package;
@@ -29,6 +30,7 @@ use crate::core::Summary;
 use crate::core::Workspace;
 use crate::core::dependency::DepKind;
 use crate::core::registry::PackageRegistry;
+use crate::ops::resolve_ws;
 use crate::sources::source::QueryKind;
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::edit_distance;
@@ -38,6 +40,7 @@ use crate::util::toml_mut::dependency::Dependency;
 use crate::util::toml_mut::dependency::GitSource;
 use crate::util::toml_mut::dependency::MaybeWorkspace;
 use crate::util::toml_mut::dependency::PathSource;
+use crate::util::toml_mut::dependency::RegistrySource;
 use crate::util::toml_mut::dependency::Source;
 use crate::util::toml_mut::dependency::WorkspaceSource;
 use crate::util::toml_mut::manifest::DepTable;
@@ -471,6 +474,13 @@ fn resolve_dependency(
                 src = src.set_version(v);
             }
             dependency = dependency.set_source(src);
+        } else if let Some((registry, public_source)) =
+            get_public_dependency(spec, manifest, ws, section, gctx, &dependency)?
+        {
+            if let Some(registry) = registry {
+                dependency = dependency.set_registry(registry);
+            }
+            dependency = dependency.set_source(public_source);
         } else {
             let latest =
                 get_latest_dependency(spec, &dependency, honor_rust_version, gctx, registry)?;
@@ -501,6 +511,113 @@ fn resolve_dependency(
         dependency = dependency.clear_version();
     }
 
+    let query = query_dependency(ws, gctx, &mut dependency)?;
+    let dependency = populate_available_features(dependency, &query, registry)?;
+
+    Ok(dependency)
+}
+
+fn get_public_dependency(
+    spec: &Package,
+    manifest: &LocalManifest,
+    ws: &Workspace<'_>,
+    section: &DepTable,
+    gctx: &GlobalContext,
+    dependency: &Dependency,
+) -> CargoResult<Option<(Option<String>, Source)>> {
+    if spec
+        .manifest()
+        .unstable_features()
+        .require(Feature::public_dependency())
+        .is_err()
+    {
+        return Ok(None);
+    }
+
+    let (package_set, resolve) = resolve_ws(ws, true)?;
+
+    let mut latest = None;
+
+    for (_, path, dep) in manifest.get_dependencies(ws, ws.unstable_features()) {
+        if path != *section {
+            continue;
+        }
+
+        let Some(mut dep) = dep.ok() else {
+            continue;
+        };
+
+        let dep = query_dependency(ws, gctx, &mut dep)?;
+        let Some(dep_pkgid) = package_set
+            .package_ids()
+            .filter(|package_id| {
+                package_id.name() == dep.package_name()
+                    && dep.version_req().matches(package_id.version())
+            })
+            .max_by_key(|x| x.version())
+        else {
+            continue;
+        };
+
+        let pkg_ids_and_reqs = resolve.deps(dep_pkgid).filter_map(|(id, deps)| {
+            deps.iter()
+                .find(|dep| {
+                    dep.is_public()
+                        && dep.kind() == DepKind::Normal
+                        && dep.package_name() == dependency.name.as_str()
+                })
+                .map(|dep| (id, dep.version_req().clone()))
+        });
+
+        for (pkg_id, req) in pkg_ids_and_reqs {
+            if let Some((old_pkg_id, _)) = &latest
+                && *old_pkg_id >= pkg_id
+            {
+                continue;
+            }
+            latest = Some((pkg_id, req))
+        }
+    }
+
+    let Some((pkg_id, version_req)) = latest else {
+        return Ok(None);
+    };
+
+    let source = pkg_id.source_id();
+    if source.is_git() {
+        Ok(Some((
+            Option::<String>::None,
+            Source::Git(GitSource::new(source.as_encoded_url().to_string())),
+        )))
+    } else if let Some(path) = source.local_path() {
+        Ok(Some((None, Source::Path(PathSource::new(path)))))
+    } else {
+        let toml_source = match version_req {
+            crate::util::OptVersionReq::Any => Source::Registry(RegistrySource::new(format!(
+                "={}",
+                pkg_id.version().to_string()
+            ))),
+            crate::util::OptVersionReq::Req(version_req)
+            | crate::util::OptVersionReq::Locked(_, version_req)
+            | crate::util::OptVersionReq::Precise(_, version_req) => {
+                Source::Registry(RegistrySource::new(version_req.to_string()))
+            }
+        };
+        Ok(Some((
+            source
+                .alt_registry_key()
+                .map(|x| x.to_owned())
+                .filter(|_| !source.is_crates_io()),
+            toml_source,
+        )))
+    }
+}
+
+fn query_dependency(
+    ws: &Workspace<'_>,
+    gctx: &GlobalContext,
+    dependency: &mut Dependency,
+) -> CargoResult<crate::core::Dependency> {
     let query = dependency.query(gctx)?;
     let query = match query {
         MaybeWorkspace::Workspace(_workspace) => {
@@ -511,7 +628,7 @@ fn resolve_dependency(
                 ws.unstable_features(),
             )?;
             if let Some(features) = dep.features.clone() {
-                dependency = dependency.set_inherited_features(features);
+                *dependency = dependency.clone().set_inherited_features(features);
             }
             let query = dep.query(gctx)?;
             match query {
@@ -523,10 +640,7 @@ fn resolve_dependency(
         }
         MaybeWorkspace::Other(query) => query,
     };
-
-    let dependency = populate_available_features(dependency, &query, registry)?;
-
-    Ok(dependency)
+    Ok(query)
 }
 
 fn fuzzy_lookup(
@@ -640,8 +754,11 @@ fn get_existing_dependency(
     }
 
     let mut possible: Vec<_> = manifest
-        .get_dependency_versions(dep_key, ws, unstable_features)
-        .map(|(path, dep)| {
+        .get_dependencies(ws, unstable_features)
+        .filter_map(|(key, path, dep)| {
+            if key.as_str() != dep_key {
+                return None;
+            }
             let key = if path == *section {
                 (Key::Existing, true)
             } else if dep.is_err() {
@@ -654,7 +771,7 @@ fn get_existing_dependency(
                 };
                 (key, path.target().is_some())
             };
-            (key, dep)
+            Some((key, dep))
         })
         .collect();
     possible.sort_by_key(|(key, _)| *key);

src/cargo/util/toml_mut/manifest.rs

@@ -327,12 +327,11 @@ impl LocalManifest {
     }
 
     /// Lookup a dependency.
-    pub fn get_dependency_versions<'s>(
+    pub fn get_dependencies<'s>(
         &'s self,
-        dep_key: &'s str,
         ws: &'s Workspace<'_>,
         unstable_features: &'s Features,
-    ) -> impl Iterator<Item = (DepTable, CargoResult<Dependency>)> + 's {
+    ) -> impl Iterator<Item = (String, DepTable, CargoResult<Dependency>)> + 's {
         let crate_root = self.path.parent().expect("manifest path is absolute");
         self.get_sections()
             .into_iter()
@@ -341,13 +340,7 @@ impl LocalManifest {
                 Some(
                     table
                         .into_iter()
-                        .filter_map(|(key, item)| {
-                            if key.as_str() == dep_key {
-                                Some((table_path.clone(), key, item))
-                            } else {
-                                None
-                            }
-                        })
+                        .map(|(key, item)| (table_path.clone(), key, item))
                         .collect::<Vec<_>>(),
                 )
             })
@@ -361,7 +354,7 @@ impl LocalManifest {
                     &dep_key,
                     &dep_item,
                 );
-                (table_path, dep)
+                (dep_key, table_path, dep)
             })
     }
 

tests/testsuite/cargo_add/mod.rs

@@ -134,6 +134,7 @@ mod preserve_features_unsorted;
 mod preserve_sorted;
 mod preserve_unsorted;
 mod public;
+mod public_common_version;
 mod quiet;
 mod registry;
 mod rename;

tests/testsuite/cargo_add/public_common_version/in/Cargo.toml

@@ -0,0 +1,10 @@
+cargo-features = ["public-dependency"]
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies]
+my-package = "0.1.0"

tests/testsuite/cargo_add/public_common_version/mod.rs

@@ -0,0 +1,35 @@
+use crate::prelude::*;
+use cargo_test_support::Project;
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::current_dir;
+use cargo_test_support::file;
+use cargo_test_support::registry::Dependency;
+use cargo_test_support::str;
+
+#[cargo_test]
+fn case() {
+    cargo_test_support::registry::init();
+    cargo_test_support::registry::Package::new("my-package-dep", "0.1.0").publish();
+    cargo_test_support::registry::Package::new("my-package-dep", "0.2.0").publish();
+    cargo_test_support::registry::Package::new("my-package", "0.1.0")
+        .add_dep(Dependency::new("my-package-dep", "0.1.0").public(true))
+        .publish();
+    cargo_test_support::registry::Package::new("my-package", "0.2.0")
+        .add_dep(Dependency::new("my-package-dep", "0.2.0").public(true))
+        .publish();
+    let project = Project::from_template(current_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line("my-package-dep")
+        .current_dir(cwd)
+        .masquerade_as_nightly_cargo(&["public-dependency"])
+        .assert()
+        .success()
+        .stdout_eq(str![""])
+        .stderr_eq(file!["stderr.term.svg"]);
+
+    assert_ui().subset_matches(current_dir!().join("out"), &project_root);
+}

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

@@ -0,0 +1,11 @@
+cargo-features = ["public-dependency"]
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies]
+my-package = "0.1.0"
+my-package-dep = "^0.1.0"