Cleanup internal imports (#243)

* Cleanup internal imports

Imports now only come from two modules: `crate`, which contains all public symbols, and `crate::internal`, which contains all the internal symbols. I'm surprised the split works out so cleanly.

All internal apis are now `pub(crate)`. Some of these will become public again in the uv fork.

I used nightly rustfmt to group the imports.

* Fix more lints

* typo

* typo

* And in the test, too

* Revert internal api abstraction

* Revert "Revert internal api abstraction"

This reverts commit dd9b290.

Author: konstin

benches/large_case.rs

@@ -1,16 +1,11 @@
 // SPDX-License-Identifier: MPL-2.0
 use std::time::Duration;
 
-extern crate criterion;
-use self::criterion::*;
-
-use pubgrub::package::Package;
-use pubgrub::range::Range;
-use pubgrub::solver::{resolve, OfflineDependencyProvider};
-use pubgrub::version::SemanticVersion;
-use pubgrub::version_set::VersionSet;
+use criterion::*;
 use serde::de::Deserialize;
 
+use pubgrub::{resolve, OfflineDependencyProvider, Package, Range, SemanticVersion, VersionSet};
+
 fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>(
     b: &mut Bencher,
     case: &'a str,

examples/unsat_root_message_no_version.rs

@@ -1,12 +1,12 @@
 // SPDX-License-Identifier: MPL-2.0
 
+use std::fmt::{self, Display};
+
 use pubgrub::{
-    resolve, Derived, OfflineDependencyProvider, PubGrubError, Range, Reporter, SemanticVersion,
+    resolve, DefaultStringReporter, Derived, External, Map, OfflineDependencyProvider,
+    PubGrubError, Range, ReportFormatter, Reporter, SemanticVersion, Term,
 };
 
-use pubgrub::{DefaultStringReporter, External, Map, ReportFormatter, Term};
-use std::fmt::{self, Display};
-
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum Package {
     Root,

src/error.rs

@@ -4,8 +4,7 @@
 
 use thiserror::Error;
 
-use crate::report::DerivationTree;
-use crate::solver::DependencyProvider;
+use crate::{DependencyProvider, DerivationTree};
 
 /// There is no solution for this set of dependencies.
 pub type NoSolutionError<DP> = DerivationTree<
@@ -21,10 +20,8 @@ pub enum PubGrubError<DP: DependencyProvider> {
     #[error("No solution")]
     NoSolution(NoSolutionError<DP>),
 
-    /// Error arising when the implementer of
-    /// [DependencyProvider]
-    /// returned an error in the method
-    /// [get_dependencies](crate::solver::DependencyProvider::get_dependencies).
+    /// Error arising when the implementer of [DependencyProvider] returned an error in the method
+    /// [get_dependencies](DependencyProvider::get_dependencies).
     #[error("Retrieving dependencies of {package} {version} failed")]
     ErrorRetrievingDependencies {
         /// Package whose dependencies we want.
@@ -49,15 +46,13 @@ pub enum PubGrubError<DP: DependencyProvider> {
         version: DP::V,
     },
 
-    /// Error arising when the implementer of
-    /// [DependencyProvider]
-    /// returned an error in the method
-    /// [choose_version](crate::solver::DependencyProvider::choose_version).
+    /// Error arising when the implementer of [DependencyProvider] returned an error in the method
+    /// [choose_version](DependencyProvider::choose_version).
     #[error("Decision making failed")]
     ErrorChoosingPackageVersion(#[source] DP::Err),
 
     /// Error arising when the implementer of [DependencyProvider]
-    /// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel).
+    /// returned an error in the method [should_cancel](DependencyProvider::should_cancel).
     #[error("We should cancel")]
     ErrorInShouldCancel(#[source] DP::Err),
 

src/internal/arena.rs

@@ -10,7 +10,7 @@ use std::ops::{Index, Range};
 /// that we actually don't need since it is phantom.
 ///
 /// <https://github.com/rust-lang/rust/issues/26925>
-pub struct Id<T> {
+pub(crate) struct Id<T> {
     raw: u32,
     _ty: PhantomData<fn() -> T>,
 }
@@ -48,7 +48,7 @@ impl<T> fmt::Debug for Id<T> {
 }
 
 impl<T> Id<T> {
-    pub fn into_raw(self) -> usize {
+    pub(crate) fn into_raw(self) -> usize {
         self.raw as usize
     }
     fn from(n: u32) -> Self {
@@ -57,7 +57,7 @@ impl<T> Id<T> {
             _ty: PhantomData,
         }
     }
-    pub fn range_to_iter(range: Range<Self>) -> impl Iterator<Item = Self> {
+    pub(crate) fn range_to_iter(range: Range<Self>) -> impl Iterator<Item = Self> {
         let start = range.start.raw;
         let end = range.end.raw;
         (start..end).map(Self::from)
@@ -71,7 +71,7 @@ impl<T> Id<T> {
 /// to have references between those items.
 /// They are all dropped at once when the arena is dropped.
 #[derive(Clone, PartialEq, Eq)]
-pub struct Arena<T> {
+pub(crate) struct Arena<T> {
     data: Vec<T>,
 }
 
@@ -91,17 +91,17 @@ impl<T> Default for Arena<T> {
 }
 
 impl<T> Arena<T> {
-    pub fn new() -> Self {
+    pub(crate) fn new() -> Self {
         Self { data: Vec::new() }
     }
 
-    pub fn alloc(&mut self, value: T) -> Id<T> {
+    pub(crate) fn alloc(&mut self, value: T) -> Id<T> {
         let raw = self.data.len();
         self.data.push(value);
         Id::from(raw as u32)
     }
 
-    pub fn alloc_iter<I: Iterator<Item = T>>(&mut self, values: I) -> Range<Id<T>> {
+    pub(crate) fn alloc_iter<I: Iterator<Item = T>>(&mut self, values: I) -> Range<Id<T>> {
         let start = Id::from(self.data.len() as u32);
         values.for_each(|v| {
             self.alloc(v);

src/internal/core.rs

@@ -6,22 +6,15 @@
 use std::collections::HashSet as Set;
 use std::sync::Arc;
 
-use crate::error::NoSolutionError;
-use crate::internal::arena::Arena;
-use crate::internal::incompatibility::{Incompatibility, Relation};
-use crate::internal::partial_solution::SatisfierSearch::{
-    DifferentDecisionLevels, SameDecisionLevels,
+use crate::internal::{
+    Arena, DecisionLevel, IncompDpId, Incompatibility, PartialSolution, Relation, SatisfierSearch,
+    SmallVec,
 };
-use crate::internal::partial_solution::{DecisionLevel, PartialSolution};
-use crate::internal::small_vec::SmallVec;
-use crate::report::DerivationTree;
-use crate::solver::DependencyProvider;
-use crate::type_aliases::{IncompDpId, Map};
-use crate::version_set::VersionSet;
+use crate::{DependencyProvider, DerivationTree, Map, NoSolutionError, VersionSet};
 
 /// Current state of the PubGrub algorithm.
 #[derive(Clone)]
-pub struct State<DP: DependencyProvider> {
+pub(crate) struct State<DP: DependencyProvider> {
     root_package: DP::P,
     root_version: DP::V,
 
@@ -40,10 +33,10 @@ pub struct State<DP: DependencyProvider> {
 
     /// Partial solution.
     /// TODO: remove pub.
-    pub partial_solution: PartialSolution<DP>,
+    pub(crate) partial_solution: PartialSolution<DP>,
 
     /// The store is the reference storage for all incompatibilities.
-    pub incompatibility_store: Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
+    pub(crate) incompatibility_store: Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
 
     /// This is a stack of work to be done in `unit_propagation`.
     /// It can definitely be a local variable to that method, but
@@ -53,7 +46,7 @@ pub struct State<DP: DependencyProvider> {
 
 impl<DP: DependencyProvider> State<DP> {
     /// Initialization of PubGrub state.
-    pub fn init(root_package: DP::P, root_version: DP::V) -> Self {
+    pub(crate) 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(),
@@ -74,13 +67,13 @@ impl<DP: DependencyProvider> State<DP> {
     }
 
     /// Add an incompatibility to the state.
-    pub fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS, DP::M>) {
+    pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS, DP::M>) {
         let id = self.incompatibility_store.alloc(incompat);
         self.merge_incompatibility(id);
     }
 
     /// Add an incompatibility to the state.
-    pub fn add_incompatibility_from_dependencies(
+    pub(crate) fn add_incompatibility_from_dependencies(
         &mut self,
         package: DP::P,
         version: DP::V,
@@ -105,7 +98,7 @@ impl<DP: DependencyProvider> State<DP> {
 
     /// 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(&mut self, package: DP::P) -> Result<(), NoSolutionError<DP>> {
+    pub(crate) fn unit_propagation(&mut self, package: DP::P) -> Result<(), NoSolutionError<DP>> {
         self.unit_propagation_buffer.clear();
         self.unit_propagation_buffer.push(package);
         while let Some(current_package) = self.unit_propagation_buffer.pop() {
@@ -202,7 +195,7 @@ impl<DP: DependencyProvider> State<DP> {
                     &self.incompatibility_store,
                 );
                 match satisfier_search_result {
-                    DifferentDecisionLevels {
+                    SatisfierSearch::DifferentDecisionLevels {
                         previous_satisfier_level,
                     } => {
                         let package = package.clone();
@@ -214,7 +207,7 @@ impl<DP: DependencyProvider> State<DP> {
                         log::info!("backtrack to {:?}", previous_satisfier_level);
                         return Ok((package, current_incompat_id));
                     }
-                    SameDecisionLevels { satisfier_cause } => {
+                    SatisfierSearch::SameDecisionLevels { satisfier_cause } => {
                         let prior_cause = Incompatibility::prior_cause(
                             current_incompat_id,
                             satisfier_cause,
@@ -248,10 +241,10 @@ impl<DP: DependencyProvider> State<DP> {
 
     /// Add this incompatibility into the set of all incompatibilities.
     ///
-    /// Pub collapses identical dependencies from adjacent package versions
+    /// PubGrub collapses identical dependencies from adjacent package versions
     /// into individual incompatibilities.
     /// This substantially reduces the total number of incompatibilities
-    /// and makes it much easier for Pub to reason about multiple versions of packages at once.
+    /// and makes it much easier for PubGrub to reason about multiple versions of packages at once.
     ///
     /// For example, rather than representing
     /// foo 1.0.0 depends on bar ^1.0.0 and