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

Tilde operator without patch level #23

Closed
seyon opened this issue Dec 30, 2020 · 3 comments
Closed

Tilde operator without patch level #23

seyon opened this issue Dec 30, 2020 · 3 comments
Assignees

Comments

@seyon
Copy link

seyon commented Dec 30, 2020

We are currently using the 2.x version and wanted to update to the new 3.x version. In the process, however, our unit tests failed.
The original statement no longer works:

Tilde operator: ~1.0.0 can be written as >=1.0.0 <1.1.0 and read as »every version within minor version 1.1. The behavior of tilde operator depends on whether a patch level version is provided or not. If no patch level is provided, tilde operator behaves like the caret operator: ~1.0 is identical to ^1.0.

When parsing a version number, "0" is now always set instead of "null" for the patch default value, so that the "getPatch()->isAny()" method returns true every time. It then assumes, for example, "~7.0.0" even if you pass "~7.0" and a comparison with 7.1 then fails.

$this->patch = isset($matches['Patch']) ? new VersionNumber((int)$matches['Patch']) : new VersionNumber(0);

@theseer
Copy link
Member

theseer commented Feb 22, 2021

Finally having some time to look into this, but I'm no longer convinced this issue is valid.

  1. The description and title of this issue refer to the use of the tilde operator. That makes it deal with a version constraint, not, as referenced with a link to the code, with a version.

  2. A version is required to always have a patch level as per semver specification (read, format is - at least - required to be x.y.z) and thus it can never be null. The fact we allow that in the parsing logic of Version qualifies as a bug of its own. I'm considering to tighten this.

  3. I don't see how in your unit test you can get true for getPatch()->isAny() when coming from Version. The value is never null thus it should always return false, given the check reads return $this->value === null.

Nevertheless, the code is problematic: Version as well as VersionConstraintParser use the same VersionNumber implementation despite their different contexts. From the constraint's perspective, VersionNumber::isAny can make sense as a part in the constraint can be optional, from the version's perspective it's not and thus the method probably shouldn't be available. Given it always will return false when used with Version though, it's not exactly a wrong - just pointless.

@theseer
Copy link
Member

theseer commented Feb 22, 2021

Okay, it is a valid issue as ConstraintParser uses Version internally and thus requires the optional patch level.

@theseer
Copy link
Member

theseer commented Feb 23, 2021

Fixed with commit 9dc75c0.

@theseer theseer closed this as completed Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants