Revert "Cache on successful file processing"

This reverts commit df13bf1.
rectorphp · May 1, 2023 · 2e10efa · 2e10efa
1 parent 489b03b
commit 2e10efa
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 83 deletions.
diff --git a/packages-tests/Caching/Detector/ChangedFilesDetectorTest.php b/packages-tests/Caching/Detector/ChangedFilesDetectorTest.php
@@ -32,8 +32,7 @@ public function testHasFileChanged(): void
         $filePath = __DIR__ . '/Source/file.php';
 
         $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
-        $this->changedFilesDetector->addCachableFile($filePath);
-        $this->changedFilesDetector->cacheFileWithDependencies($filePath);
+        $this->changedFilesDetector->addFileWithDependencies($filePath, []);
 
         $this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
         $this->changedFilesDetector->invalidateFile($filePath);
@@ -47,10 +46,7 @@ public function testHasFileChanged(): void
     #[DataProvider('provideData')]
     public function testGetDependentFileInfos(string $filePath, array $dependantFiles): void
     {
-        $this->changedFilesDetector->addFileDependentFiles($filePath, $dependantFiles);
-        $this->changedFilesDetector->addCachableFile($filePath);
-        $this->changedFilesDetector->cacheFileWithDependencies($filePath);
-
+        $this->changedFilesDetector->addFileWithDependencies($filePath, $dependantFiles);
         $dependantFilePaths = $this->changedFilesDetector->getDependentFilePaths($filePath);
 
         $dependantFilesCount = count($dependantFiles);

diff --git a/packages/Caching/Detector/ChangedFilesDetector.php b/packages/Caching/Detector/ChangedFilesDetector.php
@@ -15,16 +15,6 @@
  */
 final class ChangedFilesDetector
 {
-    /**
-     * @var array<string, string[]>
-     */
-    private array $dependentFiles = [];
-
-    /**
-     * @var array<string, true>
-     */
-    private array $cachableFiles = [];
-
     public function __construct(
         private readonly FileHashComputer $fileHashComputer,
         private readonly Cache $cache
@@ -34,34 +24,13 @@ public function __construct(
     /**
      * @param string[] $dependentFiles
      */
-    public function addFileDependentFiles(string $filePath, array $dependentFiles): void
-    {
-        $filePathCacheKey = $this->getFilePathCacheKey($filePath);
-        $this->dependentFiles[$filePathCacheKey] = $dependentFiles;
-    }
-
-    public function cacheFileWithDependencies(string $filePath): void
+    public function addFileWithDependencies(string $filePath, array $dependentFiles): void
     {
         $filePathCacheKey = $this->getFilePathCacheKey($filePath);
-
-        if (! array_key_exists($filePathCacheKey, $this->cachableFiles)) {
-            return;
-        }
-
         $hash = $this->hashFile($filePath);
 
         $this->cache->save($filePathCacheKey, CacheKey::FILE_HASH_KEY, $hash);
-        $this->cache->save(
-            $filePathCacheKey . '_files',
-            CacheKey::DEPENDENT_FILES_KEY,
-            $this->dependentFiles[$filePathCacheKey],
-        );
-    }
-
-    public function addCachableFile(string $filePath): void
-    {
-        $filePathCacheKey = $this->getFilePathCacheKey($filePath);
-        $this->cachableFiles[$filePathCacheKey] = true;
+        $this->cache->save($filePathCacheKey . '_files', CacheKey::DEPENDENT_FILES_KEY, $dependentFiles);
     }
 
     public function hasFileChanged(string $filePath): bool
@@ -82,7 +51,6 @@ public function invalidateFile(string $filePath): void
     {
         $fileInfoCacheKey = $this->getFilePathCacheKey($filePath);
         $this->cache->clean($fileInfoCacheKey);
-        unset($this->cachableFiles[$fileInfoCacheKey]);
     }
 
     public function clear(): void

diff --git a/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php
@@ -436,6 +436,6 @@ private function resolveAndSaveDependentFiles(
             }
         }
 
-        $this->changedFilesDetector->addFileDependentFiles($filePath, $dependentFiles);
+        $this->changedFilesDetector->addFileWithDependencies($filePath, $dependentFiles);
     }
 }
diff --git a/packages/Parallel/WorkerRunner.php b/packages/Parallel/WorkerRunner.php
@@ -7,7 +7,6 @@
 use Clue\React\NDJson\Decoder;
 use Clue\React\NDJson\Encoder;
 use Nette\Utils\FileSystem;
-use Rector\Caching\Detector\ChangedFilesDetector;
 use Rector\Core\Application\ApplicationFileProcessor;
 use Rector\Core\Application\FileSystem\RemovedAndAddedFilesProcessor;
 use Rector\Core\Console\Style\RectorConsoleOutputStyle;
@@ -42,8 +41,7 @@ public function __construct(
         private readonly RectorConsoleOutputStyle $rectorConsoleOutputStyle,
         private readonly RemovedAndAddedFilesProcessor $removedAndAddedFilesProcessor,
         private readonly ApplicationFileProcessor $applicationFileProcessor,
-        private readonly ChangedFilesDetector $changedFilesDetector,
-        private readonly array $fileProcessors = [],
+        private readonly array $fileProcessors = []
     ) {
     }
 
@@ -87,24 +85,14 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura
             $this->applicationFileProcessor->configurePHPStanNodeScopeResolver($filePaths, $configuration);
 
             foreach ($filePaths as $filePath) {
-                $file = null;
-
                 try {
                     $file = new File($filePath, FileSystem::read($filePath));
                     $this->currentFileProvider->setFile($file);
 
                     $errorAndFileDiffs = $this->processFiles($file, $configuration, $errorAndFileDiffs);
-
-                    if ($errorAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) {
-                        $this->invalidateFile($file);
-                    } elseif (! $configuration->isDryRun()) {
-                        $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath());
-                    }
                 } catch (Throwable $throwable) {
                     ++$systemErrorsCount;
                     $systemErrors = $this->collectSystemErrors($systemErrors, $throwable, $filePath);
-
-                    $this->invalidateFile($file);
                 }
             }
 
@@ -170,13 +158,4 @@ private function collectSystemErrors(array $systemErrors, Throwable $throwable,
 
         return $systemErrors;
     }
-
-    private function invalidateFile(?File $file): void
-    {
-        if ($file === null) {
-            return;
-        }
-
-        $this->changedFilesDetector->invalidateFile($file->getFilePath());
-    }
 }
diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php
@@ -5,7 +5,6 @@
 namespace Rector\Core\Application;
 
 use PHPStan\Analyser\NodeScopeResolver;
-use Rector\Caching\Detector\ChangedFilesDetector;
 use Rector\Core\Application\FileDecorator\FileDiffFileDecorator;
 use Rector\Core\Application\FileSystem\RemovedAndAddedFilesProcessor;
 use Rector\Core\Configuration\Option;
@@ -53,8 +52,7 @@ public function __construct(
         private readonly ParameterProvider $parameterProvider,
         private readonly ScheduleFactory $scheduleFactory,
         private readonly CpuCoreCountProvider $cpuCoreCountProvider,
-        private readonly ChangedFilesDetector $changedFilesDetector,
-        private readonly array $fileProcessors = [],
+        private readonly array $fileProcessors = []
     ) {
     }
 
@@ -133,12 +131,6 @@ public function processFiles(array $files, Configuration $configuration): array
                 $systemErrorsAndFileDiffs = $this->arrayParametersMerger->merge($systemErrorsAndFileDiffs, $result);
             }
 
-            if ($systemErrorsAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) {
-                $this->changedFilesDetector->invalidateFile($file->getFilePath());
-            } elseif (! $configuration->isDryRun()) {
-                $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath());
-            }
-
             // progress bar +1
             if ($shouldShowProgressBar) {
                 $this->rectorOutputStyle->progressAdvance();

diff --git a/src/Application/FileProcessor/PhpFileProcessor.php b/src/Application/FileProcessor/PhpFileProcessor.php
@@ -6,7 +6,6 @@
 
 use Nette\Utils\Strings;
 use PHPStan\AnalysedCodeException;
-use Rector\Caching\Detector\ChangedFilesDetector;
 use Rector\ChangesReporting\ValueObjectFactory\ErrorFactory;
 use Rector\ChangesReporting\ValueObjectFactory\FileDiffFactory;
 use Rector\Core\Application\FileProcessor;
@@ -40,7 +39,6 @@ public function __construct(
         private readonly RemovedAndAddedFilesCollector $removedAndAddedFilesCollector,
         private readonly OutputStyleInterface $rectorOutputStyle,
         private readonly FileDiffFactory $fileDiffFactory,
-        private readonly ChangedFilesDetector $changedFilesDetector,
         private readonly CurrentFileProvider $currentFileProvider,
         private readonly PostFileProcessor $postFileProcessor,
         private readonly ErrorFactory $errorFactory,
@@ -67,8 +65,6 @@ public function process(File $file, Configuration $configuration): array
             return $systemErrorsAndFileDiffs;
         }
 
-        $fileHasChanged = false;
-
         // 2. change nodes with Rectors
         $rectorWithLineChanges = null;
         do {
@@ -84,19 +80,12 @@ public function process(File $file, Configuration $configuration): array
             // important to detect if file has changed
             $this->printFile($file, $configuration);
 
-            $fileHasChanged = $fileHasChanged || $file->hasChanged();
-
             if ($file->hasChanged()) {
                 $file->setFileDiff($this->fileDiffFactory->createTempFileDiff($file));
                 $rectorWithLineChanges = $file->getRectorWithLineChanges();
             }
         } while ($file->hasChanged());
 
-        // 5. add as cacheable if not changed at all
-        if (! $fileHasChanged) {
-            $this->changedFilesDetector->addCachableFile($file->getFilePath());
-        }
-
         if ($configuration->shouldShowDiffs() && $rectorWithLineChanges !== null) {
             $file->setFileDiff(
                 $this->fileDiffFactory->createFileDiffWithLineChanges(

diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php
@@ -89,6 +89,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $processResult = $this->processResultFactory->create($systemErrorsAndFileDiffs);
         $outputFormatter->report($processResult, $configuration);
 
+        // invalidate affected files
+        $this->invalidateCacheForChangedAndErroredFiles($processResult);
+
         return $this->resolveReturnCode($processResult, $configuration);
     }
 
@@ -111,6 +114,22 @@ protected function initialize(InputInterface $input, OutputInterface $output): v
         }
     }
 
+    private function invalidateCacheForChangedAndErroredFiles(ProcessResult $processResult): void
+    {
+        foreach ($processResult->getChangedFilePaths() as $changedFilePath) {
+            $this->changedFilesDetector->invalidateFile($changedFilePath);
+        }
+
+        foreach ($processResult->getErrors() as $systemError) {
+            $errorFile = $systemError->getFile();
+            if (! is_string($errorFile)) {
+                continue;
+            }
+
+            $this->changedFilesDetector->invalidateFile($errorFile);
+        }
+    }
+
     /**
      * @return ExitCode::*
      */

diff --git a/src/ValueObject/ProcessResult.php b/src/ValueObject/ProcessResult.php
@@ -63,4 +63,17 @@ public function getRemovedNodeCount(): int
     {
         return $this->removedNodeCount;
     }
+
+    /**
+     * @return string[]
+     */
+    public function getChangedFilePaths(): array
+    {
+        $fileInfos = [];
+        foreach ($this->fileDiffs as $fileDiff) {
+            $fileInfos[] = $fileDiff->getRelativeFilePath();
+        }
+
+        return array_unique($fileInfos);
+    }
 }