-
Notifications
You must be signed in to change notification settings - Fork 457
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Bleeding edge: Check vprintf/vsprintf arguments against placeholder c…
…ount
- Loading branch information
Showing
7 changed files
with
338 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
78
tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
], | ||
]); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
|
||
} |