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.

Sign up for GitHub

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

Jump to bottom

Add Intervals class to allow checking for subsets and intersections between constraints #97

Merged
merged 28 commits into from
May 19, 2020
Merged

Add Intervals class to allow checking for subsets and intersections between constraints #97

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
56b15a5
Add basic tests and implementation for isSubsetOf
Seldaek May 5, 2020
bb1549d
Add dev-* tests, fixes Constraint><Constraint implementation by spell…
Seldaek May 5, 2020
0f5aa06
Add isContiguous to optimize and pass simple multi constraint compari…
Seldaek May 5, 2020
0218e6d
Fix bug comparing against = constraints
Seldaek May 5, 2020
6726a92
Fix matching of !=x against multi constraints
Seldaek May 5, 2020
006581b
Update wording
Seldaek May 5, 2020
4d5f569
Fix handling of disjunctive multiconstraints
Seldaek May 5, 2020
6ed5932
Clean ups, more tests
Seldaek May 5, 2020
4708f31
Add getIntervals to MultiConstraint to resolve more cases
Seldaek May 5, 2020
9686233
Add additional subset test cases
naderman May 5, 2020
b007ae4
Add failing subset tests
naderman May 5, 2020
639fc07
Fix PHP 5.3 syntax
Seldaek May 6, 2020
6469a85
Proper getIntervals implementation, move isSubsetOf out of constraint…
Seldaek May 7, 2020
e5e4d89
Add support for dev-* constraints in intervals and isSubsetOf
Seldaek May 8, 2020
67b8bb5
Avoid generating invalid intervals
Seldaek May 8, 2020
5170fb7
Fix a few more issues with dev-* by adding a dev* match all when needed
Seldaek May 8, 2020
6a1fa53
Mark a few things as not subset anymore after the dev* changes
Seldaek May 8, 2020
0b0edb3
Use version_compare for safety in case constraints are not normalized…
Seldaek May 8, 2020
5aef5c2
Add Intervals::isIntersectionOf and ensure it is acting the same as -…
Seldaek May 8, 2020
07f0384
Explicitly gate generateIntervals to our internal classes
Seldaek May 8, 2020
dcb965d
Remove sorting of dev constraints to try and fix version_compare disc…
Seldaek May 8, 2020
5468efb
Use same values for zero/+inf bounds as are used in intervals zero/+inf
Seldaek May 8, 2020
4c42914
Rename isIntersectionOf to haveIntersections
Seldaek May 19, 2020
e7ee6a2
Add phpstan annotation
Seldaek May 19, 2020
aaf43b3
Minor refactoring and add phpdocs
Seldaek May 19, 2020
3d72ad3
Fix feedback
Seldaek May 19, 2020
c3e5429
Convert interval arrays to a value object
Seldaek May 19, 2020
8806417
Rename [intervals, devConstraints] return value to [numeric, branches]
Seldaek May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions src/Interval.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php

/*
* This file is part of composer/semver.
*
* (c) Composer <https://github.com/composer>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace Composer\Semver;

use Composer\Semver\Constraint\Constraint;

class Interval
{
/** @var Constraint */
private $start;
/** @var Constraint */
private $end;

public function __construct(Constraint $start, Constraint $end)
{
$this->start = $start;
$this->end = $end;
}

/**
* @return Constraint
*/
public function getStart()
{
return $this->start;
}

/**
* @return Constraint
*/
public function getEnd()
{
return $this->end;
}

/**
* @return Constraint
*/
public static function zero()
{
static $zero;

if (null === $zero) {
$zero = new Constraint('>=', '0.0.0.0-dev');
}

return $zero;
}

/**
* @return Constraint
*/
public static function positiveInfinity()
{
static $positiveInfinity;

if (null === $positiveInfinity) {
$positiveInfinity = new Constraint('<', PHP_INT_MAX.'.0.0.0');
}

return $positiveInfinity;
}

/**
* @return self
*/
public static function any()
{
return new self(self::zero(), self::positiveInfinity());
}

/**
* @return Constraint
*/
public static function anyDev()
{
static $anyDev;

if (null === $anyDev) {
// this ideally should be an EmptyConstraint but the code expects Constraint instances so
// this makes it work with less workarounds/checks above
$anyDev = new Constraint('==', 'dev*');
}

return $anyDev;
}
}
86 changes: 27 additions & 59 deletions src/Intervals.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
class Intervals
{
/**
* @phpstan-var array<string, array{'intervals': array<int, array{'start': Constraint, 'end': Constraint}>, 'devConstraints': Constraint[]}>
* @phpstan-var array<string, array{'intervals': Interval[], 'devConstraints': Constraint[]}>
*/
private static $intervalsCache = array();

Expand Down Expand Up @@ -68,11 +68,11 @@ public static function isSubsetOf(ConstraintInterface $candidate, ConstraintInte
return false;
}

if ((string) $candidateIntervals['intervals'][$index]['start'] !== (string) $interval['start']) {
if ((string) $candidateIntervals['intervals'][$index]->getStart() !== (string) $interval->getStart()) {
return false;
}

if ((string) $candidateIntervals['intervals'][$index]['end'] !== (string) $interval['end']) {
if ((string) $candidateIntervals['intervals'][$index]->getEnd() !== (string) $interval->getEnd()) {
return false;
}
}
Expand Down Expand Up @@ -110,10 +110,10 @@ public static function haveIntersections(ConstraintInterface $a, ConstraintInter
*
* if the returned intervals array is empty it means the constraint matches nothing in the numeric range (0 - +inf)
* if the returned devConstraints array is empty it means no dev-* versions are matched
* if a constraint matches all possible dev-* versions, devConstraints will contain self::anyDev() as a constraint
* if a constraint matches all possible dev-* versions, devConstraints will contain Interval::anyDev() as a constraint
*
* @return array
Seldaek marked this conversation as resolved.
Show resolved Hide resolved
* @phpstan-return array{'intervals': array<int, array{'start': Constraint, 'end': Constraint}>, 'devConstraints': Constraint[]}
* @phpstan-return array{'intervals': Interval[], 'devConstraints': Constraint[]}
*/
public static function get(ConstraintInterface $constraint)
{
Expand All @@ -127,12 +127,12 @@ public static function get(ConstraintInterface $constraint)
}

/**
* @phpstan-return array{'intervals': array<int, array{'start': Constraint, 'end': Constraint}>, 'devConstraints': Constraint[]}
* @phpstan-return array{'intervals': Interval[], 'devConstraints': Constraint[]}
*/
private static function generateIntervals(ConstraintInterface $constraint, $stopOnFirstValidInterval = false)
{
if ($constraint instanceof EmptyConstraint) {
return array('intervals' => array(array('start' => self::zero(), 'end' => self::positiveInfinity())), 'devConstraints' => array(self::anyDev()));
return array('intervals' => array(new Interval(Interval::zero(), Interval::positiveInfinity())), 'devConstraints' => array(Interval::anyDev()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imho we have the same optimization possibility here as we'd have with a ConstraintFactory: we don't need a new instance of Interval over and over again if the interval is exactly the same. We could reuse existing interval instances, right? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but IMO that's still best done with the memoization on get(), because the expensive stuff here is computing intervals for a constraint, not storing them. Especially as they'll get used at poolbuilder time and then it can be cleared as it becomes useless. Feel free to PR if you have time though :P

}

if ($constraint instanceof Constraint) {
Expand Down Expand Up @@ -164,7 +164,7 @@ private static function generateIntervals(ConstraintInterface $constraint, $stop
}

foreach ($dev as $i => $c) {
if ($c === self::anyDev()) {
if ($c === Interval::anyDev()) {
$dev = array($c);
break;
}
Expand Down Expand Up @@ -210,7 +210,7 @@ private static function generateIntervals(ConstraintInterface $constraint, $stop
}

foreach ($group2 as $j2 => $c2) {
if ((string) $c2 === (string) $c || $c2 === self::anyDev()) {
if ((string) $c2 === (string) $c || $c2 === Interval::anyDev()) {
$otherGroupMatches++;
}

Expand Down Expand Up @@ -242,8 +242,8 @@ private static function generateIntervals(ConstraintInterface $constraint, $stop
$borders = array();
foreach ($intervalGroups as $group) {
foreach ($group as $interval) {
$borders[] = array('version' => $interval['start']->getVersion(), 'operator' => $interval['start']->getOperator(), 'side' => 'start');
$borders[] = array('version' => $interval['end']->getVersion(), 'operator' => $interval['end']->getOperator(), 'side' => 'end');
$borders[] = array('version' => $interval->getStart()->getVersion(), 'operator' => $interval->getStart()->getOperator(), 'side' => 'start');
$borders[] = array('version' => $interval->getEnd()->getVersion(), 'operator' => $interval->getEnd()->getOperator(), 'side' => 'end');
}
}

Expand All @@ -262,44 +262,47 @@ private static function generateIntervals(ConstraintInterface $constraint, $stop
$index = 0;
$activationThreshold = $constraint->isConjunctive() ? \count($intervalGroups) : 1;
$active = false;
$start = null;
foreach ($borders as $border) {
if ($border['side'] === 'start') {
$activeIntervals++;
} else {
$activeIntervals--;
}
if (!$active && $activeIntervals >= $activationThreshold) {
$intervals[$index] = array('start' => new Constraint($border['operator'], $border['version']));
$start = new Constraint($border['operator'], $border['version']);
$active = true;
}
if ($active && $activeIntervals < $activationThreshold) {
$active = false;

// filter out invalid intervals like > x - <= x, or >= x - < x
if (
version_compare($intervals[$index]['start']->getVersion(), $border['version'], '=')
version_compare($start->getVersion(), $border['version'], '=')
&& (
($intervals[$index]['start']->getOperator() === '>' && $border['operator'] === '<=')
|| ($intervals[$index]['start']->getOperator() === '>=' && $border['operator'] === '<')
($start->getOperator() === '>' && $border['operator'] === '<=')
|| ($start->getOperator() === '>=' && $border['operator'] === '<')
)
) {
unset($intervals[$index]);
} else {
$intervals[$index]['end'] = new Constraint($border['operator'], $border['version']);
$intervals[$index] = new Interval($start, new Constraint($border['operator'], $border['version']));
$index++;

if ($stopOnFirstValidInterval) {
break;
}
}

$start = null;
}
}

return array('intervals' => $intervals, 'devConstraints' => $dev);
}

/**
* @phpstan-return array{'intervals': array<int, array{'start': Constraint, 'end': Constraint}>, 'devConstraints': Constraint[]}
* @phpstan-return array{'intervals': Interval[], 'devConstraints': Constraint[]}
*/
private static function generateSingleConstraintIntervals(Constraint $constraint)
{
Expand All @@ -309,64 +312,29 @@ private static function generateSingleConstraintIntervals(Constraint $constraint

// != dev-foo means any numeric version may match
if ($op === '!=') {
$intervals[] = array('start' => self::zero(), 'end' => self::positiveInfinity());
$intervals[] = new Interval(Interval::zero(), Interval::positiveInfinity());
}

return array('intervals' => $intervals, 'devConstraints' => array($constraint));
}

if ($op[0] === '>') { // > & >=
naderman marked this conversation as resolved.
Show resolved Hide resolved
return array('intervals' => array(array('start' => $constraint, 'end' => self::positiveInfinity())), 'devConstraints' => array());
return array('intervals' => array(new Interval($constraint, Interval::positiveInfinity())), 'devConstraints' => array());
}
if ($op[0] === '<') { // < & <=
return array('intervals' => array(array('start' => self::zero(), 'end' => $constraint)), 'devConstraints' => array());
return array('intervals' => array(new Interval(Interval::zero(), $constraint)), 'devConstraints' => array());
}
if ($op === '!=') {
// convert !=x to intervals of 0 - <x && >x - +inf + dev*
return array('intervals' => array(
array('start' => self::zero(), 'end' => new Constraint('<', $constraint->getVersion())),
array('start' => new Constraint('>', $constraint->getVersion()), 'end' => self::positiveInfinity()),
), 'devConstraints' => array(self::anyDev()));
new Interval(Interval::zero(), new Constraint('<', $constraint->getVersion())),
new Interval(new Constraint('>', $constraint->getVersion()), Interval::positiveInfinity()),
), 'devConstraints' => array(Interval::anyDev()));
}

// convert ==x to an interval of >=x - <=x
return array('intervals' => array(
array('start' => new Constraint('>=', $constraint->getVersion()), 'end' => new Constraint('<=', $constraint->getVersion())),
new Interval(new Constraint('>=', $constraint->getVersion()), new Constraint('<=', $constraint->getVersion())),
), 'devConstraints' => array());
}

public static function zero()
{
static $zero;

if (null === $zero) {
$zero = new Constraint('>=', '0.0.0.0-dev');
}

return $zero;
}

public static function positiveInfinity()
{
static $positiveInfinity;

if (null === $positiveInfinity) {
$positiveInfinity = new Constraint('<', PHP_INT_MAX.'.0.0.0');
}

return $positiveInfinity;
}

public static function anyDev()
{
static $anyDev;

if (null === $anyDev) {
// this ideally should be an EmptyConstraint but the code expects Constraint instances so
// this makes it work with less workarounds/checks above
$anyDev = new Constraint('==', 'dev*');
}

return $anyDev;
}
}
3 changes: 3 additions & 0 deletions tests/IntervalsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public function testGetIntervals($expected, $constraint)
$result = Intervals::get($constraint);
if (is_array($result)) {
array_walk_recursive($result, function (&$c) {
if ($c instanceof Interval) {
$c = array('start' => (string) $c->getStart(), 'end' => (string) $c->getEnd());
}
if ($c instanceof ConstraintInterface) {
$c = (string) $c;
}
Expand Down