Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] intersects(r1, r2, options) function returns true if the not intersected range < 0.0.0 and 0.x are supplied #521

Closed
1 task done
sounisi5011 opened this issue Mar 1, 2023 · 10 comments · Fixed by #538
Labels
Bug thing that needs fixing Priority 1 high priority issue

Comments

@sounisi5011
Copy link

sounisi5011 commented Mar 1, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Range < 0.0.0 (or range < 0.0.0-0 if the includePrerelease option is true) should not intersect with all ranges. However, the intersects(r1, r2, options) function returns true when comparing ranges < 0.0.0 (< 0.0.0-0) and X-Range.

Expected Behavior

  • Returns false if compared to range < 0.0.0 if the includePrerelease option is false
  • Returns false if compared to range < 0.0.0-0 if the includePrerelease option is true

Steps To Reproduce

const semver = require('semver'); // Import semver@7.3.8

// This returns `false`, which is as expected since the two ranges do not intersect
semver.intersects('< 1.0.0', '1.0.0', { includePrerelease: false }) === false

// These also return `false`
semver.intersects('< 1.0.0', '1.x', { includePrerelease: false }) === false
semver.intersects('< 0.0.0', '1.x', { includePrerelease: false }) === false
semver.intersects('< 0.0.0', '0.1.x', { includePrerelease: false }) === false
semver.intersects('< 0.0.0', '0.0.0', { includePrerelease: false }) === false

// These return `true` because they intersect if a pre-release is included
semver.intersects('< 1.0.0', '1.x', { includePrerelease: true }) === true
semver.intersects('< 0.0.0', '0.x', { includePrerelease: true }) === true

// However, when compared to the X-Range, this returns `true` instead of `false`
semver.intersects('< 0.0.0', '0.0.x', { includePrerelease: false }) !== false
semver.intersects('< 0.0.0', '0.x', { includePrerelease: false }) !== false
semver.intersects('< 0.0.0', '0', { includePrerelease: false }) !== false
semver.intersects('< 0.0.0-0', '0.0.x', { includePrerelease: true }) !== false
semver.intersects('< 0.0.0-0', '0.x', { includePrerelease: true }) !== false
semver.intersects('< 0.0.0-0', '0', { includePrerelease: true }) !== false

// Whether the range `< 0.0.0` is an X-range or not does not affect the result
semver.intersects('< 0', '0.0.0', { includePrerelease: false }) === false
semver.intersects('< 0.x', '0.0.0', { includePrerelease: false }) === false
semver.intersects('< 0.0.x', '0.0.0', { includePrerelease: false }) === false
semver.intersects('< 0', '0.0.x', { includePrerelease: false }) !== false
semver.intersects('< 0.x', '0.0.x', { includePrerelease: false }) !== false
semver.intersects('< 0.0.x', '0.0.x', { includePrerelease: false }) !== false

Environment

@sounisi5011 sounisi5011 added Bug thing that needs fixing Needs Triage needs an initial review labels Mar 1, 2023
@isaacs
Copy link
Contributor

isaacs commented Mar 19, 2023

Technically this should be true if including pre releases, because 0.0.0-0 is less than 0.0.0.

@sounisi5011
Copy link
Author

sounisi5011 commented Mar 19, 2023

Technically this should be true if including pre releases, because 0.0.0-0 is less than 0.0.0.
#521 (comment)

If the includePrerelease option is true, then yes. But this behavior does not change whether the includePrerelease option is true or false.

I forgot the includePrerelease option existed, so I will add it to the steps to reproduce. Thank you!

@isaacs
Copy link
Contributor

isaacs commented Mar 20, 2023

Yeah, that definitely seems like a bug to me. Was just noting the edge case so we don't make a new bug when fixing this one 😬

@wraithgar wraithgar added Priority 1 high priority issue and removed Needs Triage needs an initial review labels Apr 6, 2023
@wraithgar
Copy link
Member

With includePrerelease can anything be less than 0.0.0-0?

@jakebailey
Copy link

I would assume not; "the rules" say that in prerelease parts, any valid number comes before any valid string, and 0 is the smallest number (negative numbers don't exist 🙈).

@wraithgar
Copy link
Member

Thanks for confirming, that was my understanding also.

@wraithgar
Copy link
Member

If includePrerelease is true and either range is <0.0.0-0 we always return false
If includePrerelease is false and either range starts with <0.0.0 we always return false

I think that covers everything?

@wraithgar
Copy link
Member

Found another bug while testing a fix for this:

semver.intersects('< 0.0.0', '<0.1.x', { includePrerelease: true })

ends up calling

https://github.com/npm/node-semver/blob/main/ranges/intersects.js#L5

which doesn't pass those options to the intersects method itself

@ljharb
Copy link

ljharb commented Apr 6, 2023

nice catch

@wraithgar
Copy link
Member

#538

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants