From 965a85c13dfa37a554409ff6da192881de2115eb Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Jul 2025 09:44:26 +0200 Subject: [PATCH 1/2] stream_get_contents() does not return FALSE unless an invalid position is provided --- ...GetContentsFunctionReturnTypeExtension.php | 41 +++++++++++++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 15 +++++++ tests/PHPStan/Rules/Methods/data/bug-3396.php | 20 +++++++++ 3 files changed, 76 insertions(+) create mode 100644 src/Type/Php/StreamGetContentsFunctionReturnTypeExtension.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-3396.php diff --git a/src/Type/Php/StreamGetContentsFunctionReturnTypeExtension.php b/src/Type/Php/StreamGetContentsFunctionReturnTypeExtension.php new file mode 100644 index 0000000000..791f0d85ab --- /dev/null +++ b/src/Type/Php/StreamGetContentsFunctionReturnTypeExtension.php @@ -0,0 +1,41 @@ +getName() === 'stream_get_contents'; + } + + public function getTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope): ?Type + { + if (count($functionCall->getArgs()) >= 3) { + return null; + } + + // stream_get_contents() does not return FALSE unless an invalid offset is provided. + $returnType = ParametersAcceptorSelector::selectFromArgs( + $scope, + $functionCall->getArgs(), + $functionReflection->getVariants(), + )->getReturnType(); + + return TypeCombinator::remove($returnType, new ConstantBooleanType(false)); + } + +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index 34b9b08a28..d9fd398db6 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3572,4 +3572,19 @@ public function testBug13171(): void ]); } + public function testBug3396(): void + { + $this->checkThisOnly = false; + $this->checkNullables = false; + $this->checkUnionTypes = true; + $this->checkExplicitMixed = false; + + $this->analyse([__DIR__ . '/data/bug-3396.php'], [ + [ + 'Parameter #1 $s of method Bug3396\HelloWorld::takesString() expects string, string|false given.', + 18, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-3396.php b/tests/PHPStan/Rules/Methods/data/bug-3396.php new file mode 100644 index 0000000000..17670e97b5 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-3396.php @@ -0,0 +1,20 @@ +takesString(stream_get_contents($stream)); + $this->takesString(stream_get_contents($stream, 1)); + $this->takesString(stream_get_contents($stream, 1, 1)); + } +} From 50b21566f6ae81c05e61b702071d50092d76d50e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Jul 2025 09:52:41 +0200 Subject: [PATCH 2/2] Cleanup newly detected dead code --- src/Command/AnalyseCommand.php | 3 --- src/Parallel/Process.php | 12 +++--------- src/Process/ProcessPromise.php | 3 --- src/Testing/ErrorFormatterTestCase.php | 4 ---- .../Command/AnalyseApplicationIntegrationTest.php | 4 ---- tests/PHPStan/Command/CommandHelperTest.php | 6 ------ .../BaselineNeonErrorFormatterTest.php | 4 ---- 7 files changed, 3 insertions(+), 33 deletions(-) diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index cdf46e635f..926aa66408 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -746,9 +746,6 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult $stream = $streamOutput->getStream(); rewind($stream); $baselineContents = stream_get_contents($stream); - if ($baselineContents === false) { - throw new ShouldNotHappenException(); - } try { DirectoryCreator::ensureDirectoryExists($baselineFileDirectory, 0644); diff --git a/src/Parallel/Process.php b/src/Parallel/Process.php index 0f51442a56..36799c274d 100644 --- a/src/Parallel/Process.php +++ b/src/Parallel/Process.php @@ -9,7 +9,6 @@ use React\Stream\WritableStreamInterface; use Throwable; use function fclose; -use function is_string; use function rewind; use function sprintf; use function stream_get_contents; @@ -73,16 +72,11 @@ public function start(callable $onData, callable $onError, callable $onExit): vo $output = ''; rewind($this->stdOut); - $stdOut = stream_get_contents($this->stdOut); - if (is_string($stdOut)) { - $output .= $stdOut; - } + $output .= stream_get_contents($this->stdOut); rewind($this->stdErr); - $stdErr = stream_get_contents($this->stdErr); - if (is_string($stdErr)) { - $output .= $stdErr; - } + $output .= stream_get_contents($this->stdErr); + $onExit($exitCode, $output); fclose($this->stdOut); fclose($this->stdErr); diff --git a/src/Process/ProcessPromise.php b/src/Process/ProcessPromise.php index c4b152cd8b..31c9f0b761 100644 --- a/src/Process/ProcessPromise.php +++ b/src/Process/ProcessPromise.php @@ -69,9 +69,6 @@ public function run(): PromiseInterface } if ($exitCode === 0) { - if ($stdOut === false) { - $stdOut = ''; - } $this->deferred->resolve($stdOut); return; } diff --git a/src/Testing/ErrorFormatterTestCase.php b/src/Testing/ErrorFormatterTestCase.php index 8512285772..2d0b672f32 100644 --- a/src/Testing/ErrorFormatterTestCase.php +++ b/src/Testing/ErrorFormatterTestCase.php @@ -75,10 +75,6 @@ protected function getOutputContent(bool $decorated = false, bool $verbose = fal rewind($this->getOutputStream($decorated, $verbose)->getStream()); $contents = stream_get_contents($this->getOutputStream($decorated, $verbose)->getStream()); - if ($contents === false) { - throw new ShouldNotHappenException(); - } - return $this->rtrimMultiline($contents); } diff --git a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php index 32f51da20c..b13cb49ef9 100644 --- a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php +++ b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php @@ -93,10 +93,6 @@ private function runPath(string $path, int $expectedStatusCode): string rewind($output->getStream()); $contents = stream_get_contents($output->getStream()); - if ($contents === false) { - throw new ShouldNotHappenException(); - } - $this->assertSame($expectedStatusCode, $statusCode, $contents); return $contents; diff --git a/tests/PHPStan/Command/CommandHelperTest.php b/tests/PHPStan/Command/CommandHelperTest.php index 4267588e0c..702b5ac4ae 100644 --- a/tests/PHPStan/Command/CommandHelperTest.php +++ b/tests/PHPStan/Command/CommandHelperTest.php @@ -136,9 +136,6 @@ public function testBegin( if (!$expectException) { rewind($output->getStream()); $contents = stream_get_contents($output->getStream()); - if ($contents === false) { - throw new ShouldNotHappenException(); - } $this->fail($contents); } } @@ -146,9 +143,6 @@ public function testBegin( rewind($output->getStream()); $contents = stream_get_contents($output->getStream()); - if ($contents === false) { - throw new ShouldNotHappenException(); - } $this->assertStringContainsString($expectedOutput, $contents); if (isset($result)) { diff --git a/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php b/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php index 4712721634..48b4a54e55 100644 --- a/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php +++ b/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php @@ -437,10 +437,6 @@ public function testEndOfFileNewlines( rewind($outputStream->getStream()); $content = stream_get_contents($outputStream->getStream()); - if ($content === false) { - throw new ShouldNotHappenException(); - } - if ($expectedNewlinesCount > 0) { Assert::assertSame(str_repeat("\n", $expectedNewlinesCount), substr($content, -$expectedNewlinesCount)); }