feat!: associated types instead of generics for P and VS (pubgrub-rs#190)

* feat!: associated types instead of generics for P and VS

* mabe V as well?

* more source

* more refs to Version trait

Author: Eh2406

benches/large_case.rs

@@ -17,7 +17,7 @@ fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>(
 ) where
     <VS as VersionSet>::V: Deserialize<'a>,
 {
-    let dependency_provider: OfflineDependencyProvider<P, VS> = ron::de::from_str(&case).unwrap();
+    let dependency_provider: OfflineDependencyProvider<P, VS> = ron::de::from_str(case).unwrap();
 
     b.iter(|| {
         for p in dependency_provider.packages() {

examples/caching_dependency_provider.rs

@@ -2,23 +2,19 @@
 
 use std::cell::RefCell;
 
-use pubgrub::package::Package;
 use pubgrub::range::Range;
 use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider};
-use pubgrub::version_set::VersionSet;
 
 type NumVS = Range<u32>;
 
 // An example implementing caching dependency provider that will
 // store queried dependencies in memory and check them before querying more from remote.
-struct CachingDependencyProvider<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> {
+struct CachingDependencyProvider<DP: DependencyProvider> {
     remote_dependencies: DP,
-    cached_dependencies: RefCell<OfflineDependencyProvider<P, VS>>,
+    cached_dependencies: RefCell<OfflineDependencyProvider<DP::P, DP::VS>>,
 }
 
-impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>
-    CachingDependencyProvider<P, VS, DP>
-{
+impl<DP: DependencyProvider> CachingDependencyProvider<DP> {
     pub fn new(remote_dependencies_provider: DP) -> Self {
         CachingDependencyProvider {
             remote_dependencies: remote_dependencies_provider,
@@ -27,15 +23,13 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>
     }
 }
 
-impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvider<P, VS>
-    for CachingDependencyProvider<P, VS, DP>
-{
+impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP> {
     // Caches dependencies if they were already queried
     fn get_dependencies(
         &self,
-        package: &P,
-        version: &VS::V,
-    ) -> Result<Dependencies<P, VS>, DP::Err> {
+        package: &DP::P,
+        version: &DP::V,
+    ) -> Result<Dependencies<DP::P, DP::VS>, DP::Err> {
         let mut cache = self.cached_dependencies.borrow_mut();
         match cache.get_dependencies(package, version) {
             Ok(Dependencies::Unknown) => {
@@ -58,17 +52,21 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
         }
     }
 
-    fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, DP::Err> {
+    fn choose_version(&self, package: &DP::P, range: &DP::VS) -> Result<Option<DP::V>, DP::Err> {
         self.remote_dependencies.choose_version(package, range)
     }
 
     type Priority = DP::Priority;
 
-    fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
+    fn prioritize(&self, package: &DP::P, range: &DP::VS) -> Self::Priority {
         self.remote_dependencies.prioritize(package, range)
     }
 
     type Err = DP::Err;
+
+    type P = DP::P;
+    type V = DP::V;
+    type VS = DP::VS;
 }
 
 fn main() {

src/error.rs

@@ -4,58 +4,94 @@
 
 use thiserror::Error;
 
-use crate::package::Package;
 use crate::report::DerivationTree;
-use crate::version_set::VersionSet;
+use crate::solver::DependencyProvider;
 
 /// Errors that may occur while solving dependencies.
-#[derive(Error, Debug)]
-pub enum PubGrubError<P: Package, VS: VersionSet, E: std::error::Error> {
+#[derive(Error)]
+pub enum PubGrubError<DP>
+where
+    DP: DependencyProvider,
+{
     /// There is no solution for this set of dependencies.
     #[error("No solution")]
-    NoSolution(DerivationTree<P, VS>),
+    NoSolution(DerivationTree<DP::P, DP::VS>),
 
     /// Error arising when the implementer of
-    /// [DependencyProvider](crate::solver::DependencyProvider)
+    /// [DependencyProvider]
     /// returned an error in the method
     /// [get_dependencies](crate::solver::DependencyProvider::get_dependencies).
     #[error("Retrieving dependencies of {package} {version} failed")]
     ErrorRetrievingDependencies {
         /// Package whose dependencies we want.
-        package: P,
+        package: DP::P,
         /// Version of the package for which we want the dependencies.
-        version: VS::V,
+        version: DP::V,
         /// Error raised by the implementer of
-        /// [DependencyProvider](crate::solver::DependencyProvider).
-        source: E,
+        /// [DependencyProvider].
+        source: DP::Err,
     },
 
     /// Error arising when the implementer of
-    /// [DependencyProvider](crate::solver::DependencyProvider)
+    /// [DependencyProvider]
     /// returned a dependency on the requested package.
     /// This technically means that the package directly depends on itself,
     /// and is clearly some kind of mistake.
     #[error("{package} {version} depends on itself")]
     SelfDependency {
         /// Package whose dependencies we want.
-        package: P,
+        package: DP::P,
         /// Version of the package for which we want the dependencies.
-        version: VS::V,
+        version: DP::V,
     },
 
     /// Error arising when the implementer of
-    /// [DependencyProvider](crate::solver::DependencyProvider)
+    /// [DependencyProvider]
     /// returned an error in the method
     /// [choose_version](crate::solver::DependencyProvider::choose_version).
     #[error("Decision making failed")]
-    ErrorChoosingPackageVersion(E),
+    ErrorChoosingPackageVersion(#[source] DP::Err),
 
-    /// Error arising when the implementer of [DependencyProvider](crate::solver::DependencyProvider)
+    /// Error arising when the implementer of [DependencyProvider]
     /// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel).
     #[error("We should cancel")]
-    ErrorInShouldCancel(E),
+    ErrorInShouldCancel(#[source] DP::Err),
 
     /// Something unexpected happened.
     #[error("{0}")]
     Failure(String),
 }
+
+impl<DP> std::fmt::Debug for PubGrubError<DP>
+where
+    DP: DependencyProvider,
+{
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::NoSolution(arg0) => f.debug_tuple("NoSolution").field(arg0).finish(),
+            Self::ErrorRetrievingDependencies {
+                package,
+                version,
+                source,
+            } => f
+                .debug_struct("ErrorRetrievingDependencies")
+                .field("package", package)
+                .field("version", version)
+                .field("source", source)
+                .finish(),
+            Self::SelfDependency { package, version } => f
+                .debug_struct("SelfDependency")
+                .field("package", package)
+                .field("version", version)
+                .finish(),
+            Self::ErrorChoosingPackageVersion(arg0) => f
+                .debug_tuple("ErrorChoosingPackageVersion")
+                .field(arg0)
+                .finish(),
+            Self::ErrorInShouldCancel(arg0) => {
+                f.debug_tuple("ErrorInShouldCancel").field(arg0).finish()
+            }
+            Self::Failure(arg0) => f.debug_tuple("Failure").field(arg0).finish(),
+        }
+    }
+}

src/internal/core.rs

@@ -3,55 +3,57 @@
 //! Core model and functions
 //! to write a functional PubGrub algorithm.
 
-use std::error::Error;
+use std::collections::HashSet as Set;
 use std::sync::Arc;
 
 use crate::error::PubGrubError;
 use crate::internal::arena::Arena;
-use crate::internal::incompatibility::{IncompId, Incompatibility, Relation};
+use crate::internal::incompatibility::{Incompatibility, Relation};
 use crate::internal::partial_solution::SatisfierSearch::{
     DifferentDecisionLevels, SameDecisionLevels,
 };
 use crate::internal::partial_solution::{DecisionLevel, PartialSolution};
 use crate::internal::small_vec::SmallVec;
-use crate::package::Package;
 use crate::report::DerivationTree;
-use crate::type_aliases::{DependencyConstraints, Map, Set};
+use crate::solver::DependencyProvider;
+use crate::type_aliases::{DependencyConstraints, IncompDpId, Map};
 use crate::version_set::VersionSet;
 
 /// Current state of the PubGrub algorithm.
 #[derive(Clone)]
-pub struct State<P: Package, VS: VersionSet, Priority: Ord + Clone> {
-    root_package: P,
-    root_version: VS::V,
+pub struct State<DP: DependencyProvider> {
+    root_package: DP::P,
+    root_version: DP::V,
 
-    incompatibilities: Map<P, Vec<IncompId<P, VS>>>,
+    #[allow(clippy::type_complexity)]
+    incompatibilities: Map<DP::P, Vec<IncompDpId<DP>>>,
 
     /// Store the ids of incompatibilities that are already contradicted.
     /// For each one keep track of the decision level when it was found to be contradicted.
     /// These will stay contradicted until we have backtracked beyond its associated decision level.
-    contradicted_incompatibilities: Map<IncompId<P, VS>, DecisionLevel>,
+    contradicted_incompatibilities: Map<IncompDpId<DP>, DecisionLevel>,
 
     /// All incompatibilities expressing dependencies,
     /// with common dependents merged.
-    merged_dependencies: Map<(P, P), SmallVec<IncompId<P, VS>>>,
+    #[allow(clippy::type_complexity)]
+    merged_dependencies: Map<(DP::P, DP::P), SmallVec<IncompDpId<DP>>>,
 
     /// Partial solution.
     /// TODO: remove pub.
-    pub partial_solution: PartialSolution<P, VS, Priority>,
+    pub partial_solution: PartialSolution<DP>,
 
     /// The store is the reference storage for all incompatibilities.
-    pub incompatibility_store: Arena<Incompatibility<P, VS>>,
+    pub incompatibility_store: Arena<Incompatibility<DP::P, DP::VS>>,
 
     /// This is a stack of work to be done in `unit_propagation`.
     /// It can definitely be a local variable to that method, but
     /// this way we can reuse the same allocation for better performance.
-    unit_propagation_buffer: SmallVec<P>,
+    unit_propagation_buffer: SmallVec<DP::P>,
 }
 
-impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
+impl<DP: DependencyProvider> State<DP> {
     /// Initialization of PubGrub state.
-    pub fn init(root_package: P, root_version: VS::V) -> Self {
+    pub fn init(root_package: DP::P, root_version: DP::V) -> Self {
         let mut incompatibility_store = Arena::new();
         let not_root_id = incompatibility_store.alloc(Incompatibility::not_root(
             root_package.clone(),
@@ -72,38 +74,38 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
     }
 
     /// Add an incompatibility to the state.
-    pub fn add_incompatibility(&mut self, incompat: Incompatibility<P, VS>) {
+    pub fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS>) {
         let id = self.incompatibility_store.alloc(incompat);
         self.merge_incompatibility(id);
     }
 
     /// Add an incompatibility to the state.
     pub fn add_incompatibility_from_dependencies(
         &mut self,
-        package: P,
-        version: VS::V,
-        deps: &DependencyConstraints<P, VS>,
-    ) -> std::ops::Range<IncompId<P, VS>> {
+        package: DP::P,
+        version: DP::V,
+        deps: &DependencyConstraints<DP::P, DP::VS>,
+    ) -> std::ops::Range<IncompDpId<DP>> {
         // Create incompatibilities and allocate them in the store.
         let new_incompats_id_range =
             self.incompatibility_store
                 .alloc_iter(deps.iter().map(|dep| {
                     Incompatibility::from_dependency(
                         package.clone(),
-                        VS::singleton(version.clone()),
+                        <DP::VS as VersionSet>::singleton(version.clone()),
                         dep,
                     )
                 }));
         // Merge the newly created incompatibilities with the older ones.
-        for id in IncompId::range_to_iter(new_incompats_id_range.clone()) {
+        for id in IncompDpId::<DP>::range_to_iter(new_incompats_id_range.clone()) {
             self.merge_incompatibility(id);
         }
         new_incompats_id_range
     }
 
     /// Unit propagation is the core mechanism of the solving algorithm.
     /// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
-    pub fn unit_propagation<E: Error>(&mut self, package: P) -> Result<(), PubGrubError<P, VS, E>> {
+    pub fn unit_propagation(&mut self, package: DP::P) -> Result<(), PubGrubError<DP>> {
         self.unit_propagation_buffer.clear();
         self.unit_propagation_buffer.push(package);
         while let Some(current_package) = self.unit_propagation_buffer.pop() {
@@ -179,10 +181,10 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
     /// Return the root cause and the backtracked model.
     /// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
     #[allow(clippy::type_complexity)]
-    fn conflict_resolution<E: Error>(
+    fn conflict_resolution(
         &mut self,
-        incompatibility: IncompId<P, VS>,
-    ) -> Result<(P, IncompId<P, VS>), PubGrubError<P, VS, E>> {
+        incompatibility: IncompDpId<DP>,
+    ) -> Result<(DP::P, IncompDpId<DP>), PubGrubError<DP>> {
         let mut current_incompat_id = incompatibility;
         let mut current_incompat_changed = false;
         loop {
@@ -229,7 +231,7 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
     /// Backtracking.
     fn backtrack(
         &mut self,
-        incompat: IncompId<P, VS>,
+        incompat: IncompDpId<DP>,
         incompat_changed: bool,
         decision_level: DecisionLevel,
     ) {
@@ -257,7 +259,7 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
     /// (provided that no other version of foo exists between 1.0.0 and 2.0.0).
     /// We could collapse them into { foo (1.0.0 ∪ 1.1.0), not bar ^1.0.0 }
     /// without having to check the existence of other versions though.
-    fn merge_incompatibility(&mut self, mut id: IncompId<P, VS>) {
+    fn merge_incompatibility(&mut self, mut id: IncompDpId<DP>) {
         if let Some((p1, p2)) = self.incompatibility_store[id].as_dependency() {
             // If we are a dependency, there's a good chance we can be merged with a previous dependency
             let deps_lookup = self
@@ -295,8 +297,8 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
 
     // Error reporting #########################################################
 
-    fn build_derivation_tree(&self, incompat: IncompId<P, VS>) -> DerivationTree<P, VS> {
-        let mut all_ids = Set::default();
+    fn build_derivation_tree(&self, incompat: IncompDpId<DP>) -> DerivationTree<DP::P, DP::VS> {
+        let mut all_ids: Set<IncompDpId<DP>> = Set::default();
         let mut shared_ids = Set::default();
         let mut stack = vec![incompat];
         while let Some(i) = stack.pop() {

src/internal/incompatibility.rs

@@ -161,7 +161,7 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
                 .unwrap()
                 .unwrap_positive()
                 .union(other.get(p1).unwrap().unwrap_positive()), // It is safe to `simplify` here
-            (&p2, dep_term.map_or(&VS::empty(), |v| v.unwrap_negative())),
+            (p2, dep_term.map_or(&VS::empty(), |v| v.unwrap_negative())),
         ));
     }
 
@@ -310,7 +310,6 @@ pub mod tests {
     use super::*;
     use crate::range::Range;
     use crate::term::tests::strategy as term_strat;
-    use crate::type_aliases::Map;
     use proptest::prelude::*;
 
     proptest! {