Improve handling of "full" version ranges (#868)

Reduces the number of implementation branches handling `Range:full`,
deferring it to `PackageRange`.
Improves some user-facing messages, e.g. saying `all versions of
<package>` instead of `<package>*`.
Changes the member names of the `PackageRangeKind` enum — they were not
very clear.
This commit is contained in:
Zanie Blue 2024-01-10 15:03:55 -06:00 committed by GitHub
parent a65c55ff4a
commit 811332eacc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 65 additions and 89 deletions

View file

@ -289,12 +289,12 @@ fn dependency_excludes_range_of_compatible_versions() -> Result<()> {
c>1.0.0,<2.0.0
c>2.0.0
and c==1.0.0 depends on a<2.0.0, we can conclude that c<2.0.0 depends on a<2.0.0.
And because c==2.0.0 depends on a>=3.0.0 we can conclude that c depends on one of:
And because c==2.0.0 depends on a>=3.0.0 we can conclude that all versions of c depends on one of:
a<2.0.0
a>=3.0.0
And because we know from (1) that a<2.0.0 depends on b==1.0.0, we can conclude that a!=3.0.0, c*, b!=1.0.0 are incompatible.
And because a==3.0.0 depends on b==3.0.0 we can conclude that c depends on one of:
And because we know from (1) that a<2.0.0 depends on b==1.0.0, we can conclude that a!=3.0.0, all versions of c, b!=1.0.0 are incompatible.
And because a==3.0.0 depends on b==3.0.0 we can conclude that all versions of c depends on one of:
b<=1.0.0
b>=3.0.0
@ -438,7 +438,7 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() -> Result<(
c>1.0.0,<2.0.0
c>2.0.0
and c==1.0.0 depends on a<2.0.0, we can conclude that c<2.0.0 depends on a<2.0.0.
And because c==2.0.0 depends on a>=3.0.0 we can conclude that c depends on one of:
And because c==2.0.0 depends on a>=3.0.0 we can conclude that all versions of c depends on one of:
a<2.0.0
a>=3.0.0
@ -448,7 +448,7 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() -> Result<(
depends on one of:
b<=1.0.0
b>=3.0.0
we can conclude that c depends on one of:
we can conclude that all versions of c depends on one of:
b<=1.0.0
b>=3.0.0
@ -1537,12 +1537,12 @@ fn requires_transitive_prerelease_and_stable_dependency_many_versions() -> Resul
Because there are no versions of b that satisfy any of:
b<1.0.0
b>1.0.0
and b==1.0.0 depends on c, we can conclude that b depends on c.
And because there are no versions of c that satisfy c>=2.0.0b1 we can conclude that b depends on c<2.0.0b1.
and b==1.0.0 depends on c, we can conclude that all versions of b depends on c.
And because there are no versions of c that satisfy c>=2.0.0b1 we can conclude that all versions of b depends on c<2.0.0b1.
And because a==1.0.0 depends on c>=2.0.0b1 and there are no versions of a that satisfy any of:
a<1.0.0
a>1.0.0
we can conclude that b* and a* are incompatible.
we can conclude that all versions of b and all versions of a are incompatible.
And because root depends on b and root depends on a, we can conclude that the requirements are unsatisfiable.
hint: c was requested with a pre-release marker (e.g., c>=2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`)
@ -2068,7 +2068,7 @@ fn requires_transitive_incompatible_with_root_version() -> Result<()> {
Because there are no versions of a that satisfy any of:
a<1.0.0
a>1.0.0
and a==1.0.0 depends on b==2.0.0, we can conclude that a depends on b==2.0.0.
and a==1.0.0 depends on b==2.0.0, we can conclude that all versions of a depends on b==2.0.0.
And because root depends on a and root depends on b==1.0.0, we can conclude that the requirements are unsatisfiable.
"###);
});
@ -2147,11 +2147,11 @@ fn requires_transitive_incompatible_with_transitive() -> Result<()> {
Because there are no versions of b that satisfy any of:
b<1.0.0
b>1.0.0
and b==1.0.0 depends on c==2.0.0, we can conclude that b depends on c==2.0.0.
and b==1.0.0 depends on c==2.0.0, we can conclude that all versions of b depends on c==2.0.0.
And because a==1.0.0 depends on c==1.0.0 and there are no versions of a that satisfy any of:
a<1.0.0
a>1.0.0
we can conclude that a* and b* are incompatible.
we can conclude that all versions of a and all versions of b are incompatible.
And because root depends on b and root depends on a, we can conclude that the requirements are unsatisfiable.
"###);
});

View file

@ -42,88 +42,55 @@ impl ReportFormatter<PubGrubPackage, Range<PubGrubVersion>> for PubGrubReportFor
format!(
"there are no versions of {} that satisfy {}",
package,
PackageRange::requires(package, &set)
PackageRange::compatibility(package, &set)
)
}
}
External::UnavailableDependencies(package, set) => {
let set = self.simplify_set(set, package);
if set.as_ref() == &Range::full() {
format!("dependencies of {package} are unavailable")
} else {
format!(
"dependencies of {}are unavailable",
Padded::new("", &PackageRange::requires(package, &set), " ")
Padded::new("", &PackageRange::compatibility(package, &set), " ")
)
}
}
External::UnusableDependencies(package, set, reason) => {
if let Some(reason) = reason {
if matches!(package, PubGrubPackage::Root(_)) {
format!("{package} dependencies are unusable: {reason}")
} else {
let set = self.simplify_set(set, package);
if set.as_ref() == &Range::full() {
format!("dependencies of {package} are unusable: {reason}")
} else {
format!(
"dependencies of {}are unusable: {reason}",
Padded::new("", &PackageRange::requires(package, &set), " ")
Padded::new("", &PackageRange::compatibility(package, &set), " ")
)
}
}
} else {
let set = self.simplify_set(set, package);
if set.as_ref() == &Range::full() {
format!("dependencies of {package} are unusable")
} else {
format!(
"dependencies of {}are unusable",
Padded::new("", &PackageRange::requires(package, &set), " ")
Padded::new("", &PackageRange::compatibility(package, &set), " ")
)
}
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
let package_set = self.simplify_set(package_set, package);
let dependency_set = self.simplify_set(dependency_set, dependency);
if package_set.as_ref() == &Range::full()
&& dependency_set.as_ref() == &Range::full()
{
format!("{package} depends on {dependency}")
} else if package_set.as_ref() == &Range::full() {
format!(
"{package} depends on {}",
PackageRange::depends(dependency, &dependency_set)
)
} else if dependency_set.as_ref() == &Range::full() {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
format!("{package} depends on {dependency}")
} else {
format!(
"{}depends on {dependency}",
Padded::new("", &PackageRange::requires(package, &package_set), " ")
)
}
} else {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
format!(
"{package} depends on {}",
PackageRange::depends(dependency, &dependency_set)
PackageRange::dependency(dependency, &dependency_set)
)
} else {
format!(
"{}depends on {}",
Padded::new("", &PackageRange::requires(package, &package_set), " "),
PackageRange::depends(dependency, &dependency_set)
Padded::new("", &PackageRange::compatibility(package, &package_set), " "),
PackageRange::dependency(dependency, &dependency_set)
)
}
}
}
}
}
/// Try to print terms of an incompatibility in a human-readable way.
fn format_terms(&self, terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>) -> String {
@ -137,7 +104,10 @@ impl ReportFormatter<PubGrubPackage, Range<PubGrubVersion>> for PubGrubReportFor
.unwrap_or(&vec![])
.iter(),
);
format!("{} cannot be used", PackageRange::requires(package, &range))
format!(
"{} cannot be used",
PackageRange::compatibility(package, &range)
)
}
[(package @ PubGrubPackage::Package(..), Term::Negative(range))] => {
let range = range.simplify(
@ -146,7 +116,10 @@ impl ReportFormatter<PubGrubPackage, Range<PubGrubVersion>> for PubGrubReportFor
.unwrap_or(&vec![])
.iter(),
);
format!("{} must be used", PackageRange::requires(package, &range))
format!(
"{} must be used",
PackageRange::compatibility(package, &range)
)
}
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => self.format_external(
&External::FromDependencyOf((*p1).clone(), r1.clone(), (*p2).clone(), r2.clone()),
@ -431,7 +404,7 @@ impl std::fmt::Display for PubGrubHint {
"hint".bold().cyan(),
":".bold(),
package.bold(),
PackageRange::requires(package, range).bold()
PackageRange::compatibility(package, range).bold()
)
}
}
@ -447,13 +420,13 @@ struct PackageTerm<'a> {
impl std::fmt::Display for PackageTerm<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match &self.term {
Term::Positive(set) => write!(f, "{}", PackageRange::requires(self.package, set)),
Term::Positive(set) => write!(f, "{}", PackageRange::compatibility(self.package, set)),
Term::Negative(set) => {
if let Some(version) = set.as_singleton() {
let package = self.package;
write!(f, "{package}!={version}")
} else {
write!(f, "!( {} )", PackageRange::requires(self.package, set))
write!(f, "!( {} )", PackageRange::compatibility(self.package, set))
}
}
}
@ -472,8 +445,8 @@ impl PackageTerm<'_> {
/// The kind of version ranges being displayed in [`PackageRange`]
#[derive(Debug)]
enum PackageRangeKind {
Depends,
Requires,
Dependency,
Compatibility,
}
/// A [`Range`] and [`PubGrubPackage`] combination for display.
@ -492,31 +465,34 @@ impl std::fmt::Display for PackageRange<'_> {
let segments: Vec<_> = self.range.iter().collect();
if segments.len() > 1 {
match self.kind {
PackageRangeKind::Depends => write!(f, "one of:")?,
PackageRangeKind::Requires => write!(f, "any of:")?,
PackageRangeKind::Dependency => write!(f, "one of:")?,
PackageRangeKind::Compatibility => write!(f, "any of:")?,
}
}
for segment in &segments {
if segments.len() > 1 {
write!(f, "\n ")?;
}
write!(f, "{}", self.package)?;
let package = self.package;
match segment {
(Bound::Unbounded, Bound::Unbounded) => write!(f, "*")?,
(Bound::Unbounded, Bound::Included(v)) => write!(f, "<={v}")?,
(Bound::Unbounded, Bound::Excluded(v)) => write!(f, "<{v}")?,
(Bound::Included(v), Bound::Unbounded) => write!(f, ">={v}")?,
(Bound::Unbounded, Bound::Unbounded) => match self.kind {
PackageRangeKind::Dependency => write!(f, "{package}")?,
PackageRangeKind::Compatibility => write!(f, "all versions of {package}")?,
},
(Bound::Unbounded, Bound::Included(v)) => write!(f, "{package}<={v}")?,
(Bound::Unbounded, Bound::Excluded(v)) => write!(f, "{package}<{v}")?,
(Bound::Included(v), Bound::Unbounded) => write!(f, "{package}>={v}")?,
(Bound::Included(v), Bound::Included(b)) => {
if v == b {
write!(f, "=={v}")?;
write!(f, "{package}=={v}")?;
} else {
write!(f, ">={v},<={b}")?;
write!(f, "{package}>={v},<={b}")?;
}
}
(Bound::Included(v), Bound::Excluded(b)) => write!(f, ">={v},<{b}")?,
(Bound::Excluded(v), Bound::Unbounded) => write!(f, ">{v}")?,
(Bound::Excluded(v), Bound::Included(b)) => write!(f, ">{v},<={b}")?,
(Bound::Excluded(v), Bound::Excluded(b)) => write!(f, ">{v},<{b}")?,
(Bound::Included(v), Bound::Excluded(b)) => write!(f, "{package}>={v},<{b}")?,
(Bound::Excluded(v), Bound::Unbounded) => write!(f, "{package}>{v}")?,
(Bound::Excluded(v), Bound::Included(b)) => write!(f, "{package}>{v},<={b}")?,
(Bound::Excluded(v), Bound::Excluded(b)) => write!(f, "{package}>{v},<{b}")?,
};
}
if segments.len() > 1 {
@ -528,25 +504,25 @@ impl std::fmt::Display for PackageRange<'_> {
}
impl PackageRange<'_> {
fn requires<'a>(
fn compatibility<'a>(
package: &'a PubGrubPackage,
range: &'a Range<PubGrubVersion>,
) -> PackageRange<'a> {
PackageRange {
package,
range,
kind: PackageRangeKind::Requires,
kind: PackageRangeKind::Compatibility,
}
}
fn depends<'a>(
fn dependency<'a>(
package: &'a PubGrubPackage,
range: &'a Range<PubGrubVersion>,
) -> PackageRange<'a> {
PackageRange {
package,
range,
kind: PackageRangeKind::Depends,
kind: PackageRangeKind::Dependency,
}
}
}