From 568210bd301f94a0d4b1e5a0808c374c1b9cf11b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=BD=C3=A1=C4=8Dek?= Date: Thu, 15 Feb 2024 13:33:14 +0100 Subject: [PATCH] Introduce strict array_filter call (require callback method) --- README.md | 1 + rules.neon | 10 ++ src/Rules/Functions/ArrayFilterStrictRule.php | 126 ++++++++++++++++++ .../Functions/ArrayFilterStrictRuleTest.php | 58 ++++++++ .../Functions/data/array-filter-strict.php | 36 +++++ 5 files changed, 231 insertions(+) create mode 100644 src/Rules/Functions/ArrayFilterStrictRule.php create mode 100644 tests/Rules/Functions/ArrayFilterStrictRuleTest.php create mode 100644 tests/Rules/Functions/data/array-filter-strict.php diff --git a/README.md b/README.md index 246c0c84..389adcbd 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ parameters: strictCalls: false switchConditionsMatchingType: false noVariableVariables: false + strictArrayFilter: false ``` ## Enabling rules one-by-one diff --git a/rules.neon b/rules.neon index 46632c0c..bc874af0 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: @@ -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 diff --git a/src/Rules/Functions/ArrayFilterStrictRule.php b/src/Rules/Functions/ArrayFilterStrictRule.php new file mode 100644 index 00000000..44f5310e --- /dev/null +++ b/src/Rules/Functions/ArrayFilterStrictRule.php @@ -0,0 +1,126 @@ + + */ +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 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; +});