Skip to content

Commit

Permalink
Introduce strict array_filter call (require callback method)
Browse files Browse the repository at this point in the history
  • Loading branch information
kamil-zacek committed Feb 26, 2024
1 parent 7a50e96 commit cbdf0ed
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ parameters:
strictCalls: false
switchConditionsMatchingType: false
noVariableVariables: false
strictArrayFilter: false
```

## Enabling rules one-by-one
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
],
"require": {
"php": "^7.2 || ^8.0",
"phpstan/phpstan": "^1.10.34"
"phpstan/phpstan": "^1.10.42"
},
"require-dev": {
"nikic/php-parser": "^4.13.0",
Expand Down
10 changes: 10 additions & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ parameters:
strictCalls: %strictRules.allRules%
switchConditionsMatchingType: %strictRules.allRules%
noVariableVariables: %strictRules.allRules%
strictArrayFilter: [%strictRules.allRules%, %featureToggles.bleedingEdge%]

parametersSchema:
strictRules: structure([
Expand All @@ -45,6 +46,7 @@ parametersSchema:
strictCalls: anyOf(bool(), arrayOf(bool()))
switchConditionsMatchingType: anyOf(bool(), arrayOf(bool()))
noVariableVariables: anyOf(bool(), arrayOf(bool()))
strictArrayFilter: anyOf(bool(), arrayOf(bool()))
])

conditionalTags:
Expand Down Expand Up @@ -78,6 +80,8 @@ conditionalTags:
phpstan.rules.rule: %strictRules.overwriteVariablesWithLoop%
PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule:
phpstan.rules.rule: %strictRules.overwriteVariablesWithLoop%
PHPStan\Rules\Functions\ArrayFilterStrictRule:
phpstan.rules.rule: %strictRules.strictArrayFilter%
PHPStan\Rules\Functions\ClosureUsesThisRule:
phpstan.rules.rule: %strictRules.closureUsesThis%
PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule:
Expand Down Expand Up @@ -184,6 +188,12 @@ services:
-
class: PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule

-
class: PHPStan\Rules\Functions\ArrayFilterStrictRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
reportMaybes: %reportMaybes%

-
class: PHPStan\Rules\Functions\ClosureUsesThisRule

Expand Down
131 changes: 131 additions & 0 deletions src/Rules/Functions/ArrayFilterStrictRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use PHPStan\Analyser\ArgumentsNormalizer;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Type;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;

/**
* @implements Rule<FuncCall>
*/
class ArrayFilterStrictRule implements Rule
{

/** @var ReflectionProvider */
private $reflectionProvider;

/** @var bool */
private $treatPhpDocTypesAsCertain;

/** @var bool */
private $reportMaybes;

public function __construct(
ReflectionProvider $reflectionProvider,
bool $treatPhpDocTypesAsCertain,
bool $reportMaybes
)
{
$this->reflectionProvider = $reflectionProvider;
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->reportMaybes = $reportMaybes;
}

public function getNodeType(): string
{
return FuncCall::class;
}

/**
* @param FuncCall $node
* @return RuleError[] errors
*/
public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Name) {
return [];
}

if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
return [];
}

$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);

if ($functionReflection->getName() !== 'array_filter') {
return [];
}

$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
$scope,
$node->getArgs(),
$functionReflection->getVariants(),
$functionReflection->getNamedArgumentsVariants()
);

$normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $node);

if ($normalizedFuncCall === null) {
return [];
}

$args = $normalizedFuncCall->getArgs();
if (count($args) === 0) {
return [];
}

if (count($args) === 1) {
return [RuleErrorBuilder::message('Call to function array_filter() requires parameter #2 to be passed to avoid loose comparison semantics.')->build()];
}

$nativeCallbackType = $scope->getNativeType($args[1]->value);

if ($this->treatPhpDocTypesAsCertain) {
$callbackType = $scope->getType($args[1]->value);
} else {
$callbackType = $nativeCallbackType;
}

if ($this->isCallbackTypeNull($callbackType)) {
$message = 'Call to function array_filter() requires parameter #2 to be callable, %s given.';
$errorBuilder = RuleErrorBuilder::message(sprintf(
$message,
$callbackType->describe(VerbosityLevel::typeOnly())
));

if ($nativeCallbackType->isNull()->no() && $this->treatPhpDocTypesAsCertain) {
$errorBuilder->tip('Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.');
}

return [$errorBuilder->build()];
}

return [];
}

private function isCallbackTypeNull(Type $callbackType): bool
{
if ($callbackType->isNull()->yes()) {
return true;
}

if ($callbackType->isNull()->no()) {
return false;
}

return $this->reportMaybes;
}

}
58 changes: 58 additions & 0 deletions tests/Rules/Functions/ArrayFilterStrictRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

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

/**
* @extends RuleTestCase<ArrayFilterStrictRule>
*/
class ArrayFilterStrictRuleTest extends RuleTestCase
{

/** @var bool */
private $treatPhpDocTypesAsCertain;

/** @var bool */
private $reportMaybes;

protected function getRule(): Rule
{
return new ArrayFilterStrictRule($this->createReflectionProvider(), $this->treatPhpDocTypesAsCertain, $this->reportMaybes);
}

protected function shouldTreatPhpDocTypesAsCertain(): bool
{
return $this->treatPhpDocTypesAsCertain;
}

public function testRule(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->reportMaybes = true;
$this->analyse([__DIR__ . '/data/array-filter-strict.php'], [
[
'Call to function array_filter() requires parameter #2 to be passed to avoid loose comparison semantics.',
15,
],
[
'Call to function array_filter() requires parameter #2 to be passed to avoid loose comparison semantics.',
25,
],
[
'Call to function array_filter() requires parameter #2 to be passed to avoid loose comparison semantics.',
26,
],
[
'Call to function array_filter() requires parameter #2 to be callable, null given.',
28,
],
[
'Call to function array_filter() requires parameter #2 to be callable, (Closure)|null given.',
34,
],
]);
}

}
36 changes: 36 additions & 0 deletions tests/Rules/Functions/data/array-filter-strict.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types = 1);

namespace ArrayFilterStrict;

/** @var list<int> $list */
$list = [1, 2, 3];

/** @var array<string, int> $array */
$array = ["a" => 1, "b" => 2, "c" => 3];

array_filter([1, 2, 3], function (int $value): bool {
return $value > 1;
});

array_filter([1, 2, 3]);

array_filter([1, 2, 3], function (int $value): bool {
return $value > 1;
}, ARRAY_FILTER_USE_KEY);

array_filter([1, 2, 3], function (int $value): int {
return $value;
});

array_filter($list);
array_filter($array);

array_filter($array, null);

array_filter($list, 'intval');

/** @var bool $bool */
$bool = doFoo();
array_filter($list, foo() ? null : function (int $value): bool {
return $value > 1;
});

0 comments on commit cbdf0ed

Please sign in to comment.