Improve constant string union handling for concat and encapsed string

phpstan · Dec 6, 2022 · dd092dd · dd092dd
1 parent 2ca6118
commit dd092dd
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 80 deletions.
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -66,8 +66,6 @@
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\Accessory\AccessoryArrayListType;
 use PHPStan\Type\Accessory\AccessoryLiteralStringType;
-use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
-use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
 use PHPStan\Type\Accessory\HasOffsetValueType;
 use PHPStan\Type\Accessory\NonEmptyArrayType;
 use PHPStan\Type\ArrayType;
@@ -1091,63 +1089,25 @@ private function resolveType(Expr $node): Type
 		} elseif ($node instanceof String_) {
 			return $this->initializerExprTypeResolver->getType($node, InitializerExprContext::fromScope($this));
 		} elseif ($node instanceof Node\Scalar\Encapsed) {
-			$parts = [];
-			foreach ($node->parts as $part) {
-				if ($part instanceof EncapsedStringPart) {
-					$parts[] = new ConstantStringType($part->value);
-					continue;
-				}
+			$resultType = null;
 
-				$partStringType = $this->getType($part)->toString();
-				if ($partStringType instanceof ErrorType) {
-					return new ErrorType();
-				}
-
-				$parts[] = $partStringType;
-			}
+			foreach ($node->parts as $part) {
+				$partType = $part instanceof EncapsedStringPart
+					? new ConstantStringType($part->value)
+					: $this->getType($part);
+				if ($resultType === null) {
+					$resultType = $partType;
 
-			$constantString = new ConstantStringType('');
-			foreach ($parts as $part) {
-				if ($part instanceof ConstantStringType) {
-					$constantString = $constantString->append($part);
 					continue;
 				}
 
-				$isNonEmpty = false;
-				$isNonFalsy = false;
-				$isLiteralString = true;
-				foreach ($parts as $partType) {
-					if ($partType->isNonFalsyString()->yes()) {
-						$isNonFalsy = true;
-					}
-					if ($partType->isNonEmptyString()->yes()) {
-						$isNonEmpty = true;
-					}
-					if ($partType->isLiteralString()->yes()) {
-						continue;
-					}
-					$isLiteralString = false;
-				}
-
-				$accessoryTypes = [];
-				if ($isNonFalsy === true) {
-					$accessoryTypes[] = new AccessoryNonFalsyStringType();
-				} elseif ($isNonEmpty === true) {
-					$accessoryTypes[] = new AccessoryNonEmptyStringType();
+				$resultType = $this->initializerExprTypeResolver->resolveConcatType($resultType, $partType);
+				if (! $resultType instanceof ConstantStringType && ! $resultType instanceof UnionType) {
+					return $resultType;
 				}
-
-				if ($isLiteralString === true) {
-					$accessoryTypes[] = new AccessoryLiteralStringType();
-				}
-				if (count($accessoryTypes) > 0) {
-					$accessoryTypes[] = new StringType();
-					return new IntersectionType($accessoryTypes);
-				}
-
-				return new StringType();
 			}
 
-			return $constantString;
+			return $resultType ?? new ConstantStringType('');
 		} elseif ($node instanceof DNumber) {
 			return $this->initializerExprTypeResolver->getType($node, InitializerExprContext::fromScope($this));
 		} elseif ($node instanceof Expr\CallLike && $node->isFirstClassCallable()) {

diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php
@@ -62,6 +62,7 @@
 use PHPStan\Type\TypeWithClassName;
 use PHPStan\Type\UnionType;
 use function array_keys;
+use function array_merge;
 use function count;
 use function dirname;
 use function in_array;
@@ -353,8 +354,16 @@ public function getType(Expr $expr, InitializerExprContext $context): Type
 	 */
 	public function getConcatType(Expr $left, Expr $right, callable $getTypeCallback): Type
 	{
-		$leftStringType = $getTypeCallback($left)->toString();
-		$rightStringType = $getTypeCallback($right)->toString();
+		$leftType = $getTypeCallback($left);
+		$rightType = $getTypeCallback($right);
+
+		return $this->resolveConcatType($leftType, $rightType);
+	}
+
+	public function resolveConcatType(Type $left, Type $right): Type
+	{
+		$leftStringType = $left->toString();
+		$rightStringType = $right->toString();
 		if (TypeCombinator::union(
 			$leftStringType,
 			$rightStringType,
@@ -374,34 +383,33 @@ public function getConcatType(Expr $left, Expr $right, callable $getTypeCallback
 			return $leftStringType->append($rightStringType);
 		}
 
+		$leftConstantStrings = TypeUtils::getConstantStrings($leftStringType);
+		$rightConstantStrings = TypeUtils::getConstantStrings($rightStringType);
+		$combinedConstantStringsCount = count($leftConstantStrings) * count($rightConstantStrings);
+
 		// we limit the number of union-types for performance reasons
-		if ($leftStringType instanceof UnionType && count($leftStringType->getTypes()) <= 16 && $rightStringType instanceof ConstantStringType) {
-			$constantStrings = TypeUtils::getConstantStrings($leftStringType);
-			if (count($constantStrings) > 0) {
-				$strings = [];
-				foreach ($constantStrings as $constantString) {
-					if ($constantString->getValue() === '') {
-						$strings[] = $rightStringType;
+		if ($combinedConstantStringsCount > 0 && $combinedConstantStringsCount <= 16) {
+			$strings = [];
 
-						continue;
-					}
-					$strings[] = $constantString->append($rightStringType);
+			foreach ($leftConstantStrings as $leftConstantString) {
+				if ($leftConstantString->getValue() === '') {
+					$strings = array_merge($strings, $rightConstantStrings);
+
+					continue;
 				}
-				return TypeCombinator::union(...$strings);
-			}
-		}
-		if ($rightStringType instanceof UnionType && count($rightStringType->getTypes()) <= 16 && $leftStringType instanceof ConstantStringType) {
-			$constantStrings = TypeUtils::getConstantStrings($rightStringType);
-			if (count($constantStrings) > 0) {
-				$strings = [];
-				foreach ($constantStrings as $constantString) {
-					if ($constantString->getValue() === '') {
-						$strings[] = $leftStringType;
+
+				foreach ($rightConstantStrings as $rightConstantString) {
+					if ($rightConstantString->getValue() === '') {
+						$strings[] = $leftConstantString;
 
 						continue;
 					}
-					$strings[] = $leftStringType->append($constantString);
+
+					$strings[] = $leftConstantString->append($rightConstantString);
 				}
+			}
+
+			if (count($strings) > 0) {
 				return TypeCombinator::union(...$strings);
 			}
 		}

diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
@@ -2996,15 +2996,15 @@ public function dataBinaryOperations(): array
 				'$decrementedFooString',
 			],
 			[
-				'literal-string&non-falsy-string',
+				"'barbar'|'barfoo'|'foobar'|'foofoo'",
 				'$conditionalString . $conditionalString',
 			],
 			[
-				'literal-string&non-falsy-string',
+				"'baripsum'|'barlorem'|'fooipsum'|'foolorem'",
 				'$conditionalString . $anotherConditionalString',
 			],
 			[
-				'literal-string&non-falsy-string',
+				"'ipsumbar'|'ipsumfoo'|'lorembar'|'loremfoo'",
 				'$anotherConditionalString . $conditionalString',
 			],
 			[

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -810,7 +810,7 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6584.php');
 
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/weird-strlen-cases.php');
-		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6439.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/data/constant-string-unions.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6748.php');
 
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/array-fill-keys.php');

diff --git a/tests/PHPStan/Analyser/data/bug-6439.php → .../Analyser/data/constant-string-unions.php b/tests/PHPStan/Analyser/data/bug-6439.php → .../Analyser/data/constant-string-unions.php
@@ -1,10 +1,10 @@
 <?php
 
-namespace Bug6439;
+namespace ConstantStringUnions;
 
 use function PHPStan\Testing\assertType;
 
-
+// See https://github.com/phpstan/phpstan/issues/6439
 class HelloWorld
 {
 	public function unionOnLeft(string $name, ?int $gesperrt = null, ?int $adaid = null):void
@@ -33,6 +33,28 @@ public function unionOnRight(string $name, ?int $gesperrt = null, ?int $adaid =
 		assertType("'branch-a general'|'branch-b branch-a general'|'branch-b general'|'general'", $string);
 	}
 
+	public function unionOnBoth():void
+	{
+		$left = rand() ? 'a' : 'b';
+		$right = rand() ? 'x' : 'y';
+		assertType("'ax'|'ay'|'bx'|'by'", $left . $right);
+	}
+
+	public function encapsedString():void
+	{
+		$str = rand() ? 'a' : 'b';
+		$int = rand() ? 1 : 0;
+		$float = rand() ? 1.0 : 2.0;
+		$bool = (bool) rand();
+		$nullable = rand() ? 'a' : null;
+		assertType("'.a.'|'.b.'", ".$str.");
+		assertType("'.0.'|'.1.'", ".$int.");
+		assertType("'.1.'|'.2.'", ".$float.");
+		assertType("'..'|'.1.'", ".$bool.");
+		assertType("'..'|'.a.'", ".$nullable.");
+		assertType("'.a.0.'|'.a.1.'|'.b.0.'|'.b.1.'", ".$str.$int.");
+	}
+
 	/**
 	 * @param '1'|'2'|'3'|'4'|'5'|'6'|'7'|'8'|'9'|'10'|'11'|'12'|'13'|'14'|'15' $s15
 	 * @param '1'|'2'|'3'|'4'|'5'|'6'|'7'|'8'|'9'|'10'|'11'|'12'|'13'|'14'|'15'|'16' $s16