diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 4c934840f12..f7ce07fa273 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -9404,7 +9404,7 @@ 'print' => ['int', 'arg'=>'string'], 'print_r' => ['string', 'value'=>'mixed'], 'print_r\'1' => ['true', 'value'=>'mixed', 'return='=>'bool'], -'printf' => ['int', 'format'=>'string', '...values='=>'string|int|float'], +'printf' => ['int<0, max>', 'format'=>'string', '...values='=>'string|int|float'], 'proc_close' => ['int', 'process'=>'resource'], 'proc_get_status' => ['array{command: string, pid: int, running: bool, signaled: bool, stopped: bool, exitcode: int, termsig: int, stopsig: int}', 'process'=>'resource'], 'proc_nice' => ['bool', 'priority'=>'int'], @@ -14365,7 +14365,7 @@ 'VarnishStat::getSnapshot' => ['array'], 'version_compare' => ['bool', 'version1'=>'string', 'version2'=>'string', 'operator'=>'\'<\'|\'lt\'|\'<=\'|\'le\'|\'>\'|\'gt\'|\'>=\'|\'ge\'|\'==\'|\'=\'|\'eq\'|\'!=\'|\'<>\'|\'ne\''], 'version_compare\'1' => ['int', 'version1'=>'string', 'version2'=>'string'], -'vfprintf' => ['int', 'stream'=>'resource', 'format'=>'string', 'values'=>'array'], +'vfprintf' => ['int<0, max>', 'stream'=>'resource', 'format'=>'string', 'values'=>'array'], 'virtual' => ['bool', 'uri'=>'string'], 'vpopmail_add_alias_domain' => ['bool', 'domain'=>'string', 'aliasdomain'=>'string'], 'vpopmail_add_alias_domain_ex' => ['bool', 'olddomain'=>'string', 'newdomain'=>'string'], @@ -14384,8 +14384,8 @@ 'vpopmail_error' => ['string'], 'vpopmail_passwd' => ['bool', 'user'=>'string', 'domain'=>'string', 'password'=>'string', 'apop='=>'bool'], 'vpopmail_set_user_quota' => ['bool', 'user'=>'string', 'domain'=>'string', 'quota'=>'string'], -'vprintf' => ['int', 'format'=>'string', 'values'=>'array'], -'vsprintf' => ['string', 'format'=>'string', 'values'=>'array'], +'vprintf' => ['int<0, max>', 'format'=>'string', 'values'=>'array'], +'vsprintf' => ['string', 'format'=>'string', 'values'=>'array'], 'Vtiful\Kernel\Chart::__construct' => ['void', 'handle'=>'resource', 'type'=>'int'], 'Vtiful\Kernel\Chart::axisNameX' => ['Vtiful\Kernel\Chart', 'name'=>'string'], 'Vtiful\Kernel\Chart::axisNameY' => ['Vtiful\Kernel\Chart', 'name'=>'string'], diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php index ba35d9f0e7e..5df7b6c4da4 100644 --- a/dictionaries/CallMap_historical.php +++ b/dictionaries/CallMap_historical.php @@ -13555,7 +13555,7 @@ 'print' => ['int', 'arg'=>'string'], 'print_r' => ['string', 'value'=>'mixed'], 'print_r\'1' => ['true', 'value'=>'mixed', 'return='=>'bool'], - 'printf' => ['int', 'format'=>'string', '...values='=>'string|int|float'], + 'printf' => ['int<0, max>', 'format'=>'string', '...values='=>'string|int|float'], 'proc_close' => ['int', 'process'=>'resource'], 'proc_get_status' => ['array{command: string, pid: int, running: bool, signaled: bool, stopped: bool, exitcode: int, termsig: int, stopsig: int}|false', 'process'=>'resource'], 'proc_nice' => ['bool', 'priority'=>'int'], @@ -15318,7 +15318,7 @@ 'variant_xor' => ['mixed', 'left'=>'mixed', 'right'=>'mixed'], 'version_compare' => ['bool', 'version1'=>'string', 'version2'=>'string', 'operator'=>'\'<\'|\'lt\'|\'<=\'|\'le\'|\'>\'|\'gt\'|\'>=\'|\'ge\'|\'==\'|\'=\'|\'eq\'|\'!=\'|\'<>\'|\'ne\''], 'version_compare\'1' => ['int', 'version1'=>'string', 'version2'=>'string'], - 'vfprintf' => ['int', 'stream'=>'resource', 'format'=>'string', 'values'=>'array'], + 'vfprintf' => ['int<0, max>', 'stream'=>'resource', 'format'=>'string', 'values'=>'array'], 'virtual' => ['bool', 'uri'=>'string'], 'vpopmail_add_alias_domain' => ['bool', 'domain'=>'string', 'aliasdomain'=>'string'], 'vpopmail_add_alias_domain_ex' => ['bool', 'olddomain'=>'string', 'newdomain'=>'string'], @@ -15337,8 +15337,8 @@ 'vpopmail_error' => ['string'], 'vpopmail_passwd' => ['bool', 'user'=>'string', 'domain'=>'string', 'password'=>'string', 'apop='=>'bool'], 'vpopmail_set_user_quota' => ['bool', 'user'=>'string', 'domain'=>'string', 'quota'=>'string'], - 'vprintf' => ['int', 'format'=>'string', 'values'=>'array'], - 'vsprintf' => ['string', 'format'=>'string', 'values'=>'array'], + 'vprintf' => ['int<0, max>', 'format'=>'string', 'values'=>'array'], + 'vsprintf' => ['string', 'format'=>'string', 'values'=>'array'], 'w32api_deftype' => ['bool', 'typename'=>'string', 'member1_type'=>'string', 'member1_name'=>'string', '...args='=>'string'], 'w32api_init_dtype' => ['resource', 'typename'=>'string', 'value'=>'', '...args='=>''], 'w32api_invoke_function' => ['', 'funcname'=>'string', 'argument'=>'', '...args='=>''], diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php index 5bdf54dae6e..7aa6e9e24ec 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php @@ -2,6 +2,11 @@ namespace Psalm\Internal\Provider\ReturnTypeProvider; +use ArgumentCountError; +use Psalm\Issue\InvalidArgument; +use Psalm\Issue\TooFewArguments; +use Psalm\Issue\TooManyArguments; +use Psalm\IssueBuffer; use Psalm\Plugin\EventHandler\Event\FunctionReturnTypeProviderEvent; use Psalm\Plugin\EventHandler\FunctionReturnTypeProviderInterface; use Psalm\Type; @@ -12,8 +17,10 @@ use Psalm\Type\Atomic\TNonEmptyString; use Psalm\Type\Atomic\TNumeric; use Psalm\Type\Union; +use ValueError; use function array_fill; +use function array_pop; use function count; use function sprintf; @@ -28,40 +35,175 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface public static function getFunctionIds(): array { return [ + 'printf', 'sprintf', ]; } - public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): Union + public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): ?Union { $statements_source = $event->getStatementsSource(); - $node_type_provider = $statements_source->getNodeTypeProvider(); - $call_args = $event->getCallArgs(); + + // it makes no sense to use sprintf/printf when there is only 1 arg (the format) + // as it wouldn't have any placeholders + if (count($call_args) === 1) { + IssueBuffer::maybeAdd( + new TooFewArguments( + 'Too few arguments for ' . $event->getFunctionId() . ', expecting at least 2 arguments', + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + return null; + } + + $node_type_provider = $statements_source->getNodeTypeProvider(); foreach ($call_args as $index => $call_arg) { $type = $node_type_provider->getType($call_arg->value); + if ($type === null && $event->getFunctionId() === 'printf') { + break; + } + if ($type === null) { continue; } if ($index === 0 && $type->isSingleStringLiteral()) { - // use empty string dummies to check if the format itself produces a non-empty return value - // faster than validating the pattern and checking all args separately - $dummy = array_fill(0, count($call_args) - 1, ''); - if (sprintf($type->getSingleStringLiteral()->value, ...$dummy) !== '') { + $args_count = count($call_args) - 1; + $dummy = array_fill(0, $args_count, ''); + + // check if we have enough/too many arguments and a valid format + $initial_result = null; + while (count($dummy) > -1) { + try { + // before PHP 8, an uncatchable Warning is thrown if too few arguments are passed + // which is ignored and handled below instead + $result = @sprintf($type->getSingleStringLiteral()->value, ...$dummy); + if ($initial_result === null) { + $initial_result = $result; + } + } catch (ValueError $value_error) { + // PHP 8 + // the format is invalid + IssueBuffer::maybeAdd( + new InvalidArgument( + 'Argument 1 of ' . $event->getFunctionId() . ' is invalid - ' + . $value_error->getMessage(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + break 2; + } catch (ArgumentCountError $error) { + // PHP 8 + if (count($dummy) >= $args_count) { + IssueBuffer::maybeAdd( + new TooFewArguments( + 'Too few arguments for ' . $event->getFunctionId(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + break 2; + } + + // we are in the next iteration, so we have 1 placeholder less here + // otherwise we would have reported an error above already + if (count($dummy) + 1 === $args_count) { + break; + } + + IssueBuffer::maybeAdd( + new TooManyArguments( + 'Too many arguments for the number of placeholders in ' . $event->getFunctionId(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + break; + } + + /** + * PHP 7 + * + * @psalm-suppress DocblockTypeContradiction + */ + if ($result === false && count($dummy) >= $args_count) { + IssueBuffer::maybeAdd( + new TooFewArguments( + 'Too few arguments for ' . $event->getFunctionId(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + return Type::getFalse(); + } + + /** + * PHP 7 + * + * @psalm-suppress DocblockTypeContradiction + */ + if ($result === false && count($dummy) + 1 !== $args_count) { + IssueBuffer::maybeAdd( + new TooManyArguments( + 'Too many arguments for the number of placeholders in ' . $event->getFunctionId(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + + break; + } + + // for PHP 7, since it doesn't throw above + // abort if it's empty, since we checked everything + if (array_pop($dummy) === null) { + break; + } + } + + if ($event->getFunctionId() === 'printf') { + // printf only has the format validated above + // don't change the return type + return null; + } + + /** + * PHP 7 can have false here + * + * @psalm-suppress RedundantConditionGivenDocblockType + */ + if ($initial_result !== null && $initial_result !== false && $initial_result !== '') { return Type::getNonEmptyString(); } } + if ($index === 0 && $event->getFunctionId() === 'printf') { + // printf only has the format validated above + // don't change the return type + break; + } + if ($index === 0) { continue; } // if the function has more arguments than the pattern has placeholders, this could be a false positive // if the param is not used in the pattern - // however we would need to analyze the format arg to check that - // can be done eventually to also implement https://github.com/vimeo/psalm/issues/9818 - // and https://github.com/vimeo/psalm/issues/9817 + // however this is already reported above and returned, so this cannot happen if ($type->isNonEmptyString() || $type->isInt() || $type->isFloat()) { return Type::getNonEmptyString(); } @@ -91,6 +233,6 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev return Type::getNonEmptyString(); } - return Type::getString(); + return null; } } diff --git a/stubs/CoreGenericFunctions.phpstub b/stubs/CoreGenericFunctions.phpstub index 30ac82c3c1f..9277cb3041c 100644 --- a/stubs/CoreGenericFunctions.phpstub +++ b/stubs/CoreGenericFunctions.phpstub @@ -1285,10 +1285,11 @@ function preg_quote(string $str, ?string $delimiter = null) : string {} * * @psalm-flow ($format, $values) -> return */ -function sprintf(string $format, ...$values) : string {} +function sprintf(string $format, ...$values) {} /** * @psalm-pure + * @param array $values * @return string|false * @psalm-ignore-falsable-return * @@ -1308,20 +1309,25 @@ function wordwrap(string $string, int $width = 75, string $break = "\n", bool $c * @psalm-pure * * @param string|int|float $values + * @return int<0, max> * * @psalm-taint-specialize * @psalm-flow ($format, $values) -> return * @psalm-taint-sink html $format * @psalm-taint-sink html $values */ -function printf(string $format, ...$values) : string {} +function printf(string $format, ...$values) {} /** + * @param array $values + * @return int<0, max> + * + * @psalm-pure * @psalm-taint-specialize * @psalm-taint-sink html $format * @psalm-taint-sink html $values */ -function vprintf(string $format, array $values) : int {} +function vprintf(string $format, array $values) {} /** * @psalm-pure diff --git a/tests/ReturnTypeProvider/SprintfTest.php b/tests/ReturnTypeProvider/SprintfTest.php index b0759da0da4..d66bd664aed 100644 --- a/tests/ReturnTypeProvider/SprintfTest.php +++ b/tests/ReturnTypeProvider/SprintfTest.php @@ -3,10 +3,12 @@ namespace Psalm\Tests\ReturnTypeProvider; use Psalm\Tests\TestCase; +use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; class SprintfTest extends TestCase { + use InvalidCodeAnalysisTestTrait; use ValidCodeAnalysisTestTrait; public function providerValidCodeParse(): iterable @@ -120,5 +122,68 @@ public function providerValidCodeParse(): iterable '$val===' => 'string', ], ]; + + yield 'printfSimple' => [ + 'code' => ' [ + '$val===' => 'int<0, max>', + ], + ]; + } + + public function providerInvalidCodeParse(): iterable + { + return [ + 'sprintfOnlyFormat' => [ + 'code' => ' 'TooFewArguments', + ], + 'sprintfTooFewArguments' => [ + 'code' => ' 'TooFewArguments', + ], + 'sprintfTooManyArguments' => [ + 'code' => ' 'TooManyArguments', + ], + 'sprintfInvalidFormat' => [ + 'code' => ' 'InvalidArgument', + ], + 'printfOnlyFormat' => [ + 'code' => ' 'TooFewArguments', + ], + 'printfTooFewArguments' => [ + 'code' => ' 'TooFewArguments', + ], + 'printfTooManyArguments' => [ + 'code' => ' 'TooManyArguments', + ], + 'printfInvalidFormat' => [ + 'code' => ' 'InvalidArgument', + ], + ]; } }