Improve sprintf return type inference

phpstan · Jun 19, 2024 · a0824ef · a0824ef
1 parent f513931
commit a0824ef
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 90 deletions.
diff --git a/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
@@ -21,11 +21,10 @@
 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
@@ -48,70 +47,90 @@ 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 (array_key_exists(1, $matches) && ($matches[1] !== '')) {
-						// invalid positional argument
-						if ($matches[1] === '0$') {
-							return null;
-						}
-						$checkArg = intval(substr($matches[1], 0, -1));
-					} else {
-						$checkArg = 1;
-					}
+		$formatStrings = $formatType->getConstantStrings();
+		if (count($formatStrings) === 0) {
+			return null;
+		}
 
-					// constant string specifies a numbered argument that does not exist
-					if (!array_key_exists($checkArg, $args)) {
-						return null;
-					}
+		$isNonEmpty = false;
+		$isNonFalsy = false;
+		$isNumeric = false;
+		foreach ($formatStrings 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;
+			}
 
-					// 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->isString()->yes()) {
-						$singlePlaceholderEarlyReturn = $checkArgType;
-					} elseif ($matches[2] !== 's') {
-						$singlePlaceholderEarlyReturn = new IntersectionType([
-							new StringType(),
-							new AccessoryNumericStringType(),
-						]);
-					}
+			// invalid positional argument
+			if (array_key_exists('argnum', $matches) && $matches['argnum'] === '0$') {
+				return null;
+			}
 
-					continue;
+			if (array_key_exists('width', $matches)) {
+				if ($matches['width'] > 1) {
+					$isNonFalsy = true;
+				} elseif ($matches['width'] > 0) {
+					$isNonEmpty = true;
 				}
+			}
 
-				$singlePlaceholderEarlyReturn = null;
+			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 ($i === 0) { // skip format type
+				continue;
+			}
+
+			if ($functionReflection->getName() === 'vsprintf') {
+				if ($argType->isIterableAtLeastOnce()->yes()) {
+					$isNonEmpty = true;
+				}
+				continue;
+			}
 
-			if ($singlePlaceholderEarlyReturn !== null) {
-				return $singlePlaceholderEarlyReturn;
+			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
@@ -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('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<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));
 	}
 }
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,24 @@
+<?php // lint >= 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;
+	}
+}