Refactored constraints to be less fragile

.github/workflows/test.yml

@@ -25,5 +25,8 @@ jobs:
       - name: Install Composer dependencies
         run: composer install --prefer-dist --optimize-autoloader --no-progress
 
+      - name: Check coding standards
+        run: vendor/bin/php-cs-fixer fix --rules=@Symfony src --dry-run
+
       - name: Run tests
         run: vendor/bin/phpunit tests/

src/Commands/BuildComposerJson.php

@@ -6,14 +6,49 @@
 
 use App\ConstraintParser;
 use App\Container;
+use App\ReleaseManager;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Filesystem\Filesystem;
 
 #[AsCommand(name: 'build:composer')]
-final class BuildComposerJson extends BuildBase
+final class BuildComposerJson extends Command
 {
+    public function __construct(
+        protected readonly Filesystem $fileSystem,
+        protected readonly ReleaseManager $releaseManager,
+    ) {
+        parent::__construct();
+    }
+
+    protected function configure(): void
+    {
+        $this
+            ->addArgument(
+                'release',
+                description: 'The release category. Enter "legacy" for Drupal 7 and "current" for Drupal 8+.',
+                default: 'current'
+            );
+    }
+
+    protected function getInputSettings(InputInterface $input): array
+    {
+        return match ($input->getArgument('release')) {
+            '7.x', 'legacy' => [
+                'updateEndpoint' => '7.x',
+                'release' => 'legacy',
+                'file' => Container::baseDir().'/legacy.json',
+            ],
+            default => [
+                'updateEndpoint' => 'current',
+                'release' => 'current',
+                'file' => Container::baseDir().'/current.json',
+            ],
+        };
+    }
+
     private function generateConstraints(
         string $releaseCategory,
         string $updateEndpoint,
@@ -33,11 +68,10 @@ private function generateConstraints(
                 ->getUpdateData($name, $updateEndpoint);
 
             if (!$constraint = ConstraintParser::format($project)) {
-                $output->write('<comment>No valid constraints found!</comment>');
-
+                $output->write('<comment>No valid constraints found!</comment>'.PHP_EOL);
                 continue;
             }
-            $output->write('<info>Generated constraint:</info> ' . $constraint . PHP_EOL);
+            $output->write('<info>Generated constraint:</info> '.$constraint.PHP_EOL);
 
             $conflicts[$namespacedName] = $constraint;
         }
@@ -67,7 +101,7 @@ public function execute(InputInterface $input, OutputInterface $output): int
             'conflict' => $constraints,
         ];
 
-        $content = json_encode($composer, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE) . "\n";
+        $content = json_encode($composer, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)."\n";
 
         $this->fileSystem->dumpFile($file, $content);
 

src/ConstraintParser.php

@@ -10,14 +10,17 @@
 use Composer\Semver\Constraint\ConstraintInterface;
 use Composer\Semver\Constraint\MatchAllConstraint;
 use Composer\Semver\Constraint\MultiConstraint;
+use Composer\Semver\Semver;
 use Composer\Semver\VersionParser;
 use UnexpectedValueException;
 
 final class ConstraintParser
 {
+    use SemanticVersionTrait;
+
     private static function constraintToString(Constraint $constraint): string
     {
-        return $constraint->getOperator() . $constraint->getVersion();
+        return $constraint->getOperator().$constraint->getVersion();
     }
 
     public static function format(Project $project, string $separator = '|'): ?string
@@ -37,17 +40,21 @@ public static function format(Project $project, string $separator = '|'): ?strin
             if ($constraint instanceof Constraint) {
                 $parts[] = self::constraintToString($constraint);
             }
+
+            if ($constraint instanceof MatchAllConstraint) {
+                $parts[] = (string) $constraint;
+            }
         }
 
         return implode($separator, $parts);
     }
 
-    private static function getNormalizedVersion(UpdateRelease $release): ?string
+    private static function getNormalizedVersion(string $version): ?string
     {
         $versionParser = new VersionParser();
 
         try {
-            $version = $release->getSemanticVersion();
+            $version = self::convertLegacyVersionToSemantic($version);
 
             // Dev releases are never marked as insecure, so we can safely
             // ignore them.
@@ -61,6 +68,7 @@ private static function getNormalizedVersion(UpdateRelease $release): ?string
             return $version;
         } catch (UnexpectedValueException) {
         }
+
         return null;
     }
 
@@ -72,119 +80,108 @@ private static function filterReleases(Project $project): array
         $releases = [];
 
         foreach ($project->getReleases() as $release) {
-            if (!$version = self::getNormalizedVersion($release)) {
+            if (!$version = self::getNormalizedVersion($release->getVersion())) {
+                continue;
+            }
+
+            if (!self::isSupportedBranch($project, $release->getVersion())) {
                 continue;
             }
             $releases[$version] = $release;
         }
+
         return $releases;
     }
 
-    private static function isNewSecurityRelease(UpdateRelease $current, ?UpdateRelease $previous): bool
+    private static function isSupportedBranch(Project $project, string $version): bool
     {
-        if ($current->isSecurityRelease()) {
-            return true;
-        }
-        // Some releases are not marked as 'Security update' (usually alpha and beta releases).
-        // Make sure these are taken into account by checking if previous release was marked as
-        // 'insecure' but the current one is not.
-        if (!$current->isInsecure() && $previous?->isInsecure()) {
-            return true;
+        foreach ($project->getSupportedBranches() as $branch) {
+            if (str_starts_with($version, $branch)) {
+                return true;
+            }
         }
 
         return false;
     }
 
-    private static function reduceGroups(array $group, array $releases): array
+    private static function getBranchFromVersion(array $supportedBranches, string $version): string
     {
-        // Filter out group if project has no insecure releases after the first item
-        // of the given group.
-        // For example, given the following constraints '<1.0.0, >1.2.0, <2.0.1', the '<1.0.0'
-        // group is redundant since there's no security releases before the 1.0.0 release.
-        return array_filter($group, function (ConstraintInterface $constraint) use ($releases) {
-            if (!$constraint instanceof Constraint) {
-                return true;
-            }
-            $version = $constraint->getVersion();
-
-            reset($releases);
-            // Move the internal pointer to given constraint version.
-            while (current($releases)) {
-                if (key($releases) === $version) {
-                    break;
-                }
-                next($releases);
+        foreach ($supportedBranches as $branch) {
+            if (Semver::satisfies($version, '~'.$branch)) {
+                return $branch;
             }
-            // Loop through the remaining releases to check if there are any insecure
-            // releases.
-            while ($item = next($releases)) {
-                if ($item->isInsecure()) {
-                    return true;
-                }
+        }
+
+        foreach ($supportedBranches as $branch) {
+            if (Semver::satisfies($version, '^'.$branch)) {
+                return $branch;
             }
-            return false;
-        });
+        }
+
+        return '0.0.0';
     }
 
     public static function createConstraints(Project $project): array
     {
-        $constraintGroups = $groups = [];
-        /** @var UpdateRelease|null $previous */
-        $insecureRelease = $latestSecurityUpdate = $previous = null;
+        // Mark unsupported projects as insecure.
+        if ($project->isUnsupported() || !$project->getSupportedBranches()) {
+            return [new MatchAllConstraint()];
+        }
+        $insecureGroups = $constraints = $branches = [];
+        $latestSecurityUpdate = null;
 
         $releases = self::filterReleases($project);
-        $group = 0;
+        $supportedBranches = $project->getNormalizedSupportedBranches();
 
-        // Collect and group all known releases between two insecure releases.
         foreach (array_reverse($releases) as $version => $release) {
-            // Some projects have security updates where previous releases are not marked as
+            $branch = self::getBranchFromVersion($supportedBranches, $version);
+
+            // Some projects have security releases where previous releases are not marked as
             // insecure. Capture the latest known security release.
             if ($release->isSecurityRelease()) {
                 $latestSecurityUpdate = $version;
             }
 
-            if (self::isNewSecurityRelease($release, $previous)) {
-                $group++;
-            }
-
             if (!$release->isInsecure()) {
-                $constraintGroups[$group][] = $version;
+                $branches[$branch][] = $version;
             } else {
-                $insecureRelease = $version;
+                // Mark branch as insecure if there is at least one release marked as insecure.
+                $insecureGroups[$branch] = $version;
             }
-            $previous = $release;
         }
 
-        $constraintGroups = array_reverse($constraintGroups);
+        // Filter out branches without known security releases.
+        $branches = array_filter(array_reverse($branches), function (string $group) use ($insecureGroups) {
+            return isset($insecureGroups[$group]);
+        }, ARRAY_FILTER_USE_KEY);
 
-        // Compare the first item of current group against the last item of the next group and
-        // group them together, like '>{next groups last item}, <{current groups first item}'.
-        while ($current = current($constraintGroups)) {
-            $lowerBound = $upperBound = new Constraint('<', reset($current));
+        foreach ($branches as $branch => $versions) {
+            $constraints[] = MultiConstraint::create([
+                new Constraint('>=', $branch),
+                new Constraint('<', reset($versions)),
+            ]);
+        }
 
-            if (!$next = next($constraintGroups)) {
-                $groups[] = $lowerBound;
-                break;
+        if (!$constraints) {
+            // Mark all previous releases as insecure if project has at least one security release, but
+            // has no other releases marked as insecure. This can cause some false positives since there
+            // is no way of telling what versions are *actually* insecure.
+            if ($latestSecurityUpdate) {
+                return [new Constraint('<', $latestSecurityUpdate)];
             }
-            $lowerBound =  new Constraint('>', end($next));
 
-            $groups[] = MultiConstraint::create([$lowerBound, $upperBound]);
+            // Mark the whole project as insecure if project has branches marked as insecure, but no
+            // constraints were generated.
+            if (count($insecureGroups) > 0) {
+                return [new Constraint('<=', end($insecureGroups))];
+            }
         }
 
-        $groups = self::reduceGroups($groups, $releases);
+        usort($supportedBranches, 'version_compare');
 
-        if (!$groups) {
-            // Mark the whole project as insecure if no constraints were generated. The project
-            // is most likely abandoned and has publicly known security issue(s).
-            return $previous ? [new Constraint('<=', $previous->getSemanticVersion())] : [new MatchAllConstraint()];
-        }
+        // Use the oldest supported version as baseline lower bound.
+        $constraints[] = new Constraint('<', reset($supportedBranches));
 
-        // Mark all previous releases as insecure if project has at least one security release, but
-        // has no other releases marked as insecure. This can cause some false positives since there
-        // is no way of telling what versions are *actually* insecure.
-        if (!$insecureRelease && $latestSecurityUpdate) {
-            return [new Constraint('<', $latestSecurityUpdate)];
-        }
-        return array_reverse($groups);
+        return array_reverse($constraints);
     }
 }

src/Container.php

@@ -4,11 +4,12 @@
 
 final class Container
 {
-    protected array $service = [];
+    private array $service = [];
 
     public function add(string $name, mixed $object): self
     {
         $this->service[$name] = $object;
+
         return $this;
     }
 

src/DTO/Base.php

@@ -20,9 +20,7 @@ protected static function validate(Collection $collectionConstraint, mixed $data
                 $messages[] = 'Field '.$violation->getPropertyPath().': '.$violation->getMessage();
             }
 
-            throw new \UnexpectedValueException(
-                sprintf("Malformed data: %s\nData given:\n %s", implode(",\n", $messages), print_r($data, true))
-            );
+            throw new \UnexpectedValueException(sprintf("Malformed data: %s\nData given:\n %s", implode(",\n", $messages), print_r($data, true)));
         }
     }
 }