Merge pull request #11349 from danog/more_detailed_progress

More detailed progress for taint graph resolution

Author: danog

UPGRADING.md

@@ -1,3 +1,23 @@
+# Upgrading from Psalm 6 to Psalm 7
+
+## Changed
+
+- [BC] The `startScanningFiles`, `startAnalyzingFiles`, `startAlteringFiles` of `Psalm\Progress\Progress` and subclasses were removed and replaced with a new `startPhase` method, taking a `Psalm\Progress\Phase` enum case.
+
+- [BC] The `start` method was removed, use `expand`, instead; the progress is reset to 0 when changing the current phase.  
+
+- [BC] Method `doesTerminalSupportUtf8` of class `Psalm\Progress\Progress` became final
+
+- [BC] Method debug() of class Psalm\Progress\Progress changed from concrete to abstract
+
+- [BC] Method alterFileDone() of class Psalm\Progress\Progress changed from concrete to abstract
+
+- [BC] Method expand() of class Psalm\Progress\Progress changed from concrete to abstract
+
+- [BC] Method taskDone() of class Psalm\Progress\Progress changed from concrete to abstract
+
+- [BC] Method finish() of class Psalm\Progress\Progress changed from concrete to abstract
+
 # Upgrading from Psalm 5 to Psalm 6
 ## Changed
 

src/Psalm/Internal/Analyzer/ProjectAnalyzer.php

@@ -47,6 +47,7 @@
 use Psalm\Issue\UnusedProperty;
 use Psalm\Issue\UnusedVariable;
 use Psalm\Plugin\EventHandler\Event\AfterCodebasePopulatedEvent;
+use Psalm\Progress\Phase;
 use Psalm\Progress\Progress;
 use Psalm\Progress\VoidProgress;
 use Psalm\Report;
@@ -461,7 +462,7 @@ public function check(string $base_dir, bool $is_diff = false): void
         }
 
         $this->progress->write($this->generatePHPVersionMessage());
-        $this->progress->startScanningFiles();
+        $this->progress->startPhase(Phase::SCAN);
 
         $diff_no_files = false;
 
@@ -519,7 +520,7 @@ public function check(string $base_dir, bool $is_diff = false): void
             $this->config->eventDispatcher->dispatchAfterCodebasePopulated($event);
         }
 
-        $this->progress->startAnalyzingFiles();
+        $this->progress->startPhase(Phase::ANALYSIS);
 
         $this->codebase->analyzer->analyzeFiles(
             $this,
@@ -875,15 +876,15 @@ public function checkDir(string $dir_name): void
         $this->checkDirWithConfig($dir_name, $this->config, true);
 
         $this->progress->write($this->generatePHPVersionMessage());
-        $this->progress->startScanningFiles();
+        $this->progress->startPhase(Phase::SCAN);
 
         $this->config->initializePlugins($this);
 
         $this->codebase->scanFiles($this->scanThreads);
 
         $this->config->visitStubFiles($this->codebase, $this->progress);
 
-        $this->progress->startAnalyzingFiles();
+        $this->progress->startPhase(Phase::ANALYSIS);
 
         $this->codebase->analyzer->analyzeFiles(
             $this,
@@ -975,15 +976,15 @@ public function checkFile(string $file_path): void
         $this->file_reference_provider->loadReferenceCache();
 
         $this->progress->write($this->generatePHPVersionMessage());
-        $this->progress->startScanningFiles();
+        $this->progress->startPhase(Phase::SCAN);
 
         $this->config->initializePlugins($this);
 
         $this->codebase->scanFiles($this->scanThreads);
 
         $this->config->visitStubFiles($this->codebase, $this->progress);
 
-        $this->progress->startAnalyzingFiles();
+        $this->progress->startPhase(Phase::ANALYSIS);
 
         $this->codebase->analyzer->analyzeFiles(
             $this,
@@ -999,7 +1000,7 @@ public function checkFile(string $file_path): void
     public function checkPaths(array $paths_to_check): void
     {
         $this->progress->write($this->generatePHPVersionMessage());
-        $this->progress->startScanningFiles();
+        $this->progress->startPhase(Phase::SCAN);
 
         $this->config->visitPreloadedStubFiles($this->codebase, $this->progress);
         $this->visitAutoloadFiles();
@@ -1031,7 +1032,7 @@ public function checkPaths(array $paths_to_check): void
 
         $this->config->eventDispatcher->dispatchAfterCodebasePopulated($event);
 
-        $this->progress->startAnalyzingFiles();
+        $this->progress->startPhase(Phase::ANALYSIS);
 
         $this->codebase->analyzer->analyzeFiles(
             $this,

src/Psalm/Internal/Cli/Psalm.php

@@ -13,6 +13,7 @@
 use Psalm\Exception\ConfigException;
 use Psalm\Internal\Analyzer\ProjectAnalyzer;
 use Psalm\Internal\CliUtils;
+use Psalm\Internal\Codebase\InternalCallMapHandler;
 use Psalm\Internal\Codebase\ReferenceMapGenerator;
 use Psalm\Internal\Composer;
 use Psalm\Internal\ErrorHandler;
@@ -389,6 +390,9 @@ public static function run(array $argv): void
             $config->addPluginPath($plugin_path);
         }
 
+        // Prime cache
+        InternalCallMapHandler::getCallMap();
+
         if ($paths_to_check === null) {
             $project_analyzer->check($current_dir, $is_diff);
         } elseif ($paths_to_check) {

src/Psalm/Internal/Codebase/Analyzer.php

@@ -25,6 +25,7 @@
 use Psalm\Internal\Provider\FileProvider;
 use Psalm\Internal\Provider\FileStorageProvider;
 use Psalm\IssueBuffer;
+use Psalm\Progress\Phase;
 use Psalm\Progress\Progress;
 use Psalm\Type;
 use Psalm\Type\Union;
@@ -247,7 +248,7 @@ public function analyzeFiles(
         $scanned_files = $codebase->scanner->getScannedFiles();
 
         if ($codebase->taint_flow_graph) {
-            $codebase->taint_flow_graph->connectSinksAndSources();
+            $codebase->taint_flow_graph->connectSinksAndSources($codebase->progress);
         }
 
         $this->progress->finish();
@@ -280,7 +281,7 @@ public function analyzeFiles(
         }
 
         if ($alter_code) {
-            $this->progress->startAlteringFiles();
+            $this->progress->startPhase(Phase::ALTERING);
 
             $project_analyzer->prepareMigration();
 
@@ -296,7 +297,7 @@ public function analyzeFiles(
 
     private function doAnalysis(ProjectAnalyzer $project_analyzer, int $pool_size): void
     {
-        $this->progress->start(count($this->files_to_analyze));
+        $this->progress->expand(count($this->files_to_analyze));
 
         ksort($this->files_to_analyze);
 

src/Psalm/Internal/Codebase/TaintFlowGraph.php

@@ -31,6 +31,8 @@
 use Psalm\Issue\TaintedUserSecret;
 use Psalm\Issue\TaintedXpath;
 use Psalm\IssueBuffer;
+use Psalm\Progress\Phase;
+use Psalm\Progress\Progress;
 use Psalm\Type\TaintKind;
 
 use function array_diff;
@@ -202,8 +204,10 @@ public function getIssueTrace(DataFlowNode $source): array
         return [$node];
     }
 
-    public function connectSinksAndSources(): void
+    public function connectSinksAndSources(Progress $progress): void
     {
+        $progress->startPhase(Phase::TAINT_GRAPH_RESOLUTION);
+
         $visited_source_ids = [];
 
         $sources = $this->sources;
@@ -217,6 +221,7 @@ public function connectSinksAndSources(): void
             $new_sources = [];
 
             ksort($sources);
+            $progress->expand(count($sources));
 
             foreach ($sources as $source) {
                 $source_taints = $source->taints;
@@ -234,6 +239,8 @@ public function connectSinksAndSources(): void
                         $visited_source_ids,
                     )];
                 }
+
+                $progress->taskDone(0);
             }
 
             $sources = $new_sources;

src/Psalm/Internal/LanguageServer/Progress.php

@@ -5,6 +5,7 @@
 namespace Psalm\Internal\LanguageServer;
 
 use Override;
+use Psalm\Progress\Phase;
 use Psalm\Progress\Progress as Base;
 
 use function str_replace;
@@ -30,6 +31,31 @@ public function debug(string $message): void
         }
     }
 
+    #[Override]
+    public function startPhase(Phase $phase): void
+    {
+    }
+
+    #[Override]
+    public function alterFileDone(string $file_name): void
+    {
+    }
+
+    #[Override]
+    public function expand(int $number_of_tasks): void
+    {
+    }
+
+    #[Override]
+    public function taskDone(int $level): void
+    {
+    }
+
+    #[Override]
+    public function finish(): void
+    {
+    }
+
     #[Override]
     public function write(string $message): void
     {

src/Psalm/Internal/Preloader.php

@@ -4,11 +4,12 @@
 
 namespace Psalm\Internal;
 
+use Psalm\Progress\Phase;
 use Psalm\Progress\Progress;
 
 use function class_exists;
 
-use const PHP_EOL;
+use function count;
 
 /** @internal */
 final class Preloader
@@ -21,13 +22,15 @@ public static function preload(?Progress $progress = null, bool $hasJit = false)
         }
 
         if ($hasJit) {
-            $progress?->write("JIT compilation in progress... ");
+            $progress?->startPhase(Phase::JIT_COMPILATION);
+            $progress?->expand(count(PreloaderList::CLASSES)+1);
         }
         foreach (PreloaderList::CLASSES as $class) {
+            $progress?->taskDone(0);
             class_exists($class);
         }
         if ($hasJit) {
-            $progress?->write("Done.".PHP_EOL.PHP_EOL);
+            $progress?->finish();
         }
         self::$preloaded = true;
     }

src/Psalm/Progress/DebugProgress.php

@@ -25,21 +25,30 @@ public function debug(string $message): void
     }
 
     #[Override]
-    public function startScanningFiles(): void
+    public function startPhase(Phase $phase): void
+    {
+        $this->write(match ($phase) {
+            Phase::SCAN => "\nScanning files...\n\n",
+            Phase::ANALYSIS => "\nAnalyzing files...\n",
+            Phase::ALTERING => "\nUpdating files...\n",
+            Phase::TAINT_GRAPH_RESOLUTION => "\nResolving taint graph...\n",
+            Phase::JIT_COMPILATION => "\nJIT compilation in progress...\n",
+        });
+    }
+    
+    #[Override]
+    public function expand(int $number_of_tasks): void
     {
-        $this->write("\n" . 'Scanning files...' . "\n\n");
     }
 
     #[Override]
-    public function startAnalyzingFiles(): void
+    public function taskDone(int $level): void
     {
-        $this->write("\n" . 'Analyzing files...' . "\n");
     }
 
     #[Override]
-    public function startAlteringFiles(): void
+    public function finish(): void
     {
-        $this->write('Updating files...' . "\n");
     }
 
     #[Override]

src/Psalm/Progress/DefaultProgress.php

@@ -6,8 +6,8 @@
 
 use Override;
 
+use function hrtime;
 use function max;
-use function microtime;
 use function str_repeat;
 use function strlen;
 
@@ -17,32 +17,35 @@ class DefaultProgress extends LongProgress
 
     // Update the progress bar at most once per 0.1 seconds.
     // This reduces flickering and reduces the amount of time spent writing to STDERR and updating the terminal.
-    private const PROGRESS_BAR_SAMPLE_INTERVAL = 0.1;
+    private const PROGRESS_BAR_SAMPLE_INTERVAL_NANOSECONDS = 100_000_000;
 
-    /** @var float the last time when the progress bar UI was updated */
-    private float $previous_update_time = 0.0;
+    /** the last time when the progress bar UI was updated (seconds) */
+    private int $previous_update_seconds = 0;
+
+    /** the last time when the progress bar UI was updated (nanoseconds) */
+    private int $previous_update_nseconds = 0;
 
     #[Override]
     public function taskDone(int $level): void
     {
         if ($this->fixed_size && $this->number_of_tasks > self::TOO_MANY_FILES) {
             ++$this->progress;
 
-            // Source for rate limiting:
-            // https://github.com/phan/phan/blob/9a788581ee1a4e1c35bebf89c435fd8a238c1d17/src/Phan/CLI.php
-            $time = microtime(true);
+            [$seconds, $nseconds] = hrtime();
 
             // If not enough time has elapsed, then don't update the progress bar.
             // Making the update frequency based on time (instead of the number of files)
             // prevents the terminal from rapidly flickering while processing small/empty files,
             // and reduces the time spent writing to stderr.
-            if ($time - $this->previous_update_time < self::PROGRESS_BAR_SAMPLE_INTERVAL) {
-                // Make sure to output the section for 100% completion regardless of limits, to avoid confusion.
-                if ($this->progress !== $this->number_of_tasks) {
-                    return;
-                }
+            // Make sure to output the section for 100% completion regardless of limits, to avoid confusion.
+            if ($seconds === $this->previous_update_seconds
+                && ($nseconds - $this->previous_update_nseconds < self::PROGRESS_BAR_SAMPLE_INTERVAL_NANOSECONDS)
+                && $this->progress !== $this->number_of_tasks
+            ) {
+                return;
             }
-            $this->previous_update_time = $time;
+            $this->previous_update_seconds = $seconds;
+            $this->previous_update_nseconds = $nseconds;
 
             $inner_progress = self::renderInnerProgressBar(
                 self::NUMBER_OF_COLUMNS,
@@ -103,8 +106,7 @@ public function finish(): void
     {
         if ($this->number_of_tasks > self::TOO_MANY_FILES) {
             $this->write(str_repeat(' ', self::NUMBER_OF_COLUMNS + strlen($this->getOverview()) + 1) . "\r");
-        } else {
-            parent::finish();
         }
+        parent::finish();
     }
 }