Don't change DependencyProvider

Author: konstin

examples/caching_dependency_provider.rs

@@ -4,6 +4,7 @@ use std::cell::RefCell;
 
 use pubgrub::range::Range;
 use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider};
+use pubgrub::type_aliases::DependencyConstraints;
 
 type NumVS = Range<u32>;
 
@@ -29,37 +30,27 @@ impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependenc
         &self,
         package: &DP::P,
         version: &DP::V,
-    ) -> Result<Dependencies<impl IntoIterator<Item = (DP::P, DP::VS)> + Clone, Self::M>, DP::Err>
-    {
+    ) -> Result<Dependencies<DependencyConstraints<DP::P, DP::VS>, DP::M>, DP::Err> {
         let mut cache = self.cached_dependencies.borrow_mut();
         match cache.get_dependencies(package, version) {
             Ok(Dependencies::Unavailable(_)) => {
-                // Code below to end the borrow.
-            }
-            Ok(dependencies) => {
-                return Ok(match dependencies {
-                    Dependencies::Unavailable(reason) => Dependencies::Unavailable(reason),
-                    Dependencies::Available(available) => Dependencies::Available(
-                        available.into_iter().collect::<Vec<(Self::P, Self::VS)>>(),
-                    ),
-                })
+                let dependencies = self.remote_dependencies.get_dependencies(package, version);
+                match dependencies {
+                    Ok(Dependencies::Available(dependencies)) => {
+                        cache.add_dependencies(
+                            package.clone(),
+                            version.clone(),
+                            dependencies.clone(),
+                        );
+                        Ok(Dependencies::Available(dependencies))
+                    }
+                    Ok(Dependencies::Unavailable(reason)) => Ok(Dependencies::Unavailable(reason)),
+                    error @ Err(_) => error,
+                }
             }
+            Ok(dependencies) => Ok(dependencies),
             Err(_) => unreachable!(),
         }
-        let dependencies = self.remote_dependencies.get_dependencies(package, version);
-        match dependencies {
-            Ok(Dependencies::Available(dependencies)) => {
-                cache.add_dependencies(package.clone(), version.clone(), dependencies.clone());
-                Ok(Dependencies::Available(
-                    dependencies
-                        .clone()
-                        .into_iter()
-                        .collect::<Vec<(Self::P, Self::VS)>>(),
-                ))
-            }
-            Ok(Dependencies::Unavailable(reason)) => Ok(Dependencies::Unavailable(reason)),
-            Err(err) => Err(err),
-        }
     }
 
     fn choose_version(&self, package: &DP::P, range: &DP::VS) -> Result<Option<DP::V>, DP::Err> {

src/lib.rs

@@ -73,7 +73,7 @@
 //! # use pubgrub::solver::{DependencyProvider, Dependencies};
 //! # use pubgrub::version::SemanticVersion;
 //! # use pubgrub::range::Range;
-//! # use pubgrub::type_aliases::Map;
+//! # use pubgrub::type_aliases::{DependencyConstraints, Map};
 //! # use std::error::Error;
 //! # use std::borrow::Borrow;
 //! # use std::convert::Infallible;
@@ -96,8 +96,8 @@
 //!         &self,
 //!         package: &String,
 //!         version: &SemanticVersion,
-//!     ) -> Result<Dependencies<impl IntoIterator<Item = (String, SemVS)> + Clone, Self::M>, Infallible> {
-//!         Ok(Dependencies::Available([]))
+//!     ) -> Result<Dependencies<DependencyConstraints<String, SemVS>, Self::M>, Infallible> {
+//!         Ok(Dependencies::Available(DependencyConstraints::default()))
 //!     }
 //!
 //!     type Err = Infallible;

src/solver.rs

@@ -71,7 +71,7 @@ use crate::error::PubGrubError;
 use crate::internal::core::State;
 use crate::internal::incompatibility::Incompatibility;
 use crate::package::Package;
-use crate::type_aliases::{Map, SelectedDependencies};
+use crate::type_aliases::{DependencyConstraints, Map, SelectedDependencies};
 use crate::version_set::VersionSet;
 use log::{debug, info};
 
@@ -280,10 +280,7 @@ pub trait DependencyProvider {
         &self,
         package: &Self::P,
         version: &Self::V,
-    ) -> Result<
-        Dependencies<impl IntoIterator<Item = (Self::P, Self::VS)> + Clone, Self::M>,
-        Self::Err,
-    >;
+    ) -> Result<Dependencies<DependencyConstraints<Self::P, Self::VS>, Self::M>, Self::Err>;
 
     /// This is called fairly regularly during the resolution,
     /// if it returns an Err then resolution will be terminated.
@@ -357,8 +354,8 @@ impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
 
     /// Lists dependencies of a given package and version.
     /// Returns [None] if no information is available regarding that package and version pair.
-    fn dependencies(&self, package: &P, version: &VS::V) -> Option<&Map<P, VS>> {
-        self.dependencies.get(package)?.get(version)
+    fn dependencies(&self, package: &P, version: &VS::V) -> Option<DependencyConstraints<P, VS>> {
+        self.dependencies.get(package)?.get(version).cloned()
     }
 }
 
@@ -396,17 +393,12 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OfflineDependencyProvide
         &self,
         package: &P,
         version: &VS::V,
-    ) -> Result<
-        Dependencies<impl IntoIterator<Item = (Self::P, Self::VS)> + Clone, Self::M>,
-        Self::Err,
-    > {
+    ) -> Result<Dependencies<DependencyConstraints<Self::P, Self::VS>, Self::M>, Self::Err> {
         Ok(match self.dependencies(package, version) {
             None => {
                 Dependencies::Unavailable("its dependencies could not be determined".to_string())
             }
-            Some(dependencies) => {
-                Dependencies::Available(dependencies.iter().map(|(p, vs)| (p.clone(), vs.clone())))
-            }
+            Some(dependencies) => Dependencies::Available(dependencies),
         })
     }
 }

src/type_aliases.rs

@@ -14,6 +14,13 @@ pub type Set<V> = rustc_hash::FxHashSet<V>;
 pub type SelectedDependencies<DP> =
     Map<<DP as DependencyProvider>::P, <DP as DependencyProvider>::V>;
 
+/// Holds information about all possible versions a given package can accept.
+/// There is a difference in semantics between an empty map
+/// inside [DependencyConstraints] and [Dependencies::Unavailable](crate::solver::Dependencies::Unavailable):
+/// the former means the package has no dependency and it is a known fact,
+/// while the latter means they could not be fetched by the [DependencyProvider].
+pub type DependencyConstraints<P, VS> = Map<P, VS>;
+
 pub(crate) type IncompDpId<DP> = IncompId<
     <DP as DependencyProvider>::P,
     <DP as DependencyProvider>::VS,

tests/proptest.rs

@@ -11,7 +11,7 @@ use pubgrub::package::Package;
 use pubgrub::range::Range;
 use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter};
 use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider};
-use pubgrub::type_aliases::SelectedDependencies;
+use pubgrub::type_aliases::{DependencyConstraints, SelectedDependencies};
 #[cfg(feature = "serde")]
 use pubgrub::version::SemanticVersion;
 use pubgrub::version_set::VersionSet;
@@ -37,7 +37,7 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OldestVersionsDependency
         &self,
         p: &P,
         v: &VS::V,
-    ) -> Result<Dependencies<impl IntoIterator<Item = (P, VS)> + Clone, Self::M>, Infallible> {
+    ) -> Result<Dependencies<DependencyConstraints<Self::P, Self::VS>, Self::M>, Infallible> {
         self.0.get_dependencies(p, v)
     }
 
@@ -90,8 +90,7 @@ impl<DP: DependencyProvider> DependencyProvider for TimeoutDependencyProvider<DP
         &self,
         p: &DP::P,
         v: &DP::V,
-    ) -> Result<Dependencies<impl IntoIterator<Item = (DP::P, DP::VS)> + Clone, DP::M>, DP::Err>
-    {
+    ) -> Result<Dependencies<DependencyConstraints<DP::P, DP::VS>, DP::M>, DP::Err> {
         self.dp.get_dependencies(p, v)
     }