Skip to content

Commit

Permalink
Avoid loss of data in Range.simplify (#13)
Browse files Browse the repository at this point in the history
* Add `Range.is_singleton`

* Do not simplify singletons

* Do not return null sets

* Tweak docs
  • Loading branch information
zanieb authored and konstin committed Mar 6, 2024
1 parent eb53f3e commit 3c2a410
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
7 changes: 0 additions & 7 deletions examples/unsat_root_message_no_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
format!("dependencies of {package} at version {set} are unavailable")
}
}
External::UnusableDependencies(package, set, ..) => {
if set == &Range::full() {
format!("dependencies of {package} are unusable")
} else {
format!("dependencies of {package} at version {set} are unusable")
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
if package_set == &Range::full() && dependency_set == &Range::full() {
format!("{package} depends on {dependency}")
Expand Down
35 changes: 27 additions & 8 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,21 +436,26 @@ impl<V: Ord + Clone> Range<V> {
Self { segments: output }.check_invariants()
}

/// Returns a simpler Range that contains the same versions
/// Returns a simpler Range that contains the same versions.
///
/// For every one of the Versions provided in versions the existing range and
/// the simplified range will agree on whether it is contained.
/// For every one of the Versions provided in versions the existing range and the simplified range will agree on whether it is contained.
/// The simplified version may include or exclude versions that are not in versions as the implementation wishes.
/// For example:
/// - If all the versions are contained in the original than the range will be simplified to `full`.
/// - If none of the versions are contained in the original than the range will be simplified to `empty`.
///
/// If versions are not sorted the correctness of this function is not guaranteed.
/// If none of the versions are contained in the original than the range will be returned unmodified.
/// If the range includes a single version, it will be returned unmodified.
/// If all the versions are contained in the original than the range will be simplified to `full`.
///
/// If the given versions are not sorted the correctness of this function is not guaranteed.
pub fn simplify<'s, I, BV>(&self, versions: I) -> Self
where
I: Iterator<Item = BV> + 's,
BV: Borrow<V> + 's,
{
// Do not simplify singletons
if self.is_singleton() {
return self.clone();
}

#[cfg(debug_assertions)]
let mut last: Option<BV> = None;
// Return the segment index in the range for each version in the range, None otherwise
Expand All @@ -477,7 +482,13 @@ impl<V: Ord + Clone> Range<V> {
}
Some(None)
});
let kept_segments = group_adjacent_locations(version_locations);
let mut kept_segments = group_adjacent_locations(version_locations).peekable();

// Do not return null sets
if kept_segments.peek().is_none() {
return self.clone();
}

self.keep_segments(kept_segments)
}

Expand All @@ -498,6 +509,14 @@ impl<V: Ord + Clone> Range<V> {
}
Self { segments }.check_invariants()
}

/// Whether the range is a single version
pub fn is_singleton(&self) -> bool {
match self.segments.as_slice() {
[(Included(v1), Included(v2))] => v1 == v2,
_ => false,
}
}
}

impl<T: Debug + Display + Clone + Eq + Ord> VersionSet for Range<T> {
Expand Down
4 changes: 2 additions & 2 deletions tests/examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ fn confusing_with_lots_of_holes() {
};
assert_eq!(
&DefaultStringReporter::report(&derivation_tree),
r#"Because there is no available version for bar and foo ==1, ==2, ==3, ==4, ==5 depends on bar, foo ==1, ==2, ==3, ==4, ==5 is forbidden.
And because there is no version of foo in <1, >1, <2, >2, <3, >3, <4, >4, <5, >5 and root ==1 depends on foo, root ==1 is forbidden."#
r#"Because there is no available version for bar and foo ==1 | ==2 | ==3 | ==4 | ==5 depends on bar, foo ==1 | ==2 | ==3 | ==4 | ==5 is forbidden.
And because there is no version of foo in <1 | >1, <2 | >2, <3 | >3, <4 | >4, <5 | >5 and root ==1 depends on foo, root ==1 is forbidden."#
);
derivation_tree.collapse_no_versions();
assert_eq!(
Expand Down

0 comments on commit 3c2a410

Please sign in to comment.