From e8d6200cb8322ed9318b8f25528213a601603d1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Tue, 12 Mar 2024 20:56:38 +0100 Subject: [PATCH 1/2] feat: Group by source the required and provided extensions Instead of first dividing by category (required or provided) and then by source, we no longer devide by category. This allows to better inspect what the status is for a given extension with the `info:requirements` command (see !1273). --- .../RequirementsBuilder.php | 85 +++++++++++-------- .../RequirementsBuilderTest.php | 4 +- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/RequirementChecker/RequirementsBuilder.php b/src/RequirementChecker/RequirementsBuilder.php index d4518edc9..1cf8a9339 100644 --- a/src/RequirementChecker/RequirementsBuilder.php +++ b/src/RequirementChecker/RequirementsBuilder.php @@ -19,14 +19,36 @@ use function array_map; use function array_unique; use function natsort; +use function strcmp; use function strnatcmp; use function uksort; +use function usort; final class RequirementsBuilder { + /** + * @var list + */ private array $predefinedRequirements = []; + + /** + * @var array> + */ private array $requiredExtensions = []; + + /** + * @var array> + */ private array $providedExtensions = []; + + /** + * @var array + */ + private array $allExtensions = []; + + /** + * @var array> + */ private array $conflictingExtensions = []; public function addRequirement(Requirement $requirement): void @@ -37,11 +59,13 @@ public function addRequirement(Requirement $requirement): void public function addRequiredExtension(Extension $extension, ?string $source): void { $this->requiredExtensions[$extension->name][] = $source; + $this->allExtensions[$extension->name][$source] = [$source, RequirementType::EXTENSION]; } public function addProvidedExtension(Extension $extension, ?string $source): void { $this->providedExtensions[$extension->name][] = $source; + $this->allExtensions[$extension->name][$source] = [$source, RequirementType::PROVIDED_EXTENSION]; } public function addConflictingExtension(Extension $extension, ?string $source): void @@ -53,21 +77,17 @@ public function all(): Requirements { $requirements = $this->predefinedRequirements; - foreach ($this->getUnfilteredSortedRequiredExtensions() as $extensionName => $sources) { - foreach ($sources as $source) { - $requirements[] = Requirement::forRequiredExtension( - $extensionName, - $source, - ); - } - } - - foreach ($this->getSortedProvidedExtensions() as $extensionName => $sources) { - foreach ($sources as $source) { - $requirements[] = Requirement::forProvidedExtension( - $extensionName, - $source, - ); + foreach ($this->getSortedRequiredAndProvidedExtensions() as $extensionName => $sources) { + foreach ($sources as [$source, $type]) { + $requirements[] = RequirementType::EXTENSION === $type + ? Requirement::forRequiredExtension( + $extensionName, + $source, + ) + : Requirement::forProvidedExtension( + $extensionName, + $source, + ); } } @@ -109,33 +129,28 @@ public function build(): Requirements } /** - * @return array> + * @return array> */ - private function getUnfilteredSortedRequiredExtensions(): array + private function getSortedRequiredAndProvidedExtensions(): array { return array_map( - self::createSortedDistinctList(...), - self::sortByExtensionName( - $this->requiredExtensions, - ), - ); - } + static function (array $sources): array { + usort( + $sources, + static fn (array $sourceTypePairA, array $sourceTypePairB) => strcmp( + (string) $sourceTypePairA[0], + (string) $sourceTypePairB[0], + ), + ); - /** - * @return array> - */ - private function getSortedProvidedExtensions(): array - { - return array_map( - self::createSortedDistinctList(...), - self::sortByExtensionName( - $this->providedExtensions, - ), + return $sources; + }, + self::sortByExtensionName($this->allExtensions), ); } /** - * @return array> + * @return array> */ private function getSortedRequiredExtensions(): array { @@ -151,7 +166,7 @@ private function getSortedRequiredExtensions(): array } /** - * @return array> + * @return array> */ private function getSortedConflictedExtensions(): array { diff --git a/tests/RequirementChecker/RequirementsBuilderTest.php b/tests/RequirementChecker/RequirementsBuilderTest.php index 559c0ae71..7331cea88 100644 --- a/tests/RequirementChecker/RequirementsBuilderTest.php +++ b/tests/RequirementChecker/RequirementsBuilderTest.php @@ -179,9 +179,9 @@ public function test_it_removes_extension_requirements_if_they_are_provided(): v $expectedAllRequirements = new Requirements([ Requirement::forRequiredExtension('http', 'package1'), Requirement::forRequiredExtension('http', 'package2'), + Requirement::forProvidedExtension('http', 'package3'), Requirement::forRequiredExtension('openssl', 'package3'), Requirement::forRequiredExtension('phar', 'package1'), - Requirement::forProvidedExtension('http', 'package3'), ]); $this->assertBuiltRequirementsEquals($expectedBuiltRequirements); @@ -218,9 +218,9 @@ public function test_it_does_not_remove_extension_conflicts_if_they_are_provided $expectedAllRequirements = new Requirements([ Requirement::forRequiredExtension('http', 'package1'), Requirement::forRequiredExtension('http', 'package2'), + Requirement::forProvidedExtension('http', 'package3'), Requirement::forRequiredExtension('openssl', 'package3'), Requirement::forRequiredExtension('phar', 'package1'), - Requirement::forProvidedExtension('http', 'package3'), ]); $this->assertBuiltRequirementsEquals($expectedBuiltRequirements); From 93ca74b194ccf4796969525ca5d2e247274e3298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Tue, 12 Mar 2024 21:06:21 +0100 Subject: [PATCH 2/2] fix mutation --- src/RequirementChecker/RequirementsBuilder.php | 2 +- tests/RequirementChecker/RequirementsBuilderTest.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/RequirementChecker/RequirementsBuilder.php b/src/RequirementChecker/RequirementsBuilder.php index 1cf8a9339..d8a5b4a2d 100644 --- a/src/RequirementChecker/RequirementsBuilder.php +++ b/src/RequirementChecker/RequirementsBuilder.php @@ -138,7 +138,7 @@ static function (array $sources): array { usort( $sources, static fn (array $sourceTypePairA, array $sourceTypePairB) => strcmp( - (string) $sourceTypePairA[0], + $sourceTypePairA[0], (string) $sourceTypePairB[0], ), ); diff --git a/tests/RequirementChecker/RequirementsBuilderTest.php b/tests/RequirementChecker/RequirementsBuilderTest.php index 7331cea88..f98cad507 100644 --- a/tests/RequirementChecker/RequirementsBuilderTest.php +++ b/tests/RequirementChecker/RequirementsBuilderTest.php @@ -68,6 +68,10 @@ public function test_it_can_build_requirements_from_required_extensions(): void new Extension('http'), 'package2', ); + $this->builder->addRequiredExtension( + new Extension('http'), + null, + ); $this->builder->addRequiredExtension( new Extension('phar'), 'package1', @@ -83,6 +87,7 @@ public function test_it_can_build_requirements_from_required_extensions(): void ); $expected = new Requirements([ + Requirement::forRequiredExtension('http', null), Requirement::forRequiredExtension('http', 'package1'), Requirement::forRequiredExtension('http', 'package2'), Requirement::forRequiredExtension('openssl', 'package3'),