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

### What does this PR try to resolve?

This change considers public dependencies when adding a new dependency.
For example, if you depend on `foo`, which depends publicly on `bar
1.0`, and you run `cargo add bar`, you will now get `bar 1.0` even if
`bar 2.0` is available.

Fixes #13038

### How to test and review this PR?

The test suite has been updated with an example scenario. More tests
might be warrented to see how this interacts with other features though.

Author: epage

src/cargo/ops/cargo_add/mod.rs

@@ -20,16 +20,20 @@ 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;
+use crate::core::PackageId;
 use crate::core::Registry;
 use crate::core::Shell;
 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::OptVersionReq;
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::edit_distance;
 use crate::util::style;
@@ -38,6 +42,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 +476,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 +513,125 @@ 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: Option<(PackageId, OptVersionReq)> = 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 mut pkg_ids_and_reqs = Vec::new();
+        let mut pkg_id_queue = VecDeque::new();
+        let mut examined = BTreeSet::new();
+        pkg_id_queue.push_back(dep_pkgid);
+
+        while let Some(dep_pkgid) = pkg_id_queue.pop_front() {
+            let got_deps = resolve.deps(dep_pkgid).filter_map(|(id, deps)| {
+                deps.iter()
+                    .find(|dep| dep.is_public() && dep.kind() == DepKind::Normal)
+                    .map(|dep| (id, dep))
+            });
+
+            for (pkg_id, got_dep) in got_deps {
+                if got_dep.package_name() == dependency.name.as_str() {
+                    pkg_ids_and_reqs.push((pkg_id, got_dep.version_req().clone()));
+                }
+
+                if examined.insert(pkg_id.clone()) {
+                    pkg_id_queue.push_back(pkg_id)
+                }
+            }
+        }
+
+        for (pkg_id, req) in pkg_ids_and_reqs {
+            if let Some((old_pkg_id, _)) = &latest
+                && old_pkg_id.version() >= pkg_id.version()
+            {
+                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(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 +642,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 +654,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 +768,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 +785,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/in/src/lib.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/mod.rs

@@ -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"