diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 4d8707567e..396a0acd13 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -56,5 +56,6 @@ parameters: checkParameterCastableToStringFunctions: true narrowPregMatches: true uselessReturnValue: true + printfArrayParameters: true stubFiles: - ../stubs/bleedingEdge/Rule.stub diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 844d09cdf1..4f9da39886 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -26,6 +26,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.magicConstantOutOfContext% PHPStan\Rules\Functions\UselessFunctionReturnValueRule: phpstan.rules.rule: %featureToggles.uselessReturnValue% + PHPStan\Rules\Functions\PrintfArrayParametersRule: + phpstan.rules.rule: %featureToggles.printfArrayParameters% rules: - PHPStan\Rules\Api\ApiInstantiationRule @@ -298,3 +300,6 @@ services: - class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule + + - + class: PHPStan\Rules\Functions\PrintfArrayParametersRule diff --git a/conf/config.neon b/conf/config.neon index 1e91656943..21aa21dfd3 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -91,6 +91,7 @@ parameters: checkParameterCastableToStringFunctions: false narrowPregMatches: false uselessReturnValue: false + printfArrayParameters: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 0cd1faa9b1..8459195463 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -86,6 +86,7 @@ parametersSchema: checkParameterCastableToStringFunctions: bool() narrowPregMatches: bool() uselessReturnValue: bool() + printfArrayParameters: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php new file mode 100644 index 0000000000..fa190ddfa0 --- /dev/null +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -0,0 +1,188 @@ + + */ +class PrintfArrayParametersRule implements Rule +{ + + public function __construct( + private PrintfHelper $printfHelper, + private ReflectionProvider $reflectionProvider, + ) + { + } + + 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); + $name = $functionReflection->getName(); + if (!in_array($name, ['vprintf', 'vsprintf'], true)) { + return []; + } + + $args = $node->getArgs(); + $argsCount = count($args); + if ($argsCount < 1) { + return []; // caught by CallToFunctionParametersRule + } + + $formatArgType = $scope->getType($args[0]->value); + $placeHoldersCounts = []; + foreach ($formatArgType->getConstantStrings() as $formatString) { + $format = $formatString->getValue(); + + $placeHoldersCounts[] = $this->printfHelper->getPrintfPlaceholdersCount($format); + } + + if ($placeHoldersCounts === []) { + return []; + } + + $minCount = min($placeHoldersCounts); + $maxCount = max($placeHoldersCounts); + if ($minCount === $maxCount) { + $placeHoldersCount = new ConstantIntegerType($minCount); + } else { + $placeHoldersCount = IntegerRangeType::fromInterval($minCount, $maxCount); + + if (!$placeHoldersCount instanceof IntegerRangeType && !$placeHoldersCount instanceof ConstantIntegerType) { + return []; + } + } + + $formatArgsCounts = []; + if (isset($args[1])) { + $formatArgsType = $scope->getType($args[1]->value); + + $constantArrays = $formatArgsType->getConstantArrays(); + foreach ($constantArrays as $constantArray) { + $formatArgsCounts[] = $constantArray->getArraySize(); + } + + if ($constantArrays === []) { + $formatArgsCounts[] = $formatArgsType->getArraySize(); + } + } + + if ($formatArgsCounts === []) { + $formatArgsCount = new ConstantIntegerType(0); + } else { + $formatArgsCount = TypeCombinator::union(...$formatArgsCounts); + + if (!$formatArgsCount instanceof IntegerRangeType && !$formatArgsCount instanceof ConstantIntegerType) { + return []; + } + } + + if (!$this->placeholdersMatchesArgsCount($placeHoldersCount, $formatArgsCount)) { + + if ($placeHoldersCount instanceof IntegerRangeType) { + $placeholders = $this->getIntegerRangeAsString($placeHoldersCount); + $singlePlaceholder = false; + } else { + $placeholders = $placeHoldersCount->getValue(); + $singlePlaceholder = $placeholders === 1; + } + + if ($formatArgsCount instanceof IntegerRangeType) { + $values = $this->getIntegerRangeAsString($formatArgsCount); + $singleValue = false; + } else { + $values = $formatArgsCount->getValue(); + $singleValue = $values === 1; + } + + return [ + RuleErrorBuilder::message(sprintf( + sprintf( + '%s, %s.', + $singlePlaceholder ? 'Call to %s contains %d placeholder' : 'Call to %s contains %s placeholders', + $singleValue ? '%d value given' : '%s values given', + ), + $name, + $placeholders, + $values, + ))->identifier(sprintf('argument.%s', $name))->build(), + ]; + } + + return []; + } + + private function placeholdersMatchesArgsCount(ConstantIntegerType|IntegerRangeType $placeHoldersCount, ConstantIntegerType|IntegerRangeType $formatArgsCount): bool + { + if ($placeHoldersCount instanceof ConstantIntegerType) { + if ($formatArgsCount instanceof ConstantIntegerType) { + return $placeHoldersCount->getValue() === $formatArgsCount->getValue(); + } + + // Zero placeholders + array + if ($placeHoldersCount->getValue() === 0) { + return true; + } + + return false; + } + + if ( + $formatArgsCount instanceof IntegerRangeType + && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($placeHoldersCount)->yes() + ) { + if ($formatArgsCount->getMin() !== null && $formatArgsCount->getMax() !== null) { + // constant array + return $placeHoldersCount->isSuperTypeOf($formatArgsCount)->yes(); + } + + // general array + return IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($formatArgsCount)->yes(); + } + + return false; + } + + private function getIntegerRangeAsString(IntegerRangeType $range): string + { + if ($range->getMin() !== null && $range->getMax() !== null) { + return $range->getMin() . '-' . $range->getMax(); + } elseif ($range->getMin() !== null) { + return $range->getMin() . ' or more'; + } elseif ($range->getMax() !== null) { + return $range->getMax() . ' or less'; + } + + throw new ShouldNotHappenException(); + } + +} diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php new file mode 100644 index 0000000000..c1aa4a41aa --- /dev/null +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -0,0 +1,78 @@ + + */ +class PrintfArrayParametersRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new PrintfArrayParametersRule( + new PrintfHelper(new PhpVersion(PHP_VERSION_ID)), + $this->createReflectionProvider(), + ); + } + + public function testFile(): void + { + $this->analyse([__DIR__ . '/data/vprintf.php'], [ + [ + 'Call to vsprintf contains 2 placeholders, 1 value given.', + 10, + ], + [ + 'Call to vsprintf contains 0 placeholders, 1 value given.', + 11, + ], + [ + 'Call to vsprintf contains 1 placeholder, 2 values given.', + 12, + ], + [ + 'Call to vsprintf contains 2 placeholders, 1 value given.', + 13, + ], + [ + 'Call to vsprintf contains 2 placeholders, 0 values given.', + 14, + ], + [ + 'Call to vsprintf contains 2 placeholders, 0 values given.', + 15, + ], + [ + 'Call to vsprintf contains 4 placeholders, 0 values given.', + 16, + ], + [ + 'Call to vsprintf contains 5 placeholders, 2 values given.', + 18, + ], + [ + 'Call to vsprintf contains 1 placeholder, 2 values given.', + 21, + ], + [ + 'Call to vsprintf contains 1 placeholder, 1-2 values given.', + 29, + ], + [ + 'Call to vprintf contains 2 placeholders, 1 value given.', + 34, + ], + [ + 'Call to vsprintf contains 1-2 placeholders, 0 or more values given.', + 53, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php new file mode 100644 index 0000000000..7c30ff32d6 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -0,0 +1,64 @@ +