Skip to content

Report invalid printf placeholder #4478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.1.x
Choose a base branch
from
Open

Report invalid printf placeholder #4478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3b0fd8
Report invalid printf placeholder
VincentLanglet Oct 24, 2025
5603955
Change error message
VincentLanglet Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Rules/Functions/PrintfArrayParametersRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 === []) {
Expand Down
31 changes: 22 additions & 9 deletions src/Rules/Functions/PrintfHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, non-empty-list<PrintfPlaceholder>> 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('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
}

/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
private function parsePlaceholders(string $specifiersPattern, string $format): array
/**
* @phpstan-return array<int, non-empty-list<PrintfPlaceholder>>|null parameter index => placeholders
*/
private function parsePlaceholders(string $specifiersPattern, string $format): ?array
{
$addSpecifier = '';
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
Expand All @@ -50,7 +52,7 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a

$specifiers = sprintf($specifiersPattern, $addSpecifier);

$pattern = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $specifiers . '~';
$pattern = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $specifiers . '?~';

$matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER);

Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/Rules/Functions/PrintfParameterTypeRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
9 changes: 9 additions & 0 deletions src/Rules/Functions/PrintfParametersRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]);
}

}
16 changes: 15 additions & 1 deletion tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
];
Expand All @@ -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,
],
]);
}

}
9 changes: 9 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-1889.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

printf('%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%', 71, 73, 70, 56, 57, 97, 1, 0, 1, 0, 128, 255, 0, 192, 192, 192, 0, 0, 0, 33, 249, 4, 1, 0, 0, 0, 0, 44, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 68, 1, 0, 59);

printf('%c%c%c%', 71, 73, 70, 80);

vsprintf('%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%', [71, 73, 70, 56, 57, 97, 1, 0, 1, 0, 128, 255, 0, 192, 192, 192, 0, 0, 0, 33, 249, 4, 1, 0, 0, 0, 0, 44, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 68, 1, 0, 59]);

vsprintf('%c%c%c%', [71, 73, 70, 80]);
4 changes: 2 additions & 2 deletions tests/PHPStan/Rules/Functions/data/printf.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
sprintf('%s %s', 'foo'); // one parameter missing
sprintf('foo', 'foo'); // one parameter over
sprintf('foo %s', 'foo', 'bar'); // one parameter over
sprintf('%2$s %1$s %% %1$s %%%', 'one'); // one parameter missing
sprintf('%2$s %1$s %% %1$s %%', 'one'); // one parameter missing
sprintf('%2$s %%'); // two parameters required
sprintf('%2$s %1$s %1$s %s %s %s %s'); // four parameters required
sprintf('%2$s %1$s %% %s %s %s %s %%% %%%%', 'one', 'two', 'three', 'four'); // ok
sprintf('%2$s %1$s %% %s %s %s %s %% %%%%', 'one', 'two', 'three', 'four'); // ok
sprintf("%'.9d %1$'.9d %0.3f %d %d %d", 123, 456);
sprintf('%-4s', 'foo'); // ok
sprintf('%%s %s', 'foo', 'bar'); // one parameter over
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Rules/Functions/data/vprintf.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ function doFoo($message, array $arr) {
vsprintf('%s %s', ['foo']); // one parameter missing
vsprintf('foo', ['foo']); // one parameter over
vsprintf('foo %s', ['foo', 'bar']); // one parameter over
vsprintf('%2$s %1$s %% %1$s %%%', ['one']); // one parameter missing
vsprintf('%2$s %1$s %% %1$s %%', ['one']); // one parameter missing
vsprintf('%2$s %%'); // two parameters required
vsprintf('%2$s %%', []); // two parameters required
vsprintf('%2$s %1$s %1$s %s %s %s %s'); // four parameters required
vsprintf('%2$s %1$s %% %s %s %s %s %%% %%%%', ['one', 'two', 'three', 'four']); // ok
vsprintf('%2$s %1$s %% %s %s %s %s %% %%%%', ['one', 'two', 'three', 'four']); // ok
vsprintf("%'.9d %1$'.9d %0.3f %d %d %d", [123, 456]); // five parameters required

vsprintf('%-4s', ['foo']); // ok
Expand Down
Loading