Sprintf improve return param type validation

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<string|int|float>'],
 '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<string|int|float>'],
+'vsprintf' => ['string', 'format'=>'string', 'values'=>'array<string|int|float>'],
 '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'],

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<string|int|float>'],
     '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<string|int|float>'],
+    'vsprintf' => ['string', 'format'=>'string', 'values'=>'array<string|int|float>'],
     '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='=>''],

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;
     }
 }

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<string|int|float> $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<string|int|float> $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

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' => '<?php
+                $val = printf("%s", "hello");
+            ',
+            'assertions' => [
+                '$val===' => 'int<0, max>',
+            ],
+        ];
+    }
+
+    public function providerInvalidCodeParse(): iterable
+    {
+        return [
+            'sprintfOnlyFormat' => [
+                'code' => '<?php
+                    $x = sprintf("hello");
+                ',
+                'error_message' => 'TooFewArguments',
+            ],
+            'sprintfTooFewArguments' => [
+                'code' => '<?php
+                    $x = sprintf("%s hello %d", "a");
+                ',
+                'error_message' => 'TooFewArguments',
+            ],
+            'sprintfTooManyArguments' => [
+                'code' => '<?php
+                    $x = sprintf("%s hello", "a", "b");
+                ',
+                'error_message' => 'TooManyArguments',
+            ],
+            'sprintfInvalidFormat' => [
+                'code' => '<?php
+                    $x = sprintf(\'"%" hello\', "a");
+                ',
+                'error_message' => 'InvalidArgument',
+            ],
+            'printfOnlyFormat' => [
+                'code' => '<?php
+                    printf("hello");
+                ',
+                'error_message' => 'TooFewArguments',
+            ],
+            'printfTooFewArguments' => [
+                'code' => '<?php
+                    printf("%s hello %d", "a");
+                ',
+                'error_message' => 'TooFewArguments',
+            ],
+            'printfTooManyArguments' => [
+                'code' => '<?php
+                    printf("%s hello", "a", "b");
+                ',
+                'error_message' => 'TooManyArguments',
+            ],
+            'printfInvalidFormat' => [
+                'code' => '<?php
+                    printf(\'"%" hello\', "a");
+                ',
+                'error_message' => 'InvalidArgument',
+            ],
+        ];
     }
 }