From c225fe6ab6e7c8d0f0be345a8f4fa6c82ffe70ea Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 11:38:28 +0700 Subject: [PATCH 1/8] [Diffs] Handle --no-diffs usage to show file have been changed if any with --dry-run + --no-diffs --- src/Application/ApplicationFileProcessor.php | 9 +++++++-- src/Application/FileProcessor.php | 2 +- .../Output/ConsoleOutputFormatter.php | 4 ++-- src/Console/Command/ProcessCommand.php | 4 ++++ src/Console/Command/WorkerCommand.php | 1 + src/Parallel/Application/ParallelFileProcessor.php | 14 +++++++++++--- src/Parallel/ValueObject/Bridge.php | 5 +++++ src/ValueObject/FileProcessResult.php | 8 +++++++- src/ValueObject/ProcessResult.php | 11 +++++++++++ .../Output/GitHubOutputFormatterTest.php | 5 +++-- 10 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php index a9bff22b3b1..d2caa1ae89e 100644 --- a/src/Application/ApplicationFileProcessor.php +++ b/src/Application/ApplicationFileProcessor.php @@ -61,7 +61,7 @@ public function run(Configuration $configuration, InputInterface $input): Proces // no files found if ($filePaths === []) { - return new ProcessResult([], []); + return new ProcessResult([], [], 0); } $this->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..3cba01305dd 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -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..0f86746e00d 100644 --- a/src/ChangesReporting/Output/ConsoleOutputFormatter.php +++ b/src/ChangesReporting/Output/ConsoleOutputFormatter.php @@ -144,9 +144,9 @@ private function normalizePathsToRelativeWithLine(string $errorMessage): string private function createSuccessMessage(ProcessResult $processResult, Configuration $configuration): string { - $changeCount = count($processResult->getFileDiffs()); + $changeCount = $processResult->getTotalChanged(); - if ($changeCount === 0) { + if ($changeCount === 0 && ! $processResult->hasChanged()) { return 'Rector is done!'; } diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php index c77ddac6427..4e98f14d76b 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->hasChanged()) { + return ExitCode::CHANGED_CODE; + } + return ExitCode::SUCCESS; } diff --git a/src/Console/Command/WorkerCommand.php b/src/Console/Command/WorkerCommand.php index 3fedc0dbf9d..3694d3f8105 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::HAS_CHANGED => $processResult->hasChanged(), ], ]); }); diff --git a/src/Parallel/Application/ParallelFileProcessor.php b/src/Parallel/Application/ParallelFileProcessor.php index 971652f80f7..e22a5525b3e 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,14 @@ function (array $json) use ( &$reachedInternalErrorsCountLimit, $processIdentifier, &$fileChunksBudgetPerProcess, - &$processSpawner + &$processSpawner, + &$totalChanged ): void { + + if ($json[Bridge::HAS_CHANGED]) { + ++$totalChanged; + } + // decode arrays to objects foreach ($json[Bridge::SYSTEM_ERRORS] as $jsonError) { if (is_string($jsonError)) { @@ -269,6 +277,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..2dde3b0181b 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 HAS_CHANGED = 'has_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..bf629a35aa7 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,14 @@ public function addSystemErrors(array $systemErrors): void $this->systemErrors = [...$this->systemErrors, ...$systemErrors]; } + + public function hasChanged(): bool + { + return $this->totalChanged > 0; + } + + 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() ); } From 12f50f2e9c7635f0d5c4025e722dae0829d4c45c Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 11:55:07 +0700 Subject: [PATCH 2/8] fix phpstan --- src/Application/FileProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 3cba01305dd..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; From 02c004e186e59381da1e92fbea7fc1653c6136fb Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 15:06:50 +0700 Subject: [PATCH 3/8] Fix total changed --- src/Console/Command/WorkerCommand.php | 2 +- src/Parallel/Application/ParallelFileProcessor.php | 4 +--- src/Parallel/ValueObject/Bridge.php | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Console/Command/WorkerCommand.php b/src/Console/Command/WorkerCommand.php index 3694d3f8105..4ed1aac0ee9 100644 --- a/src/Console/Command/WorkerCommand.php +++ b/src/Console/Command/WorkerCommand.php @@ -158,7 +158,7 @@ private function runWorker( Bridge::FILES_COUNT => count($filePaths), Bridge::SYSTEM_ERRORS => $processResult->getSystemErrors(), Bridge::SYSTEM_ERRORS_COUNT => count($processResult->getSystemErrors()), - Bridge::HAS_CHANGED => $processResult->hasChanged(), + Bridge::TOTAL_CHANGED => $processResult->getTotalChanged(), ], ]); }); diff --git a/src/Parallel/Application/ParallelFileProcessor.php b/src/Parallel/Application/ParallelFileProcessor.php index e22a5525b3e..d8c1d48f834 100644 --- a/src/Parallel/Application/ParallelFileProcessor.php +++ b/src/Parallel/Application/ParallelFileProcessor.php @@ -191,9 +191,7 @@ function (array $json) use ( &$totalChanged ): void { - if ($json[Bridge::HAS_CHANGED]) { - ++$totalChanged; - } + $totalChanged += $json[Bridge::TOTAL_CHANGED]; // decode arrays to objects foreach ($json[Bridge::SYSTEM_ERRORS] as $jsonError) { diff --git a/src/Parallel/ValueObject/Bridge.php b/src/Parallel/ValueObject/Bridge.php index 2dde3b0181b..b35cd93af83 100644 --- a/src/Parallel/ValueObject/Bridge.php +++ b/src/Parallel/ValueObject/Bridge.php @@ -37,5 +37,5 @@ final class Bridge /** * @var string */ - public const HAS_CHANGED = 'has_changed'; + public const TOTAL_CHANGED = 'total_changed'; } From ba601f6b6bfa815da5da7f6193a22cd391e32f32 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 15:08:53 +0700 Subject: [PATCH 4/8] clean up --- src/ChangesReporting/Output/ConsoleOutputFormatter.php | 2 +- src/Console/Command/ProcessCommand.php | 2 +- src/ValueObject/ProcessResult.php | 5 ----- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ChangesReporting/Output/ConsoleOutputFormatter.php b/src/ChangesReporting/Output/ConsoleOutputFormatter.php index 0f86746e00d..4efe600dacf 100644 --- a/src/ChangesReporting/Output/ConsoleOutputFormatter.php +++ b/src/ChangesReporting/Output/ConsoleOutputFormatter.php @@ -146,7 +146,7 @@ private function createSuccessMessage(ProcessResult $processResult, Configuratio { $changeCount = $processResult->getTotalChanged(); - if ($changeCount === 0 && ! $processResult->hasChanged()) { + if ($changeCount === 0) { return 'Rector is done!'; } diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php index 4e98f14d76b..d023f4f3a80 100644 --- a/src/Console/Command/ProcessCommand.php +++ b/src/Console/Command/ProcessCommand.php @@ -214,7 +214,7 @@ private function resolveReturnCode(ProcessResult $processResult, Configuration $ return ExitCode::CHANGED_CODE; } - if ($processResult->hasChanged()) { + if ($processResult->getTotalChanged() > 0) { return ExitCode::CHANGED_CODE; } diff --git a/src/ValueObject/ProcessResult.php b/src/ValueObject/ProcessResult.php index bf629a35aa7..9f562735ab2 100644 --- a/src/ValueObject/ProcessResult.php +++ b/src/ValueObject/ProcessResult.php @@ -53,11 +53,6 @@ public function addSystemErrors(array $systemErrors): void $this->systemErrors = [...$this->systemErrors, ...$systemErrors]; } - public function hasChanged(): bool - { - return $this->totalChanged > 0; - } - public function getTotalChanged(): int { return $this->totalChanged; From aeaa3fc74edff6080d234705e172fc8735548a9e Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 15:38:21 +0700 Subject: [PATCH 5/8] Fix phpstan --- src/Parallel/Application/ParallelFileProcessor.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Parallel/Application/ParallelFileProcessor.php b/src/Parallel/Application/ParallelFileProcessor.php index d8c1d48f834..406f663dc04 100644 --- a/src/Parallel/Application/ParallelFileProcessor.php +++ b/src/Parallel/Application/ParallelFileProcessor.php @@ -190,7 +190,13 @@ function (array $json) use ( &$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 From 85073d14764549703e5bd9e63d171440799b82b3 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 17:23:24 +0700 Subject: [PATCH 6/8] Add e2e test for show files changed without diff --- .github/workflows/e2e_with_no_diffs.yaml | 49 +++++++++++++++++++ .../.gitignore | 1 + .../composer.json | 7 +++ .../expected-output.diff | 1 + .../rector.php | 16 ++++++ .../src/AlwaysTrue.php | 12 +++++ .../src/DeadConstructor.php | 8 +++ e2e/e2eTestRunner.php | 4 ++ 8 files changed, 98 insertions(+) create mode 100644 .github/workflows/e2e_with_no_diffs.yaml create mode 100644 e2e/applied-rule-removed-node-no-diffs/.gitignore create mode 100644 e2e/applied-rule-removed-node-no-diffs/composer.json create mode 100644 e2e/applied-rule-removed-node-no-diffs/expected-output.diff create mode 100644 e2e/applied-rule-removed-node-no-diffs/rector.php create mode 100644 e2e/applied-rule-removed-node-no-diffs/src/AlwaysTrue.php create mode 100644 e2e/applied-rule-removed-node-no-diffs/src/DeadConstructor.php diff --git a/.github/workflows/e2e_with_no_diffs.yaml b/.github/workflows/e2e_with_no_diffs.yaml new file mode 100644 index 00000000000..9b0257f564f --- /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: + - '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..6f92b7124f2 --- /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 \ No newline at end of file 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 @@ + Date: Tue, 11 Nov 2025 17:25:06 +0700 Subject: [PATCH 7/8] Add e2e test for show files changed without diff --- .github/workflows/e2e_with_no_diffs.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e_with_no_diffs.yaml b/.github/workflows/e2e_with_no_diffs.yaml index 9b0257f564f..3006f84a593 100644 --- a/.github/workflows/e2e_with_no_diffs.yaml +++ b/.github/workflows/e2e_with_no_diffs.yaml @@ -24,7 +24,7 @@ jobs: matrix: php_version: ['8.2'] directory: - - 'applied-rule-removed-node-no-diffs' + - 'e2e/applied-rule-removed-node-no-diffs' name: End to end test - ${{ matrix.directory }} From e56d730f1d20739562182f3073f1201578b596ab Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 11 Nov 2025 17:25:25 +0700 Subject: [PATCH 8/8] eol --- e2e/applied-rule-removed-node-no-diffs/expected-output.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/applied-rule-removed-node-no-diffs/expected-output.diff b/e2e/applied-rule-removed-node-no-diffs/expected-output.diff index 6f92b7124f2..79b8967aa37 100644 --- a/e2e/applied-rule-removed-node-no-diffs/expected-output.diff +++ b/e2e/applied-rule-removed-node-no-diffs/expected-output.diff @@ -1 +1 @@ -[OK] 2 files would have been changed (dry-run) by Rector \ No newline at end of file +[OK] 2 files would have been changed (dry-run) by Rector