From f3b0fd8dfbb26d3f1274572f42e006fbf46686a9 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 24 Oct 2025 23:48:19 +0200 Subject: [PATCH 1/2] Report invalid printf placeholder --- .../Functions/PrintfArrayParametersRule.php | 12 ++++++- src/Rules/Functions/PrintfHelper.php | 31 +++++++++++++------ .../Functions/PrintfParameterTypeRule.php | 5 +++ src/Rules/Functions/PrintfParametersRule.php | 9 ++++++ .../PrintfArrayParametersRuleTest.php | 14 +++++++++ .../Functions/PrintfParametersRuleTest.php | 14 +++++++++ .../PHPStan/Rules/Functions/data/bug-1889.php | 9 ++++++ tests/PHPStan/Rules/Functions/data/printf.php | 4 +-- .../PHPStan/Rules/Functions/data/vprintf.php | 4 +-- 9 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-1889.php diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 1d7e093116..8ce448ba71 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -66,7 +66,17 @@ public function processNode(Node $node, Scope $scope): array foreach ($formatArgType->getConstantStrings() as $formatString) { $format = $formatString->getValue(); - $placeHoldersCounts[] = $this->printfHelper->getPrintfPlaceholdersCount($format); + $count = $this->printfHelper->getPrintfPlaceholdersCount($format); + if ($count === null) { + return [ + RuleErrorBuilder::message(sprintf( + 'Call to %s contains an invalid placeholder.', + $name, + ))->identifier(sprintf('argument.%s', $name))->build(), + ]; + } + + $placeHoldersCounts[] = $count; } if ($placeHoldersCounts === []) { diff --git a/src/Rules/Functions/PrintfHelper.php b/src/Rules/Functions/PrintfHelper.php index 73cb4af220..65334d578f 100644 --- a/src/Rules/Functions/PrintfHelper.php +++ b/src/Rules/Functions/PrintfHelper.php @@ -24,24 +24,26 @@ public function __construct(private PhpVersion $phpVersion) { } - public function getPrintfPlaceholdersCount(string $format): int + public function getPrintfPlaceholdersCount(string $format): ?int { return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format); } /** @phpstan-return array> parameter index => placeholders */ - public function getPrintfPlaceholders(string $format): array + public function getPrintfPlaceholders(string $format): ?array { return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format); } - public function getScanfPlaceholdersCount(string $format): int + public function getScanfPlaceholdersCount(string $format): ?int { return $this->getPlaceholdersCount('(?[cdDeEfinosuxX%s]|\[[^\]]+\])', $format); } - /** @phpstan-return array> parameter index => placeholders */ - private function parsePlaceholders(string $specifiersPattern, string $format): array + /** + * @phpstan-return array>|null parameter index => placeholders + */ + private function parsePlaceholders(string $specifiersPattern, string $format): ?array { $addSpecifier = ''; if ($this->phpVersion->supportsHhPrintfSpecifier()) { @@ -50,7 +52,7 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a $specifiers = sprintf($specifiersPattern, $addSpecifier); - $pattern = '~(?%*)%(?:(?\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?\*)?-?\d*(?:\.(?:\d+|(?\*))?)?' . $specifiers . '~'; + $pattern = '~(?%*)%(?:(?\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?\*)?-?\d*(?:\.(?:\d+|(?\*))?)?' . $specifiers . '?~'; $matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER); @@ -89,13 +91,19 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a $showValueSuffix = true; } + $specifier = $placeholder['specifier'] ?? ''; + if ($specifier === '') { + // A placeholder is invalid. + return null; + } + $parsedPlaceholders[] = new PrintfPlaceholder( sprintf('"%s"', $placeholder[0]) . ($showValueSuffix ? ' (value)' : ''), isset($placeholder['position']) && $placeholder['position'] !== '' ? $placeholder['position'] - 1 : $parameterIdx++, $placeholderNumber, - $this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? ''), + $this->getAcceptingTypeBySpecifier($specifier), ); } @@ -124,9 +132,14 @@ private function getAcceptingTypeBySpecifier(string $specifier): string return 'mixed'; } - private function getPlaceholdersCount(string $specifiersPattern, string $format): int + private function getPlaceholdersCount(string $specifiersPattern, string $format): ?int { - $paramIndices = array_keys($this->parsePlaceholders($specifiersPattern, $format)); + $placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format); + if ($placeholdersMap === null) { + return null; + } + + $paramIndices = array_keys($placeholdersMap); return $paramIndices === [] ? 0 diff --git a/src/Rules/Functions/PrintfParameterTypeRule.php b/src/Rules/Functions/PrintfParameterTypeRule.php index d91da4590e..974e5ea420 100644 --- a/src/Rules/Functions/PrintfParameterTypeRule.php +++ b/src/Rules/Functions/PrintfParameterTypeRule.php @@ -92,6 +92,11 @@ public function processNode(Node $node, Scope $scope): array $formatString = $formatArgTypeStrings[0]; $format = $formatString->getValue(); $placeholderMap = $this->printfHelper->getPrintfPlaceholders($format); + if ($placeholderMap === null) { + // Already reported by PrintfParametersRule. + return []; + } + $errors = []; $typeAllowedByCallToFunctionParametersRule = TypeCombinator::union( new StringAlwaysAcceptingObjectWithToStringType(), diff --git a/src/Rules/Functions/PrintfParametersRule.php b/src/Rules/Functions/PrintfParametersRule.php index 44b02b14e9..a81db2773b 100644 --- a/src/Rules/Functions/PrintfParametersRule.php +++ b/src/Rules/Functions/PrintfParametersRule.php @@ -86,6 +86,15 @@ public function processNode(Node $node, Scope $scope): array $tempPlaceHoldersCount = $this->printfHelper->getScanfPlaceholdersCount($format); } + if ($tempPlaceHoldersCount === null) { + return [ + RuleErrorBuilder::message(sprintf( + 'Call to %s contains an invalid placeholder.', + $name, + ))->identifier(sprintf('argument.%s', $name))->build(), + ]; + } + if ($maxPlaceHoldersCount === null) { $maxPlaceHoldersCount = $tempPlaceHoldersCount; } elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) { diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php index d799e58579..5b9d0e9a36 100644 --- a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -71,4 +71,18 @@ public function testFile(): void ]); } + public function testBug1889(): void + { + $this->analyse([__DIR__ . '/data/bug-1889.php'], [ + [ + 'Call to vsprintf contains an invalid placeholder.', + 7, + ], + [ + 'Call to vsprintf contains an invalid placeholder.', + 9, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index d60b1e0be6..11f7d1a79a 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -123,4 +123,18 @@ public function testBug2342(): void ]); } + public function testBug1889(): void + { + $this->analyse([__DIR__ . '/data/bug-1889.php'], [ + [ + 'Call to printf contains an invalid placeholder.', + 3, + ], + [ + 'Call to printf contains an invalid placeholder.', + 5, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-1889.php b/tests/PHPStan/Rules/Functions/data/bug-1889.php new file mode 100644 index 0000000000..37a4496b1b --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-1889.php @@ -0,0 +1,9 @@ + Date: Fri, 24 Oct 2025 23:59:12 +0200 Subject: [PATCH 2/2] Change error message --- tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index 11f7d1a79a..9f6af54439 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -103,7 +103,7 @@ public function testBug4717(): void { $errors = [ [ - 'Call to sprintf contains 1 placeholder, 2 values given.', + 'Call to sprintf contains an invalid placeholder.', 5, ], ];