From 67eedad253a607072aecb1d4322a955fbe0d41eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Sat, 16 Dec 2023 15:06:27 +0100 Subject: [PATCH] refactor: Make the Composer artifacts null rather than allowing empty objects (#1283) --- src/Composer/Artifact/ComposerFile.php | 14 +-- src/Composer/Artifact/ComposerFiles.php | 23 ++--- src/Configuration/Configuration.php | 46 +++++----- src/Configuration/ExportableConfiguration.php | 24 ++--- src/Console/Command/Compile.php | 6 +- tests/Composer/Artifact/ComposerFileTest.php | 90 ------------------- tests/Composer/Artifact/ComposerFilesTest.php | 20 ++--- tests/Configuration/ConfigurationTest.php | 36 ++++---- tests/Console/Command/CompileTest.php | 10 +-- 9 files changed, 76 insertions(+), 193 deletions(-) delete mode 100644 tests/Composer/Artifact/ComposerFileTest.php diff --git a/src/Composer/Artifact/ComposerFile.php b/src/Composer/Artifact/ComposerFile.php index 4da5194f4..f02ff555e 100644 --- a/src/Composer/Artifact/ComposerFile.php +++ b/src/Composer/Artifact/ComposerFile.php @@ -14,23 +14,11 @@ namespace KevinGH\Box\Composer\Artifact; -use Webmozart\Assert\Assert; - final readonly class ComposerFile { - public static function createEmpty(): self - { - return new self(null, []); - } - public function __construct( - public ?string $path, + public string $path, public array $decodedContents, ) { - Assert::nullOrNotEmpty($path); - - if (null === $path) { - Assert::same([], $decodedContents); - } } } diff --git a/src/Composer/Artifact/ComposerFiles.php b/src/Composer/Artifact/ComposerFiles.php index 6ab9f62b0..9234f0f65 100644 --- a/src/Composer/Artifact/ComposerFiles.php +++ b/src/Composer/Artifact/ComposerFiles.php @@ -20,33 +20,24 @@ final readonly class ComposerFiles { - public static function createEmpty(): self - { - return new self( - ComposerFile::createEmpty(), - ComposerFile::createEmpty(), - ComposerFile::createEmpty(), - ); - } - public function __construct( - private ComposerFile $composerJson, - private ComposerFile $composerLock, - private ComposerFile $installedJson, + private ?ComposerFile $composerJson = null, + private ?ComposerFile $composerLock = null, + private ?ComposerFile $installedJson = null, ) { } - public function getComposerJson(): ComposerFile + public function getComposerJson(): ?ComposerFile { return $this->composerJson; } - public function getComposerLock(): ComposerFile + public function getComposerLock(): ?ComposerFile { return $this->composerLock; } - public function getInstalledJson(): ComposerFile + public function getInstalledJson(): ?ComposerFile { return $this->installedJson; } @@ -59,7 +50,7 @@ public function getPaths(): array return array_values( array_filter( array_map( - static fn (ComposerFile $file): ?string => $file->path, + static fn (?ComposerFile $file): ?string => $file?->path, [$this->composerJson, $this->composerLock, $this->installedJson], ), ), diff --git a/src/Configuration/Configuration.php b/src/Configuration/Configuration.php index b5ea6eb77..9fbccd78a 100644 --- a/src/Configuration/Configuration.php +++ b/src/Configuration/Configuration.php @@ -226,7 +226,7 @@ public static function create(?string $file, stdClass $raw): self $excludeComposerFiles = self::retrieveExcludeComposerFiles($raw, $logger); - $mainScriptPath = self::retrieveMainScriptPath($raw, $basePath, $composerFiles->getComposerJson()->decodedContents, $logger); + $mainScriptPath = self::retrieveMainScriptPath($raw, $basePath, $composerFiles->getComposerJson()?->decodedContents, $logger); $mainScriptContents = self::retrieveMainScriptContents($mainScriptPath); [$tmpOutputPath, $outputPath] = self::retrieveOutputPath($raw, $basePath, $mainScriptPath, $logger); @@ -255,8 +255,8 @@ public static function create(?string $file, stdClass $raw): self $checkRequirements = self::retrieveCheckRequirements( $raw, - null !== $composerFiles->getComposerJson()->path, - null !== $composerFiles->getComposerLock()->path, + null !== $composerFiles->getComposerJson()?->path, + null !== $composerFiles->getComposerLock()?->path, false === $isStubGenerated && null === $stubPath, $logger, ); @@ -265,8 +265,8 @@ public static function create(?string $file, stdClass $raw): self $devPackages = ComposerConfiguration::retrieveDevPackages( $basePath, - new DecodedComposerJson($composerFiles->getComposerJson()->decodedContents), - new DecodedComposerLock($composerFiles->getComposerLock()->decodedContents), + new DecodedComposerJson($composerFiles->getComposerJson()?->decodedContents ?? []), + new DecodedComposerLock($composerFiles->getComposerLock()?->decodedContents ?? []), $excludeDevPackages, ); @@ -422,8 +422,8 @@ private function __construct( private readonly ?string $file, private readonly string $alias, private readonly string $basePath, - private readonly ComposerFile $composerJson, - private readonly ComposerFile $composerLock, + private readonly ?ComposerFile $composerJson, + private readonly ?ComposerFile $composerLock, private readonly array $files, private readonly array $binaryFiles, private readonly bool $autodiscoveredFiles, @@ -495,12 +495,12 @@ public function getBasePath(): string return $this->basePath; } - public function getComposerJson(): ComposerFile + public function getComposerJson(): ?ComposerFile { return $this->composerJson; } - public function getComposerLock(): ComposerFile + public function getComposerLock(): ?ComposerFile { return $this->composerLock; } @@ -848,7 +848,7 @@ private static function collectFiles( if ($autodiscoverFiles || $forceFilesAutodiscovery) { [$filesToAppend, $directories] = self::retrieveAllDirectoriesToInclude( $basePath, - $composerFiles->getComposerJson()->decodedContents, + $composerFiles->getComposerJson()?->decodedContents, $devPackages, $composerFiles->getPaths(), $excludedPaths, @@ -912,7 +912,7 @@ private static function collectBinaryFiles( array $devPackages, ConfigurationLogger $logger, ): array { - $binaryFiles = self::retrieveFiles($raw, self::FILES_BIN_KEY, $basePath, ComposerFiles::createEmpty(), $alwaysExcludedPaths, $logger); + $binaryFiles = self::retrieveFiles($raw, self::FILES_BIN_KEY, $basePath, new ComposerFiles(), $alwaysExcludedPaths, $logger); $binaryDirectories = self::retrieveDirectories( $raw, @@ -952,8 +952,8 @@ private static function retrieveFiles( $excludedFiles = array_flip($excludedFiles); $files = array_filter([ - $composerFiles->getComposerJson()->path, - $composerFiles->getComposerLock()->path, + $composerFiles->getComposerJson()?->path, + $composerFiles->getComposerLock()?->path, ]); if (false === isset($raw->{$key})) { @@ -1541,22 +1541,22 @@ private static function retrieveDumpAutoload(stdClass $raw, ComposerFiles $compo self::checkIfDefaultValue($logger, $raw, self::DUMP_AUTOLOAD_KEY, null); $canDumpAutoload = ( - null !== $composerFiles->getComposerJson()->path + null !== $composerFiles->getComposerJson()?->path && ( // The composer.lock and installed.json are optional (e.g. if there is no dependencies installed) // but when one is present, the other must be as well otherwise the dumped autoloader will be broken ( - null === $composerFiles->getComposerLock()->path - && null === $composerFiles->getInstalledJson()->path + null === $composerFiles->getComposerLock()?->path + && null === $composerFiles->getInstalledJson()?->path ) || ( - null !== $composerFiles->getComposerLock()->path - && null !== $composerFiles->getInstalledJson()->path + null !== $composerFiles->getComposerLock()?->path + && null !== $composerFiles->getInstalledJson()?->path ) || ( - null === $composerFiles->getComposerLock()->path - && null !== $composerFiles->getInstalledJson()->path - && [] === $composerFiles->getInstalledJson()->decodedContents + null === $composerFiles->getComposerLock()?->path + && null !== $composerFiles->getInstalledJson()?->path + && [] === $composerFiles->getInstalledJson()?->decodedContents ) ) ); @@ -1815,12 +1815,12 @@ private static function retrieveComposerFiles(string $basePath): ComposerFiles ); } - private static function retrieveComposerFile(string $path): ComposerFile + private static function retrieveComposerFile(string $path): ?ComposerFile { $json = new Json(); if (false === file_exists($path) || false === is_file($path) || false === is_readable($path)) { - return ComposerFile::createEmpty(); + return null; } try { diff --git a/src/Configuration/ExportableConfiguration.php b/src/Configuration/ExportableConfiguration.php index 3a1fd3b13..b6c3c679c 100644 --- a/src/Configuration/ExportableConfiguration.php +++ b/src/Configuration/ExportableConfiguration.php @@ -52,14 +52,18 @@ public static function create(Configuration $configuration): self $normalizePath($configuration->getConfigurationFile()), $configuration->getAlias(), $configuration->getBasePath(), - new ComposerFile( - $normalizePath($composerJson->path), - $composerJson->decodedContents, - ), - new ComposerFile( - $normalizePath($composerLock->path), - $composerLock->decodedContents, - ), + null === $composerJson + ? null + : new ComposerFile( + $normalizePath($composerJson->path), + $composerJson->decodedContents, + ), + null === $composerLock + ? null + : new ComposerFile( + $normalizePath($composerLock->path), + $composerLock->decodedContents, + ), $normalizePaths($configuration->getFiles()), $normalizePaths($configuration->getBinaryFiles()), $configuration->hasAutodiscoveredFiles(), @@ -116,8 +120,8 @@ private function __construct( private ?string $file, private string $alias, private string $basePath, - private ComposerFile $composerJson, - private ComposerFile $composerLock, + private ?ComposerFile $composerJson, + private ?ComposerFile $composerLock, private array $files, private array $binaryFiles, private bool $autodiscoveredFiles, diff --git a/src/Console/Command/Compile.php b/src/Console/Command/Compile.php index 6e19266ac..53b936d66 100644 --- a/src/Console/Command/Compile.php +++ b/src/Console/Command/Compile.php @@ -567,8 +567,8 @@ private static function registerRequirementsChecker(Configuration $config, Box $ ); $checkFiles = RequirementsDumper::dump( - new DecodedComposerJson($config->getComposerJson()->decodedContents), - new DecodedComposerLock($config->getComposerLock()->decodedContents), + new DecodedComposerJson($config->getComposerJson()?->decodedContents ?? []), + new DecodedComposerLock($config->getComposerLock()?->decodedContents ?? []), $config->getCompressionAlgorithm(), ); @@ -682,7 +682,7 @@ private static function checkComposerFiles(Box $box, Configuration $config, Comp if ($config->excludeComposerFiles()) { $box->removeComposerArtefacts( ComposerConfiguration::retrieveVendorDir( - new DecodedComposerJson($config->getComposerJson()->decodedContents), + new DecodedComposerJson($config->getComposerJson()?->decodedContents ?? []), ), ); } diff --git a/tests/Composer/Artifact/ComposerFileTest.php b/tests/Composer/Artifact/ComposerFileTest.php deleted file mode 100644 index 73fbb823a..000000000 --- a/tests/Composer/Artifact/ComposerFileTest.php +++ /dev/null @@ -1,90 +0,0 @@ - - * Théo Fidry - * - * This source file is subject to the MIT license that is bundled - * with this source code in the file LICENSE. - */ - -namespace KevinGH\Box\Composer\Artifact; - -use Closure; -use LogicException; -use PHPUnit\Framework\Attributes\CoversClass; -use PHPUnit\Framework\Attributes\DataProvider; -use PHPUnit\Framework\TestCase; - -/** - * @internal - */ -#[CoversClass(ComposerFile::class)] -class ComposerFileTest extends TestCase -{ - #[DataProvider('validInstantiatorsProvider')] - public function test_it_can_be_created(Closure $create, ?string $expectedPath, array $expectedContents): void - { - /** @var ComposerFile $actual */ - $actual = $create(); - - self::assertInstanceOf(ComposerFile::class, $actual); - - self::assertSame($expectedPath, $actual->path); - self::assertSame($expectedContents, $actual->decodedContents); - } - - #[DataProvider('invalidInstantiatorsProvider')] - public function test_it_cannot_be_created_with_invalid_values(Closure $create, string $errorMessage): void - { - try { - $create(); - - self::fail('Expected exception to be thrown.'); - } catch (LogicException $exception) { - self::assertSame($errorMessage, $exception->getMessage()); - } - } - - public static function validInstantiatorsProvider(): iterable - { - yield [ - static fn (): ComposerFile => new ComposerFile(null, []), - null, - [], - ]; - - yield [ - static fn (): ComposerFile => ComposerFile::createEmpty(), - null, - [], - ]; - - yield [ - static fn (): ComposerFile => new ComposerFile('path/to/foo', ['foo' => 'bar']), - 'path/to/foo', - ['foo' => 'bar'], - ]; - } - - public static function invalidInstantiatorsProvider(): iterable - { - yield [ - static function (): void { - new ComposerFile('', []); - }, - 'Expected a non-empty value. Got: ""', - ]; - - yield [ - static function (): void { - new ComposerFile(null, ['foo' => 'bar']); - }, - 'Expected a value identical to array. Got: array', - ]; - } -} diff --git a/tests/Composer/Artifact/ComposerFilesTest.php b/tests/Composer/Artifact/ComposerFilesTest.php index 925899670..682688752 100644 --- a/tests/Composer/Artifact/ComposerFilesTest.php +++ b/tests/Composer/Artifact/ComposerFilesTest.php @@ -28,9 +28,9 @@ class ComposerFilesTest extends TestCase #[DataProvider('validInstantiatorsProvider')] public function test_it_can_be_created( Closure $create, - ComposerFile $expectedComposerJson, - ComposerFile $expectedComposerLock, - ComposerFile $expectedInstalledJson, + ?ComposerFile $expectedComposerJson, + ?ComposerFile $expectedComposerLock, + ?ComposerFile $expectedInstalledJson, array $expectedPaths, ): void { /** @var ComposerFiles $actual */ @@ -67,7 +67,7 @@ public static function validInstantiatorsProvider(): iterable yield (static function (): array { $json = new ComposerFile('path/to/composer.json', ['name' => 'composer.json']); - $lock = ComposerFile::createEmpty(); + $lock = null; $installed = new ComposerFile('path/to/installed.json', ['name' => 'installed.json']); return [ @@ -82,12 +82,12 @@ public static function validInstantiatorsProvider(): iterable ]; })(); - yield (static fn (): array => [ - static fn (): ComposerFiles => ComposerFiles::createEmpty(), - ComposerFile::createEmpty(), - ComposerFile::createEmpty(), - ComposerFile::createEmpty(), + yield [ + static fn () => new ComposerFiles(), + null, + null, + null, [], - ])(); + ]; } } diff --git a/tests/Configuration/ConfigurationTest.php b/tests/Configuration/ConfigurationTest.php index c4935c398..b2de07d04 100644 --- a/tests/Configuration/ConfigurationTest.php +++ b/tests/Configuration/ConfigurationTest.php @@ -313,7 +313,7 @@ public function test_the_base_path_value_is_normalized(): void self::assertSame([], $this->config->getWarnings()); } - #[DataProvider('jsonFilesProvider')] + #[DataProvider('composerFilesProvider')] public function test_it_attempts_to_get_and_decode_the_json_and_lock_files( callable $setup, ?string $expectedJsonPath, @@ -323,23 +323,19 @@ public function test_it_attempts_to_get_and_decode_the_json_and_lock_files( ): void { $setup(); - if (null !== $expectedJsonPath) { - $expectedJsonPath = $this->tmp.DIRECTORY_SEPARATOR.$expectedJsonPath; - } - - if (null !== $expectedLockPath) { - $expectedLockPath = $this->tmp.DIRECTORY_SEPARATOR.$expectedLockPath; - } - - $expectedJson = new ComposerFile( - $expectedJsonPath, - $expectedJsonContents ?? [], - ); + $expectedJson = null === $expectedJsonPath + ? null + : new ComposerFile( + $this->tmp.DIRECTORY_SEPARATOR.$expectedJsonPath, + $expectedJsonContents ?? [], + ); - $expectedLock = new ComposerFile( - $expectedLockPath, - $expectedLockContents ?? [], - ); + $expectedLock = null === $expectedLockPath + ? null + : new ComposerFile( + $this->tmp.DIRECTORY_SEPARATOR.$expectedLockPath, + $expectedLockContents ?? [], + ); $this->reloadConfig(); @@ -2970,8 +2966,8 @@ public function test_it_can_be_created_with_only_default_values(): void self::assertSame([], $this->config->getBinaryFiles()); self::assertSame([], $this->config->getCompactors()->toArray()); self::assertFalse($this->config->hasAutodiscoveredFiles()); - self::assertNotNull($this->config->getComposerJson()); - self::assertNotNull($this->config->getComposerLock()); + self::assertNull($this->config->getComposerJson()); + self::assertNull($this->config->getComposerLock()); self::assertSame(CompressionAlgorithm::NONE, $this->config->getCompressionAlgorithm()); self::assertSame($this->tmp.'/box.json', $this->config->getConfigurationFile()); self::assertEquals( @@ -3230,7 +3226,7 @@ public static function unormalizedCustomBannerProvider(): iterable ]; } - public static function jsonFilesProvider(): iterable + public static function composerFilesProvider(): iterable { yield [ static function (): void {}, diff --git a/tests/Console/Command/CompileTest.php b/tests/Console/Command/CompileTest.php index 8b37a4028..92654cf50 100644 --- a/tests/Console/Command/CompileTest.php +++ b/tests/Console/Command/CompileTest.php @@ -1187,14 +1187,8 @@ public function test_it_can_build_a_phar_file_in_debug_mode(): void -file: "box.json" -alias: "index.phar" -basePath: "/path/to" - -composerJson: KevinGH\Box\Composer\Artifact\ComposerFile {#140 - +path: null - +decodedContents: [] - } - -composerLock: KevinGH\Box\Composer\Artifact\ComposerFile {#140 - +path: null - +decodedContents: [] - } + -composerJson: null + -composerLock: null -files: [] -binaryFiles: [] -autodiscoveredFiles: true