Skip to content

Improve sprintf return type inference #3168

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

Closed
wants to merge 8 commits into from
Closed
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
5 changes: 5 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,11 @@ parameters:
count: 1
path: src/Type/Php/ReflectionMethodConstructorThrowTypeExtension.php

-
message: "#^Strict comparison using \\=\\=\\= between string and false will always evaluate to false\\.$#"
count: 1
path: src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantStringType is error\\-prone and deprecated\\. Use Type\\:\\:getConstantStrings\\(\\) instead\\.$#"
count: 1
Expand Down
171 changes: 111 additions & 60 deletions src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@

namespace PHPStan\Type\Php;

use ArgumentCountError;
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Internal\CombinationsHelper;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Reflection\InitializerExprTypeResolver;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
use PHPStan\Type\IntegerRangeType;
use PHPStan\Type\IntersectionType;
Expand All @@ -22,16 +23,17 @@
use function array_shift;
use function count;
use function in_array;
use function intval;
use function is_string;
use function preg_match;
use function sprintf;
use function substr;
use function str_contains;
use function vsprintf;

class SprintfFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
{

private const MAX_INTERPOLATION_RETRIES = 10;

public function isFunctionSupported(FunctionReflection $functionReflection): bool
{
return in_array($functionReflection->getName(), ['sprintf', 'vsprintf'], true);
Expand All @@ -49,74 +51,123 @@ public function getTypeFromFunctionCall(
}

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

if (count($formatType->getConstantStrings()) > 0) {
$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;
}

// constant string specifies a numbered argument that does not exist
if (!array_key_exists($checkArg, $args)) {
return null;
}
if (
$functionReflection->getName() === 'vsprintf'
&& count($args) === 2
&& $scope->getType($args[1]->value)->isIterableAtLeastOnce()->no()
) {
return $formatType;
}

// 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);
$formatStrings = $formatType->getConstantStrings();
if (count($formatStrings) === 0) {
return null;
}

if (!array_key_exists(2, $matches)) {
throw new ShouldNotHappenException();
}
$isNonEmpty = false;
$isNonFalsy = false;
$isNumeric = false;
foreach ($formatStrings as $constantString) {
$constantParts = $this->getFormatConstantParts($constantString->getValue());
if ($constantParts !== null) {
if ($constantParts->isNonFalsyString()->yes()) {
$isNonFalsy = true;
} elseif ($constantParts->isNonEmptyString()->yes()) {
$isNonEmpty = true;
}
}

if ($matches[2] === 's' && $checkArgType->isString()->yes()) {
$singlePlaceholderEarlyReturn = $checkArgType;
} elseif ($matches[2] !== 's') {
$singlePlaceholderEarlyReturn = new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
]);
}
// The printf format is %[argnum$][flags][width][.precision]specifier.
if (preg_match('/^(?<!%)%(?P<argnum>[0-9]*\$)?[0-9]*\.?[0-9]*(?P<specifier>[a-zA-Z])$/', $constantString->getValue(), $matches) !== 1) {
continue;
}

continue;
}
// invalid positional argument
if (array_key_exists('argnum', $matches) && $matches['argnum'] === '0$') {
return null;
}

$singlePlaceholderEarlyReturn = null;
if (array_key_exists('specifier', $matches) && str_contains('bdeEfFgGhHouxX', $matches['specifier'])) {
$isNumeric = true;
break;
}
}

$argTypes = [];
foreach ($args as $i => $arg) {
$argType = $scope->getType($arg->value);
$argTypes[] = $argType;

if ($singlePlaceholderEarlyReturn !== null) {
return $singlePlaceholderEarlyReturn;
if ($i === 0) { // skip format type
continue;
}

if ($functionReflection->getName() === 'vsprintf') {
if ($argType->isIterableAtLeastOnce()->yes()) {
$isNonEmpty = true;
}
continue;
}

if ($argType->toString()->isNonFalsyString()->yes()) {
$isNonFalsy = true;
} elseif ($argType->toString()->isNonEmptyString()->yes()) {
$isNonEmpty = true;
}
}

$accessories = [];
if ($isNumeric) {
$accessories[] = new AccessoryNumericStringType();
}
if ($isNonFalsy) {
$accessories[] = new AccessoryNonFalsyStringType();
} elseif ($isNonEmpty) {
$accessories[] = new AccessoryNonEmptyStringType();
}
$returnType = new StringType();
if (count($accessories) > 0) {
$accessories[] = new StringType();
$returnType = new IntersectionType($accessories);
}

return $this->getConstantType($argTypes, $returnType, $functionReflection, $scope);
}

private function getFormatConstantParts(string $format): ?ConstantStringType
{
$dummyValues = [];
for ($i = 0; $i < self::MAX_INTERPOLATION_RETRIES; $i++) {
$dummyValues[] = '';

if ($formatType->isNonFalsyString()->yes()) {
$returnType = new IntersectionType([
new StringType(),
new AccessoryNonFalsyStringType(),
]);
} elseif ($formatType->isNonEmptyString()->yes()) {
$returnType = new IntersectionType([
new StringType(),
new AccessoryNonEmptyStringType(),
]);
} else {
$returnType = new StringType();
try {
$formatted = @sprintf($format, ...$dummyValues);
if ($formatted === false) {
continue;
}
return new ConstantStringType($formatted);
} catch (ArgumentCountError) {
continue;
} catch (Throwable) {
return null;
}
}

return null;
}

/**
* @param Type[] $argTypes
*/
private function getConstantType(array $argTypes, Type $fallbackReturnType, FunctionReflection $functionReflection, Scope $scope): Type
{
$values = [];
$combinationsCount = 1;
foreach ($args as $arg) {
$argType = $scope->getType($arg->value);
foreach ($argTypes as $argType) {
$constantScalarValues = $argType->getConstantScalarValues();

if (count($constantScalarValues) === 0) {
Expand All @@ -128,23 +179,23 @@ public function getTypeFromFunctionCall(
}

if (count($constantScalarValues) === 0) {
return $returnType;
return $fallbackReturnType;
}

$values[] = $constantScalarValues;
$combinationsCount *= count($constantScalarValues);
}

if ($combinationsCount > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) {
return $returnType;
return $fallbackReturnType;
}

$combinations = CombinationsHelper::combinations($values);
$returnTypes = [];
foreach ($combinations as $combination) {
$format = array_shift($combination);
if (!is_string($format)) {
return $returnType;
return $fallbackReturnType;
}

try {
Expand All @@ -154,12 +205,12 @@ public function getTypeFromFunctionCall(
$returnTypes[] = $scope->getTypeFromValue(@vsprintf($format, $combination));
}
} catch (Throwable) {
return $returnType;
return $fallbackReturnType;
}
}

if (count($returnTypes) > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) {
return $returnType;
return $fallbackReturnType;
}

return TypeCombinator::union(...$returnTypes);
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-7387-php8.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php // lint >= 8.0

namespace Bug7387Php8;

use function PHPStan\Testing\assertType;

class HelloWorld
{
public function specifiers(int $i) {
// https://3v4l.org/fmVIg
assertType('non-falsy-string&numeric-string', sprintf('%14h', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14H', $i));
}

}
Loading