Avoid changing the public API for the dependencies internal iterator

Author: konstin

examples/caching_dependency_provider.rs

@@ -4,7 +4,6 @@ use std::cell::RefCell;
 
 use pubgrub::range::Range;
 use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider};
-use pubgrub::type_aliases::DependencyConstraints;
 
 type NumVS = Range<u32>;
 
@@ -30,7 +29,7 @@ impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependenc
         &self,
         package: &DP::P,
         version: &DP::V,
-    ) -> Result<Dependencies<DependencyConstraints<DP::P, DP::VS>, DP::M>, DP::Err> {
+    ) -> Result<Dependencies<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(_)) => {

src/lib.rs

@@ -96,7 +96,7 @@
 //!         &self,
 //!         package: &String,
 //!         version: &SemanticVersion,
-//!     ) -> Result<Dependencies<DependencyConstraints<String, SemVS>, Self::M>, Infallible> {
+//!     ) -> Result<Dependencies<String, SemVS, Self::M>, Infallible> {
 //!         Ok(Dependencies::Available(DependencyConstraints::default()))
 //!     }
 //!

src/solver.rs

@@ -158,10 +158,10 @@ pub fn resolve<DP: DependencyProvider>(
                     ));
                     continue;
                 }
-                Dependencies::Available(x) if x.clone().into_iter().any(|(d, _)| &d == p) => {
+                Dependencies::Available(x) if x.contains_key(p) => {
                     return Err(PubGrubError::SelfDependency {
                         package: p.clone(),
-                        version: v.clone(),
+                        version: v,
                     });
                 }
                 Dependencies::Available(x) => x,
@@ -189,11 +189,11 @@ pub fn resolve<DP: DependencyProvider>(
 /// An enum used by [DependencyProvider] that holds information about package dependencies.
 /// For each [Package] there is a set of versions allowed as a dependency.
 #[derive(Clone)]
-pub enum Dependencies<T, M: Eq + Clone + Debug + Display> {
+pub enum Dependencies<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     /// Package dependencies are unavailable with the reason why they are missing.
     Unavailable(M),
     /// Container for all available package versions.
-    Available(T),
+    Available(DependencyConstraints<P, VS>),
 }
 
 /// Trait that allows the algorithm to retrieve available packages and their dependencies.
@@ -280,7 +280,7 @@ pub trait DependencyProvider {
         &self,
         package: &Self::P,
         version: &Self::V,
-    ) -> Result<Dependencies<DependencyConstraints<Self::P, Self::VS>, Self::M>, Self::Err>;
+    ) -> Result<Dependencies<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.
@@ -304,7 +304,7 @@ pub trait DependencyProvider {
 )]
 #[cfg_attr(feature = "serde", serde(transparent))]
 pub struct OfflineDependencyProvider<P: Package, VS: VersionSet> {
-    dependencies: Map<P, BTreeMap<VS::V, Map<P, VS>>>,
+    dependencies: Map<P, BTreeMap<VS::V, DependencyConstraints<P, VS>>>,
 }
 
 impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
@@ -393,7 +393,7 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OfflineDependencyProvide
         &self,
         package: &P,
         version: &VS::V,
-    ) -> Result<Dependencies<DependencyConstraints<Self::P, Self::VS>, Self::M>, Self::Err> {
+    ) -> Result<Dependencies<P, VS, Self::M>, Infallible> {
         Ok(match self.dependencies(package, version) {
             None => {
                 Dependencies::Unavailable("its dependencies could not be determined".to_string())

src/type_aliases.rs

@@ -10,7 +10,8 @@ pub type Map<K, V> = rustc_hash::FxHashMap<K, V>;
 /// Set implementation used by the library.
 pub type Set<V> = rustc_hash::FxHashSet<V>;
 
-/// Concrete dependencies picked by the library during [resolve](crate::solver::resolve).
+/// Concrete dependencies picked by the library during [resolve](crate::solver::resolve)
+/// from [DependencyConstraints].
 pub type SelectedDependencies<DP> =
     Map<<DP as DependencyProvider>::P, <DP as DependencyProvider>::V>;
 

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::{DependencyConstraints, SelectedDependencies};
+use pubgrub::type_aliases::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<DependencyConstraints<Self::P, Self::VS>, Self::M>, Infallible> {
+    ) -> Result<Dependencies<P, VS, Self::M>, Infallible> {
         self.0.get_dependencies(p, v)
     }
 
@@ -90,7 +90,7 @@ impl<DP: DependencyProvider> DependencyProvider for TimeoutDependencyProvider<DP
         &self,
         p: &DP::P,
         v: &DP::V,
-    ) -> Result<Dependencies<DependencyConstraints<DP::P, DP::VS>, DP::M>, DP::Err> {
+    ) -> Result<Dependencies<DP::P, DP::VS, DP::M>, DP::Err> {
         self.dp.get_dependencies(p, v)
     }
 
@@ -352,8 +352,8 @@ fn retain_dependencies<N: Package + Ord, VS: VersionSet>(
             smaller_dependency_provider.add_dependencies(
                 n.clone(),
                 v.clone(),
-                deps.into_iter().filter_map(|(dep, range)| {
-                    if !retain(n, v, &dep) {
+                deps.iter().filter_map(|(dep, range)| {
+                    if !retain(n, v, dep) {
                         None
                     } else {
                         Some((dep.clone(), range.clone()))

tests/sat_dependency_provider.rs

@@ -69,10 +69,10 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
                 Dependencies::Unavailable(_) => panic!(),
                 Dependencies::Available(d) => d,
             };
-            for (p1, range) in deps {
+            for (p1, range) in &deps {
                 let empty_vec = vec![];
                 let mut matches: Vec<varisat::Lit> = all_versions_by_p
-                    .get(&p1)
+                    .get(p1)
                     .unwrap_or(&empty_vec)
                     .iter()
                     .filter(|(v1, _)| range.contains(v1))