Skip to content

Commit

Permalink
Merge pull request #9904 from kkmuffme/sprintf-additional-validations…
Browse files Browse the repository at this point in the history 
…-and-bugfix

Sprintf additional validations and bugfix
  • Loading branch information
@orklah
orklah authored Jun 12, 2023
2 parents 68559b1 + 8a8aeb6 commit 2c50745
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 1 deletion.
73 changes: 72 additions & 1 deletion src/Psalm/Internal/Provider/ReturnTypeProvider/SprintfReturnTypeProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use function array_fill;
use function array_pop;
use function count;
use function preg_match;
use function sprintf;

/**
Expand Down Expand Up @@ -63,7 +64,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
$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') {
if ($type === null && $index === 0 && $event->getFunctionId() === 'printf') {
break;
}

Expand All @@ -72,6 +73,56 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
}

if ($index === 0 && $type->isSingleStringLiteral()) {
if ($type->getSingleStringLiteral()->value === '') {
IssueBuffer::maybeAdd(
new InvalidArgument(
'Argument 1 of ' . $event->getFunctionId() . ' must not be an empty string',
$event->getCodeLocation(),
$event->getFunctionId(),
),
$statements_source->getSuppressedIssues(),
);

if ($event->getFunctionId() === 'printf') {
return Type::getInt(false, 0);
}

return Type::getString('');
}

// there are probably additional formats that return an empty string, this is just a starting point
if (preg_match('/^%(?:\d+\$)?[-+]?0(?:\.0)?s$/', $type->getSingleStringLiteral()->value) === 1) {
IssueBuffer::maybeAdd(
new InvalidArgument(
'The pattern of argument 1 of ' . $event->getFunctionId()
. ' will always return an empty string',
$event->getCodeLocation(),
$event->getFunctionId(),
),
$statements_source->getSuppressedIssues(),
);

if ($event->getFunctionId() === 'printf') {
return Type::getInt(false, 0);
}

return Type::getString('');
}

// these placeholders are too complex to handle for now
if (preg_match(
'/%(?:\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();
}

$args_count = count($call_args) - 1;
$dummy = array_fill(0, $args_count, '');

Expand All @@ -84,6 +135,20 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
$result = @sprintf($type->getSingleStringLiteral()->value, ...$dummy);
if ($initial_result === null) {
$initial_result = $result;

if ($result === $type->getSingleStringLiteral()->value) {
IssueBuffer::maybeAdd(
new InvalidArgument(
'Argument 1 of ' . $event->getFunctionId()
. ' does not contain any placeholders',
$event->getCodeLocation(),
$event->getFunctionId(),
),
$statements_source->getSuppressedIssues(),
);

return null;
}
}
} catch (ValueError $value_error) {
// PHP 8
Expand Down Expand Up @@ -189,6 +254,12 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
if ($initial_result !== null && $initial_result !== false && $initial_result !== '') {
return Type::getNonEmptyString();
}

// if we didn't have a valid result
// the pattern is invalid or not yet supported by the return type provider
if ($initial_result === null || $initial_result === false) {
return null;
}
}

if ($index === 0 && $event->getFunctionId() === 'printf') {
Expand Down
69 changes: 69 additions & 0 deletions tests/ReturnTypeProvider/SprintfTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,57 @@ public function providerValidCodeParse(): iterable
'$val===' => 'int<0, max>',
],
];

yield 'sprintfEmptyStringFormat' => [
'code' => '<?php
$val = sprintf("", "abc");
',
'assertions' => [
'$val===' => '\'\'',
],
'ignored_issues' => [
'InvalidArgument',
],
];

yield 'sprintfPaddedEmptyStringFormat' => [
'code' => '<?php
$val = sprintf("%0.0s", "abc");
',
'assertions' => [
'$val===' => '\'\'',
],
'ignored_issues' => [
'InvalidArgument',
],
];

yield 'sprintfComplexPlaceholderNotYetSupported1' => [
'code' => '<?php
$val = sprintf(\'%*.0s\', 0, "abc");
',
'assertions' => [
'$val===' => 'string',
],
];

yield 'sprintfComplexPlaceholderNotYetSupported2' => [
'code' => '<?php
$val = sprintf(\'%0.*s\', 0, "abc");
',
'assertions' => [
'$val===' => 'string',
],
];

yield 'sprintfComplexPlaceholderNotYetSupported3' => [
'code' => '<?php
$val = sprintf(\'%*.*s\', 0, 0, "abc");
',
'assertions' => [
'$val===' => 'string',
],
];
}

public function providerInvalidCodeParse(): iterable
Expand Down Expand Up @@ -184,6 +235,24 @@ public function providerInvalidCodeParse(): iterable
',
'error_message' => 'InvalidArgument',
],
'sprintfEmptyFormat' => [
'code' => '<?php
$x = sprintf("", "abc");
',
'error_message' => 'InvalidArgument',
],
'sprintfFormatWithoutPlaceholders' => [
'code' => '<?php
$x = sprintf("hello", "abc");
',
'error_message' => 'InvalidArgument',
],
'sprintfPaddedComplexEmptyStringFormat' => [
'code' => '<?php
$x = sprintf("%1$+0.0s", "abc");
',
'error_message' => 'InvalidArgument',
],
];
}
}

0 comments on commit 2c50745

Please sign in to comment.