Skip to content
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.

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

vprintf/vsprintf require a non-empty-array argument #3126

Merged
merged 14 commits into from
Jul 19, 2024
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@ parameters:
checkParameterCastableToStringFunctions: true
narrowPregMatches: true
uselessReturnValue: true
printfArrayParameters: true
stubFiles:
- ../stubs/bleedingEdge/Rule.stub
5 changes: 5 additions & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -298,3 +300,6 @@ services:

-
class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule

-
class: PHPStan\Rules\Functions\PrintfArrayParametersRule
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ parameters:
checkParameterCastableToStringFunctions: false
narrowPregMatches: false
uselessReturnValue: false
printfArrayParameters: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ parametersSchema:
checkParameterCastableToStringFunctions: bool()
narrowPregMatches: bool()
uselessReturnValue: bool()
printfArrayParameters: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
188 changes: 188 additions & 0 deletions src/Rules/Functions/PrintfArrayParametersRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\IntegerRangeType;
use PHPStan\Type\TypeCombinator;
use function count;
use function in_array;
use function max;
use function min;
use function sprintf;

/**
* @implements Rule<Node\Expr\FuncCall>
*/
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();
}

}
78 changes: 78 additions & 0 deletions tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<PrintfArrayParametersRule>
*/
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,
],
]);
}

}
64 changes: 64 additions & 0 deletions tests/PHPStan/Rules/Functions/data/vprintf.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

namespace PrintfArrayParametersRuleTest;

function doFoo($message, array $arr) {
vsprintf($message, 'foo'); // skip - format not a literal string

vsprintf('%s', ['foo']); // ok
vsprintf('%s %% %% %s', ['foo', 'bar']); // ok
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 %%'); // 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("%'.9d %1$'.9d %0.3f %d %d %d", [123, 456]); // five parameters required

vsprintf('%-4s', ['foo']); // ok
vsprintf('%%s %s', ['foo', 'bar']); // one parameter over


if (rand(0,1)) {
$args = ['foo'];
} else {
$args = ['foo', 'bar'];
}
vsprintf('%-4s', $args); // one path with wrong number of args


vprintf('%s', ['foo']); // ok
vprintf('%s %% %% %s', ['foo', 'bar']); // ok
vprintf('%s %s', ['foo']); // one parameter missing
vprintf('abc'); // caught by CallToFunctionParametersRule
vsprintf('abc', []); // ok
vsprintf('abc', $arr); // ok

if (rand(0,1)) {
$format = '%s';
$args = ['foo'];
} else {
$format = '%s%s';
$args = ['foo', 'bar'];
}
vsprintf($format, $args); // ok

if (rand(0,1)) {
$format = '%s';
} else {
$format = '%s%s';
}
vsprintf($format, $arr); // need at least non-empty-array

if (rand(0,1)) {
$format = '%s';
} else {
$format = '%s%s';
}
if ($arr !== []) {
vsprintf($format, $arr); // ok
}

}
Loading