From 97fe86c4e158b5a57c5150aa5055c38b5a809aab Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Wed, 17 Mar 2021 01:28:18 +0200 Subject: [PATCH] Fix BC break introduced in 4.6.3 (#5410) Fixes vimeo/psalm#5405 Requires new patch release. This PR reverts changes to the signature of `IssueBuffer::finish()` and introduces separate method to be used to capture `$_SERVER` --- src/Psalm/IssueBuffer.php | 68 ++++++++++++++++++++++-------------- src/psalm-refactor.php | 6 ++-- src/psalm.php | 6 ++-- src/psalter.php | 6 ++-- tests/Config/PluginTest.php | 2 +- tests/IssueBufferTest.php | 2 +- tests/ProjectCheckerTest.php | 4 +-- tests/ReportOutputTest.php | 2 +- 8 files changed, 54 insertions(+), 42 deletions(-) diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index 1f234427c22..cd0ab77fd32 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -1,52 +1,54 @@ */ + private static $server = []; + /** * @param string[] $suppressed_issues * @@ -441,7 +446,6 @@ public static function addIssues(array $issues_data): void */ public static function finish( ProjectAnalyzer $project_analyzer, - BuildInfoCollector $build_info_collector, bool $is_full, float $start_time, bool $add_stats = false, @@ -543,12 +547,13 @@ function (IssueData $d1, IssueData $d2) : int { } } - $source_control_info = null; - $build_info = $build_info_collector->collect(); if ($codebase->config->eventDispatcher->after_analysis || $codebase->config->eventDispatcher->legacy_after_analysis ) { + $source_control_info = null; + $build_info = (new BuildInfoCollector(self::$server))->collect(); + try { $source_control_info = (new \Psalm\Internal\ExecutionEnvironment\GitInfoCollector())->collect(); } catch (\RuntimeException $e) { @@ -861,4 +866,13 @@ public static function bubbleUp(CodeIssue $e): void self::$recorded_issues[self::$recording_level][] = $e; } + + /** + * @internal + * @param array $server + */ + final public static function captureServer(array $server): void + { + self::$server = $server; + } } diff --git a/src/psalm-refactor.php b/src/psalm-refactor.php index 6376b897625..155e19f8fe2 100644 --- a/src/psalm-refactor.php +++ b/src/psalm-refactor.php @@ -42,7 +42,7 @@ require_once __DIR__ . '/command_functions.php'; require_once __DIR__ . '/Psalm/Internal/Composer.php'; require_once __DIR__ . '/Psalm/Internal/IncludeCollector.php'; -require_once __DIR__ . '/' . 'Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php'; +require_once __DIR__ . '/' . 'Psalm/IssueBuffer.php'; ( /** @param array $argv */ @@ -162,7 +162,7 @@ function ($arg) use ($valid_long_options): void { $vendor_dir = \Psalm\getVendorDir($current_dir); // capture environment before registering autoloader (it may destroy it) - $build_info_collector = new BuildInfoCollector($_SERVER); + IssueBuffer::captureServer($_SERVER); $include_collector = new IncludeCollector(); $first_autoloader = $include_collector->runAndCollect( @@ -309,6 +309,6 @@ function () use ($current_dir, $options, $vendor_dir): ?\Composer\Autoload\Class $project_analyzer->check($current_dir); - IssueBuffer::finish($project_analyzer, $build_info_collector, false, $start_time); + IssueBuffer::finish($project_analyzer, false, $start_time); } )($argv); diff --git a/src/psalm.php b/src/psalm.php index 239ab11d164..f9b0dce3a05 100644 --- a/src/psalm.php +++ b/src/psalm.php @@ -6,7 +6,6 @@ use Psalm\Internal\Analyzer\ProjectAnalyzer; use Psalm\Internal\Composer; use Psalm\Internal\ErrorHandler; -use Psalm\Internal\ExecutionEnvironment\BuildInfoCollector; use Psalm\Internal\IncludeCollector; use Psalm\Internal\Provider; use Psalm\Progress\DebugProgress; @@ -61,7 +60,7 @@ require_once __DIR__ . '/command_functions.php' ; require_once __DIR__ . '/Psalm/Internal/Composer.php'; require_once __DIR__ . '/' . 'Psalm/Internal/IncludeCollector.php'; -require_once __DIR__ . '/' . 'Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php'; +require_once __DIR__ . '/' . 'Psalm/IssueBuffer.php'; ( /** @@ -284,7 +283,7 @@ function ($arg) use ($valid_long_options, $valid_short_options): void { $vendor_dir = \Psalm\getVendorDir($current_dir); // capture environment before registering autoloader (it may destroy it) - $build_info_collector = new BuildInfoCollector($_SERVER); + IssueBuffer::captureServer($_SERVER); $include_collector = new IncludeCollector(); $first_autoloader = $include_collector->runAndCollect( @@ -842,7 +841,6 @@ function (array $edges) { if (!isset($options['i'])) { IssueBuffer::finish( $project_analyzer, - $build_info_collector, !$paths_to_check, $start_time, isset($options['stats']), diff --git a/src/psalter.php b/src/psalter.php index f294f33f773..8f0740ba584 100644 --- a/src/psalter.php +++ b/src/psalter.php @@ -53,7 +53,7 @@ require_once __DIR__ . '/command_functions.php'; require_once __DIR__ . '/Psalm/Internal/Composer.php'; require_once __DIR__ . '/Psalm/Internal/IncludeCollector.php'; -require_once __DIR__ . '/' . 'Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php'; +require_once __DIR__ . '/' . 'Psalm/IssueBuffer.php'; ( /** @param array $argv */ @@ -224,7 +224,7 @@ function ($arg) use ($valid_long_options): void { $vendor_dir = \Psalm\getVendorDir($current_dir); // capture environment before registering autoloader (it may destroy it) - $build_info_collector = new BuildInfoCollector($_SERVER); + IssueBuffer::captureServer($_SERVER); $include_collector = new IncludeCollector(); $first_autoloader = $include_collector->runAndCollect( @@ -515,6 +515,6 @@ function (string $line) : bool { } } - IssueBuffer::finish($project_analyzer, $build_info_collector, false, $start_time); + IssueBuffer::finish($project_analyzer, false, $start_time); } )($argv); diff --git a/tests/Config/PluginTest.php b/tests/Config/PluginTest.php index 95990ff35f3..7676de2c947 100644 --- a/tests/Config/PluginTest.php +++ b/tests/Config/PluginTest.php @@ -956,7 +956,7 @@ public function testAfterAnalysisHooks(): void $this->project_analyzer->check('tests/fixtures/DummyProject', true); ob_start(); - \Psalm\IssueBuffer::finish($this->project_analyzer, new BuildInfoCollector([]), true, microtime(true)); + \Psalm\IssueBuffer::finish($this->project_analyzer, true, microtime(true)); ob_end_clean(); } diff --git a/tests/IssueBufferTest.php b/tests/IssueBufferTest.php index 5ecff4368af..8b1bc3a87c2 100644 --- a/tests/IssueBufferTest.php +++ b/tests/IssueBufferTest.php @@ -81,7 +81,7 @@ public function testFinishDoesNotCorruptInternalState(): void $projectAnalzyer->generated_report_options = []; \ob_start(); - IssueBuffer::finish($projectAnalzyer, new BuildInfoCollector([]), false, \microtime(true), false, $baseline); + IssueBuffer::finish($projectAnalzyer, false, \microtime(true), false, $baseline); $output = \ob_get_clean(); $this->assertStringNotContainsString("ERROR", $output, "all issues baselined"); IssueBuffer::clear(); diff --git a/tests/ProjectCheckerTest.php b/tests/ProjectCheckerTest.php index a6cdc186eef..f093ba06676 100644 --- a/tests/ProjectCheckerTest.php +++ b/tests/ProjectCheckerTest.php @@ -157,7 +157,7 @@ public function testCheckAfterNoChange(): void $this->project_analyzer->check('tests/fixtures/DummyProject', true); ob_start(); - \Psalm\IssueBuffer::finish($this->project_analyzer, new BuildInfoCollector([]), true, microtime(true)); + \Psalm\IssueBuffer::finish($this->project_analyzer, true, microtime(true)); ob_end_clean(); $this->assertSame( @@ -201,7 +201,7 @@ public function testCheckAfterFileChange(): void $this->project_analyzer->check('tests/fixtures/DummyProject', true); ob_start(); - \Psalm\IssueBuffer::finish($this->project_analyzer, new BuildInfoCollector([]), true, microtime(true)); + \Psalm\IssueBuffer::finish($this->project_analyzer, true, microtime(true)); ob_end_clean(); $this->assertSame( diff --git a/tests/ReportOutputTest.php b/tests/ReportOutputTest.php index dfdad3c186f..80e08c2cce3 100644 --- a/tests/ReportOutputTest.php +++ b/tests/ReportOutputTest.php @@ -1053,7 +1053,7 @@ public function testEmptyReportIfNotError(): void ); ob_start(); - IssueBuffer::finish($this->project_analyzer, new BuildInfoCollector([]), true, 0); + IssueBuffer::finish($this->project_analyzer, true, 0); ob_end_clean(); $this->assertFileExists(__DIR__ . '/test-report.json'); $this->assertSame('[]