Refactor PrintfParametersRule

phpstan · Jul 16, 2024 · 14d0951 · 14d0951
1 parent c5f4a47
commit 14d0951
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 81 deletions.
diff --git 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
@@ -0,0 +1,75 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Functions;
+
+use Nette\Utils\Strings;
+use PHPStan\Php\PhpVersion;
+use function array_filter;
+use function count;
+use function max;
+use function sprintf;
+use function strlen;
+use const PREG_SET_ORDER;
+
+final class PrintfHelper
+{
+
+	public function __construct(private PhpVersion $phpVersion)
+	{
+	}
+
+	public function getPrintfPlaceholdersCount(string $format): int
+	{
+		return $this->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 = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $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
@@ -2,30 +2,40 @@
 
 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<Node\Expr\FuncCall>
  */
 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 = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $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
@@ -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