Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More detailed progress for taint graph resolution #11349

Merged
merged 7 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
17 changes: 9 additions & 8 deletions src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Internal/Cli/Psalm.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 4 additions & 3 deletions src/Psalm/Internal/Codebase/Analyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -280,7 +281,7 @@ public function analyzeFiles(
}

if ($alter_code) {
$this->progress->startAlteringFiles();
$this->progress->startPhase(Phase::ALTERING);

$project_analyzer->prepareMigration();

Expand All @@ -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);

Expand Down
9 changes: 8 additions & 1 deletion src/Psalm/Internal/Codebase/TaintFlowGraph.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -217,6 +221,7 @@ public function connectSinksAndSources(): void
$new_sources = [];

ksort($sources);
$progress->expand(count($sources));

foreach ($sources as $source) {
$source_taints = $source->taints;
Expand All @@ -234,6 +239,8 @@ public function connectSinksAndSources(): void
$visited_source_ids,
)];
}

$progress->taskDone(0);
}

$sources = $new_sources;
Expand Down
26 changes: 26 additions & 0 deletions src/Psalm/Internal/LanguageServer/Progress.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Psalm\Internal\LanguageServer;

use Override;
use Psalm\Progress\Phase;
use Psalm\Progress\Progress as Base;

use function str_replace;
Expand All @@ -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
{
Expand Down
9 changes: 6 additions & 3 deletions src/Psalm/Internal/Preloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
21 changes: 15 additions & 6 deletions src/Psalm/Progress/DebugProgress.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
32 changes: 17 additions & 15 deletions src/Psalm/Progress/DefaultProgress.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,46 @@

use Override;

use function hrtime;
use function max;
use function microtime;
use function str_repeat;
use function strlen;

class DefaultProgress extends LongProgress

Check failure on line 14 in src/Psalm/Progress/DefaultProgress.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Method Psalm\Progress\LongProgress#startScanningFiles() was removed

Check failure on line 14 in src/Psalm/Progress/DefaultProgress.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Method Psalm\Progress\LongProgress#startAnalyzingFiles() was removed

Check failure on line 14 in src/Psalm/Progress/DefaultProgress.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Method Psalm\Progress\LongProgress#startAlteringFiles() was removed

Check failure on line 14 in src/Psalm/Progress/DefaultProgress.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Method Psalm\Progress\LongProgress#start() was removed
{
private const TOO_MANY_FILES = 1_500;

// 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,
Expand Down Expand Up @@ -103,8 +106,7 @@
{
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();
}
}
Loading
Loading