Skip to content

Simplify SprintfFunctionDynamicReturnTypeExtension #3188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 45 additions & 35 deletions src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,50 +49,56 @@ public function getTypeFromFunctionCall(
}

$formatType = $scope->getType($args[0]->value);
if (count($args) === 1) {
return $this->getConstantType($args, null, $functionReflection, $scope);
}

if (count($formatType->getConstantStrings()) > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the loop is the same as before, just the if statement was unwrapped

$singlePlaceholderEarlyReturn = null;
foreach ($formatType->getConstantStrings() as $constantString) {
// The printf format is %[argnum$][flags][width][.precision]
if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*([sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) {
if ($matches[1] !== '') {
// invalid positional argument
if ($matches[1] === '0$') {
return null;
}
$checkArg = intval(substr($matches[1], 0, -1));
} else {
$checkArg = 1;
}
$formatStrings = $formatType->getConstantStrings();
if (count($formatStrings) === 0) {
return null;
}

// constant string specifies a numbered argument that does not exist
if (!array_key_exists($checkArg, $args)) {
$singlePlaceholderEarlyReturn = null;
foreach ($formatType->getConstantStrings() as $constantString) {
// The printf format is %[argnum$][flags][width][.precision]
if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*([sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) {
if ($matches[1] !== '') {
// invalid positional argument
if ($matches[1] === '0$') {
return null;
}
$checkArg = intval(substr($matches[1], 0, -1));
} else {
$checkArg = 1;
}

// if the format string is just a placeholder and specified an argument
// of stringy type, then the return value will be of the same type
$checkArgType = $scope->getType($args[$checkArg]->value);

if ($matches[2] === 's' && $checkArgType->isString()->yes()) {
$singlePlaceholderEarlyReturn = $checkArgType;
} elseif ($matches[2] !== 's') {
$singlePlaceholderEarlyReturn = new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
]);
}
// constant string specifies a numbered argument that does not exist
if (!array_key_exists($checkArg, $args)) {
return null;
}

continue;
// if the format string is just a placeholder and specified an argument
// of stringy type, then the return value will be of the same type
$checkArgType = $scope->getType($args[$checkArg]->value);

if ($matches[2] === 's' && $checkArgType->isString()->yes()) {
$singlePlaceholderEarlyReturn = $checkArgType;
} elseif ($matches[2] !== 's') {
$singlePlaceholderEarlyReturn = new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
]);
}

$singlePlaceholderEarlyReturn = null;
break;
continue;
}

if ($singlePlaceholderEarlyReturn !== null) {
return $singlePlaceholderEarlyReturn;
}
$singlePlaceholderEarlyReturn = null;
break;
}

if ($singlePlaceholderEarlyReturn !== null) {
return $singlePlaceholderEarlyReturn;
}

if ($formatType->isNonFalsyString()->yes()) {
Expand All @@ -115,11 +121,15 @@ public function getTypeFromFunctionCall(
/**
* @param Arg[] $args
*/
private function getConstantType(array $args, Type $fallbackReturnType, FunctionReflection $functionReflection, Scope $scope): Type
private function getConstantType(array $args, ?Type $fallbackReturnType, FunctionReflection $functionReflection, Scope $scope): ?Type
{
$values = [];
$combinationsCount = 1;
foreach ($args as $arg) {
if ($arg->unpack) {
return $fallbackReturnType;
}

$argType = $scope->getType($arg->value);
$constantScalarValues = $argType->getConstantScalarValues();

Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/nsrt/non-empty-string.php
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,9 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni
assertType('non-empty-string', preg_quote($nonEmpty));

assertType('string', sprintf($s));
assertType('non-empty-string', sprintf($nonEmpty));
assertType('string', sprintf($nonEmpty));
assertType('string', vsprintf($s, []));
assertType('non-empty-string', vsprintf($nonEmpty, []));
assertType('string', vsprintf($nonEmpty, []));

assertType('0', strlen(''));
assertType('5', strlen('hallo'));
Expand Down
13 changes: 10 additions & 3 deletions tests/PHPStan/Analyser/nsrt/non-falsy-string.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function concat(string $s, string $nonFalsey, $numericS, $nonEmpty, $literalStri
* @param non-empty-array<non-falsy-string> $arrayOfNonFalsey
* @param non-empty-array $nonEmptyArray
*/
function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArray)
function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArray, array $arr)
{
assertType('string', implode($nonFalsey, []));
assertType('non-falsy-string', implode($nonFalsey, $nonEmptyArray));
Expand Down Expand Up @@ -104,8 +104,15 @@ function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArra

assertType('non-falsy-string', preg_quote($nonFalsey));

assertType('non-falsy-string', sprintf($nonFalsey));
assertType('non-falsy-string', vsprintf($nonFalsey, []));
assertType('string', sprintf($nonFalsey));
assertType("'foo'", sprintf('foo'));
assertType("string", sprintf(...$arr));
assertType("non-falsy-string", sprintf('%s', ...$arr)); // should be 'string'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the wrong non-falsy-string types will be fixed after we integrated the remaining parts of #3168

assertType('string', vsprintf($nonFalsey, []));
assertType('string', vsprintf($nonFalsey, []));
assertType("non-falsy-string", vsprintf('foo', [])); // should be 'foo'
assertType("non-falsy-string", vsprintf('%s', ...$arr)); // should be 'string'
assertType("string", vsprintf(...$arr));

assertType('int<1, max>', strlen($nonFalsey));

Expand Down
11 changes: 11 additions & 0 deletions tests/PHPStan/Analyser/nsrt/printf-errors-php8.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php // lint >= 8.0

namespace PrintFErrorsPhp8;

use function PHPStan\Testing\assertType;

function doFoo()
{
assertType("string", sprintf('%s')); // error
assertType("string", vsprintf('%s')); // error
}