From 3fcd9f27de7916c1ec1035dfecd9dfca7a89d891 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 19:45:15 +0200 Subject: [PATCH 01/15] Disabled equal/not-equal mutators that change `===` to `==` and vice-versa Most `===` comparisons in the codebase are between same types and should stay like that: disabling the usage of `==` and `!=` and favoring `===` and `!==` should be a better way to deal with this. --- infection.json.dist | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/infection.json.dist b/infection.json.dist index b883a216..f558ef6e 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -7,5 +7,10 @@ }, "logs": { "text": "infection-log.txt" + }, + "mutators": { + "@default": true, + "IdenticalEqual": false, + "NotIdenticalNotEqual": false } } \ No newline at end of file From ba6e6c94bd85916fd836da02754c14cac78de1cd Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 19:49:53 +0200 Subject: [PATCH 02/15] Removed unnecessary string cast operations on `sprintf()` parameters --- src/Command/AssertBackwardsCompatible.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Command/AssertBackwardsCompatible.php b/src/Command/AssertBackwardsCompatible.php index 641811a0..bd9615fb 100644 --- a/src/Command/AssertBackwardsCompatible.php +++ b/src/Command/AssertBackwardsCompatible.php @@ -151,7 +151,7 @@ public function execute(InputInterface $input, OutputInterface $output) : int $toRevision = $this->parseRevision->fromStringForRepository($input->getOption('to'), $sourceRepo); - $stdErr->writeln(sprintf('Comparing from %s to %s...', (string) $fromRevision, (string) $toRevision)); + $stdErr->writeln(sprintf('Comparing from %s to %s...', $fromRevision, $toRevision)); $fromPath = $this->git->checkout($sourceRepo, $fromRevision); $toPath = $this->git->checkout($sourceRepo, $toRevision); From aa7212de127ff4ed552b494df2d009c3823d23db Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 19:50:23 +0200 Subject: [PATCH 03/15] Asserting on the return value of `Input#getOption()` rather than blindly casting it to `string` --- src/Command/AssertBackwardsCompatible.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Command/AssertBackwardsCompatible.php b/src/Command/AssertBackwardsCompatible.php index bd9615fb..d1d5def4 100644 --- a/src/Command/AssertBackwardsCompatible.php +++ b/src/Command/AssertBackwardsCompatible.php @@ -206,10 +206,11 @@ private function printOutcomeAndExit(Changes $changes, OutputInterface $stdErr) */ private function parseRevisionFromInput(InputInterface $input, CheckedOutRepository $repository) : Revision { - return $this->parseRevision->fromStringForRepository( - (string) $input->getOption('from'), - $repository - ); + $from = $input->getOption('from'); + + Assert::that($from)->string(); + + return $this->parseRevision->fromStringForRepository($from, $repository); } private function determineFromRevisionFromRepository( From 8b8bcdc23c1f0b7c5637bc88fea6c0d650c91b93 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 21:16:43 +0200 Subject: [PATCH 04/15] Removed cast, preferring explicit `__toString()` call --- src/Formatter/SymfonyConsoleTextFormatter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Formatter/SymfonyConsoleTextFormatter.php b/src/Formatter/SymfonyConsoleTextFormatter.php index 41341476..5c66a534 100644 --- a/src/Formatter/SymfonyConsoleTextFormatter.php +++ b/src/Formatter/SymfonyConsoleTextFormatter.php @@ -22,7 +22,7 @@ public function write(Changes $changes) : void { /** @var Change $change */ foreach ($changes as $change) { - $this->output->writeln((string) $change); + $this->output->writeln($change->__toString()); } } } From 055345047806b1db6bbfd96e9698c8d49a0ecbba Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 21:37:34 +0200 Subject: [PATCH 05/15] Replaced `uniqid` callable with just straight 'uniqid' (string) --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 32456300..2d31c415 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -11,7 +11,6 @@ use function is_dir; use function sprintf; use function sys_get_temp_dir; -use function uniqid; final class GitCheckoutRevisionToTemporaryPath implements PerformCheckoutOfRevision { @@ -20,9 +19,7 @@ final class GitCheckoutRevisionToTemporaryPath implements PerformCheckoutOfRevis public function __construct(?callable $uniquenessFunction = null) { - $this->uniquenessFunction = $uniquenessFunction ?? function (string $nonUniqueThing) : string { - return uniqid($nonUniqueThing, true); - }; + $this->uniquenessFunction = $uniquenessFunction ?? 'uniqid'; } /** From e2cfc0691bc695b77272655ef406462d6d5cba19 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 21:39:24 +0200 Subject: [PATCH 06/15] Removing unnecessary string casts --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 2d31c415..6e3c38e3 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -30,8 +30,8 @@ public function checkout(CheckedOutRepository $sourceRepository, Revision $revis { $checkoutDirectory = $this->generateTemporaryPathFor($revision); - (new Process(['git', 'clone', (string) $sourceRepository, $checkoutDirectory]))->mustRun(); - (new Process(['git', 'checkout', (string) $revision]))->setWorkingDirectory($checkoutDirectory)->mustRun(); + (new Process(['git', 'clone', $sourceRepository, $checkoutDirectory]))->mustRun(); + (new Process(['git', 'checkout', $revision]))->setWorkingDirectory($checkoutDirectory)->mustRun(); return CheckedOutRepository::fromPath($checkoutDirectory); } From b35362f42a2ec6780e68c1e2eb6a5aa3f25076e1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 21:41:44 +0200 Subject: [PATCH 07/15] Removed unnecessary cast operation --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 6e3c38e3..7b073b39 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -42,7 +42,7 @@ public function checkout(CheckedOutRepository $sourceRepository, Revision $revis */ public function remove(CheckedOutRepository $checkedOutRepository) : void { - (new Process(['rm', '-rf', (string) $checkedOutRepository]))->mustRun(); + (new Process(['rm', '-rf', $checkedOutRepository]))->mustRun(); } /** From fffd4fd0a00906a0830091cae482a3b55592bf44 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:28:44 +0200 Subject: [PATCH 08/15] Removing `is_dir()` check, since we only care about the target location not existing at all --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 7b073b39..861cdf27 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -53,7 +53,7 @@ private function generateTemporaryPathFor(Revision $revision) : string $uniquePathGenerator = $this->uniquenessFunction; $checkoutDirectory = sys_get_temp_dir() . '/api-compare-' . $uniquePathGenerator((string) $revision . '_'); - if (file_exists($checkoutDirectory) || is_dir($checkoutDirectory)) { + if (file_exists($checkoutDirectory)) { throw new RuntimeException(sprintf( 'Tried to check out revision "%s" to directory "%s" which already exists', (string) $revision, From da2c7bc44c783cad816c37d22e1ffe970270e004 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:29:18 +0200 Subject: [PATCH 09/15] Removed string cast on operation that already produces a string --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 861cdf27..2e327d99 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -51,7 +51,7 @@ public function remove(CheckedOutRepository $checkedOutRepository) : void private function generateTemporaryPathFor(Revision $revision) : string { $uniquePathGenerator = $this->uniquenessFunction; - $checkoutDirectory = sys_get_temp_dir() . '/api-compare-' . $uniquePathGenerator((string) $revision . '_'); + $checkoutDirectory = sys_get_temp_dir() . '/api-compare-' . $uniquePathGenerator($revision . '_'); if (file_exists($checkoutDirectory)) { throw new RuntimeException(sprintf( From f247348704a89533f916a353edf63fe67d34bc52 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:30:15 +0200 Subject: [PATCH 10/15] Preferring explicit `->__toString()` call over string cast --- src/Git/GitParseRevision.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Git/GitParseRevision.php b/src/Git/GitParseRevision.php index f6633fd9..dca5270a 100644 --- a/src/Git/GitParseRevision.php +++ b/src/Git/GitParseRevision.php @@ -17,7 +17,7 @@ public function fromStringForRepository(string $something, CheckedOutRepository { return Revision::fromSha1( (new Process(['git', 'rev-parse', $something])) - ->setWorkingDirectory((string) $repository) + ->setWorkingDirectory($repository->__toString()) ->mustRun() ->getOutput() ); From c11aa8ad520004296a9f822cbd5e3a0667ae9a36 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:31:47 +0200 Subject: [PATCH 11/15] Removing all string casts that can be replaced by `__toString()` calls This makes it easier to detect string casts via "find usages", and removes any space for implicit type juggling on nullable values. --- src/Command/AssertBackwardsCompatible.php | 5 +++-- src/Formatter/MarkdownPipedToSymfonyConsoleFormatter.php | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Command/AssertBackwardsCompatible.php b/src/Command/AssertBackwardsCompatible.php index d1d5def4..88d680ac 100644 --- a/src/Command/AssertBackwardsCompatible.php +++ b/src/Command/AssertBackwardsCompatible.php @@ -164,11 +164,11 @@ public function execute(InputInterface $input, OutputInterface $output) : int ), $this->makeComposerInstallationReflector->__invoke( $fromPath->__toString(), - $this->locateDependencies->__invoke((string) $fromPath) + $this->locateDependencies->__invoke($fromPath->__toString()) ), $this->makeComposerInstallationReflector->__invoke( $toPath->__toString(), - $this->locateDependencies->__invoke((string) $toPath) + $this->locateDependencies->__invoke($toPath->__toString()) ) ); @@ -219,6 +219,7 @@ private function determineFromRevisionFromRepository( ) : Revision { $versions = $this->getVersions->fromRepository($repository); + // @TODO add a test around the 0 limit Assert::that($versions->count()) ->greaterThan(0, 'Could not detect any released versions for the given repository'); diff --git a/src/Formatter/MarkdownPipedToSymfonyConsoleFormatter.php b/src/Formatter/MarkdownPipedToSymfonyConsoleFormatter.php index 5805e8f1..28ec3eb3 100644 --- a/src/Formatter/MarkdownPipedToSymfonyConsoleFormatter.php +++ b/src/Formatter/MarkdownPipedToSymfonyConsoleFormatter.php @@ -57,7 +57,7 @@ private function convertFilteredChangesToMarkdownBulletList(callable $filterFunc { return array_map( function (Change $change) : string { - return ' - ' . str_replace(['ADDED: ', 'CHANGED: ', 'REMOVED: '], '', trim((string) $change)) . "\n"; + return ' - ' . str_replace(['ADDED: ', 'CHANGED: ', 'REMOVED: '], '', trim($change->__toString())) . "\n"; }, array_filter($changes, $filterFunction) ); From 16e800897e46f227e3cc9da3d59bdacee0805933 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:34:46 +0200 Subject: [PATCH 12/15] Removing all string casts that can be replaced by `__toString()` calls This makes it easier to detect string casts via "find usages", and removes any space for implicit type juggling on nullable values. --- src/Git/GetVersionCollectionFromGitRepository.php | 2 +- src/Git/GitCheckoutRevisionToTemporaryPath.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Git/GetVersionCollectionFromGitRepository.php b/src/Git/GetVersionCollectionFromGitRepository.php index 3de1b9c9..177d71b8 100644 --- a/src/Git/GetVersionCollectionFromGitRepository.php +++ b/src/Git/GetVersionCollectionFromGitRepository.php @@ -26,7 +26,7 @@ final class GetVersionCollectionFromGitRepository implements GetVersionCollectio public function fromRepository(CheckedOutRepository $checkedOutRepository) : VersionsCollection { $output = (new Process(['git', 'tag', '-l'])) - ->setWorkingDirectory((string) $checkedOutRepository) + ->setWorkingDirectory($checkedOutRepository->__toString()) ->mustRun() ->getOutput(); diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index 2e327d99..c1671b6e 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -56,7 +56,7 @@ private function generateTemporaryPathFor(Revision $revision) : string if (file_exists($checkoutDirectory)) { throw new RuntimeException(sprintf( 'Tried to check out revision "%s" to directory "%s" which already exists', - (string) $revision, + $revision->__toString(), $checkoutDirectory )); } From 9f396f39df87db79fe2e5e5f730119f6083f3cd2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:46:47 +0200 Subject: [PATCH 13/15] Verifying that collections with 1, 2 and 3 versions are correctly accepted too --- .../Command/AssertBackwardsCompatibleTest.php | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/test/unit/Command/AssertBackwardsCompatibleTest.php b/test/unit/Command/AssertBackwardsCompatibleTest.php index d5d74dbd..7ff6cc3c 100644 --- a/test/unit/Command/AssertBackwardsCompatibleTest.php +++ b/test/unit/Command/AssertBackwardsCompatibleTest.php @@ -317,11 +317,11 @@ public function testExecuteWithDefaultRevisionsNotProvidedAndNoDetectedTags() : $this->compare->execute($this->input, $this->output); } - public function testExecuteWithDefaultRevisionsNotProvided() : void + /** @dataProvider validVersionsCollections */ + public function testExecuteWithDefaultRevisionsNotProvided(VersionsCollection $versions) : void { $fromSha = sha1('fromRevision', false); $toSha = sha1('toRevision', false); - $versions = new VersionsCollection(Version::fromString('1.0.0'), Version::fromString('1.0.1')); $pickedVersion = Version::fromString('1.0.0'); $this->input->expects(self::any())->method('getOption')->willReturnMap([ @@ -382,4 +382,21 @@ public function testExecuteWithDefaultRevisionsNotProvided() : void self::assertSame(0, $this->compare->execute($this->input, $this->output)); } + + /** @return VersionsCollection[][] */ + public function validVersionsCollections() : array + { + return [ + [new VersionsCollection( + Version::fromString('1.0.0'), + Version::fromString('1.0.1'), + Version::fromString('1.0.2') + )], + [new VersionsCollection( + Version::fromString('1.0.0'), + Version::fromString('1.0.1') + )], + [new VersionsCollection(Version::fromString('1.0.0'))], + ]; + } } From ec861eba67069dbfbe5e04c3c0076fb34a84a463 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:49:13 +0200 Subject: [PATCH 14/15] Verifying that no mutants are passing under the radar in CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2dfbb4a1..b9ed15d7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -51,7 +51,7 @@ jobs: - stage: Mutation Tests php: 7.2 - script: vendor/bin/infection + script: vendor/bin/infection --min-msi=100 --min-covered-msi=100 - stage: Verify BC Breaks php: 7.2 From 3b853b21eeae7ac9020d7fadcc6b8a44594651e1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 23 Aug 2018 22:50:44 +0200 Subject: [PATCH 15/15] Automated CS check fixes --- src/Git/GitCheckoutRevisionToTemporaryPath.php | 1 - test/unit/Command/AssertBackwardsCompatibleTest.php | 6 ++++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Git/GitCheckoutRevisionToTemporaryPath.php b/src/Git/GitCheckoutRevisionToTemporaryPath.php index c1671b6e..041c143d 100644 --- a/src/Git/GitCheckoutRevisionToTemporaryPath.php +++ b/src/Git/GitCheckoutRevisionToTemporaryPath.php @@ -8,7 +8,6 @@ use Symfony\Component\Process\Exception\RuntimeException as ProcessRuntimeException; use Symfony\Component\Process\Process; use function file_exists; -use function is_dir; use function sprintf; use function sys_get_temp_dir; diff --git a/test/unit/Command/AssertBackwardsCompatibleTest.php b/test/unit/Command/AssertBackwardsCompatibleTest.php index 7ff6cc3c..ed6731da 100644 --- a/test/unit/Command/AssertBackwardsCompatibleTest.php +++ b/test/unit/Command/AssertBackwardsCompatibleTest.php @@ -391,11 +391,13 @@ public function validVersionsCollections() : array Version::fromString('1.0.0'), Version::fromString('1.0.1'), Version::fromString('1.0.2') - )], + ), + ], [new VersionsCollection( Version::fromString('1.0.0'), Version::fromString('1.0.1') - )], + ), + ], [new VersionsCollection(Version::fromString('1.0.0'))], ]; }