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 authored and ondrejmirtes committed Apr 6, 2024
1 parent 4723149 commit 568210b
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 0 deletions.
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
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%
checkNullables: %checkNullables%

-
class: PHPStan\Rules\Functions\ClosureUsesThisRule

Expand Down
126 changes: 126 additions & 0 deletions src/Rules/Functions/ArrayFilterStrictRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?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\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 $checkNullables;

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

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

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 = 'Parameter #2 of array_filter() cannot be null to avoid loose comparison semantics (%s given).';
$errorBuilder = RuleErrorBuilder::message(sprintf(
$message,
$callbackType->describe(VerbosityLevel::typeOnly())
));

if (!$this->isCallbackTypeNull($nativeCallbackType) && $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->checkNullables;
}

}
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 $checkNullables;

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

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

public function testRule(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->checkNullables = 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,
],
[
'Parameter #2 of array_filter() cannot be null to avoid loose comparison semantics (null given).',
28,
],
[
'Parameter #2 of array_filter() cannot be null to avoid loose comparison semantics ((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 568210b

Please sign in to comment.