From ee657d988134c835750583a2f7465a50585a78b6 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Thu, 1 Dec 2022 05:16:08 +0900 Subject: [PATCH 1/3] Make file_get_contents() a function with special side effects --- resources/functionMetadata.php | 3 +- ...unctionStatementWithoutSideEffectsRule.php | 53 +++++++++++++++---- ...ionStatementWithoutSideEffectsRuleTest.php | 26 +++++++++ ...ion-call-statement-no-side-effects-8.0.php | 23 ++++++++ ...unction-call-statement-no-side-effects.php | 11 +++- 5 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php diff --git a/resources/functionMetadata.php b/resources/functionMetadata.php index ad3dfb0ea5..4aefe1f0d2 100644 --- 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], -]; \ No newline at end of file +]; diff --git a/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php b/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php index 78a3c55f14..c21cc9fbec 100644 --- a/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php +++ b/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Functions; use PhpParser\Node; +use PhpParser\Node\Arg; use PHPStan\Analyser\Scope; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; @@ -42,7 +43,48 @@ 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 ($functionName === 'file_get_contents') { + $hasNamedParameter = false; + foreach ($funcCall->getRawArgs() as $i => $arg) { + if (!$arg instanceof Arg) { + return []; + } + + $isContextParameter = false; + + if ($arg->name !== null) { + $hasNamedParameter = true; + + if ($arg->name->name === 'context') { + $isContextParameter = true; + } + } + + if (!$hasNamedParameter && $i === 2) { + $isContextParameter = true; + } + + if ($isContextParameter && !$scope->getType($arg->value)->isNull()->yes()) { + return []; + } + } + + $functionHasSideEffects = false; + } + + if (!$functionHasSideEffects || $node->expr->isFirstClassCallable()) { if (!$node->expr->isFirstClassCallable()) { $throwsType = $function->getThrowType(); if ($throwsType !== null && !$throwsType->isVoid()->yes()) { @@ -55,15 +97,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 index d53dc16668..c176d75e9f 100644 --- 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 @@ -23,7 +24,32 @@ 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, + ], ]); + + if (PHP_VERSION_ID >= 80000) { + $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.', + 12, + ], + [ + 'Call to function file_get_contents() on a separate line has no effect.', + 13, + ], + [ + 'Call to function file_get_contents() on a separate line has no effect.', + 14, + ], + ]); + } } public function testPhpDoc(): void 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 new file mode 100644 index 0000000000..5afd87dbfe --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php @@ -0,0 +1,23 @@ + [ + 'method' => 'POST', + 'header' => 'Content-Type: application/json', + 'content' => json_encode($data, JSON_THROW_ON_ERROR), + ], + ])); + file_get_contents($url, false, null); } public function doBar(string $s) From 8a7f6c527d71dcc8b179ee9678efb4f20bbaa42f Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Thu, 1 Dec 2022 11:33:41 +0900 Subject: [PATCH 2/3] Generalize handling for exception functions --- ...unctionStatementWithoutSideEffectsRule.php | 43 +++++++++++++++---- ...ionStatementWithoutSideEffectsRuleTest.php | 26 +++++++++-- ...ion-call-statement-no-side-effects-8.0.php | 19 ++++++-- ...unction-call-statement-no-side-effects.php | 4 ++ 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php b/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php index c21cc9fbec..b8f7d2271a 100644 --- a/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php +++ b/src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php @@ -9,6 +9,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\NeverType; +use PHPStan\Type\Type; use function in_array; use function sprintf; @@ -18,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) { } @@ -55,32 +63,49 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($functionName === 'file_get_contents') { + 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 []; } - $isContextParameter = false; + $isFlipParameter = false; if ($arg->name !== null) { $hasNamedParameter = true; - - if ($arg->name->name === 'context') { - $isContextParameter = true; + if ($arg->name->name === $flipParameterName) { + $isFlipParameter = true; } } - if (!$hasNamedParameter && $i === 2) { - $isContextParameter = true; + if (!$hasNamedParameter && $i === $flipParameterPosision) { + $isFlipParameter = true; } - if ($isContextParameter && !$scope->getType($arg->value)->isNull()->yes()) { - return []; + if ($isFlipParameter) { + $sideEffectFlipped = $checker($scope->getType($arg->value)); + break; } } + if ($sideEffectFlipped xor $defaultHasSideEffect) { + return []; + } + $functionHasSideEffects = false; } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php index c176d75e9f..c063323852 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php @@ -32,21 +32,41 @@ public function testRule(): void '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) { $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.', - 12, + 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.', - 13, + 17, ], [ 'Call to function file_get_contents() on a separate line has no effect.', - 14, + 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 index 5afd87dbfe..b1314fb5d5 100644 --- 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 @@ -7,17 +7,28 @@ class Foo { - public function noEffect(string $url) + /** + * @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); } - public function hasSideEffect(string $s) + /** + * @param resource $resource + */ + public function hasSideEffect(string $url, $resource) { - file_get_contents($url, false, stream_context_create(), length: 100); - file_get_contents($url, context: stream_context_create()); + 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 index e603b7bc69..d5c31a3af4 100644 --- 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 @@ -20,6 +20,10 @@ public function doFoo(string $url) ], ])); file_get_contents($url, false, null); + var_export([]); + var_export([], true); + print_r([]); + print_r([], true); } public function doBar(string $s) From 744c4dac91ae243dbaab8b7ed180b46105a3aff5 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Mon, 5 Dec 2022 06:00:11 +0900 Subject: [PATCH 3/3] Make early return instead of nesting conditional Fixes SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed --- ...ionStatementWithoutSideEffectsRuleTest.php | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php index c063323852..163038e8d9 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php @@ -42,34 +42,36 @@ public function testRule(): void ], ]); - if (PHP_VERSION_ID >= 80000) { - $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, - ], - ]); + 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, + ], + ]); } public function testPhpDoc(): void