Supports functions whose side effects are flipped by parameters

phpstan · Jan 8, 2023 · 50cea22 · 50cea22
1 parent 41a67d8
commit 50cea22
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 13 deletions.
diff --git a/resources/functionMetadata.php b/resources/functionMetadata.php
@@ -862,7 +862,6 @@
 	'fgetss' => ['hasSideEffects' => true],
 	'file' => ['hasSideEffects' => false],
 	'file_exists' => ['hasSideEffects' => false],
-	'file_get_contents' => ['hasSideEffects' => false],
 	'file_put_contents' => ['hasSideEffects' => true],
 	'fileatime' => ['hasSideEffects' => false],
 	'filectime' => ['hasSideEffects' => false],
@@ -1552,4 +1551,4 @@
 	'zlib_encode' => ['hasSideEffects' => false],
 	'zlib_get_coding_type' => ['hasSideEffects' => false],
 
-];
+];
diff --git a/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php b/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php
@@ -3,11 +3,13 @@
 namespace PHPStan\Rules\Functions;
 
 use PhpParser\Node;
+use PhpParser\Node\Arg;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\NeverType;
+use PHPStan\Type\Type;
 use function in_array;
 use function sprintf;
 
@@ -17,6 +19,13 @@
 class CallToFunctionStatementWithoutSideEffectsRule implements Rule
 {
 
+	private const SIDE_EFFECT_FLIP_PARAMETERS = [
+		// functionName => [name, pos, testName, defaultHasSideEffect]
+		'file_get_contents' => ['context', 2, 'isNotNull', false],
+		'print_r' => ['return', 1, 'isTruthy', true],
+		'var_export' => ['return', 1, 'isTruthy', true],
+	];
+
 	public function __construct(private ReflectionProvider $reflectionProvider)
 	{
 	}
@@ -42,7 +51,65 @@ public function processNode(Node $node, Scope $scope): array
 		}
 
 		$function = $this->reflectionProvider->getFunction($funcCall->name, $scope);
-		if ($function->hasSideEffects()->no() || $node->expr->isFirstClassCallable()) {
+		$functionName = $function->getName();
+		$functionHasSideEffects = !$function->hasSideEffects()->no();
+
+		if (in_array($functionName, [
+			'PHPStan\\dumpType',
+			'PHPStan\\Testing\\assertType',
+			'PHPStan\\Testing\\assertNativeType',
+			'PHPStan\\Testing\\assertVariableCertainty',
+		], true)) {
+			return [];
+		}
+
+		if (isset(self::SIDE_EFFECT_FLIP_PARAMETERS[$functionName])) {
+			[
+				$flipParameterName,
+				$flipParameterPosision,
+				$testName,
+				$defaultHasSideEffect,
+			] = self::SIDE_EFFECT_FLIP_PARAMETERS[$functionName];
+
+			$sideEffectFlipped = false;
+			$hasNamedParameter = false;
+			$checker = [
+				'isNotNull' => static fn (Type $type) => $type->isNull()->no(),
+				'isTruthy' => static fn (Type $type) => $type->toBoolean()->isTrue()->yes(),
+			][$testName];
+
+			foreach ($funcCall->getRawArgs() as $i => $arg) {
+				if (!$arg instanceof Arg) {
+					return [];
+				}
+
+				$isFlipParameter = false;
+
+				if ($arg->name !== null) {
+					$hasNamedParameter = true;
+					if ($arg->name->name === $flipParameterName) {
+						$isFlipParameter = true;
+					}
+				}
+
+				if (!$hasNamedParameter && $i === $flipParameterPosision) {
+					$isFlipParameter = true;
+				}
+
+				if ($isFlipParameter) {
+					$sideEffectFlipped = $checker($scope->getType($arg->value));
+					break;
+				}
+			}
+
+			if ($sideEffectFlipped xor $defaultHasSideEffect) {
+				return [];
+			}
+
+			$functionHasSideEffects = false;
+		}
+
+		if (!$functionHasSideEffects || $node->expr->isFirstClassCallable()) {
 			if (!$node->expr->isFirstClassCallable()) {
 				$throwsType = $function->getThrowType();
 				if ($throwsType !== null && !$throwsType->isVoid()->yes()) {
@@ -55,15 +122,6 @@ public function processNode(Node $node, Scope $scope): array
 				return [];
 			}
 
-			if (in_array($function->getName(), [
-				'PHPStan\\dumpType',
-				'PHPStan\\Testing\\assertType',
-				'PHPStan\\Testing\\assertNativeType',
-				'PHPStan\\Testing\\assertVariableCertainty',
-			], true)) {
-				return [];
-			}
-
 			return [
 				RuleErrorBuilder::message(sprintf(
 					'Call to function %s() on a separate line has no effect.',

diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php
@@ -4,6 +4,7 @@
 
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
+use const PHP_VERSION_ID;
 
 /**
  * @extends RuleTestCase<CallToFunctionStatementWithoutSideEffectsRule>
@@ -23,6 +24,53 @@ public function testRule(): void
 				'Call to function sprintf() on a separate line has no effect.',
 				13,
 			],
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				14,
+			],
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				22,
+			],
+			[
+				'Call to function var_export() on a separate line has no effect.',
+				24,
+			],
+			[
+				'Call to function print_r() on a separate line has no effect.',
+				26,
+			],
+		]);
+
+		if (PHP_VERSION_ID < 80000) {
+			return;
+		}
+
+		$this->analyse([__DIR__ . '/data/function-call-statement-no-side-effects-8.0.php'], [
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				15,
+			],
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				16,
+			],
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				17,
+			],
+			[
+				'Call to function file_get_contents() on a separate line has no effect.',
+				18,
+			],
+			[
+				'Call to function var_export() on a separate line has no effect.',
+				19,
+			],
+			[
+				'Call to function print_r() on a separate line has no effect.',
+				20,
+			],
 		]);
 	}
 

diff --git a/tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php b/tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace FunctionCallStatementNoSideEffects;
+
+use PHPStan\TrinaryLogic;
+
+class Foo
+{
+
+	/**
+	 * @param ?resource $resourceOrNull
+	 */
+	public function noEffect(string $url, $resourceOrNull)
+	{
+		file_get_contents(filename: $url, offset: 1, length: 100);
+		file_get_contents($url, length: 100, offset: 1);
+		file_get_contents($url, false, null, length: 100);
+		file_get_contents($url, context: $resourceOrNull);
+		var_export([], return: true);
+		print_r([], return: true);
+	}
+
+	/**
+	 * @param resource $resource
+	 */
+	public function hasSideEffect(string $url, $resource)
+	{
+		file_get_contents($url, false, $resource, length: 100);
+		file_get_contents($url, context: $resource);
+		var_export(value: []);
+		print_r(value: []);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects.php b/tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects.php
@@ -7,10 +7,23 @@
 class Foo
 {
 
-	public function doFoo()
+	public function doFoo(string $url)
 	{
 		printf('%s', 'test');
 		sprintf('%s', 'test');
+		file_get_contents($url);
+		file_get_contents($url, false, stream_context_create([
+			'http' => [
+				'method' => 'POST',
+				'header' => 'Content-Type: application/json',
+				'content' => json_encode($data, JSON_THROW_ON_ERROR),
+			],
+		]));
+		file_get_contents($url, false, null);
+		var_export([]);
+		var_export([], true);
+		print_r([]);
+		print_r([], true);
 	}
 
 	public function doBar(string $s)