diff --git a/.github/workflows/e2e_with_no_diffs.yaml b/.github/workflows/e2e_with_no_diffs.yaml new file mode 100644 index 00000000000..3006f84a593 --- /dev/null +++ b/.github/workflows/e2e_with_no_diffs.yaml @@ -0,0 +1,49 @@ +# This workflow runs system tests: Use the Rector application from the source +# checkout to process "fixture" projects in e2e/ directory +# to see if those can be processed successfully +name: End to End tests with no diffs + +on: + pull_request: + branches: + - main + push: + branches: + - main + +env: + # see https://github.com/composer/composer/issues/9368#issuecomment-718112361 + COMPOSER_ROOT_VERSION: "dev-main" + +jobs: + end_to_end: + runs-on: ubuntu-latest + timeout-minutes: 3 + strategy: + fail-fast: false + matrix: + php_version: ['8.2'] + directory: + - 'e2e/applied-rule-removed-node-no-diffs' + + name: End to end test - ${{ matrix.directory }} + + steps: + - uses: actions/checkout@v4 + + - uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php_version }} + coverage: none + + # run in root rector-src + - run: composer install --ansi + + # run in e2e subdir + - + run: composer install --ansi + working-directory: ${{ matrix.directory }} + + # run e2e test + - run: php ../e2eTestRunner.php --no-diffs + working-directory: ${{ matrix.directory }} diff --git a/e2e/applied-rule-removed-node-no-diffs/.gitignore b/e2e/applied-rule-removed-node-no-diffs/.gitignore new file mode 100644 index 00000000000..61ead86667c --- /dev/null +++ b/e2e/applied-rule-removed-node-no-diffs/.gitignore @@ -0,0 +1 @@ +/vendor diff --git a/e2e/applied-rule-removed-node-no-diffs/composer.json b/e2e/applied-rule-removed-node-no-diffs/composer.json new file mode 100644 index 00000000000..5468cd74606 --- /dev/null +++ b/e2e/applied-rule-removed-node-no-diffs/composer.json @@ -0,0 +1,7 @@ +{ + "require": { + "php": "^8.1" + }, + "minimum-stability": "dev", + "prefer-stable": true +} diff --git a/e2e/applied-rule-removed-node-no-diffs/expected-output.diff b/e2e/applied-rule-removed-node-no-diffs/expected-output.diff new file mode 100644 index 00000000000..79b8967aa37 --- /dev/null +++ b/e2e/applied-rule-removed-node-no-diffs/expected-output.diff @@ -0,0 +1 @@ +[OK] 2 files would have been changed (dry-run) by Rector diff --git a/e2e/applied-rule-removed-node-no-diffs/rector.php b/e2e/applied-rule-removed-node-no-diffs/rector.php new file mode 100644 index 00000000000..4143a148053 --- /dev/null +++ b/e2e/applied-rule-removed-node-no-diffs/rector.php @@ -0,0 +1,16 @@ +paths([ + __DIR__ . '/src', + ]); + + $rectorConfig->rule(RemoveEmptyClassMethodRector::class); + $rectorConfig->rule(RemoveAlwaysTrueIfConditionRector::class); +}; diff --git a/e2e/applied-rule-removed-node-no-diffs/src/AlwaysTrue.php b/e2e/applied-rule-removed-node-no-diffs/src/AlwaysTrue.php new file mode 100644 index 00000000000..7b70b9e9ccd --- /dev/null +++ b/e2e/applied-rule-removed-node-no-diffs/src/AlwaysTrue.php @@ -0,0 +1,12 @@ +missConfigurationReporter->reportVendorInPaths($filePaths); @@ -124,6 +124,7 @@ public function processFiles( /** @var FileDiff[] $fileDiffs */ $fileDiffs = []; + $totalChanged = 0; foreach ($filePaths as $filePath) { if ($preFileCallback !== null) { $preFileCallback($filePath); @@ -148,6 +149,10 @@ public function processFiles( if (is_callable($postFileCallback)) { $postFileCallback(1); } + + if ($fileProcessResult->hasChanged()) { + ++$totalChanged; + } } catch (Throwable $throwable) { $this->changedFilesDetector->invalidateFile($filePath); @@ -159,7 +164,7 @@ public function processFiles( } } - return new ProcessResult($systemErrors, $fileDiffs); + return new ProcessResult($systemErrors, $fileDiffs, $totalChanged); } private function processFile(File $file, Configuration $configuration): FileProcessResult diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index a486d6c8243..9c8c87b5413 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -48,7 +48,7 @@ public function processFile(File $file, Configuration $configuration): FileProce $parsingSystemError = $this->parseFileAndDecorateNodes($file); if ($parsingSystemError instanceof SystemError) { // we cannot process this file as the parsing and type resolving itself went wrong - return new FileProcessResult([$parsingSystemError], null); + return new FileProcessResult([$parsingSystemError], null, false); } $fileHasChanged = false; @@ -100,7 +100,7 @@ public function processFile(File $file, Configuration $configuration): FileProce $file->setFileDiff($currentFileDiff); } - return new FileProcessResult([], $file->getFileDiff()); + return new FileProcessResult([], $file->getFileDiff(), $file->hasChanged()); } private function parseFileAndDecorateNodes(File $file): ?SystemError diff --git a/src/ChangesReporting/Output/ConsoleOutputFormatter.php b/src/ChangesReporting/Output/ConsoleOutputFormatter.php index 6b0e5679ef6..4efe600dacf 100644 --- a/src/ChangesReporting/Output/ConsoleOutputFormatter.php +++ b/src/ChangesReporting/Output/ConsoleOutputFormatter.php @@ -144,7 +144,7 @@ private function normalizePathsToRelativeWithLine(string $errorMessage): string private function createSuccessMessage(ProcessResult $processResult, Configuration $configuration): string { - $changeCount = count($processResult->getFileDiffs()); + $changeCount = $processResult->getTotalChanged(); if ($changeCount === 0) { return 'Rector is done!'; diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php index c77ddac6427..d023f4f3a80 100644 --- a/src/Console/Command/ProcessCommand.php +++ b/src/Console/Command/ProcessCommand.php @@ -214,6 +214,10 @@ private function resolveReturnCode(ProcessResult $processResult, Configuration $ return ExitCode::CHANGED_CODE; } + if ($processResult->getTotalChanged() > 0) { + return ExitCode::CHANGED_CODE; + } + return ExitCode::SUCCESS; } diff --git a/src/Console/Command/WorkerCommand.php b/src/Console/Command/WorkerCommand.php index 3fedc0dbf9d..4ed1aac0ee9 100644 --- a/src/Console/Command/WorkerCommand.php +++ b/src/Console/Command/WorkerCommand.php @@ -158,6 +158,7 @@ private function runWorker( Bridge::FILES_COUNT => count($filePaths), Bridge::SYSTEM_ERRORS => $processResult->getSystemErrors(), Bridge::SYSTEM_ERRORS_COUNT => count($processResult->getSystemErrors()), + Bridge::TOTAL_CHANGED => $processResult->getTotalChanged(), ], ]); }); diff --git a/src/Parallel/Application/ParallelFileProcessor.php b/src/Parallel/Application/ParallelFileProcessor.php index 971652f80f7..406f663dc04 100644 --- a/src/Parallel/Application/ParallelFileProcessor.php +++ b/src/Parallel/Application/ParallelFileProcessor.php @@ -119,6 +119,7 @@ public function process( $systemErrorsCount = 0; $reachedSystemErrorsCountLimit = false; + $totalChanged = 0; $handleErrorCallable = function (Throwable $throwable) use ( &$systemErrors, @@ -158,7 +159,8 @@ public function process( $timeoutInSeconds, $handleErrorCallable, &$fileChunksBudgetPerProcess, - &$processSpawner + &$processSpawner, + &$totalChanged ): void { $processIdentifier = Random::generate(); $workerCommandLine = $this->workerCommandLineFactory->create( @@ -185,8 +187,18 @@ function (array $json) use ( &$reachedInternalErrorsCountLimit, $processIdentifier, &$fileChunksBudgetPerProcess, - &$processSpawner + &$processSpawner, + &$totalChanged ): void { + /** @var array{ + * total_changed: int, + * system_errors: mixed[], + * file_diffs: array, + * files_count: int, + * system_errors_count: int + * } $json */ + $totalChanged += $json[Bridge::TOTAL_CHANGED]; + // decode arrays to objects foreach ($json[Bridge::SYSTEM_ERRORS] as $jsonError) { if (is_string($jsonError)) { @@ -269,6 +281,6 @@ function ($exitCode, string $stdErr) use (&$systemErrors, $processIdentifier): v )); } - return new ProcessResult($systemErrors, $fileDiffs); + return new ProcessResult($systemErrors, $fileDiffs, $totalChanged); } } diff --git a/src/Parallel/ValueObject/Bridge.php b/src/Parallel/ValueObject/Bridge.php index bc51e88a272..b35cd93af83 100644 --- a/src/Parallel/ValueObject/Bridge.php +++ b/src/Parallel/ValueObject/Bridge.php @@ -33,4 +33,9 @@ final class Bridge * @var string */ public const FILES_COUNT = 'files_count'; + + /** + * @var string + */ + public const TOTAL_CHANGED = 'total_changed'; } diff --git a/src/ValueObject/FileProcessResult.php b/src/ValueObject/FileProcessResult.php index 1d6b742f8a9..80054efe18a 100644 --- a/src/ValueObject/FileProcessResult.php +++ b/src/ValueObject/FileProcessResult.php @@ -15,7 +15,8 @@ */ public function __construct( private array $systemErrors, - private ?FileDiff $fileDiff + private ?FileDiff $fileDiff, + private bool $hasChanged ) { Assert::allIsInstanceOf($systemErrors, SystemError::class); } @@ -32,4 +33,9 @@ public function getFileDiff(): ?FileDiff { return $this->fileDiff; } + + public function hasChanged(): bool + { + return $this->hasChanged; + } } diff --git a/src/ValueObject/ProcessResult.php b/src/ValueObject/ProcessResult.php index 60cae583c6a..9f562735ab2 100644 --- a/src/ValueObject/ProcessResult.php +++ b/src/ValueObject/ProcessResult.php @@ -17,6 +17,7 @@ final class ProcessResult public function __construct( private array $systemErrors, private readonly array $fileDiffs, + private readonly int $totalChanged ) { Assert::allIsInstanceOf($systemErrors, SystemError::class); Assert::allIsInstanceOf($fileDiffs, FileDiff::class); @@ -51,4 +52,9 @@ public function addSystemErrors(array $systemErrors): void $this->systemErrors = [...$this->systemErrors, ...$systemErrors]; } + + public function getTotalChanged(): int + { + return $this->totalChanged; + } } diff --git a/tests/ChangesReporting/Output/GitHubOutputFormatterTest.php b/tests/ChangesReporting/Output/GitHubOutputFormatterTest.php index 11b3dbf4f99..98a1d8ad1a8 100644 --- a/tests/ChangesReporting/Output/GitHubOutputFormatterTest.php +++ b/tests/ChangesReporting/Output/GitHubOutputFormatterTest.php @@ -53,6 +53,7 @@ public function testReportShouldOutputErrorMessagesGrouped(): void [new RectorWithLineChange(StrStartsWithRector::class, 38)] ), ], + 1 ), new Configuration() ); @@ -62,7 +63,7 @@ public function testReportShouldOutputErrorMessagesGroupedWithNoErrors(): void { $this->expectOsOutputString('::group::Rector report' . PHP_EOL . '::endgroup::' . PHP_EOL); - $this->gitHubOutputFormatter->report(new ProcessResult([], []), new Configuration()); + $this->gitHubOutputFormatter->report(new ProcessResult([], [], 1), new Configuration()); } public function testReportShouldOutputErrorMessagesGroupedWithMultipleDiffs(): void @@ -95,7 +96,7 @@ public function testReportShouldOutputErrorMessagesGroupedWithMultipleDiffs(): v 'diff console formatted', [new RectorWithLineChange(StrStartsWithRector::class, 54)] ), - ], ), + ], 2), new Configuration() ); }