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

Prevent autoloader or plugin exit status codes of zero from incorrectly resulting in a zero status from Psalm #10098

Closed
Closed
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
64 changes: 60 additions & 4 deletions src/Psalm/Internal/Cli/Psalm.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
use function preg_match;
use function preg_replace;
use function realpath;
use function register_shutdown_function;
use function setlocale;
use function str_repeat;
use function str_replace;
Expand Down Expand Up @@ -164,6 +165,8 @@ final class Psalm
'error-level:',
];

private static ?bool $exitDetectionEnabled = null;

/**
* @param array<int,string> $argv
* @psalm-suppress ComplexMethod Maybe some of the option handling could be moved to its own function...
Expand Down Expand Up @@ -375,10 +378,15 @@ public static function run(array $argv): void
$config->addPluginPath($plugin_path);
}

if ($paths_to_check === null) {
$project_analyzer->check($current_dir, $is_diff);
} elseif ($paths_to_check) {
$project_analyzer->checkPaths($paths_to_check);
self::enableExitDetection();
try {
if ($paths_to_check === null) {
$project_analyzer->check($current_dir, $is_diff);
} elseif ($paths_to_check) {
$project_analyzer->checkPaths($paths_to_check);
}
} finally {
self::disableExitDetection();
}

if ($find_references_to) {
Expand Down Expand Up @@ -408,6 +416,54 @@ public static function run(array $argv): void
}
}

private static function enableExitDetection(): void
{
if (self::$exitDetectionEnabled === null) {
register_shutdown_function(function (): void {
if (!self::$exitDetectionEnabled) {
return;
}

$config = Config::getInstance();
if (empty($config->getPredefinedConstants())) {
$message = PHP_EOL . self::getAutoloaderExitMessage($config->autoloader);
} else {
$message = self::getUnexpectedExitMessage();
}

fwrite(STDERR, $message . PHP_EOL);

/**
* A die() or exit() call was made at an unexpected time
* (e.g., within an autoloader or plugin).
* That call might have been made with a zero status (we have no way of knowing for sure).
* We exit with a non-zero status instead to ensure that any calling scripts do not misinterpret
* the zero exit status as Psalm succeeding when it did not.
* A code of 2 is required per PsalmRunnerTrait::runPsalm().
*/
die(2);
});
}

self::$exitDetectionEnabled = true;
}

public static function disableExitDetection(): void
{
self::$exitDetectionEnabled = false;
}

public static function getAutoloaderExitMessage(?string $autoloaderPath): string
{
return "Problem running $autoloaderPath:" . PHP_EOL
. " The autoloader failed with the above output and a die() or exit() call.";
}

public static function getUnexpectedExitMessage(): string
{
return 'Psalm ended unexpectedly. This could be caused by a die() or exit() in a plugin.';
}

private static function initOutputFormat(array $options): string
{
return isset($options['output-format']) && is_string($options['output-format'])
Expand Down
3 changes: 3 additions & 0 deletions src/Psalm/Internal/Fork/Pool.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Closure;
use Exception;
use Psalm\Config;
use Psalm\Internal\Cli\Psalm;
use Throwable;

use function array_fill_keys;
Expand Down Expand Up @@ -269,6 +270,8 @@ public function __construct(

fclose($write_stream);

Psalm::disableExitDetection();

// Children exit after completing their work
exit(self::EXIT_SUCCESS);
}
Expand Down
89 changes: 89 additions & 0 deletions tests/EndToEnd/ExitStatusTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

namespace Psalm\Tests\EndToEnd;

use PHPUnit\Framework\TestCase;
use Psalm\Internal\Cli\Psalm;

use function array_pop;
use function array_shift;
use function explode;
use function realpath;
use function strlen;
use function substr;
use function trim;

class ExitStatusTest extends TestCase
{
use PsalmRunnerTrait;

public function testAutoloaderExitStatus(): void
{
$this->assert(
'Example output from failing autoloader',
Psalm::getAutoloaderExitMessage(realpath($this->getFixturePath(). 'autoloader.php')),
);
}

public function testPluginHookExitStatus(): void
{
$this->assert(
'Example output from failing hook',
Psalm::getUnexpectedExitMessage(),
);
}

private function assert(string $expectedSTDOUT, string $expectedSTDERR): void
{
$output = $this->runPsalm(['--no-cache'], $this->getFixturePath(), true);

$this->assertSame($this->getLines($expectedSTDOUT), $this->getLines($output['STDOUT']));
$this->assertSame($this->getLines($expectedSTDERR), $this->handleSTDERR($this->getLines($output['STDERR'])));
$this->assertSame(2, $output['CODE']);
}

private function getFixture(): string
{
return substr($this->getName(), strlen('test'));
}

private function getFixturePath(): string
{
$fixture = $this->getFixture();
return __DIR__ . "/../fixtures/$fixture/";
}

/**
* @return array<string>
*/
private function getLines(string $output): array
{
$lines = explode("\n", $output);

$newLines = [];
foreach ($lines as $line) {
// Trim any carriages returns on Windows
$newLines[] = trim($line);
}

return $newLines;
}

/**
* @param array<string> $lines
*/
private function handleSTDERR(array $lines): array
{
$this->assertStringStartsWith('Target PHP version: ', array_shift($lines));
$this->assertStringStartsWith('Scanning files...', array_shift($lines));

if ($lines[0] === 'Analyzing files...') {
array_shift($lines);
}

$this->assertSame('', array_shift($lines));
$this->assertSame('', array_pop($lines));

return $lines;
}
}
4 changes: 4 additions & 0 deletions tests/fixtures/AutoloaderExitStatus/autoloader.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php

// The following causes a zero exit status, which we explicitly want to ensure does NOT result in a zero exit status from Psalm itself.
die('Example output from failing autoloader');
1 change: 1 addition & 0 deletions tests/fixtures/AutoloaderExitStatus/file.php
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php
14 changes: 14 additions & 0 deletions tests/fixtures/AutoloaderExitStatus/psalm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0"?>
<psalm
autoloader="autoloader.php"
resolveFromConfigFile="true"
findUnusedCode="true"
findUnusedBaselineEntry="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
<projectFiles>
<file name="file.php" />
</projectFiles>
</psalm>
12 changes: 12 additions & 0 deletions tests/fixtures/PluginHookExitStatus/ExitStatusPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

use Psalm\Plugin\EventHandler\AfterFileAnalysisInterface;
use Psalm\Plugin\EventHandler\Event\AfterFileAnalysisEvent;

class ExitStatusPlugin implements AfterFileAnalysisInterface
{
public static function afterAnalyzeFile(AfterFileAnalysisEvent $event): void {
// The following causes a zero exit status, which we explicitly want to ensure does NOT result in a zero exit status from Psalm itself.
die('Example output from failing hook');
}
}
Empty file.
16 changes: 16 additions & 0 deletions tests/fixtures/PluginHookExitStatus/psalm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0"?>
<psalm
resolveFromConfigFile="true"
findUnusedCode="true"
findUnusedBaselineEntry="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
<projectFiles>
<file name="file.php" />
</projectFiles>
<plugins>
<plugin filename="ExitStatusPlugin.php" />
</plugins>
</psalm>
Loading