From 9b5a686c778ee26ba13d0b98cff48ad0be2f74aa Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 09:26:04 +0200 Subject: [PATCH 1/7] Improved sprintf() inference --- .../SprintfFunctionDynamicReturnTypeExtension.php | 4 ++-- tests/PHPStan/Analyser/nsrt/bug-7387.php | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index 3f37564e95..54c4cd539e 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -81,8 +81,8 @@ public function getTypeFromFunctionCall( // 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; + if ($matches[2] === 's' && $checkArgType->isScalar()->yes()) { + $singlePlaceholderEarlyReturn = $checkArgType->toString(); } elseif ($matches[2] !== 's') { $singlePlaceholderEarlyReturn = new IntersectionType([ new StringType(), diff --git a/tests/PHPStan/Analyser/nsrt/bug-7387.php b/tests/PHPStan/Analyser/nsrt/bug-7387.php index 4ab6ef987c..d5a380ca99 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7387.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7387.php @@ -23,7 +23,7 @@ public function inputTypes(int $i, float $f, string $s) { public function specifiers(int $i) { // https://3v4l.org/fmVIg - assertType('non-falsy-string', sprintf('%14s', $i)); + assertType('numeric-string', sprintf('%14s', $i)); assertType('numeric-string', sprintf('%d', $i)); @@ -45,9 +45,15 @@ public function specifiers(int $i) { } - public function positionalArgs($mixed, int $i, float $f, string $s) { + /** + * @param positive-int $posInt + * @param negative-int $negInt + */ + public function positionalArgs($mixed, int $i, float $f, string $s, int $posInt, int $negInt) { // https://3v4l.org/vVL0c - assertType('non-falsy-string', sprintf('%2$14s', $mixed, $i)); + assertType('numeric-string', sprintf('%2$14s', $mixed, $i)); + assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $posInt)); + assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $negInt)); assertType('numeric-string', sprintf('%2$.14F', $mixed, $i)); assertType('numeric-string', sprintf('%2$.14F', $mixed, $f)); From 8b93038f2bddd2236b0dd1df8b77dd2b040a626b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 09:52:45 +0200 Subject: [PATCH 2/7] fix --- .../SprintfFunctionDynamicReturnTypeExtension.php | 5 ++++- tests/PHPStan/Analyser/nsrt/bug-11201.php | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index 54c4cd539e..e30cdd15be 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -81,7 +81,10 @@ public function getTypeFromFunctionCall( // of stringy type, then the return value will be of the same type $checkArgType = $scope->getType($args[$checkArg]->value); - if ($matches[2] === 's' && $checkArgType->isScalar()->yes()) { + if ($matches[2] === 's' + && $checkArgType->isScalar()->yes() + && $checkArgType->getFiniteTypes() === [] + ) { $singlePlaceholderEarlyReturn = $checkArgType->toString(); } elseif ($matches[2] !== 's') { $singlePlaceholderEarlyReturn = new IntersectionType([ diff --git a/tests/PHPStan/Analyser/nsrt/bug-11201.php b/tests/PHPStan/Analyser/nsrt/bug-11201.php index fa935a7756..74a41fa235 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11201.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11201.php @@ -27,6 +27,10 @@ function returnsJustString(): string return rand(0,1) === 1 ? 'foo' : ''; } +function returnsBool(): bool { + return true; +} + $s = sprintf("%s", returnsNonEmptyString()); assertType('non-empty-string', $s); @@ -41,3 +45,12 @@ function returnsJustString(): string $s = sprintf('%2$s', 1234, returnsNonFalsyString()); assertType('non-falsy-string', $s); + +$s = sprintf('%20s', 'abc'); +assertType("' abc'", $s); + +$s = sprintf('%20s', true); +assertType("' 1'", $s); + +$s = sprintf('%20s', returnsBool()); +assertType("non-falsy-string", $s); From fd5b5c900c83e228d5f1da1ba85e8743777004fb Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 09:56:24 +0200 Subject: [PATCH 3/7] Update SprintfFunctionDynamicReturnTypeExtension.php --- src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index e30cdd15be..baea88afbb 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -83,7 +83,7 @@ public function getTypeFromFunctionCall( if ($matches[2] === 's' && $checkArgType->isScalar()->yes() - && $checkArgType->getFiniteTypes() === [] + && $checkArgType->getFiniteTypes() === [] // constant types will be handled in getConstantType() ) { $singlePlaceholderEarlyReturn = $checkArgType->toString(); } elseif ($matches[2] !== 's') { From b3cfcb3e48a1505a1dd037a05d220f52131c6545 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 10:48:32 +0200 Subject: [PATCH 4/7] infer constant types first --- ...intfFunctionDynamicReturnTypeExtension.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index baea88afbb..7fc56fd651 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -48,11 +48,12 @@ public function getTypeFromFunctionCall( return null; } - $formatType = $scope->getType($args[0]->value); - if (count($args) === 1) { - return $this->getConstantType($args, null, $functionReflection, $scope); + $constantType = $this->getConstantType($args, $functionReflection, $scope); + if ($constantType !== null) { + return $constantType; } + $formatType = $scope->getType($args[0]->value); $formatStrings = $formatType->getConstantStrings(); if (count($formatStrings) === 0) { return null; @@ -118,19 +119,19 @@ public function getTypeFromFunctionCall( $returnType = new StringType(); } - return $this->getConstantType($args, $returnType, $functionReflection, $scope); + return $returnType; } /** * @param Arg[] $args */ - private function getConstantType(array $args, ?Type $fallbackReturnType, FunctionReflection $functionReflection, Scope $scope): ?Type + private function getConstantType(array $args, FunctionReflection $functionReflection, Scope $scope): ?Type { $values = []; $combinationsCount = 1; foreach ($args as $arg) { if ($arg->unpack) { - return $fallbackReturnType; + return null; } $argType = $scope->getType($arg->value); @@ -145,7 +146,7 @@ private function getConstantType(array $args, ?Type $fallbackReturnType, Functio } if (count($constantScalarValues) === 0) { - return $fallbackReturnType; + return null; } $values[] = $constantScalarValues; @@ -153,7 +154,7 @@ private function getConstantType(array $args, ?Type $fallbackReturnType, Functio } if ($combinationsCount > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) { - return $fallbackReturnType; + return null; } $combinations = CombinationsHelper::combinations($values); @@ -161,7 +162,7 @@ private function getConstantType(array $args, ?Type $fallbackReturnType, Functio foreach ($combinations as $combination) { $format = array_shift($combination); if (!is_string($format)) { - return $fallbackReturnType; + return null; } try { @@ -171,12 +172,12 @@ private function getConstantType(array $args, ?Type $fallbackReturnType, Functio $returnTypes[] = $scope->getTypeFromValue(@vsprintf($format, $combination)); } } catch (Throwable) { - return $fallbackReturnType; + return null; } } if (count($returnTypes) > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) { - return $fallbackReturnType; + return null; } return TypeCombinator::union(...$returnTypes); From 33c415d44be4b300ab32f8163d897667765fa215 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 10:51:32 +0200 Subject: [PATCH 5/7] simplify --- src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index 7fc56fd651..9b0f20a91f 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -60,7 +60,7 @@ public function getTypeFromFunctionCall( } $singlePlaceholderEarlyReturn = null; - foreach ($formatType->getConstantStrings() as $constantString) { + foreach ($formatStrings 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] !== '') { From 93ce92a50e35d6c5cf45a705753f14464b43523a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 11:09:34 +0200 Subject: [PATCH 6/7] more int range tests --- tests/PHPStan/Analyser/nsrt/bug-7387.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-7387.php b/tests/PHPStan/Analyser/nsrt/bug-7387.php index d5a380ca99..9dd1349dcd 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7387.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7387.php @@ -48,12 +48,16 @@ public function specifiers(int $i) { /** * @param positive-int $posInt * @param negative-int $negInt + * @param int<1, 5> $nonZeroIntRange + * @param int<-1, 5> $intRange */ - public function positionalArgs($mixed, int $i, float $f, string $s, int $posInt, int $negInt) { + public function positionalArgs($mixed, int $i, float $f, string $s, int $posInt, int $negInt, int $nonZeroIntRange, int $intRange) { // https://3v4l.org/vVL0c assertType('numeric-string', sprintf('%2$14s', $mixed, $i)); assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $posInt)); assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $negInt)); + assertType('non-falsy-string', sprintf('%2$14s', $mixed, $intRange)); // could be numeric-string + assertType('non-falsy-string', sprintf('%2$14s', $mixed, $nonZeroIntRange)); // could be non-falsy-string&numeric-string assertType('numeric-string', sprintf('%2$.14F', $mixed, $i)); assertType('numeric-string', sprintf('%2$.14F', $mixed, $f)); From 83b20ded7e53eca4e3fb69b4bd11e647f0eea6a0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 13 Jul 2024 13:47:15 +0200 Subject: [PATCH 7/7] better int range support --- src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php | 4 +--- tests/PHPStan/Analyser/nsrt/bug-7387.php | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php index 9b0f20a91f..1125b24364 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -81,10 +81,8 @@ public function getTypeFromFunctionCall( // 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->isScalar()->yes() - && $checkArgType->getFiniteTypes() === [] // constant types will be handled in getConstantType() + && ($checkArgType->isString()->yes() || $checkArgType->isInteger()->yes()) ) { $singlePlaceholderEarlyReturn = $checkArgType->toString(); } elseif ($matches[2] !== 's') { diff --git a/tests/PHPStan/Analyser/nsrt/bug-7387.php b/tests/PHPStan/Analyser/nsrt/bug-7387.php index 9dd1349dcd..3d67bdffb3 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7387.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7387.php @@ -56,8 +56,8 @@ public function positionalArgs($mixed, int $i, float $f, string $s, int $posInt, assertType('numeric-string', sprintf('%2$14s', $mixed, $i)); assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $posInt)); assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $negInt)); - assertType('non-falsy-string', sprintf('%2$14s', $mixed, $intRange)); // could be numeric-string - assertType('non-falsy-string', sprintf('%2$14s', $mixed, $nonZeroIntRange)); // could be non-falsy-string&numeric-string + assertType('numeric-string', sprintf('%2$14s', $mixed, $intRange)); + assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $nonZeroIntRange)); assertType('numeric-string', sprintf('%2$.14F', $mixed, $i)); assertType('numeric-string', sprintf('%2$.14F', $mixed, $f));