fix(subset): check any as superset

[jameschensmith]

Summary

Adds short-circuit check if superset is *. This is currently not handled. A non-range semver has been able to get passed this not existing, but ranges came back falsey. This request addresses that shortcoming.

References

Fixes #374.

[coveralls]

Coverage remained the same at 100.0% when pulling 1e625f0 on jameschensmith:jameschensmith/subset-check-any-as-superset into 093b40f on npm:master.

[jameschensmith]

@isaacs, when you get a chance could you take a look at this request? If there's any concerns, I'll do my best to address them ASAP. The request for arborist in the References section of this request has a dependency on this being merged. Both of these would also resolve npm/cli#636 (and possibly its referenced tickets as well), so this has potential to resolve a ton of issues for users. Thank you!

[isaacs]

The suggested short-circuit path can only be applied when the includePrerelease flag is set. Otherwise, it returns an incorrect result if the first range includes a prerelease on any of its versions.

[isaacs]

This is incorrect, though. ^1.2.3-pre.0 is not a subset of * unless includePrerelease is set, because there are versions that satisfy ^1.2.3-pre.0 and do not satisfy *.

> semver.satisfies('1.2.3-pre.0', '^1.2.3-pre.0')
true
> semver.satisfies('1.2.3-pre.0', '*')
false

[isaacs]

Ah, ok, this is a bug though:

> semver.subset('^1.2.3', '*')
false

I think we need special handling of <1.2.3-0, since that range doesn't include prereleases, maybe?

[isaacs]

Ahh! This was already a problem, turns out!

> require('./').subset('^1.2.3-pre.0', '>=1')
true

That should return false in non-prerelease mode, because the first set includes 1.2.3-pre.0 and up prereleases, and the second doesn't include any prereleases.

Ok, so the solution looks like this I think:

// - If c is only the ANY comparator
//   - If C is only the ANY comparator, return true
//   - Else return false
// - If C is only the ANY comparator
//   - if in prerelease mode, return true
//   - else replace C with `[>=0.0.0]`
// ... rest of the algorithm
// - handle any prerelease ranges in c, ensuring that they don't pull in prereleases not in C

[isaacs]

This is also a bug:

> require('./').subset('*', '>=0.0.0-pre.0')
false

Because (in non-prerelease mode) >=0.0.0-pre.0 includes all versions that could possibly match * as well as any versions from 0.0.0-pre.0 through 0.0.1, it should be a superset of *. So that first bit should be:

// - If c is only the ANY comparator
//   - If C is only the ANY comparator, return true
//   - Else if in prerelease mode, return false
//   - else replace c with `[>=0.0.0]`

[jameschensmith]

Woah! Thanks for your great feedback! I had no idea about the pre-release matching, but it makes sense 😊 I see you set up another PR, requesting my review, so I'll check that out now. Thanks! 👍

[isaacs]

This is landed in #377.

Thanks for bringing up the issue! Was a pretty significant oversight, which didn't matter until Arborist started making this method load-bearing 😬