Cache unchanged files on dry run v2 (#4281)

Co-authored-by: Jiří Bok <jiri.bok@nice.com>

Author: dorrogeray

.github/workflows/e2e_consecutive_changes.yaml

@@ -22,10 +22,11 @@ jobs:
             fail-fast: false
             matrix:
                 php_version: ['8.1']
+                rector_disable_parallel: ['true', 'false']
                 directory:
                     - 'e2e/consecutive-changes-with-cache'
 
-        name: End to end test - ${{ matrix.directory }}
+        name: End to end test - ${{ matrix.directory }} [disableParallel=${{ matrix.rector_disable_parallel }}]
 
         steps:
             -   uses: actions/checkout@v3
@@ -43,10 +44,28 @@ jobs:
                 run: composer install --ansi
                 working-directory: ${{ matrix.directory }}
 
+            # run e2e test with dry run, we expect non-zero exit code
+            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-1-dry-run.diff --dry-run
+                working-directory: ${{ matrix.directory }}
+                continue-on-error: true
+                env:
+                    RECTOR_DISABLE_PARALLEL: ${{ matrix.rector_disable_parallel }}
+
+            # run e2e test with dry run once more, we expect non-zero exit code again
+            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-2-dry-run.diff --dry-run
+                working-directory: ${{ matrix.directory }}
+                continue-on-error: true
+                env:
+                    RECTOR_DISABLE_PARALLEL: ${{ matrix.rector_disable_parallel }}
+
             # run e2e test
-            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-1.diff
+            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-3.diff
                 working-directory: ${{ matrix.directory }}
+                env:
+                    RECTOR_DISABLE_PARALLEL: ${{ matrix.rector_disable_parallel }}
 
             # this tests that a 2nd run with cache and consecutive changes works, see https://github.com/rectorphp/rector-src/pull/3614#issuecomment-1507742338
-            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-2.diff
+            -   run: php ../e2eTestChangingRunnerWithCache.php -o expected-output-4.diff
                 working-directory: ${{ matrix.directory }}
+                env:
+                    RECTOR_DISABLE_PARALLEL: ${{ matrix.rector_disable_parallel }}

e2e/consecutive-changes-with-cache/expected-output-1-dry-run.diff

@@ -0,0 +1,27 @@
+1 file with changes
+===================
+
+1) src/Source.php:1
+
+    ---------- begin diff ----------
+@@ @@
+
+ $a = true;
+ $b = true;
+-
+-if ($a && $b) {
+-    return true;
++if (!$a) {
++    return;
+ }
++if (!$b) {
++    return;
++}
++return true;
+    ----------- end diff -----------
+
+Applied rules:
+ * ChangeAndIfToEarlyReturnRector
+
+
+ [OK] 1 file would have changed (dry-run) by Rector

e2e/consecutive-changes-with-cache/expected-output-2-dry-run.diff

@@ -0,0 +1,27 @@
+1 file with changes
+===================
+
+1) src/Source.php:1
+
+    ---------- begin diff ----------
+@@ @@
+
+ $a = true;
+ $b = true;
+-
+-if ($a && $b) {
+-    return true;
++if (!$a) {
++    return;
+ }
++if (!$b) {
++    return;
++}
++return true;
+    ----------- end diff -----------
+
+Applied rules:
+ * ChangeAndIfToEarlyReturnRector
+
+
+ [OK] 1 file would have changed (dry-run) by Rector

…hanges-with-cache/expected-output-1.diff …hanges-with-cache/expected-output-3.diffe2e/consecutive-changes-with-cache/expected-output-1.diff renamed to e2e/consecutive-changes-with-cache/expected-output-3.diff e2e/consecutive-changes-with-cache/expected-output-1.diff renamed to e2e/consecutive-changes-with-cache/expected-output-3.diff

@@ -10,6 +10,10 @@
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->cacheClass(FileCacheStorage::class);
 
+    if (getenv('RECTOR_DISABLE_PARALLEL') === 'true') {
+        $rectorConfig->disableParallel();
+    }
+
     $rectorConfig->paths([
         __DIR__ . '/src',
     ]);

…hanges-with-cache/expected-output-2.diff …hanges-with-cache/expected-output-4.diffe2e/consecutive-changes-with-cache/expected-output-2.diff renamed to e2e/consecutive-changes-with-cache/expected-output-4.diff e2e/consecutive-changes-with-cache/expected-output-2.diff renamed to e2e/consecutive-changes-with-cache/expected-output-4.diff

@@ -25,6 +25,10 @@
     $expectedDiff = 'expected-output.diff';
 }
 
+if (isset($argv[3]) && $argv[3] === '--dry-run') {
+    $e2eCommand .= ' --dry-run';
+}
+
 exec($e2eCommand, $output, $exitCode);
 $output = trim(implode("\n", $output));
 $output = str_replace(__DIR__, '.', $output);

e2e/consecutive-changes-with-cache/rector.php

@@ -95,7 +95,7 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura
 
                     if ($errorAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) {
                         $this->invalidateFile($file);
-                    } elseif (! $configuration->isDryRun()) {
+                    } elseif (! $configuration->isDryRun() || $errorAndFileDiffs[Bridge::FILE_DIFFS] === []) {
                         $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath());
                     }
                 } catch (Throwable $throwable) {

e2e/e2eTestChangingRunnerWithCache.php

@@ -56,6 +56,8 @@ parameters:
         - tests/DependencyInjection/config
 
     ignoreErrors:
+        - '#Cognitive complexity for "Rector\\Parallel\\WorkerRunner\:\:run\(\)" is 12, keep it under 11#'
+        - '#Cognitive complexity for "Rector\\Core\\Application\\ApplicationFileProcessor\:\:processFiles\(\)" is 13, keep it under 11#'
         - '#Cognitive complexity for "Rector\\Php80\\NodeResolver\\SwitchExprsResolver\:\:resolve\(\)" is (.*?), keep it under 11#'
 
         -

packages/Parallel/WorkerRunner.php

@@ -133,7 +133,7 @@ public function processFiles(array $files, Configuration $configuration): array
 
             if ($systemErrorsAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) {
                 $this->changedFilesDetector->invalidateFile($file->getFilePath());
-            } elseif (! $configuration->isDryRun()) {
+            } elseif (! $configuration->isDryRun() || $systemErrorsAndFileDiffs[Bridge::FILE_DIFFS] === []) {
                 $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath());
             }