From 9bd84cf4fe487e63d054227cadb5138d5c1bfee0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 19 Jul 2024 13:42:04 +0200 Subject: [PATCH] More precise `sprintf()` format arg-based return type --- ...intfFunctionDynamicReturnTypeExtension.php | 88 +++++++++++++++++-- tests/PHPStan/Analyser/nsrt/bug-7387.php | 2 +- .../Analyser/nsrt/non-empty-string.php | 32 ++++++- .../Analyser/nsrt/non-falsy-string.php | 16 +++- ...rictComparisonOfDifferentTypesRuleTest.php | 6 ++ .../Rules/Comparison/data/bug-10493.php | 24 +++++ 6 files changed, 157 insertions(+), 11 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 1125b24364..8f1782b0a5 100644 --- a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php @@ -11,6 +11,8 @@ use PHPStan\Type\Accessory\AccessoryNonEmptyStringType; use PHPStan\Type\Accessory\AccessoryNonFalsyStringType; use PHPStan\Type\Accessory\AccessoryNumericStringType; +use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntegerRangeType; use PHPStan\Type\IntersectionType; @@ -18,6 +20,7 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use Throwable; +use function array_fill; use function array_key_exists; use function array_shift; use function count; @@ -55,13 +58,32 @@ public function getTypeFromFunctionCall( $formatType = $scope->getType($args[0]->value); $formatStrings = $formatType->getConstantStrings(); - if (count($formatStrings) === 0) { - return null; - } $singlePlaceholderEarlyReturn = null; + $allPatternsNonEmpty = count($formatStrings) !== 0; + $allPatternsNonFalsy = count($formatStrings) !== 0; foreach ($formatStrings as $constantString) { - // The printf format is %[argnum$][flags][width][.precision] + $constantParts = $this->getFormatConstantParts( + $constantString->getValue(), + $functionReflection, + $functionCall, + $scope, + ); + if ($constantParts !== null) { + if ($constantParts->isNonFalsyString()->yes()) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf + // keep all bool flags as is + } elseif ($constantParts->isNonEmptyString()->yes()) { + $allPatternsNonFalsy = false; + } else { + $allPatternsNonEmpty = false; + $allPatternsNonFalsy = false; + } + } else { + $allPatternsNonEmpty = false; + $allPatternsNonFalsy = false; + } + + // The printf format is %[argnum$][flags][width][.precision]specifier. if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*([sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) { if ($matches[1] !== '') { // invalid positional argument @@ -103,12 +125,23 @@ public function getTypeFromFunctionCall( return $singlePlaceholderEarlyReturn; } - if ($formatType->isNonFalsyString()->yes()) { + $isNonEmpty = $allPatternsNonEmpty; + if ( + count($formatStrings) === 0 + && $functionReflection->getName() === 'sprintf' + && count($args) === 2 + && $formatType->isNonEmptyString()->yes() + && $scope->getType($args[1]->value)->isNonEmptyString()->yes() + ) { + $isNonEmpty = true; + } + + if ($allPatternsNonFalsy) { $returnType = new IntersectionType([ new StringType(), new AccessoryNonFalsyStringType(), ]); - } elseif ($formatType->isNonEmptyString()->yes()) { + } elseif ($isNonEmpty) { $returnType = new IntersectionType([ new StringType(), new AccessoryNonEmptyStringType(), @@ -120,6 +153,49 @@ public function getTypeFromFunctionCall( return $returnType; } + /** + * Detect constant strings in the format which neither depend on placeholders nor on given value arguments. + */ + private function getFormatConstantParts( + string $format, + FunctionReflection $functionReflection, + FuncCall $functionCall, + Scope $scope, + ): ?ConstantStringType + { + $args = $functionCall->getArgs(); + if ($functionReflection->getName() === 'sprintf') { + $valuesCount = count($args) - 1; + } elseif ( + $functionReflection->getName() === 'vsprintf' + && count($args) >= 2 + ) { + $arraySize = $scope->getType($args[1]->value)->getArraySize(); + if (!($arraySize instanceof ConstantIntegerType)) { + return null; + } + + $valuesCount = $arraySize->getValue(); + } else { + return null; + } + + if ($valuesCount <= 0) { + return null; + } + $dummyValues = array_fill(0, $valuesCount, ''); + + try { + $formatted = @vsprintf($format, $dummyValues); + if ($formatted === false) { // @phpstan-ignore identical.alwaysFalse (PHP7.2 compat) + return null; + } + return new ConstantStringType($formatted); + } catch (Throwable) { + return null; + } + } + /** * @param Arg[] $args */ diff --git a/tests/PHPStan/Analyser/nsrt/bug-7387.php b/tests/PHPStan/Analyser/nsrt/bug-7387.php index 3d67bdffb3..4623719b4d 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7387.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7387.php @@ -88,7 +88,7 @@ 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('string', vsprintf("%s", ['123'])); // could be '123' // too many arguments.. php silently allows it assertType('numeric-string', vsprintf("%4d", ['123', '456'])); } diff --git a/tests/PHPStan/Analyser/nsrt/non-empty-string.php b/tests/PHPStan/Analyser/nsrt/non-empty-string.php index 55bb8ad77c..9e6ceed668 100644 --- a/tests/PHPStan/Analyser/nsrt/non-empty-string.php +++ b/tests/PHPStan/Analyser/nsrt/non-empty-string.php @@ -303,9 +303,10 @@ class MoreNonEmptyStringFunctions /** * @param non-empty-string $nonEmpty + * @param non-falsy-string $nonFalsy * @param '1'|'2'|'5'|'10' $constUnion */ - public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUnion) + public function doFoo(string $s, string $nonEmpty, string $nonFalsy, int $i, bool $bool, $constUnion) { assertType('string', addslashes($s)); assertType('non-empty-string', addslashes($nonEmpty)); @@ -350,9 +351,20 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni assertType('string', sprintf($s)); assertType('string', sprintf($nonEmpty)); + assertType('string', sprintf($s, $nonEmpty)); + assertType('string', sprintf($nonEmpty, $s)); + assertType('string', sprintf($s, $nonFalsy)); + assertType('string', sprintf($nonFalsy, $s)); + assertType('non-empty-string', sprintf($nonEmpty, $nonEmpty)); + assertType('non-empty-string', sprintf($nonEmpty, $nonFalsy)); + assertType('non-empty-string', sprintf($nonFalsy, $nonEmpty)); assertType('string', vsprintf($s, [])); assertType('string', vsprintf($nonEmpty, [])); + assertType('non-empty-string', sprintf("%s0%s", $s, $s)); + assertType('non-empty-string', sprintf("%s0%s%s%s%s", $s, $s, $s, $s, $s)); + assertType('non-empty-string', sprintf("%s0%s%s%s%s%s", $s, $s, $s, $s, $s, $s)); + assertType('0', strlen('')); assertType('5', strlen('hallo')); assertType('int<0, 1>', strlen($bool)); @@ -374,4 +386,22 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni assertType('string', str_repeat($s, $i)); } + function multiplesPrintfFormats(string $s) { + $maybeNonEmpty = '%s'; + $maybeNonFalsy = '%s'; + $nonEmpty = '%s0'; + $nonFalsy = '%sAA'; + + if (rand(0,1)) { + $maybeNonEmpty = '%s0'; + $maybeNonFalsy = '%sAA'; + $nonEmpty = '0%s'; + $nonFalsy = 'AA%s'; + } + + assertType('string', sprintf($maybeNonEmpty, $s)); + assertType('string', sprintf($maybeNonFalsy, $s)); + assertType('non-empty-string', sprintf($nonEmpty, $s)); + assertType('non-falsy-string', sprintf($nonFalsy, $s)); + } } diff --git a/tests/PHPStan/Analyser/nsrt/non-falsy-string.php b/tests/PHPStan/Analyser/nsrt/non-falsy-string.php index fa1f1c16ed..f8c6dc40b7 100644 --- a/tests/PHPStan/Analyser/nsrt/non-falsy-string.php +++ b/tests/PHPStan/Analyser/nsrt/non-falsy-string.php @@ -107,12 +107,22 @@ function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArra assertType('string', sprintf($nonFalsey)); assertType("'foo'", sprintf('foo')); assertType("string", sprintf(...$arr)); - assertType("non-falsy-string", sprintf('%s', ...$arr)); // should be 'string' + assertType("string", sprintf('%s', ...$arr)); + + // empty array only works as long as no placeholder in the pattern 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('foo', [])); + + assertType("string", vsprintf('%s', ...$arr)); assertType("string", vsprintf(...$arr)); + assertType('non-falsy-string', vsprintf('%sAA%s', [$s, $s])); + assertType('non-falsy-string', vsprintf('%d%d', [$s, $s])); // could be non-falsy-string&numeric-string + + assertType('non-falsy-string', sprintf("%sAA%s", $s, $s)); + assertType('non-falsy-string', sprintf("%d%d", $s, $s)); // could be non-falsy-string&numeric-string + assertType('non-falsy-string', sprintf("%sAA%s%s%s%s", $s, $s, $s, $s, $s)); + assertType('non-falsy-string', sprintf("%sAA%s%s%s%s%s", $s, $s, $s, $s, $s, $s)); assertType('int<1, max>', strlen($nonFalsey)); diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 25f6c07533..e1c99901de 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 0000000000..9cf0c1334e --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-10493.php @@ -0,0 +1,24 @@ += 8.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; + } +}