diff --git a/README.md b/README.md index 74ac8d54..94310115 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ parameters: strictCalls: false switchConditionsMatchingType: false noVariableVariables: false + strictArrayFilter: false ``` Aside from introducing new custom rules, phpstan-strict-rules also [change the default values of some configuration parameters](https://github.com/phpstan/phpstan-strict-rules/blob/1.6.x/rules.neon#L1) that are present in PHPStan itself. These parameters are [documented on phpstan.org](https://phpstan.org/config-reference#stricter-analysis). diff --git a/rules.neon b/rules.neon index 9f2c6e4c..e5194fdd 100644 --- a/rules.neon +++ b/rules.neon @@ -29,6 +29,7 @@ parameters: strictCalls: %strictRules.allRules% switchConditionsMatchingType: %strictRules.allRules% noVariableVariables: %strictRules.allRules% + strictArrayFilter: [%strictRules.allRules%, %featureToggles.bleedingEdge%] parametersSchema: strictRules: structure([ @@ -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: @@ -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: @@ -188,6 +192,12 @@ services: - class: PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule + - + class: PHPStan\Rules\Functions\ArrayFilterStrictRule + arguments: + treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% + checkNullables: %checkNullables% + - class: PHPStan\Rules\Functions\ClosureUsesThisRule diff --git a/src/Rules/Functions/ArrayFilterStrictRule.php b/src/Rules/Functions/ArrayFilterStrictRule.php new file mode 100644 index 00000000..918f9dee --- /dev/null +++ b/src/Rules/Functions/ArrayFilterStrictRule.php @@ -0,0 +1,130 @@ + + */ +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.') + ->identifier('arrayFilter.strict') + ->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()) + ))->identifier('arrayFilter.strict'); + + if (!$this->isCallbackTypeNull($nativeCallbackType) && $this->treatPhpDocTypesAsCertain) { + $errorBuilder->tip('Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %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; + } + +} diff --git a/tests/Rules/Functions/ArrayFilterStrictRuleTest.php b/tests/Rules/Functions/ArrayFilterStrictRuleTest.php new file mode 100644 index 00000000..e7bcb8ac --- /dev/null +++ b/tests/Rules/Functions/ArrayFilterStrictRuleTest.php @@ -0,0 +1,58 @@ + + */ +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, + ], + ]); + } + +} diff --git a/tests/Rules/Functions/data/array-filter-strict.php b/tests/Rules/Functions/data/array-filter-strict.php new file mode 100644 index 00000000..7d7ccefa --- /dev/null +++ b/tests/Rules/Functions/data/array-filter-strict.php @@ -0,0 +1,36 @@ + $list */ +$list = [1, 2, 3]; + +/** @var array $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; +});