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.

Sign up for GitHub

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

Jump to bottom

Supports functions whose side effects are flipped by parameters #2037

Merged
merged 3 commits into from Jan 8, 2023
Merged

Supports functions whose side effects are flipped by parameters #2037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee657d9
Make file_get_contents() a function with special side effects
zonuexe Nov 30, 2022
8a7f6c5
Generalize handling for exception functions
zonuexe Dec 1, 2022
744c4da
Make early return instead of nesting conditional
zonuexe Dec 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions resources/functionMetadata.php
View file
Open in desktop
Expand Up @@ -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],
Expand Down Expand Up @@ -1552,4 +1551,4 @@
'zlib_encode' => ['hasSideEffects' => false],
'zlib_get_coding_type' => ['hasSideEffects' => false],

];
];
78 changes: 68 additions & 10 deletions src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php
View file
Open in desktop
Expand Up @@ -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;

Expand All @@ -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)
{
}
Expand All @@ -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()) {
Expand All @@ -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.',
Expand Down
48 changes: 48 additions & 0 deletions tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<CallToFunctionStatementWithoutSideEffectsRule>
Expand All @@ -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,
],
]);
}

Expand Down
34 changes: 34 additions & 0 deletions tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php
View file
Open in desktop
@@ -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: []);
}

}
15 changes: 14 additions & 1 deletion tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects.php
View file
Open in desktop
Expand Up @@ -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)
Expand Down