stream_get_contents() does not return FALSE unless an invalid position is provided

Author: staabm

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);

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);

src/Process/ProcessPromise.php

@@ -69,9 +69,6 @@ public function run(): PromiseInterface
 			}
 
 			if ($exitCode === 0) {
-				if ($stdOut === false) {
-					$stdOut = '';
-				}
 				$this->deferred->resolve($stdOut);
 				return;
 			}

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);
 	}
 

src/Type/Php/StreamGetContentsFunctionReturnTypeExtension.php

@@ -0,0 +1,41 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Type\Php;
+
+use PhpParser\Node\Expr\FuncCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\DependencyInjection\AutowiredService;
+use PHPStan\Reflection\FunctionReflection;
+use PHPStan\Reflection\ParametersAcceptorSelector;
+use PHPStan\Type\Constant\ConstantBooleanType;
+use PHPStan\Type\DynamicFunctionReturnTypeExtension;
+use PHPStan\Type\Type;
+use PHPStan\Type\TypeCombinator;
+use function count;
+
+#[AutowiredService]
+final class StreamGetContentsFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension
+{
+
+	public function isFunctionSupported(FunctionReflection $functionReflection): bool
+	{
+		return $functionReflection->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));
+	}
+
+}

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;

tests/PHPStan/Command/CommandHelperTest.php

@@ -136,19 +136,13 @@ public function testBegin(
 			if (!$expectException) {
 				rewind($output->getStream());
 				$contents = stream_get_contents($output->getStream());
-				if ($contents === false) {
-					throw new ShouldNotHappenException();
-				}
 				$this->fail($contents);
 			}
 		}
 
 		rewind($output->getStream());
 
 		$contents = stream_get_contents($output->getStream());
-		if ($contents === false) {
-			throw new ShouldNotHappenException();
-		}
 		$this->assertStringContainsString($expectedOutput, $contents);
 
 		if (isset($result)) {

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));
 		}

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,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Methods/data/bug-3396.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace Bug3396;
+
+class HelloWorld
+{
+
+	public function takesString(string $s): void
+	{
+	}
+
+	public function sayHello(): void
+	{
+		$stream = fopen("file.txt", "rb");
+		if ($stream === false) throw new \Error("wtf");
+		$this->takesString(stream_get_contents($stream));
+		$this->takesString(stream_get_contents($stream, 1));
+		$this->takesString(stream_get_contents($stream, 1, 1));
+	}
+}