Rescan files on missing class storage items

In case not all class storage items were available during a given file scan, those failed files will be rescanned later with more populated details in the codebase. There's a maximum of 10 rescans to avoid endless recursions.
vimeo · Feb 19, 2024 · 29a4b00 · 29a4b00
1 parent 3bd76e3
commit 29a4b00
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 18 deletions.
diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php
@@ -433,11 +433,16 @@ public function addFilesToAnalyze(array $files_to_analyze): void
      */
     public function scanFiles(int $threads = 1): void
     {
-        $has_changes = $this->scanner->scanFiles($this->classlikes, $threads);
-
-        if ($has_changes) {
-            $this->populator->populateCodebase();
-        }
+        $max = 10;
+        do {
+            $has_changes = $this->scanner->scanFiles($this->classlikes, $threads);
+            if ($has_changes) {
+                $this->populator->populateCodebase();
+            }
+            // in case previous scans failed, let's try again after previously found
+            // references (class-like storage items and dependencies) were populated
+            $shall_rescan = $this->scanner->prepareRescanFiles();
+        } while ($max-- > 0 && $shall_rescan);
     }
 
     public function getFileContents(string $file_path): string

diff --git a/src/Psalm/Internal/Codebase/Scanner.php b/src/Psalm/Internal/Codebase/Scanner.php
@@ -6,6 +6,7 @@
 
 use Psalm\Codebase;
 use Psalm\Config;
+use Psalm\Exception\ClassStorageNotFoundException;
 use Psalm\Internal\Analyzer\IssueData;
 use Psalm\Internal\Analyzer\ProjectAnalyzer;
 use Psalm\Internal\ErrorHandler;
@@ -102,6 +103,13 @@ final class Scanner
      */
     private array $files_to_scan = [];
 
+    /**
+     * Files marked to be scanned again, once all other references were resolved.
+     *
+     * @var array<string, string>
+     */
+    private array $files_to_rescan = [];
+
     /**
      * @var array<string, string>
      */
@@ -269,6 +277,19 @@ public function scanFiles(ClassLikes $classlikes, int $pool_size = 1): bool
         return $has_changes;
     }
 
+    /**
+     * @return bool whether there are files to be rescanned
+     */
+    public function prepareRescanFiles(): bool
+    {
+        if ($this->files_to_rescan === []) {
+            return false;
+        }
+        $this->files_to_scan = $this->files_to_rescan;
+        $this->files_to_rescan = [];
+        return true;
+    }
+
     private function shouldScan(string $file_path): bool
     {
         return $this->file_provider->fileExists($file_path)
@@ -329,6 +350,7 @@ function (): void {
 
                     $this->progress->debug('Have initialised forked process for scanning' . PHP_EOL);
                 },
+                // @todo how are failures process in pooled sub processes?! (see Pool::$task_closure)
                 $this->scanAPath(...),
                 /**
                  * @return PoolData
@@ -410,7 +432,9 @@ function (): void {
 
             foreach ($files_to_scan as $file_path => $_) {
                 $this->progress->taskDone(0);
-                $this->scanAPath($i, $file_path);
+                if (!$this->scanAPath($i, $file_path)) {
+                    $this->files_to_rescan[$file_path] = $file_path;
+                }
                 ++$i;
             }
         }
@@ -505,12 +529,13 @@ private function convertClassesToFilePaths(ClassLikes $classlikes): void
 
     /**
      * @param  array<string, class-string<FileScanner>>  $filetype_scanners
+     * @return bool whether scanning was successful
      */
     private function scanFile(
         string $file_path,
         array $filetype_scanners,
         bool $will_analyze = false,
-    ): void {
+    ): bool {
         $file_scanner = $this->getScannerForPath($file_path, $filetype_scanners, $will_analyze);
 
         if (isset($this->scanned_files[$file_path])
@@ -521,7 +546,7 @@ private function scanFile(
 
         if (!$this->file_provider->fileExists($file_path) && $this->config->mustBeIgnored($file_path)) {
             // this should not happen, but might if the file was temporary
-            return;
+            return true;
         }
 
         $file_contents = $this->file_provider->getContents($file_path);
@@ -532,16 +557,20 @@ private function scanFile(
             $this->file_storage_provider->create($file_path);
         }
 
-        $this->scanned_files[$file_path] = $will_analyze;
-
         $file_storage = $this->file_storage_provider->get($file_path);
 
-        $file_scanner->scan(
-            $this->codebase,
-            $file_storage,
-            $from_cache,
-            $this->progress,
-        );
+        try {
+            $file_scanner->scan(
+                $this->codebase,
+                $file_storage,
+                $from_cache,
+                $this->progress,
+            );
+            $this->scanned_files[$file_path] = $will_analyze;
+        } catch (ClassStorageNotFoundException) {
+            // @todo there might be more failures besides `ClassStorageNotFoundException`, that should be rescanned
+            return false;
+        }
 
         if (!$from_cache) {
             if (!$file_storage->has_visitor_issues && $this->file_storage_provider->cache) {
@@ -595,6 +624,8 @@ private function scanFile(
                 $this->codebase->classlikes->addClassAlias($unaliased_name, $aliased_name);
             }
         }
+
+        return true;
     }
 
     /**
@@ -762,9 +793,12 @@ public function isForked(): void
         $this->is_forked = true;
     }
 
-    private function scanAPath(int $_, string $file_path): void
+    /**
+     * @return bool whether scanning was successful
+     */
+    private function scanAPath(int $_, string $file_path): bool
     {
-        $this->scanFile(
+        return $this->scanFile(
             $file_path,
             $this->config->getFiletypeScanners(),
             isset($this->files_to_deep_scan[$file_path]),

diff --git a/src/Psalm/Internal/Fork/Pool.php b/src/Psalm/Internal/Fork/Pool.php
@@ -370,6 +370,7 @@ private function readResultsFromChildren(): array
                                  */
                                 posix_kill($child_pid, SIGTERM);
                             }
+                            // @todo add more meta-data why the process failed for a potential auto-recovery
                             throw new Exception($message->message);
                         } else {
                             error_log('Child should return ForkMessage - response type=' . gettype($message));