From 4e313a98de10cf2160a846ca14b231adf5540012 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 4 Jun 2024 18:31:34 +0200 Subject: [PATCH 01/14] Implement PrintfArrayParametersRule --- conf/config.level0.neon | 4 + .../Functions/PrintfArrayParametersRule.php | 111 ++++++++++++++++++ .../PrintfArrayParametersRuleTest.php | 63 ++++++++++ .../PHPStan/Rules/Functions/data/vprintf.php | 17 +++ 4 files changed, 195 insertions(+) create mode 100644 src/Rules/Functions/PrintfArrayParametersRule.php create mode 100644 tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php create mode 100644 tests/PHPStan/Rules/Functions/data/vprintf.php diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 844d09cdf1..c6e2c071fd 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -79,6 +79,7 @@ rules: - PHPStan\Rules\Functions\InvalidLexicalVariablesInClosureUseRule - PHPStan\Rules\Functions\ParamAttributesRule - PHPStan\Rules\Functions\PrintfParametersRule + - PHPStan\Rules\Functions\PrintfArrayParametersRule - PHPStan\Rules\Functions\RedefinedParametersRule - PHPStan\Rules\Functions\ReturnNullsafeByRefRule - PHPStan\Rules\Ignore\IgnoreParseErrorRule @@ -298,3 +299,6 @@ services: - class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule + + - + class: PHPStan\Rules\Functions\PrintfHelper diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php new file mode 100644 index 0000000000..018eea3e15 --- /dev/null +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -0,0 +1,111 @@ + + */ +class PrintfArrayParametersRule implements Rule +{ + + public function __construct(private PrintfHelper $printfHelper) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!($node->name instanceof Node\Name)) { + return []; + } + + $functionsArgumentPositions = [ + 'vprintf' => 0, + 'vsprintf' => 0, + ]; + $minimumNumberOfArguments = [ + 'vprintf' => 1, + 'vsprintf' => 1, + ]; + + $name = strtolower((string) $node->name); + if (!array_key_exists($name, $functionsArgumentPositions)) { + return []; + } + + $formatArgumentPosition = $functionsArgumentPositions[$name]; + + $args = $node->getArgs(); + foreach ($args as $arg) { + if ($arg->unpack) { + return []; + } + } + $argsCount = count($args); + if ($argsCount < $minimumNumberOfArguments[$name]) { + return []; // caught by CallToFunctionParametersRule + } + + $formatArgType = $scope->getType($args[$formatArgumentPosition]->value); + $maxPlaceHoldersCount = null; + foreach ($formatArgType->getConstantStrings() as $formatString) { + $format = $formatString->getValue(); + $tempPlaceHoldersCount = $this->printfHelper->getPrintfPlaceholdersCount($format); + if ($maxPlaceHoldersCount === null) { + $maxPlaceHoldersCount = $tempPlaceHoldersCount; + } elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) { + $maxPlaceHoldersCount = $tempPlaceHoldersCount; + } + } + + if ($maxPlaceHoldersCount === null) { + return []; + } + + $formatArgsCount = 0; + if (isset($args[1])) { + $formatArgs = $scope->getType($args[1]->value); + + foreach ($formatArgs->getConstantArrays() as $constantArray) { + $size = $constantArray->getArraySize(); + if (!$size instanceof ConstantIntegerType) { + return []; + } + $formatArgsCount = $size->getValue(); + } + } + + if ($formatArgsCount !== $maxPlaceHoldersCount) { + return [ + RuleErrorBuilder::message(sprintf( + sprintf( + '%s, %s.', + $maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', + $formatArgsCount === 1 ? '%d value given' : '%d values given', + ), + $name, + $maxPlaceHoldersCount, + $formatArgsCount, + ))->identifier(sprintf('argument.%s', $name))->build(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php new file mode 100644 index 0000000000..6feec57a5f --- /dev/null +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -0,0 +1,63 @@ + + */ +class PrintfArrayParametersRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new PrintfArrayParametersRule(new PrintfHelper(new PhpVersion(PHP_VERSION_ID))); + } + + public function testFile(): void + { + $this->analyse([__DIR__ . '/data/vprintf.php'], [ + [ + 'Call to vsprintf contains 2 placeholders, 1 value given.', + 6, + ], + [ + 'Call to vsprintf contains 0 placeholders, 1 value given.', + 7, + ], + [ + 'Call to vsprintf contains 1 placeholder, 2 values given.', + 8, + ], + [ + 'Call to vsprintf contains 2 placeholders, 1 value given.', + 9, + ], + [ + 'Call to vsprintf contains 2 placeholders, 0 values given.', + 10, + ], + [ + 'Call to vsprintf contains 2 placeholders, 0 values given.', + 11, + ], + [ + 'Call to vsprintf contains 4 placeholders, 0 values given.', + 12, + ], + [ + 'Call to vsprintf contains 5 placeholders, 2 values given.', + 14, + ], + [ + 'Call to vsprintf contains 1 placeholder, 2 values given.', + 17, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php new file mode 100644 index 0000000000..1681f9a858 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -0,0 +1,17 @@ + Date: Tue, 4 Jun 2024 18:36:33 +0200 Subject: [PATCH 02/14] removed not relevant arg-unpack logic --- src/Rules/Functions/PrintfArrayParametersRule.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 018eea3e15..f503ce7a7e 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -51,11 +51,6 @@ public function processNode(Node $node, Scope $scope): array $formatArgumentPosition = $functionsArgumentPositions[$name]; $args = $node->getArgs(); - foreach ($args as $arg) { - if ($arg->unpack) { - return []; - } - } $argsCount = count($args); if ($argsCount < $minimumNumberOfArguments[$name]) { return []; // caught by CallToFunctionParametersRule From 620d12fd18bbb7abc2747ae76b2ecd9bf195b89d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 4 Jun 2024 18:43:47 +0200 Subject: [PATCH 03/14] added vprintf test --- .../Functions/PrintfArrayParametersRuleTest.php | 4 ++++ tests/PHPStan/Rules/Functions/data/vprintf.php | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php index 6feec57a5f..005287481c 100644 --- a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -57,6 +57,10 @@ public function testFile(): void 'Call to vsprintf contains 1 placeholder, 2 values given.', 17, ], + [ + 'Call to vprintf contains 2 placeholders, 1 value given.', + 30, + ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php index 1681f9a858..60f10ce0d6 100644 --- a/tests/PHPStan/Rules/Functions/data/vprintf.php +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -15,3 +15,16 @@ 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); // could be error + + +vprintf('%s', ['foo']); // ok +vprintf('%s %% %% %s', ['foo', 'bar']); // ok +vprintf('%s %s', ['foo']); // one parameter missing From 9132ece528531d1788a926398c4ba447b4e65d01 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 5 Jun 2024 08:52:54 +0200 Subject: [PATCH 04/14] use constants --- .../Functions/PrintfArrayParametersRule.php | 23 +++++++++---------- .../PHPStan/Rules/Functions/data/vprintf.php | 2 ++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index f503ce7a7e..35ea77c0fd 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -18,6 +18,14 @@ */ class PrintfArrayParametersRule implements Rule { + private const FORMAT_ARGUMENT_POSITIONS = [ + 'vprintf' => 0, + 'vsprintf' => 0, + ]; + private const MINIMUM_NUMBER_OF_ARGUMENTS = [ + 'vprintf' => 1, + 'vsprintf' => 1, + ]; public function __construct(private PrintfHelper $printfHelper) { @@ -34,25 +42,16 @@ public function processNode(Node $node, Scope $scope): array return []; } - $functionsArgumentPositions = [ - 'vprintf' => 0, - 'vsprintf' => 0, - ]; - $minimumNumberOfArguments = [ - 'vprintf' => 1, - 'vsprintf' => 1, - ]; - $name = strtolower((string) $node->name); - if (!array_key_exists($name, $functionsArgumentPositions)) { + if (!array_key_exists($name, self::FORMAT_ARGUMENT_POSITIONS)) { return []; } - $formatArgumentPosition = $functionsArgumentPositions[$name]; + $formatArgumentPosition = self::FORMAT_ARGUMENT_POSITIONS[$name]; $args = $node->getArgs(); $argsCount = count($args); - if ($argsCount < $minimumNumberOfArguments[$name]) { + if ($argsCount < self::MINIMUM_NUMBER_OF_ARGUMENTS[$name]) { return []; // caught by CallToFunctionParametersRule } diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php index 60f10ce0d6..61f6771209 100644 --- a/tests/PHPStan/Rules/Functions/data/vprintf.php +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -28,3 +28,5 @@ 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 From 48ae1064fc9e7f5779e6053cae0c5bc44e90b1a2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 5 Jun 2024 09:49:48 +0200 Subject: [PATCH 05/14] support integer range array-size --- .../Functions/PrintfArrayParametersRule.php | 42 +++++++++++- .../PrintfArrayParametersRuleTest.php | 28 +++++--- .../PHPStan/Rules/Functions/data/vprintf.php | 65 +++++++++++-------- 3 files changed, 95 insertions(+), 40 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 35ea77c0fd..64c3fba375 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -7,7 +7,9 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\IntegerRangeType; use function array_key_exists; use function count; use function sprintf; @@ -18,6 +20,7 @@ */ class PrintfArrayParametersRule implements Rule { + private const FORMAT_ARGUMENT_POSITIONS = [ 'vprintf' => 0, 'vsprintf' => 0, @@ -73,15 +76,50 @@ public function processNode(Node $node, Scope $scope): array $formatArgsCount = 0; if (isset($args[1])) { - $formatArgs = $scope->getType($args[1]->value); + $formatArgsType = $scope->getType($args[1]->value); - foreach ($formatArgs->getConstantArrays() as $constantArray) { + $size = null; + $constantArrays = $formatArgsType->getConstantArrays(); + foreach ($constantArrays as $constantArray) { $size = $constantArray->getArraySize(); + + if ($size instanceof IntegerRangeType) { + break; + } if (!$size instanceof ConstantIntegerType) { return []; } $formatArgsCount = $size->getValue(); } + + if ($constantArrays === []) { + $size = $formatArgsType->getArraySize(); + } + + if ($size instanceof IntegerRangeType) { + if ($size->getMin() !== null && $size->getMax() !== null) { + $values = $size->getMin() . '-' . $size->getMax(); + } elseif ($size->getMin() !== null) { + $values = $size->getMin() . ' or more'; + } elseif ($size->getMax() !== null) { + $values = $size->getMax() . ' or less'; + } else { + throw new ShouldNotHappenException(); + } + + return [ + RuleErrorBuilder::message(sprintf( + sprintf( + '%s, %s.', + $maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', + '%s values given', + ), + $name, + $maxPlaceHoldersCount, + $values, + ))->identifier(sprintf('argument.%s', $name))->build(), + ]; + } } if ($formatArgsCount !== $maxPlaceHoldersCount) { diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php index 005287481c..26975a4498 100644 --- a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -23,43 +23,51 @@ public function testFile(): void $this->analyse([__DIR__ . '/data/vprintf.php'], [ [ 'Call to vsprintf contains 2 placeholders, 1 value given.', - 6, + 10, ], [ 'Call to vsprintf contains 0 placeholders, 1 value given.', - 7, + 11, ], [ 'Call to vsprintf contains 1 placeholder, 2 values given.', - 8, + 12, ], [ 'Call to vsprintf contains 2 placeholders, 1 value given.', - 9, + 13, ], [ 'Call to vsprintf contains 2 placeholders, 0 values given.', - 10, + 14, ], [ 'Call to vsprintf contains 2 placeholders, 0 values given.', - 11, + 15, ], [ 'Call to vsprintf contains 4 placeholders, 0 values given.', - 12, + 16, ], [ 'Call to vsprintf contains 5 placeholders, 2 values given.', - 14, + 18, ], [ 'Call to vsprintf contains 1 placeholder, 2 values given.', - 17, + 21, + ], + [ + 'Call to vsprintf contains 1 placeholder, 1-2 values given.', + 29, ], [ 'Call to vprintf contains 2 placeholders, 1 value given.', - 30, + 34, + ], + [ + 'Call to vsprintf contains 0 placeholders, 1 or more values given.', + 39, ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php index 61f6771209..d977069cdb 100644 --- a/tests/PHPStan/Rules/Functions/data/vprintf.php +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -1,32 +1,41 @@ Date: Wed, 5 Jun 2024 10:38:09 +0200 Subject: [PATCH 06/14] Utilize ReflectionProvider --- .../Functions/PrintfArrayParametersRule.php | 31 +++++++++---------- .../PrintfArrayParametersRuleTest.php | 5 ++- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 64c3fba375..0b05b0fb24 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -5,15 +5,15 @@ 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 function array_key_exists; use function count; +use function in_array; use function sprintf; -use function strtolower; /** * @implements Rule @@ -21,16 +21,10 @@ class PrintfArrayParametersRule implements Rule { - private const FORMAT_ARGUMENT_POSITIONS = [ - 'vprintf' => 0, - 'vsprintf' => 0, - ]; - private const MINIMUM_NUMBER_OF_ARGUMENTS = [ - 'vprintf' => 1, - 'vsprintf' => 1, - ]; - - public function __construct(private PrintfHelper $printfHelper) + public function __construct( + private PrintfHelper $printfHelper, + private ReflectionProvider $reflectionProvider, + ) { } @@ -45,20 +39,23 @@ public function processNode(Node $node, Scope $scope): array return []; } - $name = strtolower((string) $node->name); - if (!array_key_exists($name, self::FORMAT_ARGUMENT_POSITIONS)) { + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { return []; } - $formatArgumentPosition = self::FORMAT_ARGUMENT_POSITIONS[$name]; + $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 < self::MINIMUM_NUMBER_OF_ARGUMENTS[$name]) { + if ($argsCount < 1) { return []; // caught by CallToFunctionParametersRule } - $formatArgType = $scope->getType($args[$formatArgumentPosition]->value); + $formatArgType = $scope->getType($args[0]->value); $maxPlaceHoldersCount = null; foreach ($formatArgType->getConstantStrings() as $formatString) { $format = $formatString->getValue(); diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php index 26975a4498..bb4abd7395 100644 --- a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -15,7 +15,10 @@ class PrintfArrayParametersRuleTest extends RuleTestCase protected function getRule(): Rule { - return new PrintfArrayParametersRule(new PrintfHelper(new PhpVersion(PHP_VERSION_ID))); + return new PrintfArrayParametersRule( + new PrintfHelper(new PhpVersion(PHP_VERSION_ID)), + $this->createReflectionProvider(), + ); } public function testFile(): void From 98a9327ebb5692c482aa1f372cb464cb23f8864a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 5 Jun 2024 10:42:56 +0200 Subject: [PATCH 07/14] PrintfArrayParametersRule only in bleeding edge --- conf/bleedingEdge.neon | 1 + conf/config.level0.neon | 5 ++++- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 4d8707567e..396a0acd13 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -56,5 +56,6 @@ parameters: checkParameterCastableToStringFunctions: true narrowPregMatches: true uselessReturnValue: true + printfArrayParameters: true stubFiles: - ../stubs/bleedingEdge/Rule.stub diff --git a/conf/config.level0.neon b/conf/config.level0.neon index c6e2c071fd..a3f4ca3bb4 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -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 @@ -79,7 +81,6 @@ rules: - PHPStan\Rules\Functions\InvalidLexicalVariablesInClosureUseRule - PHPStan\Rules\Functions\ParamAttributesRule - PHPStan\Rules\Functions\PrintfParametersRule - - PHPStan\Rules\Functions\PrintfArrayParametersRule - PHPStan\Rules\Functions\RedefinedParametersRule - PHPStan\Rules\Functions\ReturnNullsafeByRefRule - PHPStan\Rules\Ignore\IgnoreParseErrorRule @@ -300,5 +301,7 @@ services: - class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule + - + class: PHPStan\Rules\Functions\PrintfArrayParametersRule - class: PHPStan\Rules\Functions\PrintfHelper diff --git a/conf/config.neon b/conf/config.neon index 1e91656943..21aa21dfd3 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -91,6 +91,7 @@ parameters: checkParameterCastableToStringFunctions: false narrowPregMatches: false uselessReturnValue: false + printfArrayParameters: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 0cd1faa9b1..8459195463 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -86,6 +86,7 @@ parametersSchema: checkParameterCastableToStringFunctions: bool() narrowPregMatches: bool() uselessReturnValue: bool() + printfArrayParameters: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() From bbbdd2f4d5d11faa0265d474254944bf2e800b64 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 5 Jun 2024 11:41:20 +0200 Subject: [PATCH 08/14] implement array counting --- .../Functions/PrintfArrayParametersRule.php | 139 ++++++++++++------ .../PrintfArrayParametersRuleTest.php | 4 +- .../PHPStan/Rules/Functions/data/vprintf.php | 25 +++- 3 files changed, 116 insertions(+), 52 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 0b05b0fb24..f566bedcc3 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Functions; +use Hoa\Stream\Test\Unit\IStream\In; use PhpParser\Node; use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; @@ -11,6 +12,7 @@ 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 sprintf; @@ -56,80 +58,78 @@ public function processNode(Node $node, Scope $scope): array } $formatArgType = $scope->getType($args[0]->value); - $maxPlaceHoldersCount = null; + $placeHoldersCounts = []; foreach ($formatArgType->getConstantStrings() as $formatString) { $format = $formatString->getValue(); - $tempPlaceHoldersCount = $this->printfHelper->getPrintfPlaceholdersCount($format); - if ($maxPlaceHoldersCount === null) { - $maxPlaceHoldersCount = $tempPlaceHoldersCount; - } elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) { - $maxPlaceHoldersCount = $tempPlaceHoldersCount; - } + + $placeHoldersCounts[] = $this->printfHelper->getPrintfPlaceholdersCount($format); } - if ($maxPlaceHoldersCount === null) { + if ($placeHoldersCounts === []) { return []; } - $formatArgsCount = 0; + $minCount = min($placeHoldersCounts); + $maxCount = max($placeHoldersCounts); + if ($minCount === $maxCount) { + $placeHoldersCount = new ConstantIntegerType($minCount); + } else { + $placeHoldersCount = IntegerRangeType::fromInterval($minCount, $maxCount); + } + + + $formatArgsCounts = []; if (isset($args[1])) { $formatArgsType = $scope->getType($args[1]->value); - $size = null; $constantArrays = $formatArgsType->getConstantArrays(); foreach ($constantArrays as $constantArray) { - $size = $constantArray->getArraySize(); - - if ($size instanceof IntegerRangeType) { - break; - } - if (!$size instanceof ConstantIntegerType) { - return []; - } - $formatArgsCount = $size->getValue(); + $formatArgsCounts[] = $constantArray->getArraySize(); } if ($constantArrays === []) { - $size = $formatArgsType->getArraySize(); + $formatArgsCounts[] = $formatArgsType->getArraySize(); } + } - if ($size instanceof IntegerRangeType) { - if ($size->getMin() !== null && $size->getMax() !== null) { - $values = $size->getMin() . '-' . $size->getMax(); - } elseif ($size->getMin() !== null) { - $values = $size->getMin() . ' or more'; - } elseif ($size->getMax() !== null) { - $values = $size->getMax() . ' or less'; - } else { - throw new ShouldNotHappenException(); - } - - return [ - RuleErrorBuilder::message(sprintf( - sprintf( - '%s, %s.', - $maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', - '%s values given', - ), - $name, - $maxPlaceHoldersCount, - $values, - ))->identifier(sprintf('argument.%s', $name))->build(), - ]; - } + if ($formatArgsCounts === []) { + $formatArgsCount = new ConstantIntegerType(0); + } else { + $formatArgsCount = TypeCombinator::union(...$formatArgsCounts); } - if ($formatArgsCount !== $maxPlaceHoldersCount) { + if (!$this->placeholdersMatchesArgsCount($placeHoldersCount, $formatArgsCount)) { + + if ($placeHoldersCount instanceof IntegerRangeType) { + $placeholders = $this->getIntegerRangeAsString($placeHoldersCount); + $singlePlaceholder = false; + } elseif ($placeHoldersCount instanceof ConstantIntegerType) { + $placeholders = $placeHoldersCount->getValue(); + $singlePlaceholder = $placeholders === 1; + } else { + throw new ShouldNotHappenException(); + } + + if ($formatArgsCount instanceof IntegerRangeType) { + $values = $this->getIntegerRangeAsString($formatArgsCount); + $singleValue = false; + } elseif ($formatArgsCount instanceof ConstantIntegerType) { + $values = $formatArgsCount->getValue(); + $singleValue = $values === 1; + } else { + throw new ShouldNotHappenException(); + } + return [ RuleErrorBuilder::message(sprintf( sprintf( '%s, %s.', - $maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders', - $formatArgsCount === 1 ? '%d value given' : '%d values given', + $singlePlaceholder ? 'Call to %s contains %d placeholder' : 'Call to %s contains %s placeholders', + $singleValue ? '%d value given' : '%s values given', ), $name, - $maxPlaceHoldersCount, - $formatArgsCount, + $placeholders, + $values, ))->identifier(sprintf('argument.%s', $name))->build(), ]; } @@ -137,4 +137,45 @@ public function processNode(Node $node, Scope $scope): array return []; } + private function placeholdersMatchesArgsCount(IntegerRangeType|ConstantIntegerType $placeHoldersCount, IntegerRangeType|ConstantIntegerType $formatArgsCount): bool + { + if ($placeHoldersCount instanceof ConstantIntegerType && $formatArgsCount instanceof ConstantIntegerType) { + return $placeHoldersCount->getValue() === $formatArgsCount->getValue(); + } + + // Zero placeholders + array + if ($placeHoldersCount instanceof ConstantIntegerType + && $placeHoldersCount->getValue() === 0 + && $formatArgsCount instanceof IntegerRangeType + ) { + return true; + } + + if ($placeHoldersCount instanceof IntegerRangeType + && $formatArgsCount instanceof IntegerRangeType + && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($placeHoldersCount)->yes() + ) { + if ($formatArgsCount->getMin() !== null && $formatArgsCount->getMax() !== null) { + // constant array + return $placeHoldersCount->isSuperTypeOf($formatArgsCount)->yes(); + } + + 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'; + } else { + throw new ShouldNotHappenException(); + } + } + } diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php index bb4abd7395..c1aa4a41aa 100644 --- a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php @@ -69,8 +69,8 @@ public function testFile(): void 34, ], [ - 'Call to vsprintf contains 0 placeholders, 1 or more values given.', - 39, + 'Call to vsprintf contains 1-2 placeholders, 0 or more values given.', + 53, ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php index d977069cdb..7c30ff32d6 100644 --- a/tests/PHPStan/Rules/Functions/data/vprintf.php +++ b/tests/PHPStan/Rules/Functions/data/vprintf.php @@ -34,8 +34,31 @@ function doFoo($message, array $arr) { 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('no-placeholder', $arr); // at least one parameter over + vsprintf($format, $arr); // ok } + } From f5654f0c1e10529d4cc41b00ac6bb1dd5cd9e584 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 5 Jun 2024 11:42:34 +0200 Subject: [PATCH 09/14] cs --- .../Functions/PrintfArrayParametersRule.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index f566bedcc3..09a6bf00cc 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -2,7 +2,6 @@ namespace PHPStan\Rules\Functions; -use Hoa\Stream\Test\Unit\IStream\In; use PhpParser\Node; use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; @@ -12,9 +11,12 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\IntegerRangeType; +use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use function count; use function in_array; +use function max; +use function min; use function sprintf; /** @@ -77,7 +79,6 @@ public function processNode(Node $node, Scope $scope): array $placeHoldersCount = IntegerRangeType::fromInterval($minCount, $maxCount); } - $formatArgsCounts = []; if (isset($args[1])) { $formatArgsType = $scope->getType($args[1]->value); @@ -137,7 +138,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - private function placeholdersMatchesArgsCount(IntegerRangeType|ConstantIntegerType $placeHoldersCount, IntegerRangeType|ConstantIntegerType $formatArgsCount): bool + private function placeholdersMatchesArgsCount(Type $placeHoldersCount, Type $formatArgsCount): bool { if ($placeHoldersCount instanceof ConstantIntegerType && $formatArgsCount instanceof ConstantIntegerType) { return $placeHoldersCount->getValue() === $formatArgsCount->getValue(); @@ -153,29 +154,31 @@ private function placeholdersMatchesArgsCount(IntegerRangeType|ConstantIntegerTy if ($placeHoldersCount instanceof IntegerRangeType && $formatArgsCount instanceof IntegerRangeType - && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($placeHoldersCount)->yes() + && 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 { + 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'; - } else { - throw new ShouldNotHappenException(); } + + throw new ShouldNotHappenException(); } } From 4b4cdcdb21d89e7bd5bec0b8ea27abc056aa559c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 17 Jul 2024 11:30:03 +0200 Subject: [PATCH 10/14] Update config.level0.neon --- conf/config.level0.neon | 2 -- 1 file changed, 2 deletions(-) diff --git a/conf/config.level0.neon b/conf/config.level0.neon index a3f4ca3bb4..4f9da39886 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -303,5 +303,3 @@ services: - class: PHPStan\Rules\Functions\PrintfArrayParametersRule - - - class: PHPStan\Rules\Functions\PrintfHelper From e1088c5d22787f2ac0ec9fa719860b55d41205ee Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 17 Jul 2024 11:44:24 +0200 Subject: [PATCH 11/14] simplify per feedback --- src/Rules/Functions/PrintfArrayParametersRule.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index 09a6bf00cc..c4a20a21e7 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -11,7 +11,6 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\IntegerRangeType; -use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use function count; use function in_array; @@ -104,21 +103,17 @@ public function processNode(Node $node, Scope $scope): array if ($placeHoldersCount instanceof IntegerRangeType) { $placeholders = $this->getIntegerRangeAsString($placeHoldersCount); $singlePlaceholder = false; - } elseif ($placeHoldersCount instanceof ConstantIntegerType) { + } else { $placeholders = $placeHoldersCount->getValue(); $singlePlaceholder = $placeholders === 1; - } else { - throw new ShouldNotHappenException(); } if ($formatArgsCount instanceof IntegerRangeType) { $values = $this->getIntegerRangeAsString($formatArgsCount); $singleValue = false; - } elseif ($formatArgsCount instanceof ConstantIntegerType) { + } else { $values = $formatArgsCount->getValue(); $singleValue = $values === 1; - } else { - throw new ShouldNotHappenException(); } return [ From 64b180ea14d428dc2bb06b1b281402aa7285aa5d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 17 Jul 2024 11:51:13 +0200 Subject: [PATCH 12/14] use ConstantIntegerType|IntegerRangeType --- .../Functions/PrintfArrayParametersRule.php | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index c4a20a21e7..cdb7001e07 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -76,6 +76,10 @@ public function processNode(Node $node, Scope $scope): array $placeHoldersCount = new ConstantIntegerType($minCount); } else { $placeHoldersCount = IntegerRangeType::fromInterval($minCount, $maxCount); + + if (!$placeHoldersCount instanceof IntegerRangeType && !$placeHoldersCount instanceof ConstantIntegerType) { + return []; + } } $formatArgsCounts = []; @@ -96,6 +100,10 @@ public function processNode(Node $node, Scope $scope): array $formatArgsCount = new ConstantIntegerType(0); } else { $formatArgsCount = TypeCombinator::union(...$formatArgsCounts); + + if (!$formatArgsCount instanceof IntegerRangeType && !$formatArgsCount instanceof ConstantIntegerType) { + return []; + } } if (!$this->placeholdersMatchesArgsCount($placeHoldersCount, $formatArgsCount)) { @@ -133,18 +141,17 @@ public function processNode(Node $node, Scope $scope): array return []; } - private function placeholdersMatchesArgsCount(Type $placeHoldersCount, Type $formatArgsCount): bool + private function placeholdersMatchesArgsCount(ConstantIntegerType|IntegerRangeType $placeHoldersCount, ConstantIntegerType|IntegerRangeType $formatArgsCount): bool { - if ($placeHoldersCount instanceof ConstantIntegerType && $formatArgsCount instanceof ConstantIntegerType) { - return $placeHoldersCount->getValue() === $formatArgsCount->getValue(); - } + if ($placeHoldersCount instanceof ConstantIntegerType) { + if ($formatArgsCount instanceof ConstantIntegerType) { + return $placeHoldersCount->getValue() === $formatArgsCount->getValue(); + } - // Zero placeholders + array - if ($placeHoldersCount instanceof ConstantIntegerType - && $placeHoldersCount->getValue() === 0 - && $formatArgsCount instanceof IntegerRangeType - ) { - return true; + // Zero placeholders + array + if ($placeHoldersCount->getValue() === 0) { + return true; + } } if ($placeHoldersCount instanceof IntegerRangeType From ced1b10e9b7c6c6f0caf790ca6b3326a9b38a766 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 17 Jul 2024 17:32:51 +0200 Subject: [PATCH 13/14] simplify --- src/Rules/Functions/PrintfArrayParametersRule.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index cdb7001e07..bf02d204b9 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -16,6 +16,7 @@ use function in_array; use function max; use function min; +use function PHPStan\dumpType; use function sprintf; /** @@ -152,10 +153,12 @@ private function placeholdersMatchesArgsCount(ConstantIntegerType|IntegerRangeTy if ($placeHoldersCount->getValue() === 0) { return true; } + + return false; } - if ($placeHoldersCount instanceof IntegerRangeType - && $formatArgsCount instanceof IntegerRangeType + if ( + $formatArgsCount instanceof IntegerRangeType && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($placeHoldersCount)->yes() ) { if ($formatArgsCount->getMin() !== null && $formatArgsCount->getMax() !== null) { From b927dbe396c00e2336c2bb1ef977d8406382430a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 17 Jul 2024 17:33:30 +0200 Subject: [PATCH 14/14] Update PrintfArrayParametersRule.php --- src/Rules/Functions/PrintfArrayParametersRule.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php index bf02d204b9..fa190ddfa0 100644 --- a/src/Rules/Functions/PrintfArrayParametersRule.php +++ b/src/Rules/Functions/PrintfArrayParametersRule.php @@ -16,7 +16,6 @@ use function in_array; use function max; use function min; -use function PHPStan\dumpType; use function sprintf; /**