From 2f785bac95d6bc47eec7b08d481f21c08db97e75 Mon Sep 17 00:00:00 2001 From: schlndh Date: Sat, 23 Nov 2024 13:03:12 +0100 Subject: [PATCH 1/3] check that values passed to array_sum/product are castable to number --- conf/config.level5.neon | 6 + .../ParameterCastableToNumberRule.php | 84 +++++++++++ .../ParameterCastableToNumberRuleTest.php | 131 ++++++++++++++++++ .../Rules/Functions/data/bug-11883.php | 14 ++ ...aram-castable-to-number-functions-enum.php | 14 ++ ...astable-to-number-functions-named-args.php | 15 ++ .../param-castable-to-number-functions.php | 45 ++++++ 7 files changed, 309 insertions(+) create mode 100644 src/Rules/Functions/ParameterCastableToNumberRule.php create mode 100644 tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-11883.php create mode 100644 tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-enum.php create mode 100644 tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-named-args.php create mode 100644 tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions.php diff --git a/conf/config.level5.neon b/conf/config.level5.neon index b89a6dbddf..60eab69397 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -5,6 +5,10 @@ parameters: checkFunctionArgumentTypes: true checkArgumentsPassedByReference: true +conditionalTags: + PHPStan\Rules\Functions\ParameterCastableToNumberRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% + rules: - PHPStan\Rules\DateTimeInstantiationRule - PHPStan\Rules\Functions\CallUserFuncRule @@ -36,3 +40,5 @@ services: treatPhpDocTypesAsCertainTip: %tips.treatPhpDocTypesAsCertain% tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Functions\ParameterCastableToNumberRule diff --git a/src/Rules/Functions/ParameterCastableToNumberRule.php b/src/Rules/Functions/ParameterCastableToNumberRule.php new file mode 100644 index 0000000000..640c73a440 --- /dev/null +++ b/src/Rules/Functions/ParameterCastableToNumberRule.php @@ -0,0 +1,84 @@ + + */ +final class ParameterCastableToNumberRule implements Rule +{ + + public function __construct( + private ReflectionProvider $reflectionProvider, + private ParameterCastableToStringCheck $parameterCastableToStringCheck, + ) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!($node->name instanceof Node\Name)) { + return []; + } + + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { + return []; + } + + $functionReflection = $this->reflectionProvider->getFunction($node->name, $scope); + $functionName = $functionReflection->getName(); + + if (!in_array($functionName, ['array_sum', 'array_product'], true)) { + return []; + } + + $origArgs = $node->getArgs(); + + if (count($origArgs) !== 1) { + return []; + } + + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $origArgs, + $functionReflection->getVariants(), + $functionReflection->getNamedArgumentsVariants(), + ); + + $errorMessage = 'Parameter %s of function %s expects an array of values castable to number, %s given.'; + $functionParameters = $parametersAcceptor->getParameters(); + $error = $this->parameterCastableToStringCheck->checkParameter( + $origArgs[0], + $scope, + $errorMessage, + static fn (Type $t) => $t->toNumber(), + $functionName, + $this->parameterCastableToStringCheck->getParameterName( + $origArgs[0], + 0, + $functionParameters[0] ?? null, + ), + ); + + return $error !== null + ? [$error] + : []; + } + +} diff --git a/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php b/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php new file mode 100644 index 0000000000..e67881005a --- /dev/null +++ b/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php @@ -0,0 +1,131 @@ + + */ +class ParameterCastableToNumberRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $broker = $this->createReflectionProvider(); + return new ParameterCastableToNumberRule($broker, new ParameterCastableToStringCheck(new RuleLevelHelper($broker, true, false, true, false, false, false))); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/param-castable-to-number-functions.php'], [ + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array> given.', + 20, + ], + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 21, + ], + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 22, + ], + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 23, + ], + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 24, + ], + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 25, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array> given.', + 27, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 28, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 29, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 30, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 31, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 32, + ], + ]); + } + + public function testNamedArguments(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->analyse([__DIR__ . '/data/param-castable-to-number-functions-named-args.php'], [ + [ + 'Parameter $array of function array_sum expects an array of values castable to number, array> given.', + 7, + ], + [ + 'Parameter $array of function array_product expects an array of values castable to number, array> given.', + 8, + ], + ]); + } + + public function testEnum(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/param-castable-to-number-functions-enum.php'], [ + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 12, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 13, + ], + ]); + } + + public function testBug11883(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-11883.php'], [ + [ + 'Parameter #1 $array of function array_sum expects an array of values castable to number, array given.', + 13, + ], + [ + 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', + 14, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Functions/data/bug-11883.php b/tests/PHPStan/Rules/Functions/data/bug-11883.php new file mode 100644 index 0000000000..a14174777b --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-11883.php @@ -0,0 +1,14 @@ += 8.1 + +namespace Bug11883; + +enum SomeEnum: int +{ + case A = 1; + case B = 2; +} + +$enums1 = [SomeEnum::A, SomeEnum::B]; + +var_dump(array_sum($enums1)); +var_dump(array_product($enums1)); diff --git a/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-enum.php b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-enum.php new file mode 100644 index 0000000000..91e9f1f686 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-enum.php @@ -0,0 +1,14 @@ += 8.1 + +namespace ParamCastableToNumberFunctionsEnum; + +enum FooEnum +{ + case A; +} + +function invalidUsages() +{ + array_sum([FooEnum::A]); + array_product([FooEnum::A]); +} diff --git a/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-named-args.php b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-named-args.php new file mode 100644 index 0000000000..4fdc546062 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions-named-args.php @@ -0,0 +1,15 @@ += 8.0 + +namespace ParamCastableToNumberFunctionsNamedArgs; + +function invalidUsages() +{ + var_dump(array_sum(array: [[0]])); + var_dump(array_product(array: [[0]])); +} + +function validUsages() +{ + var_dump(array_sum(array: [1])); + var_dump(array_product(array: [1])); +} diff --git a/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions.php b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions.php new file mode 100644 index 0000000000..9e7c5da4d2 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/param-castable-to-number-functions.php @@ -0,0 +1,45 @@ +7.7'), 5, 5.5, null])); + var_dump(array_product(['5.5', false, true, new \SimpleXMLElement('7.7'), 5, 5.5, null])); +} From 11a974fca237e97b0af5c3cb793a8aae6beac5d1 Mon Sep 17 00:00:00 2001 From: schlndh Date: Sat, 23 Nov 2024 13:29:26 +0100 Subject: [PATCH 2/3] fix tests for PHP 7.4 --- .../ParameterCastableToNumberRuleTest.php | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php b/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php index e67881005a..77b64c3a30 100644 --- a/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ParameterCastableToNumberRuleTest.php @@ -6,6 +6,8 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; +use function array_map; +use function str_replace; use const PHP_VERSION_ID; /** @@ -22,7 +24,7 @@ protected function getRule(): Rule public function testRule(): void { - $this->analyse([__DIR__ . '/data/param-castable-to-number-functions.php'], [ + $this->analyse([__DIR__ . '/data/param-castable-to-number-functions.php'], $this->hackPhp74ErrorMessages([ [ 'Parameter #1 $array of function array_sum expects an array of values castable to number, array> given.', 20, @@ -71,7 +73,7 @@ public function testRule(): void 'Parameter #1 $array of function array_product expects an array of values castable to number, array given.', 32, ], - ]); + ])); } public function testNamedArguments(): void @@ -128,4 +130,33 @@ public function testBug11883(): void ]); } + /** + * @param list $errors + * @return list + */ + private function hackPhp74ErrorMessages(array $errors): array + { + if (PHP_VERSION_ID >= 80000) { + return $errors; + } + + return array_map(static function (array $error): array { + $error[0] = str_replace( + [ + '$array of function array_sum', + '$array of function array_product', + 'array', + ], + [ + '$input of function array_sum', + '$input of function array_product', + 'array', + ], + $error[0], + ); + + return $error; + }, $errors); + } + } From 7258214d014cefabfb26462a54fdb68c9b69da7e Mon Sep 17 00:00:00 2001 From: schlndh Date: Sat, 23 Nov 2024 13:57:46 +0100 Subject: [PATCH 3/3] add separate feature toggle --- conf/bleedingEdge.neon | 1 + conf/config.level5.neon | 2 +- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 8e06b22fda..7227e369b3 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -1,5 +1,6 @@ parameters: featureToggles: bleedingEdge: true + checkParameterCastableToNumberFunctions: true skipCheckGenericClasses!: [] stricterFunctionMap: true diff --git a/conf/config.level5.neon b/conf/config.level5.neon index 60eab69397..fd3835fbf1 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -7,7 +7,7 @@ parameters: conditionalTags: PHPStan\Rules\Functions\ParameterCastableToNumberRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% + phpstan.rules.rule: %featureToggles.checkParameterCastableToNumberFunctions% rules: - PHPStan\Rules\DateTimeInstantiationRule diff --git a/conf/config.neon b/conf/config.neon index 4679d5f31c..ba4bbb2f62 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -22,6 +22,7 @@ parameters: tooWideThrowType: true featureToggles: bleedingEdge: false + checkParameterCastableToNumberFunctions: false skipCheckGenericClasses: [] stricterFunctionMap: false fileExtensions: diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 6c7f9c40e6..f73011dcad 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -28,6 +28,7 @@ parametersSchema: ]) featureToggles: structure([ bleedingEdge: bool(), + checkParameterCastableToNumberFunctions: bool(), skipCheckGenericClasses: listOf(string()), stricterFunctionMap: bool() ])