Fixed VersionParser being unable to parse its own MultiConstraint format

[Toflar]

The VersionParser currently cannot handle the output of (string) $multiConstraint.

In my case, I had to merge multiple constraints and pass them on in a string representation. When you do that, the MultiConstraint will return something like this: [>= 2.5.9.0-dev < 2.6.0.0-dev]. But if you pass this to VersionParser::parseConstraints() you'll end up getting like UnexpectedValueException: Could not parse version constraint [>= 2.5.9.0-dev: Invalid version string "[>= 2.5.9.0-dev".

It's pretty weird that the VersionParser cannot handle a format that was created by a class coming from the very same library - so I consider this a bug :)

Not sure this is the right fix but tests are still green, so at least tit doesn't break any current logic (and it also fixes my issue).

[stof]

The parseable string does not support arbitrary nesting of MultiConstraint anyway (that would require braces in the constraint syntax).

To me, ignoring the [ is the wrong fix. Maybe instead we should make MultiConstraint cast to a valid constraint string whenever possible.

[Seldaek]

Yeah this is definitely not the right fix IMO. The string is simply for human consumption.

The way to do it IMO is to do a new MultiConstraint($constraints, true|false) instead of merging as strings which is not possible as we do not have a string syntax supporting deep nesting.

[Toflar]

Background: For a customer project I needed Satis but require only the relevant versions. So I built the configuration (https://getcomposer.org/doc/articles/handling-private-packages.md#setup) dynamically. That's why I had foobar/package as a MultiConstraint($allConstraintsINeedToDownload, false). But the Composer configuration requires require to be a string. Even if I'd use Factory::create() with a PHP array it wouldn't work.

[stof]

Well, your approach won't work for that. Ignoring the [ will not magically make things work. Those [ are parenthesis-like behavior in the string representation, allowing to represent nested constraints in the human representation.
But parsable constraints don't support braces to allow arbitrary nesting.

[Toflar]

Yeah, I agree. In my case it works because they are not nested ;) But I guess we could actually introduce support for nested [] constraints in the VersionParser. Should be a matter of recursively using parseConstraints for what's inside the brackets. Not done in 5 minutes but still, it should be doable.

[Seldaek]

Should be doable yes, but then you have to support @flags and #refs also in there and it gets even more complex, all in all just not worth the trouble IMO, given you generally can serialize/flatten into pairs of A, B || C, D which parses as (A && B) || (C && D) and that is probably enough for all things you'd reasonably want to express, but when merging things together it may or may not work depending on case, if you have disjunctive constraints I think you could just merge them by getting the pretty string of the 2 constraints and concatenating with || in between.

[stof]

and if you have more complex constraints, you might try to compact them as a way to get a simpler MultiConstraint (if your constraints don't involve dev branches, this should give you a constraint that does not involve unsupported nesting)

[Toflar]

Sure, I mean I solved my problem already by stripping them because that's enough for what is logically possible in my case. I just thought I'd bring this to the table here and see if we find a general solution. At least we've discussed about it in public now and others can find it too. But I agree that it's an edge case and not worth the hassle. Unless anybody wants to give it a go but then also make sure you consider the notes by @Seldaek in #151 (comment).

Closing - thanks for the discussion @stof & @Seldaek ❤️