Improve documentation to prepare for 0.3 release (#315)

* Improve `VersionSet` documentation

* Improve report documentation

* Review

* Improve version set contract

* Rebase fix

src/report.rs

@@ -26,8 +26,7 @@ pub trait Reporter<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display>
     ) -> Self::Output;
 }
 
-/// Derivation tree resulting in the impossibility
-/// to solve the dependencies of our root package.
+/// Derivation tree resulting in the impossibility to solve the dependencies of our root package.
 #[derive(Debug, Clone)]
 pub enum DerivationTree<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     /// External incompatibility.
@@ -36,8 +35,7 @@ pub enum DerivationTree<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Disp
     Derived(Derived<P, VS, M>),
 }
 
-/// Incompatibilities that are not derived from others,
-/// they have their own reason.
+/// Incompatibility that is not derived from other incompatibilities.
 #[derive(Debug, Clone)]
 pub enum External<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     /// Initial incompatibility aiming at picking the root package for the first decision.
@@ -55,11 +53,10 @@ pub enum External<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
 pub struct Derived<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     /// Terms of the incompatibility.
     pub terms: Map<P, Term<VS>>,
-    /// Indicate if that incompatibility is present multiple times
-    /// in the derivation tree.
-    /// If that is the case, it has a unique id, provided in that option.
-    /// Then, we may want to only explain it once,
-    /// and refer to the explanation for the other times.
+    /// Indicate if the incompatibility is present multiple times in the derivation tree.
+    ///
+    /// If that is the case, the number is a unique id. This can be used to only explain this
+    /// incompatibility once, then refer to the explanation for the other times.
     pub shared_id: Option<usize>,
     /// First cause.
     pub cause1: Arc<DerivationTree<P, VS, M>>,

src/solver.rs

@@ -279,6 +279,13 @@ pub trait DependencyProvider {
     /// A common choice is [`Ranges`][version_ranges::Ranges].
     type VS: VersionSet<V = Self::V>;
 
+    /// The type returned from `prioritize`. The resolver does not care what type this is
+    /// as long as it can pick a largest one and clone it.
+    ///
+    /// [`Reverse`](std::cmp::Reverse) can be useful if you want to pick the package with
+    /// the fewest versions that match the outstanding constraint.
+    type Priority: Ord + Clone;
+
     /// Type for custom incompatibilities.
     ///
     /// There are reasons in user code outside pubgrub that can cause packages or versions
@@ -291,13 +298,6 @@ pub trait DependencyProvider {
     /// assign [`String`] as placeholder.
     type M: Eq + Clone + Debug + Display;
 
-    /// The type returned from `prioritize`. The resolver does not care what type this is
-    /// as long as it can pick a largest one and clone it.
-    ///
-    /// [`Reverse`](std::cmp::Reverse) can be useful if you want to pick the package with
-    /// 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.

src/version_set.rs

@@ -1,59 +1,67 @@
 // SPDX-License-Identifier: MPL-2.0
 
-//! As its name suggests, the [VersionSet] trait describes sets of versions.
-//!
-//! One needs to define
-//! - the associate type for versions,
-//! - two constructors for the empty set and a singleton set,
-//! - the complement and intersection set operations,
-//! - and a function to evaluate membership of versions.
-//!
-//! Two functions are automatically derived, thanks to the mathematical properties of sets.
-//! You can overwrite those implementations, but we highly recommend that you don't,
-//! except if you are confident in a correct implementation that brings much performance gains.
-//!
-//! It is also extremely important that the `Eq` trait is correctly implemented.
-//! In particular, you can only use `#[derive(Eq, PartialEq)]` if `Eq` is strictly equivalent to the
-//! structural equality, i.e. if version sets have canonical representations.
-//! Such problems may arise if your implementations of `complement()` and `intersection()` do not
-//! return canonical representations so be careful there.
-
 use std::fmt::{Debug, Display};
 
 use crate::Ranges;
 
-/// Trait describing sets of versions.
+/// A set of versions.
+///
+/// See [`Ranges`] for an implementation.
+///
+/// The methods with default implementations can be overwritten for better performance, but their
+/// output must be equal to the default implementation.
+///
+/// # Equality
+///
+/// It is important that the `Eq` trait is implemented so that if two sets contain the same
+/// versions, they are equal under `Eq`. In particular, you can only use `#[derive(Eq, PartialEq)]`
+/// if `Eq` is strictly equivalent to the structural equality, i.e. if version sets are always
+/// stored in a canonical representations. Such problems may arise if your implementations of
+/// `complement()` and `intersection()` do not return canonical representations.
+///
+/// For example, `>=1,<4 || >=2,<5` and `>=1,<4 || >=3,<5` are equal, because they can both be
+/// normalized to `>=1,<5`.
+///
+/// Note that pubgrub does not know which versions actually exist for a package, the contract
+/// is about upholding the mathematical properties of set operations, assuming all versions are
+/// possible. This is required for the solver to determine the relationship of version sets to each
+/// other.
 pub trait VersionSet: Debug + Display + Clone + Eq {
     /// Version type associated with the sets manipulated.
     type V: Debug + Display + Clone + Ord;
 
     // Constructors
-    /// Constructor for an empty set containing no version.
+
+    /// An empty set containing no version.
     fn empty() -> Self;
-    /// Constructor for a set containing exactly one version.
+
+    /// A set containing only the given version.
     fn singleton(v: Self::V) -> Self;
 
     // Operations
-    /// Compute the complement of this set.
+
+    /// The set of all version that are not in this set.
     fn complement(&self) -> Self;
-    /// Compute the intersection with another set.
+
+    /// The set of all versions that are in both sets.
     fn intersection(&self, other: &Self) -> Self;
 
-    // Membership
-    /// Evaluate membership of a version in this set.
+    /// Whether the version is part of this set.
     fn contains(&self, v: &Self::V) -> bool;
 
-    // Automatically implemented functions ###########################
+    // Automatically implemented functions
 
-    /// Constructor for the set containing all versions.
-    /// Automatically implemented as `Self::empty().complement()`.
+    /// The set containing all versions.
+    ///
+    /// The default implementation is the complement of the empty set.
     fn full() -> Self {
         Self::empty().complement()
     }
 
-    /// Compute the union with another set.
-    /// Thanks to set properties, this is automatically implemented as:
-    /// `self.complement().intersection(&other.complement()).complement()`
+    /// The set of all versions that are either (or both) of the sets.
+    ///
+    /// The default implementation is complement of the intersection of the complements of both sets
+    /// (De Morgan's law).
     fn union(&self, other: &Self) -> Self {
         self.complement()
             .intersection(&other.complement())
@@ -71,6 +79,7 @@ pub trait VersionSet: Debug + Display + Clone + Eq {
     }
 }
 
+/// [`Ranges`] contains optimized implementations of all operations.
 impl<T: Debug + Display + Clone + Eq + Ord> VersionSet for Ranges<T> {
     type V = T;