From 14d0951febe640418286b8cd364c0dbba44fb93e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 16 Jul 2024 08:55:45 +0200 Subject: [PATCH] Refactor PrintfParametersRule --- conf/config.neon | 3 + src/Rules/Functions/PrintfHelper.php | 75 +++++++++++ src/Rules/Functions/PrintfParametersRule.php | 123 ++++++------------ .../Functions/PrintfParametersRuleTest.php | 5 +- 4 files changed, 125 insertions(+), 81 deletions(-) create mode 100644 src/Rules/Functions/PrintfHelper.php diff --git a/conf/config.neon b/conf/config.neon index 51b320a7a1..1e91656943 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -1886,6 +1886,9 @@ services: - class: PHPStan\Type\Constant\OversizedArrayBuilder + - + class: PHPStan\Rules\Functions\PrintfHelper + exceptionTypeResolver: class: PHPStan\Rules\Exceptions\ExceptionTypeResolver factory: @PHPStan\Rules\Exceptions\DefaultExceptionTypeResolver diff --git a/src/Rules/Functions/PrintfHelper.php b/src/Rules/Functions/PrintfHelper.php new file mode 100644 index 0000000000..a5d4571f76 --- /dev/null +++ b/src/Rules/Functions/PrintfHelper.php @@ -0,0 +1,75 @@ +getPlaceholdersCount('(?:[bs%s]|l?[cdeEgfFGouxX])', $format); + } + + public function getScanfPlaceholdersCount(string $format): int + { + return $this->getPlaceholdersCount('(?:[cdDeEfinosuxX%s]|\[[^\]]+\])', $format); + } + + private function getPlaceholdersCount(string $specifiersPattern, string $format): int + { + $addSpecifier = ''; + if ($this->phpVersion->supportsHhPrintfSpecifier()) { + $addSpecifier .= 'hH'; + } + + $specifiers = sprintf($specifiersPattern, $addSpecifier); + + $pattern = '~(?%*)%(?:(?\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?\*)?-?\d*(?:\.(?:\d+|(?\*))?)?' . $specifiers . '~'; + + $matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER); + + if (count($matches) === 0) { + return 0; + } + + $placeholders = array_filter($matches, static fn (array $match): bool => strlen($match['before']) % 2 === 0); + + if (count($placeholders) === 0) { + return 0; + } + + $maxPositionedNumber = 0; + $maxOrdinaryNumber = 0; + foreach ($placeholders as $placeholder) { + if (isset($placeholder['width']) && $placeholder['width'] !== '') { + $maxOrdinaryNumber++; + } + + if (isset($placeholder['precision']) && $placeholder['precision'] !== '') { + $maxOrdinaryNumber++; + } + + if (isset($placeholder['position']) && $placeholder['position'] !== '') { + $maxPositionedNumber = max((int) $placeholder['position'], $maxPositionedNumber); + } else { + $maxOrdinaryNumber++; + } + } + + return max($maxPositionedNumber, $maxOrdinaryNumber); + } + +} diff --git a/src/Rules/Functions/PrintfParametersRule.php b/src/Rules/Functions/PrintfParametersRule.php index 8f5f9f2bd5..a1d8e52f35 100644 --- a/src/Rules/Functions/PrintfParametersRule.php +++ b/src/Rules/Functions/PrintfParametersRule.php @@ -2,22 +2,16 @@ namespace PHPStan\Rules\Functions; -use Nette\Utils\Strings; use PhpParser\Node; use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; -use PHPStan\Php\PhpVersion; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use function array_filter; use function array_key_exists; use function count; use function in_array; -use function max; use function sprintf; -use function strlen; -use function strtolower; -use const PREG_SET_ORDER; /** * @implements Rule @@ -25,7 +19,23 @@ class PrintfParametersRule implements Rule { - public function __construct(private PhpVersion $phpVersion) + private const FORMAT_ARGUMENT_POSITIONS = [ + 'printf' => 0, + 'sprintf' => 0, + 'sscanf' => 1, + 'fscanf' => 1, + ]; + private const MINIMUM_NUMBER_OF_ARGUMENTS = [ + 'printf' => 1, + 'sprintf' => 1, + 'sscanf' => 3, + 'fscanf' => 3, + ]; + + public function __construct( + private PrintfHelper $printfHelper, + private ReflectionProvider $reflectionProvider, + ) { } @@ -40,25 +50,17 @@ public function processNode(Node $node, Scope $scope): array return []; } - $functionsArgumentPositions = [ - 'printf' => 0, - 'sprintf' => 0, - 'sscanf' => 1, - 'fscanf' => 1, - ]; - $minimumNumberOfArguments = [ - 'printf' => 1, - 'sprintf' => 1, - 'sscanf' => 3, - 'fscanf' => 3, - ]; - - $name = strtolower((string) $node->name); - if (!array_key_exists($name, $functionsArgumentPositions)) { + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { return []; } - $formatArgumentPosition = $functionsArgumentPositions[$name]; + $functionReflection = $this->reflectionProvider->getFunction($node->name, $scope); + $name = $functionReflection->getName(); + if (!array_key_exists($name, self::FORMAT_ARGUMENT_POSITIONS)) { + return []; + } + + $formatArgumentPosition = self::FORMAT_ARGUMENT_POSITIONS[$name]; $args = $node->getArgs(); foreach ($args as $arg) { @@ -67,38 +69,44 @@ public function processNode(Node $node, Scope $scope): array } } $argsCount = count($args); - if ($argsCount < $minimumNumberOfArguments[$name]) { + if ($argsCount < self::MINIMUM_NUMBER_OF_ARGUMENTS[$name]) { return []; // caught by CallToFunctionParametersRule } $formatArgType = $scope->getType($args[$formatArgumentPosition]->value); - $placeHoldersCount = null; + $maxPlaceHoldersCount = null; foreach ($formatArgType->getConstantStrings() as $formatString) { $format = $formatString->getValue(); - $tempPlaceHoldersCount = $this->getPlaceholdersCount($name, $format); - if ($placeHoldersCount === null) { - $placeHoldersCount = $tempPlaceHoldersCount; - } elseif ($tempPlaceHoldersCount > $placeHoldersCount) { - $placeHoldersCount = $tempPlaceHoldersCount; + + if (in_array($name, ['sprintf', 'printf'], true)) { + $tempPlaceHoldersCount = $this->printfHelper->getPrintfPlaceholdersCount($format); + } else { + $tempPlaceHoldersCount = $this->printfHelper->getScanfPlaceholdersCount($format); + } + + if ($maxPlaceHoldersCount === null) { + $maxPlaceHoldersCount = $tempPlaceHoldersCount; + } elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) { + $maxPlaceHoldersCount = $tempPlaceHoldersCount; } } - if ($placeHoldersCount === null) { + if ($maxPlaceHoldersCount === null) { return []; } $argsCount -= $formatArgumentPosition; - if ($argsCount !== $placeHoldersCount + 1) { + if ($argsCount !== $maxPlaceHoldersCount + 1) { return [ RuleErrorBuilder::message(sprintf( sprintf( '%s, %s.', - $placeHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', + $maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', $argsCount - 1 === 1 ? '%d value given' : '%d values given', ), $name, - $placeHoldersCount, + $maxPlaceHoldersCount, $argsCount - 1, ))->identifier(sprintf('argument.%s', $name))->build(), ]; @@ -107,49 +115,4 @@ public function processNode(Node $node, Scope $scope): array return []; } - private function getPlaceholdersCount(string $functionName, string $format): int - { - $specifiers = in_array($functionName, ['sprintf', 'printf'], true) ? '(?:[bs%s]|l?[cdeEgfFGouxX])' : '(?:[cdDeEfinosuxX%s]|\[[^\]]+\])'; - $addSpecifier = ''; - if ($this->phpVersion->supportsHhPrintfSpecifier()) { - $addSpecifier .= 'hH'; - } - - $specifiers = sprintf($specifiers, $addSpecifier); - - $pattern = '~(?%*)%(?:(?\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?\*)?-?\d*(?:\.(?:\d+|(?\*))?)?' . $specifiers . '~'; - - $matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER); - - if (count($matches) === 0) { - return 0; - } - - $placeholders = array_filter($matches, static fn (array $match): bool => strlen($match['before']) % 2 === 0); - - if (count($placeholders) === 0) { - return 0; - } - - $maxPositionedNumber = 0; - $maxOrdinaryNumber = 0; - foreach ($placeholders as $placeholder) { - if (isset($placeholder['width']) && $placeholder['width'] !== '') { - $maxOrdinaryNumber++; - } - - if (isset($placeholder['precision']) && $placeholder['precision'] !== '') { - $maxOrdinaryNumber++; - } - - if (isset($placeholder['position']) && $placeholder['position'] !== '') { - $maxPositionedNumber = max((int) $placeholder['position'], $maxPositionedNumber); - } else { - $maxOrdinaryNumber++; - } - } - - return max($maxPositionedNumber, $maxOrdinaryNumber); - } - } diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index 4e2880fda7..252f2919ec 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -15,7 +15,10 @@ class PrintfParametersRuleTest extends RuleTestCase protected function getRule(): Rule { - return new PrintfParametersRule(new PhpVersion(PHP_VERSION_ID)); + return new PrintfParametersRule( + new PrintfHelper(new PhpVersion(PHP_VERSION_ID)), + $this->createReflectionProvider(), + ); } public function testFile(): void