mirror of
https://github.com/danog/psalm.git
synced 2024-11-30 04:39:00 +01:00
Merge pull request #10088 from kkmuffme/sprintf-error-reporting-more-correct-literal
make (s)printf error reporting more correct/literal
This commit is contained in:
orklah
GitHub
commit
96a61fcdc2
@ -4,6 +4,7 @@ namespace Psalm\Internal\Provider\ReturnTypeProvider;
|
||||
|
||||
use ArgumentCountError;
|
||||
use Psalm\Issue\InvalidArgument;
|
||||
use Psalm\Issue\RedundantFunctionCall;
|
||||
use Psalm\Issue\TooFewArguments;
|
||||
use Psalm\Issue\TooManyArguments;
|
||||
use Psalm\IssueBuffer;
|
||||
@ -25,6 +26,7 @@ use function count;
|
||||
use function is_string;
|
||||
use function preg_match;
|
||||
use function sprintf;
|
||||
use function strlen;
|
||||
|
||||
/**
|
||||
* @internal
|
||||
@ -47,6 +49,11 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface
|
||||
$statements_source = $event->getStatementsSource();
|
||||
$call_args = $event->getCallArgs();
|
||||
|
||||
// invalid - will already report an error for the params anyway
|
||||
if (count($call_args) < 1) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$has_splat_args = false;
|
||||
$node_type_provider = $statements_source->getNodeTypeProvider();
|
||||
foreach ($call_args as $call_arg) {
|
||||
@ -67,17 +74,29 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface
|
||||
// eventually this could be refined
|
||||
// to check if it's an array with literal string as first element for further checking
|
||||
if (count($call_args) === 1 && $has_splat_args === true) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new RedundantFunctionCall(
|
||||
'Using the splat operator is redundant, as v' . $event->getFunctionId()
|
||||
. ' without splat operator can be used instead of ' . $event->getFunctionId(),
|
||||
$event->getCodeLocation(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
// it makes no sense to use sprintf when there is only 1 arg (the format)
|
||||
// as it wouldn't have any placeholders
|
||||
if (count($call_args) === 1 && $event->getFunctionId() === 'sprintf') {
|
||||
// if it's a literal string, we can check it further though!
|
||||
$first_arg_type = $node_type_provider->getType($call_args[0]->value);
|
||||
if (count($call_args) === 1
|
||||
&& ($first_arg_type === null || !$first_arg_type->isSingleStringLiteral())) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new TooFewArguments(
|
||||
'Too few arguments for ' . $event->getFunctionId() . ', expecting at least 2 arguments',
|
||||
new RedundantFunctionCall(
|
||||
'Using ' . $event->getFunctionId()
|
||||
. ' with a single argument is redundant, since there are no placeholder params to be substituted',
|
||||
$event->getCodeLocation(),
|
||||
$event->getFunctionId(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
@ -89,7 +108,10 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface
|
||||
$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') {
|
||||
// printf only has the format validated above
|
||||
// don't change the return type
|
||||
break;
|
||||
}
|
||||
|
||||
@ -100,10 +122,9 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface
|
||||
if ($index === 0 && $type->isSingleStringLiteral()) {
|
||||
if ($type->getSingleStringLiteral()->value === '') {
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidArgument(
|
||||
'Argument 1 of ' . $event->getFunctionId() . ' must not be an empty string',
|
||||
new RedundantFunctionCall(
|
||||
'Calling ' . $event->getFunctionId() . ' with an empty first argument does nothing',
|
||||
$event->getCodeLocation(),
|
||||
$event->getFunctionId(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
@ -158,17 +179,48 @@ class SprintfReturnTypeProvider implements FunctionReturnTypeProviderInterface
|
||||
$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(),
|
||||
);
|
||||
if (count($call_args) > 1) {
|
||||
// we need to report this here too, since we return early without further validation
|
||||
// otherwise people who have suspended RedundantFunctionCall errors
|
||||
// will not get an error for this
|
||||
IssueBuffer::maybeAdd(
|
||||
new TooManyArguments(
|
||||
'Too many arguments for the number of placeholders in '
|
||||
. $event->getFunctionId(),
|
||||
$event->getCodeLocation(),
|
||||
$event->getFunctionId(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
}
|
||||
|
||||
return null;
|
||||
// the same error as above, but we have validated the pattern now
|
||||
if (count($call_args) === 1) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new RedundantFunctionCall(
|
||||
'Using ' . $event->getFunctionId()
|
||||
. ' with a single argument is redundant,'
|
||||
. ' since there are no placeholder params to be substituted',
|
||||
$event->getCodeLocation(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
} else {
|
||||
IssueBuffer::maybeAdd(
|
||||
new RedundantFunctionCall(
|
||||
'Argument 1 of ' . $event->getFunctionId()
|
||||
. ' does not contain any placeholders',
|
||||
$event->getCodeLocation(),
|
||||
),
|
||||
$statements_source->getSuppressedIssues(),
|
||||
);
|
||||
}
|
||||
|
||||
if ($event->getFunctionId() === 'printf') {
|
||||
return Type::getInt(false, strlen($type->getSingleStringLiteral()->value));
|
||||
}
|
||||
|
||||
return $type;
|
||||
}
|
||||
}
|
||||
} catch (ValueError $value_error) {
|
||||
|
||||
@ -142,7 +142,7 @@ class SprintfTest extends TestCase
|
||||
'$val===' => '\'\'',
|
||||
],
|
||||
'ignored_issues' => [
|
||||
'InvalidArgument',
|
||||
'RedundantFunctionCall',
|
||||
],
|
||||
];
|
||||
|
||||
@ -221,7 +221,9 @@ class SprintfTest extends TestCase
|
||||
'assertions' => [
|
||||
'$val===' => 'string',
|
||||
],
|
||||
'ignored_issues' => [],
|
||||
'ignored_issues' => [
|
||||
'RedundantFunctionCall',
|
||||
],
|
||||
'php_version' => '8.0',
|
||||
];
|
||||
|
||||
@ -251,11 +253,17 @@ class SprintfTest extends TestCase
|
||||
public function providerInvalidCodeParse(): iterable
|
||||
{
|
||||
return [
|
||||
'sprintfOnlyFormat' => [
|
||||
'sprintfOnlyFormatWithoutPlaceholders' => [
|
||||
'code' => '<?php
|
||||
$x = sprintf("hello");
|
||||
',
|
||||
'error_message' => 'TooFewArguments',
|
||||
'error_message' => 'RedundantFunctionCall',
|
||||
],
|
||||
'printfOnlyFormatWithoutPlaceholders' => [
|
||||
'code' => '<?php
|
||||
$x = sprintf("hello");
|
||||
',
|
||||
'error_message' => 'RedundantFunctionCall',
|
||||
],
|
||||
'sprintfTooFewArguments' => [
|
||||
'code' => '<?php
|
||||
@ -297,13 +305,16 @@ class SprintfTest extends TestCase
|
||||
'code' => '<?php
|
||||
$x = sprintf("", "abc");
|
||||
',
|
||||
'error_message' => 'InvalidArgument',
|
||||
'error_message' => 'RedundantFunctionCall',
|
||||
],
|
||||
'sprintfFormatWithoutPlaceholders' => [
|
||||
'code' => '<?php
|
||||
$x = sprintf("hello", "abc");
|
||||
',
|
||||
'error_message' => 'InvalidArgument',
|
||||
'error_message' => 'TooManyArguments',
|
||||
'ignored_issues' => [
|
||||
'RedundantFunctionCall',
|
||||
],
|
||||
],
|
||||
'sprintfPaddedComplexEmptyStringFormat' => [
|
||||
'code' => '<?php
|
||||
@ -311,6 +322,13 @@ class SprintfTest extends TestCase
|
||||
',
|
||||
'error_message' => 'InvalidArgument',
|
||||
],
|
||||
'printfVariableFormat' => [
|
||||
'code' => '<?php
|
||||
/** @var string $bar */
|
||||
printf($bar);
|
||||
',
|
||||
'error_message' => 'RedundantFunctionCall',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user