From 79c4f3a5310f01760ae3fbb2e10a24cf5164bd18 Mon Sep 17 00:00:00 2001 From: dngr2 Date: Tue, 18 Aug 2026 02:33:53 +0300 Subject: [PATCH 1/2] Do not compact a dev-free constraint into a bare != that matches dev The < N || > N to != N swap in compactConstraint changes dev-branch matching: a bare != N matches every dev-* version, while < N || > N matches none. So "> 1.0.0 != 2.0.0 || < 1.9.0" (matches no branch) compacted to "!= 2.0.0" (matches every branch), and Semver::satisfies flipped after compaction. Skip the swap when it would produce a bare != for a constraint whose branches do not already include all dev versions; a bounded conjunctive != (e.g. [!= 3 < 5]) still keeps a numeric bound and is unaffected. --- src/Intervals.php | 16 +++++++++++++++- tests/IntervalsTest.php | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Intervals.php b/src/Intervals.php index d889d0ad..94086de7 100644 --- a/src/Intervals.php +++ b/src/Intervals.php @@ -150,6 +150,17 @@ public static function compactConstraint(ConstraintInterface $constraint) $constraints[] = $intervals['numeric'][0]->getStart(); $hasNumericMatchAll = true; } else { + // A bare "!= N" matches every dev-* version (see generateSingleConstraintIntervals), + // whereas "< N || > N" matches none. So the swap below must be skipped when it + // would collapse the whole numeric line (except N) into a bare != for a + // constraint that matches no dev version -- otherwise "> 1 != 2 || < 1.9" + // (no branch) becomes "!= 2" (every branch). A bounded conjunctive != such + // as [!= 3 < 5] keeps a numeric bound and stays dev-free, so it is fine. + // exclude === true means the dev baseline is "all dev" (any named branches + // are subtracted separately below), so a bare != is the right baseline; + // exclude === false means "no dev", where a bare != would wrongly add them. + $branchesMatchAllDev = $intervals['branches']['exclude']; + $unEqualConstraints = array(); for ($i = 0, $count = \count($intervals['numeric']); $i < $count; $i++) { $interval = $intervals['numeric'][$i]; @@ -160,7 +171,10 @@ public static function compactConstraint(ConstraintInterface $constraint) // they are zero/+inf if ($interval->getEnd()->getOperator() === '<' && $i+1 < $count) { $nextInterval = $intervals['numeric'][$i+1]; - if ($interval->getEnd()->getVersion() === $nextInterval->getStart()->getVersion() && $nextInterval->getStart()->getOperator() === '>') { + $wouldBeBareNotEqual = 2 === $count + && (string) $interval->getStart() === (string) Interval::fromZero() + && (string) $nextInterval->getEnd() === (string) Interval::untilPositiveInfinity(); + if ($interval->getEnd()->getVersion() === $nextInterval->getStart()->getVersion() && $nextInterval->getStart()->getOperator() === '>' && ($branchesMatchAllDev || !$wouldBeBareNotEqual)) { // only add a start if we didn't already do so, can be skipped if we're looking at second // interval in [>=M, N, P, =M, !=N] already and we only want to add !=P right now diff --git a/tests/IntervalsTest.php b/tests/IntervalsTest.php index 457d8e39..1e006fc5 100644 --- a/tests/IntervalsTest.php +++ b/tests/IntervalsTest.php @@ -51,6 +51,25 @@ public function testCompactConstraint($expected, $toCompact, $conjunctive) $this->assertSame((string) $expected, (string) $new); } + public function testCompactConstraintKeepsANumericOnlyConstraintDevFree() + { + $parser = new VersionParser; + // "> 1.0.0 != 2.0.0 || < 1.9.0" covers the whole numeric line except 2.0.0 + // and matches no dev-* version. The numeric part compacts to a bare != 2.0.0, + // which on its own matches every dev branch, so compaction must not use it here. + $constraint = new MultiConstraint(array( + $parser->parseConstraints('> 1.0.0 != 2.0.0'), + $parser->parseConstraints('< 1.9.0'), + ), false); + + $compacted = Intervals::compactConstraint($constraint); + $dev = new Constraint('==', $parser->normalize('dev-foo')); + + $this->assertFalse($compacted->matches($dev), (string) $compacted); + $this->assertTrue(Intervals::isSubsetOf($compacted, $constraint)); + $this->assertTrue(Intervals::isSubsetOf($constraint, $compacted)); + } + public static function compactProvider() { return array( From eff3d0b8e47aa77d14231ffb7cb977c806172f3d Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 24 Sep 2026 16:06:22 +0200 Subject: [PATCH 2/2] Generalize the bare != compaction guard to any number of excluded points The previous check only handled two numeric intervals, so e.g. "> 1.0.0 != 2.0.0 != 3.0.0 || < 1.5" still compacted to "!= 2.0 != 3.0", which matches every dev-* version. Decide once before the loop whether the numeric part is the whole line minus single points, and skip the != swap entirely in that case unless the branches already match all dev versions. This also takes the check out of the per-interval hot path. --- src/Intervals.php | 43 ++++++++++++++++++------------- tests/IntervalsTest.php | 57 ++++++++++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 30 deletions(-) diff --git a/src/Intervals.php b/src/Intervals.php index 94086de7..a4800c5d 100644 --- a/src/Intervals.php +++ b/src/Intervals.php @@ -150,31 +150,38 @@ public static function compactConstraint(ConstraintInterface $constraint) $constraints[] = $intervals['numeric'][0]->getStart(); $hasNumericMatchAll = true; } else { - // A bare "!= N" matches every dev-* version (see generateSingleConstraintIntervals), - // whereas "< N || > N" matches none. So the swap below must be skipped when it - // would collapse the whole numeric line (except N) into a bare != for a - // constraint that matches no dev version -- otherwise "> 1 != 2 || < 1.9" - // (no branch) becomes "!= 2" (every branch). A bounded conjunctive != such - // as [!= 3 < 5] keeps a numeric bound and stays dev-free, so it is fine. - // exclude === true means the dev baseline is "all dev" (any named branches - // are subtracted separately below), so a bare != is the right baseline; - // exclude === false means "no dev", where a bare != would wrongly add them. - $branchesMatchAllDev = $intervals['branches']['exclude']; + // A bare != N matches all dev-* versions, so if the numeric part is the whole line minus some points, + // only compact it into unbounded != constraints when the branches already match all dev versions + $numeric = $intervals['numeric']; + $count = \count($numeric); + $skipNotEqual = false; + if (!$intervals['branches']['exclude'] && $count > 1 + && (string) $numeric[0]->getStart() === (string) Interval::fromZero() + && (string) $numeric[$count - 1]->getEnd() === (string) Interval::untilPositiveInfinity() + ) { + $skipNotEqual = true; + for ($i = 0; $i < $count - 1; $i++) { + if ($numeric[$i]->getEnd()->getOperator() !== '<' + || $numeric[$i + 1]->getStart()->getOperator() !== '>' + || $numeric[$i]->getEnd()->getVersion() !== $numeric[$i + 1]->getStart()->getVersion() + ) { + $skipNotEqual = false; + break; + } + } + } $unEqualConstraints = array(); - for ($i = 0, $count = \count($intervals['numeric']); $i < $count; $i++) { - $interval = $intervals['numeric'][$i]; + for ($i = 0; $i < $count; $i++) { + $interval = $numeric[$i]; // if current interval ends with < N and next interval begins with > N we can swap this out for != N // but this needs to happen as a conjunctive expression together with the start of the current interval // and end of next interval, so [>=M, N, [>=M, !=N, getEnd()->getOperator() === '<' && $i+1 < $count) { - $nextInterval = $intervals['numeric'][$i+1]; - $wouldBeBareNotEqual = 2 === $count - && (string) $interval->getStart() === (string) Interval::fromZero() - && (string) $nextInterval->getEnd() === (string) Interval::untilPositiveInfinity(); - if ($interval->getEnd()->getVersion() === $nextInterval->getStart()->getVersion() && $nextInterval->getStart()->getOperator() === '>' && ($branchesMatchAllDev || !$wouldBeBareNotEqual)) { + if (!$skipNotEqual && $interval->getEnd()->getOperator() === '<' && $i+1 < $count) { + $nextInterval = $numeric[$i+1]; + if ($interval->getEnd()->getVersion() === $nextInterval->getStart()->getVersion() && $nextInterval->getStart()->getOperator() === '>') { // only add a start if we didn't already do so, can be skipped if we're looking at second // interval in [>=M, N, P, =M, !=N] already and we only want to add !=P right now diff --git a/tests/IntervalsTest.php b/tests/IntervalsTest.php index 1e006fc5..bbc22771 100644 --- a/tests/IntervalsTest.php +++ b/tests/IntervalsTest.php @@ -51,23 +51,41 @@ public function testCompactConstraint($expected, $toCompact, $conjunctive) $this->assertSame((string) $expected, (string) $new); } - public function testCompactConstraintKeepsANumericOnlyConstraintDevFree() + /** + * @dataProvider compactDevSafetyProvider + * @param array $toCompact + * @param bool $conjunctive + */ + public function testCompactConstraintKeepsMatchedSet($toCompact, $conjunctive) { $parser = new VersionParser; - // "> 1.0.0 != 2.0.0 || < 1.9.0" covers the whole numeric line except 2.0.0 - // and matches no dev-* version. The numeric part compacts to a bare != 2.0.0, - // which on its own matches every dev branch, so compaction must not use it here. - $constraint = new MultiConstraint(array( - $parser->parseConstraints('> 1.0.0 != 2.0.0'), - $parser->parseConstraints('< 1.9.0'), - ), false); + + $parts = array(); + foreach ($toCompact as $part) { + $parts[] = $parser->parseConstraints($part); + } + $constraint = new MultiConstraint($parts, $conjunctive); $compacted = Intervals::compactConstraint($constraint); - $dev = new Constraint('==', $parser->normalize('dev-foo')); + $dev = new Constraint('==', 'dev-foo'); + + $this->assertSame($constraint->matches($dev), $compacted->matches($dev), (string) $compacted); + $this->assertTrue(Intervals::isSubsetOf($compacted, $constraint), (string) $compacted); + $this->assertTrue(Intervals::isSubsetOf($constraint, $compacted), (string) $compacted); + } - $this->assertFalse($compacted->matches($dev), (string) $compacted); - $this->assertTrue(Intervals::isSubsetOf($compacted, $constraint)); - $this->assertTrue(Intervals::isSubsetOf($constraint, $compacted)); + public static function compactDevSafetyProvider() + { + return array( + 'whole line minus one point, no dev' => array(array('> 1.0.0 != 2.0.0', '< 1.9.0'), false), + 'whole line minus two points, no dev' => array(array('> 1.0.0 != 2.0.0 != 3.0.0', '< 1.5'), false), + 'whole line minus one point, named branch' => array(array('> 1.0.0 != 2.0.0', '< 1.5', 'dev-bar'), false), + 'whole line minus two points, named branch' => array(array('> 1.0.0 != 2.0.0 != 3.0.0', '< 1.5', 'dev-bar'), false), + 'bounded != group' => array(array('> 1.0.0 != 2.0.0 < 3.0.0', '< 1.5', '>= 5.0'), false), + 'conjunctive !=, all dev' => array(array('!= 2.0', '!= 3.0'), true), + 'conjunctive != with excluded branch' => array(array('!= 2.0', '!= dev-foo'), true), + 'disjunctive != with named branch' => array(array('!= 2.0', 'dev-bar'), false), + ); } public static function compactProvider() @@ -133,6 +151,21 @@ public static function compactProvider() array('!= 1.0', '!= 1.0', '!= dev-foo', '1.0.5.*'), false ), + 'whole line minus one point without dev does not become a bare !=' => array( + '< 2.0.0.0-stable || > 2.0.0', + array('> 1.0.0 != 2.0.0', '< 1.9.0'), + false + ), + 'whole line minus two points without dev does not become a bare !=' => array( + '< 2.0.0.0-stable || > 2.0.0 < 3.0.0.0-stable || > 3.0.0', + array('> 1.0.0 != 2.0.0 != 3.0.0', '< 1.5'), + false + ), + 'whole line minus two points with named branch does not become a bare !=' => array( + '< 2.0.0.0-stable || > 2.0.0 < 3.0.0.0-stable || > 3.0.0 || dev-bar', + array('> 1.0.0 != 2.0.0 != 3.0.0', '< 1.5', 'dev-bar'), + false + ), 'conjunctive with complex negation' => array( '1.0.5.*', array('!= 1.0', '!= 1.0', '!= dev-foo', '1.0.5.*'),