diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index 6ef8e893..c4d82e20 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 use std::cell::RefCell; -use std::error::Error; use pubgrub::package::Package; use pubgrub::range::Range; @@ -37,7 +36,7 @@ impl> DependencyProvid &self, package: &P, version: &VS::V, - ) -> Result, Box> { + ) -> Result, DP::Err> { let mut cache = self.cached_dependencies.borrow_mut(); match cache.get_dependencies(package, version) { Ok(Dependencies::Unknown) => { @@ -55,16 +54,12 @@ impl> DependencyProvid error @ Err(_) => error, } } - dependencies @ Ok(_) => dependencies, - error @ Err(_) => error, + Ok(dependencies) => Ok(dependencies), + Err(_) => unreachable!(), } } - fn choose_version( - &self, - package: &P, - range: &VS, - ) -> Result, Box> { + fn choose_version(&self, package: &P, range: &VS) -> Result, DP::Err> { self.remote_dependencies.choose_version(package, range) } @@ -73,6 +68,8 @@ impl> DependencyProvid fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { self.remote_dependencies.prioritize(package, range) } + + type Err = DP::Err; } fn main() { diff --git a/src/error.rs b/src/error.rs index 15a410e3..82e07419 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,7 +10,7 @@ use crate::version_set::VersionSet; /// Errors that may occur while solving dependencies. #[derive(Error, Debug)] -pub enum PubGrubError { +pub enum PubGrubError { /// There is no solution for this set of dependencies. #[error("No solution")] NoSolution(DerivationTree), @@ -27,7 +27,7 @@ pub enum PubGrubError { version: VS::V, /// Error raised by the implementer of /// [DependencyProvider](crate::solver::DependencyProvider). - source: Box, + source: E, }, /// Error arising when the implementer of @@ -48,12 +48,12 @@ pub enum PubGrubError { /// returned an error in the method /// [choose_version](crate::solver::DependencyProvider::choose_version). #[error("Decision making failed")] - ErrorChoosingPackageVersion(Box), + ErrorChoosingPackageVersion(E), /// Error arising when the implementer of [DependencyProvider](crate::solver::DependencyProvider) /// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel). #[error("We should cancel")] - ErrorInShouldCancel(Box), + ErrorInShouldCancel(E), /// Something unexpected happened. #[error("{0}")] diff --git a/src/internal/core.rs b/src/internal/core.rs index 06e3ae21..03915663 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -4,6 +4,7 @@ //! to write a functional PubGrub algorithm. use std::collections::HashSet as Set; +use std::error::Error; use crate::error::PubGrubError; use crate::internal::arena::Arena; @@ -92,7 +93,7 @@ impl State { /// Unit propagation is the core mechanism of the solving algorithm. /// CF - pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError> { + pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError> { self.unit_propagation_buffer.clear(); self.unit_propagation_buffer.push(package); while let Some(current_package) = self.unit_propagation_buffer.pop() { @@ -154,10 +155,11 @@ impl State { /// Return the root cause and the backtracked model. /// CF - fn conflict_resolution( + #[allow(clippy::type_complexity)] + fn conflict_resolution( &mut self, incompatibility: IncompId, - ) -> Result<(P, IncompId), PubGrubError> { + ) -> Result<(P, IncompId), PubGrubError> { let mut current_incompat_id = incompatibility; let mut current_incompat_changed = false; loop { diff --git a/src/lib.rs b/src/lib.rs index 5f61fb51..11c20e80 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,13 +83,14 @@ //! # use pubgrub::type_aliases::Map; //! # use std::error::Error; //! # use std::borrow::Borrow; +//! # use std::convert::Infallible; //! # //! # struct MyDependencyProvider; //! # //! type SemVS = Range; //! //! impl DependencyProvider for MyDependencyProvider { -//! fn choose_version(&self, package: &String, range: &SemVS) -> Result, Box> { +//! fn choose_version(&self, package: &String, range: &SemVS) -> Result, Infallible> { //! unimplemented!() //! } //! @@ -102,9 +103,11 @@ //! &self, //! package: &String, //! version: &SemanticVersion, -//! ) -> Result, Box> { +//! ) -> Result, Infallible> { //! unimplemented!() //! } +//! +//! type Err = Infallible; //! } //! ``` //! diff --git a/src/solver.rs b/src/solver.rs index a1e5e06c..9ac2edfe 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -41,6 +41,7 @@ //! ## API //! //! ``` +//! # use std::convert::Infallible; //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; //! # use pubgrub::version::NumberVersion; //! # use pubgrub::error::PubGrubError; @@ -48,7 +49,7 @@ //! # //! # type NumVS = Range; //! # -//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS>> { +//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS, Infallible>> { //! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let package = "root"; //! # let version = 1; @@ -70,6 +71,7 @@ use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet as Set}; +use std::convert::Infallible; use std::error::Error; use crate::error::PubGrubError; @@ -82,11 +84,12 @@ use log::{debug, info}; /// Main function of the library. /// Finds a set of packages satisfying dependency bounds for a given package + version pair. -pub fn resolve( - dependency_provider: &impl DependencyProvider, +#[allow(clippy::type_complexity)] +pub fn resolve>( + dependency_provider: &DP, package: P, version: impl Into, -) -> Result, PubGrubError> { +) -> Result, PubGrubError> { let mut state = State::init(package.clone(), version.into()); let mut added_dependencies: Map> = Map::default(); let mut next = package; @@ -133,7 +136,7 @@ pub fn resolve( }; if !term_intersection.contains(&v) { - return Err(PubGrubError::ErrorChoosingPackageVersion( + return Err(PubGrubError::Failure( "choose_package_version picked an incompatible version".into(), )); } @@ -240,14 +243,15 @@ pub trait DependencyProvider { /// the fewest versions that match the outstanding constraint. type Priority: Ord + Clone; + /// The kind of error returned from these methods. + /// + /// Returning this signals that resolution should fail with this error. + type Err: Error; + /// Once the resolver has found the highest `Priority` package from all potential valid /// packages, it needs to know what vertion of that package to use. The most common pattern /// is to select the largest vertion that the range contains. - fn choose_version( - &self, - package: &P, - range: &VS, - ) -> Result, Box>; + fn choose_version(&self, package: &P, range: &VS) -> Result, Self::Err>; /// Retrieves the package dependencies. /// Return [Dependencies::Unknown] if its dependencies are unknown. @@ -255,14 +259,14 @@ pub trait DependencyProvider { &self, package: &P, version: &VS::V, - ) -> Result, Box>; + ) -> Result, Self::Err>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. /// This is helpful if you want to add some form of early termination like a timeout, /// or you want to add some form of user feedback if things are taking a while. /// If not provided the resolver will run as long as needed. - fn should_cancel(&self) -> Result<(), Box> { + fn should_cancel(&self) -> Result<(), Self::Err> { Ok(()) } } @@ -340,11 +344,9 @@ impl OfflineDependencyProvider { /// But, that may change in new versions if better heuristics are found. /// Versions are picked with the newest versions first. impl DependencyProvider for OfflineDependencyProvider { - fn choose_version( - &self, - package: &P, - range: &VS, - ) -> Result, Box> { + type Err = Infallible; + + fn choose_version(&self, package: &P, range: &VS) -> Result, Infallible> { Ok(self .dependencies .get(package) @@ -365,7 +367,7 @@ impl DependencyProvider for OfflineDependency &self, package: &P, version: &VS::V, - ) -> Result, Box> { + ) -> Result, Infallible> { Ok(match self.dependencies(package, version) { None => Dependencies::Unknown, Some(dependencies) => Dependencies::Known(dependencies), diff --git a/tests/proptest.rs b/tests/proptest.rs index 017efed0..d73567a7 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 -use std::{collections::BTreeSet as Set, error::Error}; +use std::collections::BTreeSet as Set; +use std::convert::Infallible; use pubgrub::error::PubGrubError; use pubgrub::package::Package; @@ -30,19 +31,11 @@ struct OldestVersionsDependencyProvider( impl DependencyProvider for OldestVersionsDependencyProvider { - fn get_dependencies( - &self, - p: &P, - v: &VS::V, - ) -> Result, Box> { + fn get_dependencies(&self, p: &P, v: &VS::V) -> Result, Infallible> { self.0.get_dependencies(p, v) } - fn choose_version( - &self, - package: &P, - range: &VS, - ) -> Result, Box> { + fn choose_version(&self, package: &P, range: &VS) -> Result, Infallible> { Ok(self .0 .versions(package) @@ -57,6 +50,8 @@ impl DependencyProvider fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { self.0.prioritize(package, range) } + + type Err = Infallible; } /// The same as DP but it has a timeout. @@ -82,15 +77,11 @@ impl TimeoutDependencyProvider { impl> DependencyProvider for TimeoutDependencyProvider { - fn get_dependencies( - &self, - p: &P, - v: &VS::V, - ) -> Result, Box> { + fn get_dependencies(&self, p: &P, v: &VS::V) -> Result, DP::Err> { self.dp.get_dependencies(p, v) } - fn should_cancel(&self) -> Result<(), Box> { + fn should_cancel(&self) -> Result<(), DP::Err> { assert!(self.start_time.elapsed().as_secs() < 60); let calls = self.call_count.get(); assert!(calls < self.max_calls); @@ -98,11 +89,7 @@ impl> DependencyProvid Ok(()) } - fn choose_version( - &self, - package: &P, - range: &VS, - ) -> Result, Box> { + fn choose_version(&self, package: &P, range: &VS) -> Result, DP::Err> { self.dp.choose_version(package, range) } @@ -111,13 +98,15 @@ impl> DependencyProvid fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { self.dp.prioritize(package, range) } + + type Err = DP::Err; } fn timeout_resolve>( dependency_provider: DP, name: P, version: impl Into, -) -> Result, PubGrubError> { +) -> Result, PubGrubError> { resolve( &TimeoutDependencyProvider::new(dependency_provider, 50_000), name, diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index 2bfb21e9..8aecefe9 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 +use std::convert::Infallible; + use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider}; @@ -135,7 +137,7 @@ impl SatResolve { pub fn check_resolve( &mut self, - res: &Result, PubGrubError>, + res: &Result, PubGrubError>, p: &P, v: &VS::V, ) {