From b685360de293adc412264ae15e5fd7265d69a219 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 19 Jun 2024 08:02:52 +0200 Subject: [PATCH] Improve `sprintf` return type inference --- ...intfFunctionDynamicReturnTypeExtension.php | 103 +++++++++++++----- tests/PHPStan/Analyser/nsrt/bug-7387.php | 102 ++++++++++------- .../Analyser/nsrt/non-empty-string.php | 2 +- .../Analyser/nsrt/non-falsy-string.php | 2 +- ...rictComparisonOfDifferentTypesRuleTest.php | 6 + .../Rules/Comparison/data/bug-10493.php | 24 ++++ 6 files changed, 169 insertions(+), 70 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-10493.php diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index fabca8b7bcd..440d231595b 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -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 @@ -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('/^(?[0-9]*\$)?(?P[0-9])*\.?[0-9]*(?P[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) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-7387.php b/tests/PHPStan/Analyser/nsrt/bug-7387.php index 5f9a4e48792..8c6e458a62d 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7387.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7387.php @@ -8,40 +8,40 @@ 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)); } @@ -49,19 +49,20 @@ 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) { @@ -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 $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)); } } diff --git a/tests/PHPStan/Analyser/nsrt/non-empty-string.php b/tests/PHPStan/Analyser/nsrt/non-empty-string.php index 60a7b9ec93c..7834742e9f3 100644 --- a/tests/PHPStan/Analyser/nsrt/non-empty-string.php +++ b/tests/PHPStan/Analyser/nsrt/non-empty-string.php @@ -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')); diff --git a/tests/PHPStan/Analyser/nsrt/non-falsy-string.php b/tests/PHPStan/Analyser/nsrt/non-falsy-string.php index ebe223f8646..7478c6ee091 100644 --- a/tests/PHPStan/Analyser/nsrt/non-falsy-string.php +++ b/tests/PHPStan/Analyser/nsrt/non-falsy-string.php @@ -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)); diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 25f6c075337..e1c99901dee 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -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'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-10493.php b/tests/PHPStan/Rules/Comparison/data/bug-10493.php new file mode 100644 index 00000000000..d5586e5f8a5 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-10493.php @@ -0,0 +1,24 @@ +old, $this->new); + + if ($return === '') { + return null; + } + + return $return; + } +}