From 3aaca358173d0a48929829cb6684d64ebb20b897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Wed, 11 Oct 2023 14:03:35 +0200 Subject: [PATCH] feat: Introduce a `--diff=diffMode` option (#1044) - Introduce a new option `--diff=list|git|gnu`. - Deprecate `--list-diff`, `--git-diff` and `--gnu-diff` in favour of the `--diff` option. --- src/Console/Command/Diff.php | 78 ++++++++++++-- src/Phar/DiffMode.php | 35 ++++++ src/Phar/PharDiff.php | 27 +++-- tests/Console/Command/DiffTest.php | 168 +++++++++++++++++++---------- 4 files changed, 235 insertions(+), 73 deletions(-) create mode 100644 src/Phar/DiffMode.php diff --git a/src/Console/Command/Diff.php b/src/Console/Command/Diff.php index b06275fe9..f706ab5ab 100644 --- a/src/Console/Command/Diff.php +++ b/src/Console/Command/Diff.php @@ -19,6 +19,7 @@ use Fidry\Console\ExitCode; use Fidry\Console\Input\IO; use KevinGH\Box\Console\PharInfoRenderer; +use KevinGH\Box\Phar\DiffMode; use KevinGH\Box\Phar\PharDiff; use KevinGH\Box\Phar\PharInfo; use Symfony\Component\Console\Input\InputArgument; @@ -30,6 +31,7 @@ use function count; // TODO: migrate to Safe API use function explode; +use function implode; use function is_string; use function sprintf; use const PHP_EOL; @@ -42,9 +44,11 @@ final class Diff implements Command private const FIRST_PHAR_ARG = 'pharA'; private const SECOND_PHAR_ARG = 'pharB'; + // TODO: replace by DiffMode::X->value once bumping to PHP 8.2 as the min version. private const LIST_FILES_DIFF_OPTION = 'list-diff'; private const GIT_DIFF_OPTION = 'git-diff'; private const GNU_DIFF_OPTION = 'gnu-diff'; + private const DIFF_OPTION = 'diff'; private const CHECK_OPTION = 'check'; private const DEFAULT_CHECKSUM_ALGO = 'sha384'; @@ -72,19 +76,32 @@ public function getConfiguration(): Configuration self::GNU_DIFF_OPTION, null, InputOption::VALUE_NONE, - 'Displays a GNU diff', + '(deprecated) Displays a GNU diff', ), new InputOption( self::GIT_DIFF_OPTION, null, InputOption::VALUE_NONE, - 'Displays a Git diff', + '(deprecated) Displays a Git diff', ), new InputOption( self::LIST_FILES_DIFF_OPTION, null, InputOption::VALUE_NONE, - 'Displays a list of file names diff (default)', + '(deprecated) Displays a list of file names diff (default)', + ), + new InputOption( + self::DIFF_OPTION, + null, + InputOption::VALUE_REQUIRED, + sprintf( + 'Displays a diff of the files. Available options are: "%s"', + implode( + '", "', + DiffMode::values(), + ), + ), + DiffMode::LIST->value, ), new InputOption( self::CHECK_OPTION, @@ -157,6 +174,50 @@ private function compareArchives(PharDiff $diff, IO $io): int return ExitCode::FAILURE; } + private static function getDiffMode(IO $io): DiffMode + { + if ($io->getOption(self::GNU_DIFF_OPTION)->asBoolean()) { + $io->writeln( + sprintf( + '⚠️ Using the option "%s" is deprecated. Use "--%s=%s" instead.', + self::GNU_DIFF_OPTION, + self::DIFF_OPTION, + DiffMode::GNU->value, + ), + ); + + return DiffMode::GNU; + } + + if ($io->getOption(self::GIT_DIFF_OPTION)->asBoolean()) { + $io->writeln( + sprintf( + '⚠️ Using the option "%s" is deprecated. Use "--%s=%s" instead.', + self::GIT_DIFF_OPTION, + self::DIFF_OPTION, + DiffMode::GIT->value, + ), + ); + + return DiffMode::GIT; + } + + if ($io->getOption(self::LIST_FILES_DIFF_OPTION)->asBoolean()) { + $io->writeln( + sprintf( + '⚠️ Using the option "%s" is deprecated. Use "--%s=%s" instead.', + self::LIST_FILES_DIFF_OPTION, + self::DIFF_OPTION, + DiffMode::LIST->value, + ), + ); + + return DiffMode::LIST; + } + + return DiffMode::from($io->getOption(self::DIFF_OPTION)->asNonEmptyString()); + } + private function compareContents(PharDiff $diff, IO $io): int { $io->comment('Comparing the two archives contents...'); @@ -167,13 +228,9 @@ private function compareContents(PharDiff $diff, IO $io): int return $diff->listChecksums($checkSumAlgorithm); } - if ($io->getOption(self::GNU_DIFF_OPTION)->asBoolean()) { - $diffResult = $diff->gnuDiff(); - } elseif ($io->getOption(self::GIT_DIFF_OPTION)->asBoolean()) { - $diffResult = $diff->gitDiff(); - } else { - $diffResult = $diff->listDiff(); - } + $diffMode = self::getDiffMode($io); + + $diffResult = $diff->diff($diffMode); if (null === $diffResult || [[], []] === $diffResult) { $io->success('The contents are identical'); @@ -254,5 +311,6 @@ private static function renderArchive(string $fileName, PharInfo $pharInfo, IO $ PharInfoRenderer::renderSignature($pharInfo, $io); PharInfoRenderer::renderMetadata($pharInfo, $io); PharInfoRenderer::renderContentsSummary($pharInfo, $io); + // TODO: checksum } } diff --git a/src/Phar/DiffMode.php b/src/Phar/DiffMode.php new file mode 100644 index 000000000..bad3736b0 --- /dev/null +++ b/src/Phar/DiffMode.php @@ -0,0 +1,35 @@ + + * 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\Phar; + +use function array_map; + +enum DiffMode: string +{ + case LIST = 'list'; + case GIT = 'git'; + case GNU = 'gnu'; + + /** + * @return list + */ + public static function values(): array + { + return array_map( + static fn (self $enum) => $enum->value, + self::cases(), + ); + } +} diff --git a/src/Phar/PharDiff.php b/src/Phar/PharDiff.php index fd705f9be..6fbbbea12 100644 --- a/src/Phar/PharDiff.php +++ b/src/Phar/PharDiff.php @@ -26,6 +26,9 @@ use function str_replace; use const DIRECTORY_SEPARATOR; +/** + * @internal + */ final class PharDiff { private readonly ParagoniePharDiff $diff; @@ -58,22 +61,28 @@ public function getPharInfoB(): PharInfo return $this->pharInfoB; } - public function gitDiff(): ?string + /** + * @return null|string|array{string[], string[]} + */ + public function diff(DiffMode $mode): null|string|array { + if (DiffMode::LIST === $mode) { + return $this->listDiff(); + } + return self::getDiff( $this->pharInfoA, $this->pharInfoB, - 'git diff --no-index', + self::getModeCommand($mode), ); } - public function gnuDiff(): ?string + private static function getModeCommand(DiffMode $mode): string { - return self::getDiff( - $this->pharInfoA, - $this->pharInfoB, - 'diff --exclude='.Extract::PHAR_META_PATH, - ); + return match ($mode) { + DiffMode::GIT => 'git diff --no-index', + DiffMode::GNU => 'diff --exclude='.Extract::PHAR_META_PATH, + }; } /** @@ -89,7 +98,7 @@ public function listChecksums(string $algo = 'sha384'): int * which are not in the second and the second array all the files present in the second PHAR but * not the first one. */ - public function listDiff(): array + private function listDiff(): array { $pharAFiles = self::collectFiles($this->pharInfoA); $pharBFiles = self::collectFiles($this->pharInfoB); diff --git a/tests/Console/Command/DiffTest.php b/tests/Console/Command/DiffTest.php index 92b796741..4b9494eb5 100644 --- a/tests/Console/Command/DiffTest.php +++ b/tests/Console/Command/DiffTest.php @@ -18,11 +18,13 @@ use Fidry\Console\DisplayNormalizer; use Fidry\Console\ExitCode; use InvalidArgumentException; +use KevinGH\Box\Phar\DiffMode; use KevinGH\Box\Phar\InvalidPhar; use KevinGH\Box\Platform; use KevinGH\Box\Test\CommandTestCase; use KevinGH\Box\Test\RequiresPharReadonlyOff; use Symfony\Component\Console\Output\OutputInterface; +use function array_splice; use function ob_get_clean; use function ob_start; use function realpath; @@ -51,20 +53,25 @@ protected function getCommand(): Command } /** - * @dataProvider listDiffPharsProvider + * @dataProvider diffPharsProvider */ - public function test_it_can_display_the_list_diff_of_two_phar_files( + public function test_it_can_display_the_diff_of_two_phar_files( string $pharAPath, string $pharBPath, + DiffMode $diffMode, ?string $expectedOutput, int $expectedStatusCode, ): void { + if (DiffMode::GIT === $diffMode) { + self::markTestSkipped('TODO'); + } + $this->commandTester->execute( [ 'command' => 'diff', 'pharA' => $pharAPath, 'pharB' => $pharBPath, - '--list-diff' => null, + '--diff' => $diffMode->value, ], ); @@ -78,13 +85,18 @@ public function test_it_can_display_the_list_diff_of_two_phar_files( self::assertSame($expectedStatusCode, $this->commandTester->getStatusCode()); } - public function test_it_displays_the_list_diff_of_two_phar_files_by_default(): void + /** + * @deprecated + */ + public function test_it_can_display_the_list_diff_of_two_phar_files(): void { + $pharPath = realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'); + $this->commandTester->execute( [ 'command' => 'diff', - 'pharA' => realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'), - 'pharB' => realpath(self::FIXTURES_DIR.'/simple-phar-bar.phar'), + 'pharA' => $pharPath, + 'pharB' => $pharPath, '--list-diff' => null, ], ); @@ -97,76 +109,89 @@ public function test_it_displays_the_list_diff_of_two_phar_files_by_default(): v // Comparing the two archives contents... - --- Files present in "simple-phar-foo.phar" but not in "simple-phar-bar.phar" - +++ Files present in "simple-phar-bar.phar" but not in "simple-phar-foo.phar" - - - foo.php [NONE] - 29.00B + ⚠️ Using the option "list-diff" is deprecated. Use "--diff=list" instead. - + bar.php [NONE] - 29.00B - - [ERROR] 2 file(s) difference + [OK] The contents are identical OUTPUT; - $this->assertSameOutput($expectedOutput, ExitCode::FAILURE); + $this->assertSameOutput( + $expectedOutput, + ExitCode::SUCCESS, + ); } /** - * @dataProvider gitDiffPharsProvider + * @deprecated */ - public function test_it_can_display_the_git_diff_of_two_phar_files( - string $pharAPath, - string $pharBPath, - ?string $expectedOutput, - int $expectedStatusCode, - ): void { + public function test_it_can_display_the_git_diff_of_two_phar_files(): void + { self::markTestSkipped('TODO'); + $pharPath = realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'); + $this->commandTester->execute( [ 'command' => 'diff', - 'pharA' => $pharAPath, - 'pharB' => $pharBPath, + 'pharA' => $pharPath, + 'pharB' => $pharPath, '--git-diff' => null, ], ); - $actualOutput = DisplayNormalizer::removeTrailingSpaces( - $this->commandTester->getDisplay(true), - ); + $expectedOutput = <<<'OUTPUT' - if (null !== $expectedOutput) { - self::assertSame($expectedOutput, $actualOutput); - } - self::assertSame($expectedStatusCode, $this->commandTester->getStatusCode()); + // Comparing the two archives... (do not check the signatures) + + [OK] The two archives are identical + + // Comparing the two archives contents... + + ⚠️ Using the option "list-diff" is deprecated. Use "--diff=list" instead. + + [OK] The contents are identical + + + OUTPUT; + + $this->assertSameOutput( + $expectedOutput, + ExitCode::SUCCESS, + ); } - /** - * @dataProvider GNUDiffPharsProvider - */ - public function test_it_can_display_the_gnu_diff_of_two_phar_files( - string $pharAPath, - string $pharBPath, - ?string $expectedOutput, - int $expectedStatusCode, - ): void { + public function test_it_can_display_the_gnu_diff_of_two_phar_files(): void + { + $pharPath = realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'); + $this->commandTester->execute( [ 'command' => 'diff', - 'pharA' => $pharAPath, - 'pharB' => $pharBPath, + 'pharA' => $pharPath, + 'pharB' => $pharPath, '--gnu-diff' => null, ], ); - $actualOutput = DisplayNormalizer::removeTrailingSpaces( - $this->commandTester->getDisplay(true), - ); + $expectedOutput = <<<'OUTPUT' - if (null !== $expectedOutput) { - self::assertSame($expectedOutput, $actualOutput); - } - self::assertSame($expectedStatusCode, $this->commandTester->getStatusCode()); + // Comparing the two archives... (do not check the signatures) + + [OK] The two archives are identical + + // Comparing the two archives contents... + + ⚠️ Using the option "gnu-diff" is deprecated. Use "--diff=gnu" instead. + + [OK] The contents are identical + + + OUTPUT; + + $this->assertSameOutput( + $expectedOutput, + ExitCode::SUCCESS, + ); } public function test_it_can_check_the_sum_of_two_phar_files(): void @@ -294,7 +319,43 @@ public function test_it_does_not_swallow_exceptions_in_debug_mode(): void ); } - private static function diffPharsProvider(): iterable + public static function diffPharsProvider(): iterable + { + foreach (self::listDiffPharsProvider() as $label => $set) { + array_splice( + $set, + 2, + 0, + [DiffMode::LIST], + ); + + yield '[list] '.$label => $set; + } + + foreach (self::gitDiffPharsProvider() as $label => $set) { + array_splice( + $set, + 2, + 0, + [DiffMode::GIT], + ); + + yield '[git] '.$label => $set; + } + + foreach (self::GNUDiffPharsProvider() as $label => $set) { + array_splice( + $set, + 2, + 0, + [DiffMode::GNU], + ); + + yield '[GNU] '.$label => $set; + } + } + + private static function commonDiffPharsProvider(): iterable { yield 'same PHAR' => [ realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'), @@ -347,9 +408,9 @@ private static function diffPharsProvider(): iterable ]; } - public static function listDiffPharsProvider(): iterable + private static function listDiffPharsProvider(): iterable { - yield from self::diffPharsProvider(); + yield from self::commonDiffPharsProvider(); yield 'different files' => [ realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'), @@ -397,7 +458,7 @@ public static function listDiffPharsProvider(): iterable public static function gitDiffPharsProvider(): iterable { - yield from self::diffPharsProvider(); + yield from self::commonDiffPharsProvider(); yield 'different files' => [ realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'), @@ -447,12 +508,11 @@ public static function gitDiffPharsProvider(): iterable public static function GNUDiffPharsProvider(): iterable { - yield from self::diffPharsProvider(); + yield from self::commonDiffPharsProvider(); yield 'different files' => [ realpath(self::FIXTURES_DIR.'/simple-phar-foo.phar'), realpath(self::FIXTURES_DIR.'/simple-phar-bar.phar'), - <<<'OUTPUT' // Comparing the two archives... (do not check the signatures)