From e5ec207e890667c499705636b7984d77c0e5ba65 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 19:33:33 +0200 Subject: [PATCH] refactor: Move the different diff methods into dedicated classes (#1058) --- src/Console/Command/Diff.php | 71 +----- src/Phar/Differ/ChecksumDiffer.php | 130 ++++++++++ src/Phar/Differ/Differ.php | 29 +++ src/Phar/Differ/DifferFactory.php | 33 +++ src/Phar/Differ/FilenameDiffer.php | 143 +++++++++++ src/Phar/Differ/ProcessCommandBasedDiffer.php | 79 ++++++ src/Phar/PharDiff.php | 228 +----------------- 7 files changed, 424 insertions(+), 289 deletions(-) create mode 100644 src/Phar/Differ/ChecksumDiffer.php create mode 100644 src/Phar/Differ/Differ.php create mode 100644 src/Phar/Differ/DifferFactory.php create mode 100644 src/Phar/Differ/FilenameDiffer.php create mode 100644 src/Phar/Differ/ProcessCommandBasedDiffer.php diff --git a/src/Console/Command/Diff.php b/src/Console/Command/Diff.php index deba7c247..9fbe263e0 100644 --- a/src/Console/Command/Diff.php +++ b/src/Console/Command/Diff.php @@ -30,12 +30,8 @@ use Symfony\Component\Filesystem\Path; use Webmozart\Assert\Assert; use function array_map; -use function count; -use function explode; use function implode; -use function is_string; use function sprintf; -use const PHP_EOL; /** * @private @@ -272,72 +268,7 @@ private function renderContentsDiff(PharDiff $diff, DiffMode $diffMode, string $ ), ); - $diffResult = $diff->diff($diffMode, $checksumAlgorithm); - - if (null === $diffResult || [[], []] === $diffResult) { - $io->writeln('No difference could be observed with this mode.'); - - return; - } - - if (is_string($diffResult)) { - // Git or GNU diff: we don't have much control on the format - $io->writeln($diffResult); - - return; - } - - $io->writeln(sprintf( - '--- Files present in "%s" but not in "%s"', - $diff->getPharInfoA()->getFileName(), - $diff->getPharInfoB()->getFileName(), - )); - $io->writeln(sprintf( - '+++ Files present in "%s" but not in "%s"', - $diff->getPharInfoB()->getFileName(), - $diff->getPharInfoA()->getFileName(), - )); - - $io->newLine(); - - self::renderPaths('-', $diff->getPharInfoA(), $diffResult[0], $io); - self::renderPaths('+', $diff->getPharInfoB(), $diffResult[1], $io); - - $io->error( - sprintf( - '%d file(s) difference', - count($diffResult[0]) + count($diffResult[1]), - ), - ); - } - - /** - * @param list $paths - */ - private static function renderPaths(string $symbol, PharInfo $pharInfo, array $paths, IO $io): void - { - $bufferedOutput = new BufferedOutput( - $io->getVerbosity(), - $io->isDecorated(), - $io->getOutput()->getFormatter(), - ); - - PharInfoRenderer::renderContent( - $bufferedOutput, - $pharInfo, - false, - false, - ); - - $lines = array_map( - static fn (string $line) => '' === $line ? '' : $symbol.' '.$line, - explode( - PHP_EOL, - $bufferedOutput->fetch(), - ), - ); - - $io->writeln($lines); + $diff->diff($diffMode, $checksumAlgorithm, $io); } private static function renderSummary(PharInfo $pharInfo, IO $io): void diff --git a/src/Phar/Differ/ChecksumDiffer.php b/src/Phar/Differ/ChecksumDiffer.php new file mode 100644 index 000000000..87cd060bd --- /dev/null +++ b/src/Phar/Differ/ChecksumDiffer.php @@ -0,0 +1,130 @@ + + * 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\Differ; + +use Fidry\Console\Input\IO; +use KevinGH\Box\Phar\PharInfo; +use UnexpectedValueException; +use ValueError; +use function implode; + +final class ChecksumDiffer implements Differ +{ + public function __construct( + private string $checksumAlgorithm, + ) { + } + + public function diff( + PharInfo $pharInfoA, + PharInfo $pharInfoB, + IO $io, + ): void { + $diff = self::computeDiff( + $pharInfoA, + $pharInfoB, + $this->checksumAlgorithm, + ); + + $io->writeln($diff ?? Differ::NO_DIFF_MESSAGE); + } + + private static function computeDiff( + PharInfo $pharInfoA, + PharInfo $pharInfoB, + string $checksumAlgorithm, + ): ?string { + $pharInfoAFileHashes = self::getFileHashesByRelativePathname( + $pharInfoA, + $checksumAlgorithm, + ); + $pharInfoBFileHashes = self::getFileHashesByRelativePathname( + $pharInfoB, + $checksumAlgorithm, + ); + $output = [ + '--- PHAR A', + '+++ PHAR B', + '@@ @@', + ]; + + foreach ($pharInfoAFileHashes as $filePath => $fileAHash) { + if (!array_key_exists($filePath, $pharInfoBFileHashes)) { + $output[] = $filePath; + $output[] = sprintf( + "\t- %s", + $fileAHash, + ); + + continue; + } + + $fileBHash = $pharInfoBFileHashes[$filePath]; + unset($pharInfoBFileHashes[$filePath]); + + if ($fileAHash === $fileBHash) { + continue; + } + + $output[] = $filePath; + $output[] = sprintf( + "\t- %s", + $fileAHash, + ); + $output[] = sprintf( + "\t+ %s", + $fileBHash, + ); + } + + foreach ($pharInfoBFileHashes as $filePath => $fileBHash) { + $output[] = $filePath; + $output[] = sprintf( + "\t+ %s", + $fileBHash, + ); + } + + return 3 === count($output) ? null : implode("\n", $output); + } + + /** + * @return array + */ + private static function getFileHashesByRelativePathname( + PharInfo $pharInfo, + string $algorithm, + ): array { + $hashFiles = []; + + try { + foreach ($pharInfo->getFiles() as $file) { + $hashFiles[$file->getRelativePathname()] = hash_file( + $algorithm, + $file->getPathname(), + ); + } + } catch (ValueError) { + throw new UnexpectedValueException( + sprintf( + 'Unexpected algorithm "%s". Please pick a registered hashing algorithm (checksum `hash_algos()`).', + $algorithm, + ), + ); + } + + return $hashFiles; + } +} diff --git a/src/Phar/Differ/Differ.php b/src/Phar/Differ/Differ.php new file mode 100644 index 000000000..4797b7c58 --- /dev/null +++ b/src/Phar/Differ/Differ.php @@ -0,0 +1,29 @@ + + * 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\Differ; + +use Fidry\Console\Input\IO; +use KevinGH\Box\Phar\PharInfo; + +interface Differ +{ + public const NO_DIFF_MESSAGE = 'No difference could be observed with this mode.'; + + public function diff( + PharInfo $pharInfoA, + PharInfo $pharInfoB, + IO $io, + ): void; +} diff --git a/src/Phar/Differ/DifferFactory.php b/src/Phar/Differ/DifferFactory.php new file mode 100644 index 000000000..29a433035 --- /dev/null +++ b/src/Phar/Differ/DifferFactory.php @@ -0,0 +1,33 @@ + + * 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\Differ; + +use KevinGH\Box\Console\Command\Extract; +use KevinGH\Box\Phar\DiffMode; + +final class DifferFactory +{ + public function create( + DiffMode $mode, + string $checksumAlgorithm, + ): Differ { + return match ($mode) { + DiffMode::FILE_NAME => new FilenameDiffer(), + DiffMode::GIT => new ProcessCommandBasedDiffer('git diff --no-index'), + DiffMode::GNU => new ProcessCommandBasedDiffer('diff --exclude='.Extract::PHAR_META_PATH), + DiffMode::CHECKSUM => new ChecksumDiffer($checksumAlgorithm), + }; + } +} diff --git a/src/Phar/Differ/FilenameDiffer.php b/src/Phar/Differ/FilenameDiffer.php new file mode 100644 index 000000000..94e444606 --- /dev/null +++ b/src/Phar/Differ/FilenameDiffer.php @@ -0,0 +1,143 @@ + + * 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\Differ; + +use Fidry\Console\Input\IO; +use KevinGH\Box\Console\PharInfoRenderer; +use KevinGH\Box\Phar\PharInfo; +use KevinGH\Box\Pharaoh\PharDiff as ParagoniePharDiff; +use SplFileInfo; +use Symfony\Component\Console\Output\BufferedOutput; +use Symfony\Component\Finder\Finder; +use function array_diff; +use function array_map; +use function array_sum; +use function explode; +use function iterator_to_array; +use function sprintf; +use function str_replace; + +final class FilenameDiffer implements Differ +{ + public function diff( + PharInfo $pharInfoA, + PharInfo $pharInfoB, + IO $io, + ): void { + $paragonieDiff = new ParagoniePharDiff($pharInfoA, $pharInfoB); + $paragonieDiff->setVerbose(true); + + $pharAFiles = self::collectFiles($pharInfoA); + $pharBFiles = self::collectFiles($pharInfoB); + + $diffResult = [ + array_diff($pharAFiles, $pharBFiles), + array_diff($pharBFiles, $pharAFiles), + ]; + + if (0 === array_sum(array_map('count', $diffResult))) { + $io->writeln(Differ::NO_DIFF_MESSAGE); + + return; + } + + self::printDiff( + $pharInfoA, + $pharInfoB, + $diffResult, + $io, + ); + } + + private static function printDiff( + PharInfo $pharInfoA, + PharInfo $pharInfoB, + array $diffResult, + IO $io, + ): void { + $io->writeln(sprintf( + '--- Files present in "%s" but not in "%s"', + $pharInfoA->getFileName(), + $pharInfoB->getFileName(), + )); + $io->writeln(sprintf( + '+++ Files present in "%s" but not in "%s"', + $pharInfoB->getFileName(), + $pharInfoA->getFileName(), + )); + + $io->newLine(); + + self::renderPaths('-', $pharInfoA, $diffResult[0], $io); + self::renderPaths('+', $pharInfoB, $diffResult[1], $io); + + $diffCount = array_sum(array_map('count', $diffResult)); + + $io->error( + sprintf( + '%d file(s) difference', + $diffCount, + ), + ); + } + + /** + * @param list $paths + */ + private static function renderPaths(string $symbol, PharInfo $pharInfo, array $paths, IO $io): void + { + $bufferedOutput = new BufferedOutput( + $io->getVerbosity(), + $io->isDecorated(), + $io->getOutput()->getFormatter(), + ); + + PharInfoRenderer::renderContent( + $bufferedOutput, + $pharInfo, + false, + false, + ); + + $lines = array_map( + static fn (string $line) => '' === $line ? '' : $symbol.' '.$line, + explode( + PHP_EOL, + $bufferedOutput->fetch(), + ), + ); + + $io->writeln($lines); + } + + /** + * @return string[] + */ + private static function collectFiles(PharInfo $pharInfo): array + { + $basePath = $pharInfo->getTmp().DIRECTORY_SEPARATOR; + + return array_map( + static fn (SplFileInfo $fileInfo): string => str_replace($basePath, '', $fileInfo->getRealPath()), + iterator_to_array( + Finder::create() + ->files() + ->in($basePath) + ->ignoreDotFiles(false), + false, + ), + ); + } +} diff --git a/src/Phar/Differ/ProcessCommandBasedDiffer.php b/src/Phar/Differ/ProcessCommandBasedDiffer.php new file mode 100644 index 000000000..f573f4745 --- /dev/null +++ b/src/Phar/Differ/ProcessCommandBasedDiffer.php @@ -0,0 +1,79 @@ + + * 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\Differ; + +use Fidry\Console\Input\IO; +use KevinGH\Box\Phar\PharInfo; +use Symfony\Component\Process\Process; + +final class ProcessCommandBasedDiffer implements Differ +{ + public function __construct(private string $command) + { + } + + public function diff(PharInfo $pharInfoA, PharInfo $pharInfoB, IO $io): void + { + $result = self::getDiff( + $pharInfoA, + $pharInfoB, + $this->command, + ); + + $io->writeln($result ?? Differ::NO_DIFF_MESSAGE); + } + + private static function getDiff(PharInfo $pharInfoA, PharInfo $pharInfoB, string $command): ?string + { + $pharInfoATmp = $pharInfoA->getTmp(); + $pharInfoBTmp = $pharInfoB->getTmp(); + + $pharInfoAFileName = $pharInfoA->getFileName(); + $pharInfoBFileName = $pharInfoB->getFileName(); + + $diffCommand = implode( + ' ', + [ + $command, + $pharInfoATmp, + $pharInfoBTmp, + ], + ); + + $diffProcess = Process::fromShellCommandline($diffCommand); + $diffProcess->run(); + + // We do not check if the process is successful as if there + // is a difference between the two files then the process + // _will_ be unsuccessful. + $diff = trim($diffProcess->getOutput()); + + if ('' === $diff) { + return null; + } + + return str_replace( + [ + $pharInfoATmp, + $pharInfoBTmp, + ], + [ + $pharInfoAFileName, + $pharInfoBFileName, + ], + $diff, + ); + } +} diff --git a/src/Phar/PharDiff.php b/src/Phar/PharDiff.php index d56eff9fb..73a5cee78 100644 --- a/src/Phar/PharDiff.php +++ b/src/Phar/PharDiff.php @@ -14,32 +14,18 @@ namespace KevinGH\Box\Phar; -use KevinGH\Box\Console\Command\Extract; -use KevinGH\Box\Pharaoh\PharDiff as ParagoniePharDiff; -use SplFileInfo; -use Symfony\Component\Finder\Finder; -use Symfony\Component\Process\Process; -use UnexpectedValueException; -use ValueError; -use function array_diff; -use function array_key_exists; +use Fidry\Console\Input\IO; +use KevinGH\Box\Phar\Differ\DifferFactory; use function array_map; -use function count; -use function hash_file; -use function implode; -use function iterator_to_array; -use function sprintf; -use function str_replace; -use const DIRECTORY_SEPARATOR; /** * @internal */ final class PharDiff { - private readonly ParagoniePharDiff $diff; private readonly PharInfo $pharInfoA; private readonly PharInfo $pharInfoB; + private readonly DifferFactory $differFactory; public function __construct(string $pathA, string $pathB) { @@ -51,10 +37,7 @@ public function __construct(string $pathA, string $pathB) $this->pharInfoA = $pharInfoA; $this->pharInfoB = $pharInfoB; - $diff = new ParagoniePharDiff($pharInfoA, $pharInfoB); - $diff->setVerbose(true); - - $this->diff = $diff; + $this->differFactory = new DifferFactory(); } public function getPharInfoA(): PharInfo @@ -72,207 +55,14 @@ public function equals(): bool return $this->pharInfoA->equals($this->pharInfoB); } - /** - * @return null|string|array{string[], string[]} - */ - public function diff(DiffMode $mode, string $checksumAlgorithm): null|string|array + public function diff(DiffMode $mode, string $checksumAlgorithm, IO $io): void { - if (DiffMode::FILE_NAME === $mode) { - return $this->listDiff(); - } - - if (DiffMode::CHECKSUM === $mode) { - return self::getChecksumDiff( + $this->differFactory + ->create($mode, $checksumAlgorithm) + ->diff( $this->pharInfoA, $this->pharInfoB, - $checksumAlgorithm, + $io, ); - } - - return self::getDiff( - $this->pharInfoA, - $this->pharInfoB, - self::getModeCommand($mode), - ); - } - - private static function getModeCommand(DiffMode $mode): string - { - return match ($mode) { - DiffMode::GIT => 'git diff --no-index', - DiffMode::GNU => 'diff --exclude='.Extract::PHAR_META_PATH, - }; - } - - /** - * @see ParagoniePharDiff::listChecksums() - */ - public function listChecksums(string $algo = 'sha384'): int - { - return $this->diff->listChecksums($algo); - } - - /** - * @return string[][] Returns two arrays of strings. The first one contains all the files present in the first PHAR - * which are not in the second and the second array all the files present in the second PHAR but - * not the first one. - */ - private function listDiff(): array - { - $pharAFiles = self::collectFiles($this->pharInfoA); - $pharBFiles = self::collectFiles($this->pharInfoB); - - return [ - array_diff($pharAFiles, $pharBFiles), - array_diff($pharBFiles, $pharAFiles), - ]; - } - - private static function getDiff(PharInfo $pharInfoA, PharInfo $pharInfoB, string $command): ?string - { - $pharInfoATmp = $pharInfoA->getTmp(); - $pharInfoBTmp = $pharInfoB->getTmp(); - - $pharInfoAFileName = $pharInfoA->getFileName(); - $pharInfoBFileName = $pharInfoB->getFileName(); - - $diffCommand = implode( - ' ', - [ - $command, - $pharInfoATmp, - $pharInfoBTmp, - ], - ); - - $diffProcess = Process::fromShellCommandline($diffCommand); - $diffProcess->run(); - - // We do not check if the process is successful as if there - // is a difference between the two files then the process - // _will_ be unsuccessful. - $diff = trim($diffProcess->getOutput()); - - if ('' === $diff) { - return null; - } - - return str_replace( - [ - $pharInfoATmp, - $pharInfoBTmp, - ], - [ - $pharInfoAFileName, - $pharInfoBFileName, - ], - $diff, - ); - } - - private static function getChecksumDiff( - PharInfo $pharInfoA, - PharInfo $pharInfoB, - string $checksumAlgorithm, - ): ?string { - $pharInfoAFileHashes = self::getFileHashesByRelativePathname( - $pharInfoA, - $checksumAlgorithm, - ); - $pharInfoBFileHashes = self::getFileHashesByRelativePathname( - $pharInfoB, - $checksumAlgorithm, - ); - $output = [ - '--- PHAR A', - '+++ PHAR B', - '@@ @@', - ]; - - foreach ($pharInfoAFileHashes as $filePath => $fileAHash) { - if (!array_key_exists($filePath, $pharInfoBFileHashes)) { - $output[] = $filePath; - $output[] = sprintf( - "\t- %s", - $fileAHash, - ); - - continue; - } - - $fileBHash = $pharInfoBFileHashes[$filePath]; - unset($pharInfoBFileHashes[$filePath]); - - if ($fileAHash === $fileBHash) { - continue; - } - - $output[] = $filePath; - $output[] = sprintf( - "\t- %s", - $fileAHash, - ); - $output[] = sprintf( - "\t+ %s", - $fileBHash, - ); - } - - foreach ($pharInfoBFileHashes as $filePath => $fileBHash) { - $output[] = $filePath; - $output[] = sprintf( - "\t+ %s", - $fileBHash, - ); - } - - return 3 === count($output) ? null : implode("\n", $output); - } - - /** - * @return string[] - */ - private static function collectFiles(PharInfo $pharInfo): array - { - $basePath = $pharInfo->getTmp().DIRECTORY_SEPARATOR; - - return array_map( - static fn (SplFileInfo $fileInfo): string => str_replace($basePath, '', $fileInfo->getRealPath()), - iterator_to_array( - Finder::create() - ->files() - ->in($basePath) - ->ignoreDotFiles(false), - false, - ), - ); - } - - /** - * @return array - */ - private static function getFileHashesByRelativePathname( - PharInfo $pharInfo, - string $algorithm, - ): array { - $hashFiles = []; - - try { - foreach ($pharInfo->getFiles() as $file) { - $hashFiles[$file->getRelativePathname()] = hash_file( - $algorithm, - $file->getPathname(), - ); - } - } catch (ValueError) { - throw new UnexpectedValueException( - sprintf( - 'Unexpected algorithm "%s". Please pick a registered hashing algorithm (checksum `hash_algos()`).', - $algorithm, - ), - ); - } - - return $hashFiles; } }