[Diffs] Handle --no-diffs usage to show file have been changed if any with --dry-run + --no-diffs

.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 }}

e2e/applied-rule-removed-node-no-diffs/.gitignore

@@ -0,0 +1 @@
+/vendor

e2e/applied-rule-removed-node-no-diffs/composer.json

@@ -0,0 +1,7 @@
+{
+    "require": {
+        "php": "^8.1"
+    },
+    "minimum-stability": "dev",
+    "prefer-stable": true
+}

e2e/applied-rule-removed-node-no-diffs/expected-output.diff

@@ -0,0 +1 @@
+[OK] 2 files would have been changed (dry-run) by Rector

e2e/applied-rule-removed-node-no-diffs/rector.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector;
+use Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->paths([
+        __DIR__ . '/src',
+    ]);
+
+    $rectorConfig->rule(RemoveEmptyClassMethodRector::class);
+    $rectorConfig->rule(RemoveAlwaysTrueIfConditionRector::class);
+};

e2e/applied-rule-removed-node-no-diffs/src/AlwaysTrue.php

@@ -0,0 +1,12 @@
+<?php
+
+final class AlwaysTrue
+{
+    public function run()
+    {
+        if (1 === 1) {
+        }
+
+        return 'no';
+    }
+}

e2e/applied-rule-removed-node-no-diffs/src/DeadConstructor.php

@@ -0,0 +1,8 @@
+<?php
+
+final class DeadConstructor
+{
+    public function __construct()
+    {
+    }
+}

e2e/e2eTestRunner.php

@@ -32,6 +32,10 @@
     $e2eCommand .= ' -a ' . $argv[2];
 }
 
+if (isset($argv[1]) && $argv[1] === '--no-diffs') {
+    $e2eCommand .= ' --no-diffs';
+}
+
 $cliOptions = 'cli-options.txt';
 if (file_exists($cliOptions)) {
     $e2eCommand .= ' ' . trim((string) file_get_contents($cliOptions));

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

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

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!';

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;
     }
 

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(),
                 ],
             ]);
         });

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<string, mixed>,
+                     *      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);
     }
 }

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';
 }

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;
+    }
 }

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;
+    }
 }

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()
         );
     }