fix(semver): fix prerelease handlings in range utils

[kt3k]

closes #4300

This fixes the case when the given ranges intersect at prerelease versions.

ex. rangeIntersects(parseRange("<7.0.0-beta.20"), parseRange(">7.0.0-beta.0"))

Current implementation tries to get min/max of comparators and compares them, but comparatorMax doesn't work with prerelease versions since it was first introduced in #3385 (It completely ignores prerelease versions)

comparatorMax seems wrong by design because the max of <7.0.0-beta.20 is something like 7.0.0-beta.19.zzzz....(infinite 'z's), and that is very inconvenient to represent in javascript.

This PR gives up the current approach for calculating the intersection. Instead this restores the original algorithm used for comparing comparators (see https://github.com/denoland/deno_std/blob/e1698647677d2e58348f3308ddeebb9c40cc0ca9/semver/mod.ts#L821-L874)

[kt3k]

comparatorMax is inherently wrong. We should remove all usages of it. We also should remove rangeMax which doesn't (can't) return correct values.

[iuioiua]

Suggested change

	* Returns true if the range of possible versions intersects with the other c1arators set of possible versions
	* @param c0 The left side c1arator
	* @param c1 The right side c1arator
	* @returns True if any part of the c1arators intersect
	* Returns true if the range of possible versions intersects with the other comparators set of possible versions
	* @param c0 The left side comparators
	* @param c1 The right side comparators
	* @returns True if any part of the comparators intersect

I'm not sure why comparators was changed to c1arators. Is this deliberate?

[iuioiua]

Can you please add one or two more tests?

[iuioiua]

comparatorMax is inherently wrong. We should remove all usages of it. We also should remove rangeMax which doesn't (can't) return correct values.

In this PR, comparatorIntersects() now depends on testRange(), which depends on comparatorMin() and comparatorMax(). How should that then be reconciled?

[nicolo-ribaudo]

comparatorMax seems wrong by design because the max of <7.0.0-beta.20 is something like 7.0.0-beta.19.zzzz....(infinite 'z's), and that is very inconvenient to represent in javascript.

If there are still use case for comparatorMax, a possible good result that is always representable is "the smallest version that is < than all the versions that would match this range".

So, for example:

• rangeMaxExclusive("=7.0.0") -> 7.0.1 (instead of 7.0.0)
• rangeMaxExclusive("<=7.9.4") -> 7.9.5 (instead of 7.9.4)
• rangeMaxExclusive("^7.0.0") -> 8.0.0-0 (instead of 7.Infinity.Infinity)
• rangeMaxExclusive("<7.0.0-beta.20") -> 7.0.0-beta.20 (instead of the current invalid result)

An alternative that would maybe make more sense for ranges that do indeed have a max, but is probably less useful because the result would need a flag indicating whether it's inclusive or exclusive, it "the smallest version that is &lte; than all the versions that would match this range":

• rangeSup("=7.0.0") -> 7.0.0, inclusive
• rangeSup("<=7.9.4") -> 7.9.4, inclusive
• rangeSup("^7.0.0") -> 8.0.0-0, exclusive
• rangeSup("<7.0.0-beta.20") -> 7.0.0-beta.20, exclusive

[kt3k]

In this PR, comparatorIntersects() now depends on testRange(), which depends on comparatorMin() and comparatorMax(). How should that then be reconciled?

Thanks for pointing it. testRange also has a bug like testRange(parse("1.0.0-3"), parseRange("<1.0.0-5")) === false (should be true). We have to remove all such usages of comparatorMax

[kt3k]

rangeMaxExclusive / rangeSup seems to make more sense than rangeMax, but I'm not sure we need to pursue that type of API.

rangeMax seems to have been introduced just for implementation purpose in #3385 (fd14b85), and there was no discussion about the utility of it as public API.

[nicolo-ribaudo]

@kt3k Out of curiosity, do these cases behave differently from npm:semver also when you pass { includePrereleases: true } to it?

[kt3k]

@nicolo-ribaudo These cases try to behave the same as the case when Range object is created with new Range(version, { includePrerelease: false }) (which is the default of Range constructor), and behave differently from new Range(version, { includePrerelease: true }). (I amend my comment #4300 (comment), the boolean value was opposite)

The difference happens when the tested version include prerelease and the range doesn't include prerelease such as version 1.0.0-rc vs range <1.0.0

We can add option to testRange to simulate the behavior of new Range(version, { includePrerelease: true }) if there's any demand for that.