Tilde operator without patch level
seyon opened this issue · 3 comments
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.
Line 114 in e478261
Finally having some time to look into this, but I'm no longer convinced this issue is valid.
-
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.
-
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. -
I don't see how in your unit test you can get
true
forgetPatch()->isAny()
when coming fromVersion
. The value is nevernull
thus it should always returnfalse
, given the check readsreturn $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.
Okay, it is a valid issue as ConstraintParser
uses Version
internally and thus requires the optional patch level.