Inconsistent application of stability flags in parseConstraint
VersionParser::parseConstraint() does not take into consideration stability flag when parsing constraint with ~ tilde or ^ caret range:
Constraint ~1.2.3@RC is parsed as >=1.2.3.0-dev and <1.3.0.0-dev
However ~1.2.3-RC is parsed as >=1.2.3.0-RC and <1.3.0.0-dev as expected.
I believe this inconsistency is a bug since ~ or ^ returns early from a conditional block but stability flags are considered again much later in the method where it apples to not equal and less/greater operators.
At the same time I see edge case with the way stability flag is handled with pre-release identifiers that are considered stable by composer:
>1.2.3@rc -> 1.2.3.0-rc
>1.2.3-beta@rc -> 1.2.3.0-beta
>1.2.3-stable@RC -> 1.2.3.0-rc
>1.2.3-patch1@RC -> 1.2.3.0-patch1-rc
I believe last constraint is invalid and as such a bug.
Hyphen ranges do not consider stability flags but also do not allow them on either side. No issue here.
Constraint with wildcards 1.2.*@rc does not allow pre-release identifiers but allows stability flags which it then ignores.
less than and less than equal with stability flag produce results that I question as well.
<1.2.0@rc -> < 1.2.0.0-rc
With that constraint as upper bound first will match alpha or beta in 1.2.0 assuming minimum-stability or flag on another part of multi-constraint allows those. Somehow this does not feel right. Is it intended outcome?
<=1.2.0@rc -> <= 1.2.0.0-rc
This will allow unstable matches but will not match stable 1.2.0.
I think you're probably right that all these are wrong, they're very edge casey, and as such probably not super critical fixes. But yeah it should be fixed if possible.
@Seldaek I was wondering if stability flags should affect constraints at all, considering they are not restricted to a single part of constraint and really handled at repository filtering level with the lowest taking the effect. It should probably have no effect on indirect constraints in dependencies so be stripped from parsed constraint entirely.
@Seldaek to clarify, I am willing to provide a PR as soon as I know the preferred direction for the change.
Yeah I'm not sure myself. Need to think about it some more and first finish my vacation ;)
Have a nice vacation.
As I was researching this for renovate I collected following composer behavior examples that are relevant here:
-
1.0.0@rcrequires precise version and only exactly1.0.0will be considered by composer. Stability flag has no effect. Pretty straightforward. -
^1.0.0-rc1allows any release candidate within range, eg1.2.0-rc1. Npm on the other hand allows release candidate only for that specific version and would not allow say1.0.1-rc1. -
^1.0.0-rc1with minimum-stability set to dev would allow any dev/alpha/beta/rc within range. It will not allow1.0.0-alpha1but1.0.1-alpha1is fair game. -
^1.0.0-rc1with minimum-stability set to stable would allow any release candidate within range. -
^1.0.0-rc1or^1.0.0@RCdeclared by dependency will not allow release candidates if minimum-stability set to stable. -
1.0.0-rc1@stablein root composer.json will not be installed at all as minimum-stability is forced to stable and required version does not match it. Minimum-stability setting has no effect. -
0.1.0@rc || 1.0.0-rc1@stablewill install1.0.0-rc1because lowest stability flag takes precedence. - Similarly,
^1.0.0@rc || ^2.0.0@alphawould allow any stability alpha or higher for the whole range, even for1.0.0
Because stability affects whole range it makes very little sense to cut out just the very edge of the range. So, unless parsed constraints are changed to deal with minimum stability within range, I believe sensible approach is not to treat stability flag as part of the version and outright strip it for the parsed constraint.
I know it's been a while here, but anyway looking at this closer again there is IMO no issue, the @<stability> notation is composer specific and only used to indicate which is the minimum-stability allowed for a given package. It has nothing to do with this library and is not considered when parsing constraints.
So I'd be of the opinion this should be closed.
It has nothing to do with this library and is not considered when parsing constraints.
I do not remember what was going on here. Composer dealing with the minimum stability is indeed unrelated. I mostly included it for reference,
Stability notation is used in this library during constraint parsing and re-applied as stability suffix to the parsed result but only with <>, !=, >, >=, <, <=
In the method it is stripped early and stored as $stabilityModifier to be applied again only here:
https://github.com/composer/semver/blob/c9b06fa2f64d9aaf2785cd325742a41f27e290aa/src/VersionParser.php#L505-L536
Inconsistency is between eg ^1.0.0@dev resulting in ^1.0.0 but then >=1.0.0@dev becomes >=1.0.0-dev
I believe sensible approach is not to treat stability flag as part of the version and outright strip it for the parsed constraint.
I discovered this while reading the code so if you rather treat this as won't fix I have no objections.