diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php index 4b770d9e8f5..f691685c094 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php @@ -22,6 +22,7 @@ use function array_fill; use function array_pop; use function count; +use function is_string; use function preg_match; use function sprintf; @@ -61,7 +62,24 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev return null; } + $has_splat_args = false; $node_type_provider = $statements_source->getNodeTypeProvider(); + foreach ($call_args as $call_arg) { + $type = $node_type_provider->getType($call_arg->value); + if ($type === null) { + continue; + } + + // if it's an array, used with splat operator + // we cannot validate it reliably below and report false positive errors + if ($type->isArray()) { + $has_splat_args = true; + break; + } + } + + // PHP 7 handling for formats that do not contain anything but placeholders + $is_falsable = true; foreach ($call_args as $index => $call_arg) { $type = $node_type_provider->getType($call_arg->value); if ($type === null && $index === 0 && $event->getFunctionId() === 'printf') { @@ -114,21 +132,17 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev '/%(?:\d+\$)?[-+]?(?:\d+|\*)(?:\.(?:\d+|\*))?[bcdouxXeEfFgGhHs]/', $type->getSingleStringLiteral()->value, ) === 1) { - if ($event->getFunctionId() === 'printf') { - return null; - } - - // the core stubs are wrong for these too, since these might be empty strings - // e.g. sprintf(\'%0.*s\', 0, "abc") - return Type::getString(); + return null; } - $args_count = count($call_args) - 1; - $dummy = array_fill(0, $args_count, ''); + // assume a random, high number for tests + $provided_placeholders_count = $has_splat_args === true ? 100 : count($call_args) - 1; + $dummy = array_fill(0, $provided_placeholders_count, ''); // check if we have enough/too many arguments and a valid format $initial_result = null; while (count($dummy) > -1) { + $result = null; try { // before PHP 8, an uncatchable Warning is thrown if too few arguments are passed // which is ignored and handled below instead @@ -166,7 +180,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev break 2; } catch (ArgumentCountError $error) { // PHP 8 - if (count($dummy) >= $args_count) { + if (count($dummy) === $provided_placeholders_count) { IssueBuffer::maybeAdd( new TooFewArguments( 'Too few arguments for ' . $event->getFunctionId(), @@ -178,49 +192,43 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev 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; + if ($result === false && count($dummy) === $provided_placeholders_count) { + // could be invalid format or too few arguments + // we cannot distinguish this in PHP 7 without additional checks + $max_dummy = array_fill(0, 100, ''); + $result = @sprintf($type->getSingleStringLiteral()->value, ...$max_dummy); + if ($result === false) { + // the format is invalid + IssueBuffer::maybeAdd( + new InvalidArgument( + 'Argument 1 of ' . $event->getFunctionId() . ' is invalid', + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); + } else { + IssueBuffer::maybeAdd( + new TooFewArguments( + 'Too few arguments for ' . $event->getFunctionId(), + $event->getCodeLocation(), + $event->getFunctionId(), + ), + $statements_source->getSuppressedIssues(), + ); } - IssueBuffer::maybeAdd( - new TooManyArguments( - 'Too many arguments for the number of placeholders in ' . $event->getFunctionId(), - $event->getCodeLocation(), - $event->getFunctionId(), - ), - $statements_source->getSuppressedIssues(), - ); - - break; + return Type::getFalse(); } - /** - * 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(); + // we can only validate the format and arg 1 when using splat + if ($has_splat_args === true) { + break; } - /** - * PHP 7 - * - * @psalm-suppress DocblockTypeContradiction - */ - if ($result === false && count($dummy) + 1 !== $args_count) { + if (is_string($result) && count($dummy) + 1 <= $provided_placeholders_count) { IssueBuffer::maybeAdd( new TooManyArguments( 'Too many arguments for the number of placeholders in ' . $event->getFunctionId(), @@ -233,7 +241,10 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev break; } - // for PHP 7, since it doesn't throw above + if (!is_string($result)) { + break; + } + // abort if it's empty, since we checked everything if (array_pop($dummy) === null) { break; @@ -246,20 +257,19 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev return null; } - /** - * PHP 7 can have false here - * - * @psalm-suppress RedundantConditionGivenDocblockType - */ if ($initial_result !== null && $initial_result !== false && $initial_result !== '') { return Type::getNonEmptyString(); } - // if we didn't have a valid result + // if we didn't have any valid result // the pattern is invalid or not yet supported by the return type provider if ($initial_result === null || $initial_result === false) { return null; } + + // the initial result is an empty string + // which means the format is valid and it depends on the args, whether it is non-empty-string or not + $is_falsable = false; } if ($index === 0 && $event->getFunctionId() === 'printf') { @@ -274,7 +284,6 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev // 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 this is already reported above and returned, so this cannot happen if ($type->isNonEmptyString() || $type->isInt() || $type->isFloat()) { return Type::getNonEmptyString(); } @@ -304,6 +313,10 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev return Type::getNonEmptyString(); } + if ($is_falsable === false) { + return Type::getString(); + } + return null; } } diff --git a/stubs/CoreGenericFunctions.phpstub b/stubs/CoreGenericFunctions.phpstub index 9277cb3041c..38cdb31b331 100644 --- a/stubs/CoreGenericFunctions.phpstub +++ b/stubs/CoreGenericFunctions.phpstub @@ -1279,9 +1279,8 @@ function preg_quote(string $str, ?string $delimiter = null) : string {} * @psalm-pure * * @param string|int|float $values - * @return ($format is non-empty-string - * ? ($values is non-empty-string|int|float ? non-empty-string : string) - * : string) + * @return (PHP_MAJOR_VERSION is 8 ? string : string|false) + * @psalm-ignore-falsable-return * * @psalm-flow ($format, $values) -> return */ @@ -1290,7 +1289,7 @@ function sprintf(string $format, ...$values) {} /** * @psalm-pure * @param array $values - * @return string|false + * @return (PHP_MAJOR_VERSION is 8 ? string : string|false) * @psalm-ignore-falsable-return * * @psalm-flow ($format, $values) -> return @@ -1309,7 +1308,8 @@ function wordwrap(string $string, int $width = 75, string $break = "\n", bool $c * @psalm-pure * * @param string|int|float $values - * @return int<0, max> + * @return (PHP_MAJOR_VERSION is 8 ? int<0, max> : int<0, max>|false) + * @psalm-ignore-falsable-return * * @psalm-taint-specialize * @psalm-flow ($format, $values) -> return @@ -1320,7 +1320,8 @@ function printf(string $format, ...$values) {} /** * @param array $values - * @return int<0, max> + * @return (PHP_MAJOR_VERSION is 8 ? int<0, max> : int<0, max>|false) + * @psalm-ignore-falsable-return * * @psalm-pure * @psalm-taint-specialize diff --git a/tests/ReturnTypeProvider/SprintfTest.php b/tests/ReturnTypeProvider/SprintfTest.php index 60e112f3eca..c98fba67597 100644 --- a/tests/ReturnTypeProvider/SprintfTest.php +++ b/tests/ReturnTypeProvider/SprintfTest.php @@ -130,6 +130,8 @@ public function providerValidCodeParse(): iterable 'assertions' => [ '$val===' => 'int<0, max>', ], + 'ignored_issues' => [], + 'php_version' => '8.0', ]; yield 'sprintfEmptyStringFormat' => [ @@ -163,6 +165,8 @@ public function providerValidCodeParse(): iterable 'assertions' => [ '$val===' => 'string', ], + 'ignored_issues' => [], + 'php_version' => '8.0', ]; yield 'sprintfComplexPlaceholderNotYetSupported2' => [ @@ -172,6 +176,8 @@ public function providerValidCodeParse(): iterable 'assertions' => [ '$val===' => 'string', ], + 'ignored_issues' => [], + 'php_version' => '8.0', ]; yield 'sprintfComplexPlaceholderNotYetSupported3' => [ @@ -181,6 +187,52 @@ public function providerValidCodeParse(): iterable 'assertions' => [ '$val===' => 'string', ], + 'ignored_issues' => [], + 'php_version' => '8.0', + ]; + + yield 'sprintfSplatUnpackingArray' => [ + 'code' => ' [ + '$val===' => 'string', + ], + 'ignored_issues' => [], + 'php_version' => '8.0', + ]; + + yield 'sprintfSplatUnpackingArrayNonEmpty' => [ + 'code' => ' [ + '$val===' => 'non-empty-string', + ], + ]; + + yield 'sprintfMultiplePlaceholdersNoErrorsIssue9941PHP7' => [ + 'code' => ' %d (%d)", 123, 456, 789); + ', + 'assertions' => [ + '$val===' => 'non-empty-string', + ], + 'ignored_issues' => [], + 'php_version' => '7.4', + ]; + + yield 'sprintfMultiplePlaceholdersNoErrorsIssue9941PHP8' => [ + 'code' => ' %d (%d)", 123, 456, 789); + ', + 'assertions' => [ + '$val===' => 'non-empty-string', + ], + 'ignored_issues' => [], + 'php_version' => '8.0', ]; }