Skip to content

Commit

Permalink
Improve sprintf return type inference
Browse files Browse the repository at this point in the history
  • Loading branch information
@staabm
staabm committed Jun 19, 2024
1 parent 1bce0b4 commit b685360
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 70 deletions.
103 changes: 73 additions & 30 deletions src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use function is_string;
use function preg_match;
use function sprintf;
use function str_contains;
use function vsprintf;

class SprintfFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
Expand All @@ -46,49 +47,91 @@ public function getTypeFromFunctionCall(
}

$formatType = $scope->getType($args[0]->value);
if (count($formatType->getConstantStrings()) > 0) {
$skip = false;
foreach ($formatType->getConstantStrings() as $constantString) {
// The printf format is %[argnum$][flags][width][.precision]
if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*[bdeEfFgGhHouxX]$/', $constantString->getValue(), $matches) === 1) {
// invalid positional argument
if (array_key_exists(1, $matches) && $matches[1] === '0$') {
return null;
}
if (count($args) === 1) {
return $formatType;
}

$isNonEmpty = false;
$isNonFalsy = false;
$isNumeric = false;

$formatStrings = $formatType->getConstantStrings();
if (count($formatStrings) === 0) {
return null;
}

continue;
foreach ($formatType->getConstantStrings() as $constantString) {
// The printf format is %[argnum$][flags][width][.precision]specifier.
if (preg_match('/^(?<!%)%(?P<argnum>[0-9]*\$)?(?P<width>[0-9])*\.?[0-9]*(?P<flags>[a-zA-Z])$/', $constantString->getValue(), $matches) !== 1) {
continue;
}

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

if (array_key_exists('width', $matches)) {
if ($matches['width'] > 1) {
$isNonFalsy = true;
} elseif ($matches['width'] > 0) {
$isNonEmpty = true;
}
}

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

$argTypes = [];
$positiveInt = IntegerRangeType::fromInterval(1, null);
foreach ($args as $i => $arg) {
$argType = $scope->getType($arg->value);
$argTypes[] = $argType;

if (!$skip) {
return new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
]);
if ($i === 0) { // skip format type
continue;
}

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

if ($argType->isNonFalsyString()->yes() || $positiveInt->isSuperTypeOf($argType)->yes()) {
$isNonFalsy = true;
} elseif (
$argType->isNonEmptyString()->yes()
|| $argType->isInteger()->yes()
|| $argType->isFloat()->yes()
|| $argType->isTrue()->yes()
) {
$isNonEmpty = true;
}
}

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();
$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);
}

$values = [];
$combinationsCount = 1;
foreach ($args as $arg) {
$argType = $scope->getType($arg->value);
foreach ($argTypes as $argType) {
$constantScalarValues = $argType->getConstantScalarValues();

if (count($constantScalarValues) === 0) {
Expand Down
102 changes: 64 additions & 38 deletions tests/PHPStan/Analyser/nsrt/bug-7387.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,60 +8,61 @@ class HelloWorld
{
public function inputTypes(int $i, float $f, string $s) {
// https://3v4l.org/iXaDX
assertType('numeric-string', sprintf('%.14F', $i));
assertType('numeric-string', sprintf('%.14F', $f));
assertType('non-empty-string&numeric-string', sprintf('%.14F', $i));
assertType('non-empty-string&numeric-string', sprintf('%.14F', $f));
assertType('numeric-string', sprintf('%.14F', $s));

assertType('numeric-string', sprintf('%1.14F', $i));
assertType('numeric-string', sprintf('%2.14F', $f));
assertType('numeric-string', sprintf('%3.14F', $s));
assertType('non-empty-string&numeric-string', sprintf('%1.14F', $i));
assertType('non-falsy-string&numeric-string', sprintf('%2.14F', $f));
assertType('non-falsy-string&numeric-string', sprintf('%3.14F', $s));

assertType('numeric-string', sprintf('%14F', $i));
assertType('numeric-string', sprintf('%14F', $f));
assertType('numeric-string', sprintf('%14F', $s));
assertType('non-falsy-string&numeric-string', sprintf('%14F', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14F', $f));
assertType('non-falsy-string&numeric-string', sprintf('%14F', $s));
}

public function specifiers(int $i) {
// https://3v4l.org/fmVIg
assertType('non-falsy-string', sprintf('%14s', $i));

assertType('numeric-string', sprintf('%d', $i));
assertType('non-empty-string&numeric-string', sprintf('%d', $i));

assertType('numeric-string', sprintf('%14b', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14b', $i));
assertType('non-falsy-string', sprintf('%14c', $i)); // binary string
assertType('numeric-string', sprintf('%14d', $i));
assertType('numeric-string', sprintf('%14e', $i));
assertType('numeric-string', sprintf('%14E', $i));
assertType('numeric-string', sprintf('%14f', $i));
assertType('numeric-string', sprintf('%14F', $i));
assertType('numeric-string', sprintf('%14g', $i));
assertType('numeric-string', sprintf('%14G', $i));
assertType('numeric-string', sprintf('%14h', $i));
assertType('numeric-string', sprintf('%14H', $i));
assertType('numeric-string', sprintf('%14o', $i));
assertType('numeric-string', sprintf('%14u', $i));
assertType('numeric-string', sprintf('%14x', $i));
assertType('numeric-string', sprintf('%14X', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14d', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14e', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14E', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14f', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14F', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14g', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14G', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14h', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14H', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14o', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14u', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14x', $i));
assertType('non-falsy-string&numeric-string', sprintf('%14X', $i));

}

public function positionalArgs($mixed, int $i, float $f, string $s) {
// https://3v4l.org/vVL0c
assertType('non-falsy-string', sprintf('%2$14s', $mixed, $i));

assertType('numeric-string', sprintf('%2$.14F', $mixed, $i));
assertType('numeric-string', sprintf('%2$.14F', $mixed, $f));
assertType('non-empty-string&numeric-string', sprintf('%2$.14F', $mixed, $i));
assertType('non-empty-string&numeric-string', sprintf('%2$.14F', $mixed, $f));
assertType('numeric-string', sprintf('%2$.14F', $mixed, $s));

assertType('numeric-string', sprintf('%2$1.14F', $mixed, $i));
assertType('numeric-string', sprintf('%2$2.14F', $mixed, $f));
assertType('numeric-string', sprintf('%2$3.14F', $mixed, $s));
assertType('non-empty-string&numeric-string', sprintf('%2$1.14F', $mixed, $i));
assertType('non-falsy-string&numeric-string', sprintf('%2$2.14F', $mixed, $f));
assertType('non-falsy-string&numeric-string', sprintf('%2$3.14F', $mixed, $s));

assertType('numeric-string', sprintf('%2$14F', $mixed, $i));
assertType('numeric-string', sprintf('%2$14F', $mixed, $f));
assertType('numeric-string', sprintf('%2$14F', $mixed, $s));
assertType('non-falsy-string&numeric-string', sprintf('%2$14F', $mixed, $i));
assertType('non-falsy-string&numeric-string', sprintf('%2$14F', $mixed, $f));
assertType('non-falsy-string&numeric-string', sprintf('%2$14F', $mixed, $s));

assertType('numeric-string', sprintf('%10$14F', $mixed, $s));
// XXX should be string because of invalid arguments count
assertType('non-falsy-string&numeric-string', sprintf('%10$14F', $mixed, $s));
}

public function invalidPositionalArgFormat($mixed, string $s) {
Expand All @@ -70,16 +71,41 @@ public function invalidPositionalArgFormat($mixed, string $s) {

public function escapedPercent(int $i) {
// https://3v4l.org/2m50L
assertType('non-falsy-string', sprintf("%%d", $i));
assertType('non-empty-string', sprintf("%%d", $i)); // could be non-falsey-string
}

public function vsprintf(array $array)
{
assertType('numeric-string', vsprintf("%4d", explode('-', '1988-8-1')));
assertType('numeric-string', vsprintf("%4d", $array));
assertType('numeric-string', vsprintf("%4d", ['123']));
assertType('non-falsy-string', vsprintf("%s", ['123']));
assertType('non-falsy-string&numeric-string', vsprintf("%4d", explode('-', '1988-8-1')));
assertType('non-falsy-string&numeric-string', vsprintf("%4d", $array));
assertType('non-falsy-string&numeric-string', vsprintf("%4d", ['123']));
assertType('non-empty-string', vsprintf("%s", ['123'])); // could be 'non-falsy-string'
// too many arguments.. php silently allows it
assertType('numeric-string', vsprintf("%4d", ['123', '456']));
assertType('non-falsy-string&numeric-string', vsprintf("%4d", ['123', '456']));
}

/**
* @param array<string> $arr
*/
public function bug11201($arr) {
assertType('string', sprintf("%s", implode(', ', array_map('intval', $arr))));
if (count($arr) > 0) {
assertType('non-falsy-string', sprintf("%s", implode(', ', array_map('intval', $arr))));
}
}

/**
* @param positive-int $positiveInt
*/
public function testNonStrings(bool $bool, int $int, float $float, $positiveInt) {
assertType('string', sprintf('%s', $bool));
if ($bool) {
assertType("'1'", sprintf('%s', $bool));
} else {
assertType("''", sprintf('%s', $bool));
}
assertType('non-empty-string', sprintf('%s', $int));
assertType('non-falsy-string', sprintf('%s', $positiveInt));
assertType('non-empty-string', sprintf('%s', $float));
}
}
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/non-empty-string.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni
assertType('string', sprintf($s));
assertType('non-empty-string', sprintf($nonEmpty));
assertType('string', vsprintf($s, []));
assertType('non-empty-string', vsprintf($nonEmpty, []));
assertType('string', vsprintf($nonEmpty, [])); // could be non-empty-string

assertType('0', strlen(''));
assertType('5', strlen('hallo'));
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/non-falsy-string.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ 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', vsprintf($nonFalsey, [])); // could be non-falsy-string

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

Expand Down
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1052,4 +1052,10 @@ public function testBug10697(): void
$this->analyse([__DIR__ . '/data/bug-10697.php'], []);
}

public function testBug10493(): void
{
$this->checkAlwaysTrueStrictComparison = true;
$this->analyse([__DIR__ . '/data/bug-10493.php'], []);
}

}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-10493.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php declare(strict_types = 1);

namespace Bug10493;

class Foo
{
public function __construct(
private readonly ?string $old,
private readonly ?string $new,
)
{
}

public function foo(): ?string
{
$return = sprintf('%s%s', $this->old, $this->new);

if ($return === '') {
return null;
}

return $return;
}
}

0 comments on commit b685360

Please sign in to comment.