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

CallToFunctionStatementWithoutSideEffectsRule checks the purity of array_filter(), array_map(), and array_reduce() #3106

Closed
wants to merge 1 commit into from
Closed

CallToFunctionStatementWithoutSideEffectsRule checks the purity of array_filter(), array_map(), and array_reduce() #3106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions resources/functionMap_bleedingEdge.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@
'bcpowmod' => ['numeric-string|null', 'base'=>'numeric-string', 'exponent'=>'numeric-string', 'modulus'=>'string', 'scale='=>'int'],
'bcsqrt' => ['numeric-string', 'operand'=>'numeric-string', 'scale='=>'int'],
'bcsub' => ['numeric-string', 'left_operand'=>'numeric-string', 'right_operand'=>'numeric-string', 'scale='=>'int'],
'array_filter' => ['array', 'input'=>'array', 'callback='=>'pure-callable(mixed,mixed):bool|pure-callable(mixed):bool', 'flag='=>'ARRAY_FILTER_USE_BOTH|ARRAY_FILTER_USE_KEY'],
'array_reduce' => ['mixed', 'input'=>'array', 'callback'=>'pure-callable(mixed,mixed):mixed', 'initial='=>'mixed'],
'array_udiff_assoc' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'key_comp_func'=>'pure-callable(mixed,mixed):int'],
'array_udiff_assoc\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|pure-callable(mixed,mixed):int', '...rest='=>'array|pure-callable(mixed,mixed):int'],
'array_udiff_uassoc' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'data_comp_func'=>'pure-callable', 'key_comp_func'=>'pure-callable(mixed,mixed):int'],
'array_udiff_uassoc\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|pure-callable(mixed,mixed):int', 'arg5'=>'array|pure-callable(mixed,mixed):int', '...rest='=>'array|pure-callable(mixed,mixed):int'],
'array_uintersect' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'data_compare_func'=>'pure-callable(mixed,mixed):int'],
'array_uintersect\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|pure-callable(mixed,mixed):int', '...rest='=>'array|pure-callable(mixed,mixed):int'],
'array_uintersect_assoc' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'data_compare_func'=>'pure-callable(mixed,mixed):int'],
'array_uintersect_assoc\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|pure-callable(mixed,mixed):int', '...rest='=>'array|pure-callable(mixed,mixed):int'],
'array_uintersect_uassoc' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'data_compare_func'=>'pure-callable(mixed,mixed):int', 'key_compare_func'=>'pure-callable(mixed,mixed):int'],
'array_uintersect_uassoc\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|pure-callable(mixed,mixed):int', 'arg5'=>'array|pure-callable(mixed,mixed):int', '...rest='=>'array|pure-callable(mixed,mixed):int'],
'Closure::bind' => ['Closure', 'old'=>'Closure', 'to'=>'?object', 'scope='=>'object|class-string|\'static\'|null'],
'Closure::bindTo' => ['Closure', 'new'=>'?object', 'newscope='=>'object|class-string|\'static\'|null'],
'error_log' => ['bool', 'message'=>'string', 'message_type='=>'0|1|2|3|4', 'destination='=>'string', 'extra_headers='=>'string'],
Expand Down
21 changes: 20 additions & 1 deletion src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\NeverType;
use PHPStan\Type\Type;
use function count;
use function in_array;
use function sprintf;

Expand All @@ -21,10 +22,12 @@ final class CallToFunctionStatementWithoutSideEffectsRule implements Rule

private const SIDE_EFFECT_FLIP_PARAMETERS = [
// functionName => [name, pos, testName]
'array_filter' => ['callback', 1, 'isPure'],
'array_map' => ['callback', 0, 'isPure'],
'array_reduce' => ['callback', 1, 'isPure'],
'print_r' => ['return', 1, 'isTruthy'],
'var_export' => ['return', 1, 'isTruthy'],
'highlight_string' => ['return', 1, 'isTruthy'],

];

public const PHPSTAN_TESTING_FUNCTIONS = [
Expand Down Expand Up @@ -76,6 +79,22 @@ public function processNode(Node $node, Scope $scope): array
$sideEffectFlipped = false;
$hasNamedParameter = false;
$checker = [
'isPure' => static function (Type $type) use ($scope) {
if ($type->isCallable()->no()) {
return false;
}
$callableParametersAcceptors = $type->getCallableParametersAcceptors($scope);
if (count($callableParametersAcceptors) === 0) {
return false;
}
foreach ($callableParametersAcceptors as $callableParametersAcceptor) {
if (!$callableParametersAcceptor->isPure()->yes()) {
return false;
}
}

return true;
},
'isNotNull' => static fn (Type $type) => $type->isNull()->no(),
'isTruthy' => static fn (Type $type) => $type->toBoolean()->isTrue()->yes(),
][$testName];
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@ public function testRule(): void
'Call to function print_r() on a separate line has no effect.',
26,
],
[
'Call to function array_filter() on a separate line has no effect.',
29,
],
[
'Call to function array_map() on a separate line has no effect.',
33,
],
[
'Call to function array_reduce() on a separate line has no effect.',
37,
],
[
'Call to function array_reduce() on a separate line has no effect.',
40,
],
]);

if (PHP_VERSION_ID < 80000) {
Expand All @@ -51,6 +67,26 @@ public function testRule(): void
'Call to function highlight_string() on a separate line has no effect.',
21,
],
[
'Call to function array_filter() on a separate line has no effect.',
22,
],
[
'Call to function array_filter() on a separate line has no effect.',
23,
],
[
'Call to function array_map() on a separate line has no effect.',
24,
],
[
'Call to function array_map() on a separate line has no effect.',
25,
],
[
'Call to function array_reduce() on a separate line has no effect.',
26,
],
]);
}

Expand Down
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-8.0.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public function noEffect(string $url, $resourceOrNull)
var_export([], return: true);
print_r([], return: true);
highlight_string($url, return: true);
array_filter([], callback: 'is_string');
Copy link
Contributor

@staabm staabm May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test cases in which the callback is impure, e.g. because of calls to impure functions or access to global variables or similar from within the callable

array_filter([], is_string(...));
array_map(array: [], callback: 'is_string');
array_map(is_string(...), []);
array_reduce([], callback: fn($carry, $item) => $carry + $item);
}

/**
Expand All @@ -31,6 +36,14 @@ public function hasSideEffect(string $url, $resource)
var_export(value: []);
print_r(value: []);
highlight_string($url);
$callback = rand() === 0 ? is_string(...) : var_dump(...);
array_filter([], callback: $callback);
array_filter([], callback: 'var_dump');
array_filter([], var_dump(...));
array_map(array: [], callback: $callback);
array_map(array: [], callback: 'var_dump');
array_map(var_dump(...), array: []);
array_reduce([], callback: $callback);
}

}
18 changes: 17 additions & 1 deletion tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class Foo
{

public function doFoo(string $url)
public function doFoo(string $url, $mixed)
{
printf('%s', 'test');
sprintf('%s', 'test');
Expand All @@ -24,6 +24,22 @@ public function doFoo(string $url)
var_export([], true);
print_r([]);
print_r([], true);
$callback = rand() === 0 ? 'is_string' : 'var_dump';
array_filter([], 'var_dump');
array_filter([], 'is_string');
array_filter([], $mixed);
array_filter([], $callback);
array_map('var_dump', []);
array_map('is_string', []);
array_map($mixed, []);
array_map($callback, []);
array_reduce([], 'var_dump');
array_reduce([], 'is_string');
array_reduce([], $mixed);
array_reduce([], $callback);
array_reduce([], function ($carry, $item) {
return $carry + $item;
});
}

public function doBar(string $s)
Expand Down
Loading