Do not merge BenevolentUnionType with UnionType

phpstan · Dec 12, 2022 · ff51d9f · ff51d9f
1 parent 4d022c6
commit ff51d9f
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 87 deletions.
diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php
@@ -108,25 +108,9 @@ public static function union(Type ...$types): Type
 			return new NeverType();
 		}
 
-		$benevolentTypes = [];
-		$benevolentUnionObject = null;
 		// transform A | (B | C) to A | B | C
 		for ($i = 0; $i < $typesCount; $i++) {
-			if ($types[$i] instanceof BenevolentUnionType) {
-				if ($types[$i] instanceof TemplateBenevolentUnionType && $benevolentUnionObject === null) {
-					$benevolentUnionObject = $types[$i];
-				}
-				$benevolentTypesCount = 0;
-				$typesInner = $types[$i]->getTypes();
-				foreach ($typesInner as $benevolentInnerType) {
-					$benevolentTypesCount++;
-					$benevolentTypes[$benevolentInnerType->describe(VerbosityLevel::value())] = $benevolentInnerType;
-				}
-				array_splice($types, $i, 1, $typesInner);
-				$typesCount += $benevolentTypesCount - 1;
-				continue;
-			}
-			if (!($types[$i] instanceof UnionType)) {
+			if (!$types[$i] instanceof UnionType || $types[$i] instanceof BenevolentUnionType) {
 				continue;
 			}
 			if ($types[$i] instanceof TemplateType) {
@@ -352,25 +336,6 @@ public static function union(Type ...$types): Type
 			return $types[0];
 		}
 
-		if ($benevolentTypes !== []) {
-			$tempTypes = $types;
-			foreach ($tempTypes as $i => $type) {
-				if (!isset($benevolentTypes[$type->describe(VerbosityLevel::value())])) {
-					break;
-				}
-
-				unset($tempTypes[$i]);
-			}
-
-			if ($tempTypes === []) {
-				if ($benevolentUnionObject instanceof TemplateBenevolentUnionType) {
-					return $benevolentUnionObject->withTypes($types);
-				}
-
-				return new BenevolentUnionType($types);
-			}
-		}
-
 		return new UnionType($types, true);
 	}
 

diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php
@@ -58,7 +58,7 @@ public function __construct(private array $types, private bool $normalized = fal
 			$throwException();
 		}
 		foreach ($types as $type) {
-			if (!($type instanceof UnionType)) {
+			if (!$type instanceof UnionType || $type instanceof BenevolentUnionType) {
 				continue;
 			}
 			if ($type instanceof TemplateType) {

diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
@@ -4908,11 +4908,11 @@ public function dataArrayFunctions(): array
 				'array_search(new stdClass, $generalStringKeys, true)',
 			],
 			[
-				'int|string|false',
+				'(int|string)|false',
 				'array_search($mixed, $array, true)',
 			],
 			[
-				'int|string|false',
+				'(int|string)|false',
 				'array_search($mixed, $array, false)',
 			],
 			[
@@ -4972,15 +4972,15 @@ public function dataArrayFunctions(): array
 				'array_search(\'id\', doFoo() ? $thisDoesNotExistAndIsMixedInUnion : false, true)',
 			],
 			[
-				'int|string|false',
+				'(int|string)|false',
 				'array_search(1, $generalIntegers, true)',
 			],
 			[
-				'int|string|false',
+				'(int|string)|false',
 				'array_search(1, $generalIntegers, false)',
 			],
 			[
-				'int|string|false',
+				'(int|string)|false',
 				'array_search(1, $generalIntegers)',
 			],
 			[
@@ -8733,11 +8733,11 @@ public function dataPhp73Functions(): array
 				'json_decode($mixed, false, 512, $integer | JSON_THROW_ON_ERROR | JSON_NUMERIC_CHECK)',
 			],
 			[
-				'int|string|null',
+				'(int|string)|null',
 				'array_key_first($mixedArray)',
 			],
 			[
-				'int|string|null',
+				'(int|string)|null',
 				'array_key_last($mixedArray)',
 			],
 			[
@@ -8817,7 +8817,7 @@ public function dataPhp73Functions(): array
 				'$hrtime3',
 			],
 			[
-				'array{int, int}|float|int',
+				'(float|int)|array{int, int}',
 				'$hrtime4',
 			],
 		];

diff --git a/tests/PHPStan/Analyser/data/array-search-php7.php b/tests/PHPStan/Analyser/data/array-search-php7.php
@@ -10,9 +10,9 @@ class Foo
 	public function mixedAndSubtractedArray($mixed, string $string): void
 	{
 		if (is_array($mixed)) {
-			assertType('int|string|false', array_search('foo', $mixed, true));
-			assertType('int|string|false', array_search('foo', $mixed));
-			assertType('int|string|false', array_search($string, $mixed, true));
+			assertType('(int|string)|false', array_search('foo', $mixed, true));
+			assertType('(int|string)|false', array_search('foo', $mixed));
+			assertType('(int|string)|false', array_search($string, $mixed, true));
 		} else {
 			assertType('mixed~array', $mixed);
 			assertType('null', array_search('foo', $mixed, true));

diff --git a/tests/PHPStan/Analyser/data/array-search-php8.php b/tests/PHPStan/Analyser/data/array-search-php8.php
@@ -10,9 +10,9 @@ class Foo
 	public function mixedAndSubtractedArray($mixed, string $string): void
 	{
 		if (is_array($mixed)) {
-			assertType('int|string|false', array_search('foo', $mixed, true));
-			assertType('int|string|false', array_search('foo', $mixed));
-			assertType('int|string|false', array_search($string, $mixed, true));
+			assertType('(int|string)|false', array_search('foo', $mixed, true));
+			assertType('(int|string)|false', array_search('foo', $mixed));
+			assertType('(int|string)|false', array_search($string, $mixed, true));
 		} else {
 			assertType('mixed~array', $mixed);
 			assertType('*NEVER*', array_search('foo', $mixed, true));

diff --git a/tests/PHPStan/Analyser/data/array-search.php b/tests/PHPStan/Analyser/data/array-search.php
@@ -10,9 +10,9 @@ class Foo
 	public function nonEmpty(array $arr, string $string): void
 	{
 		/** @var non-empty-array<string> $arr */
-		assertType('int|string|false', array_search('foo', $arr, true));
-		assertType('int|string|false', array_search('foo', $arr));
-		assertType('int|string|false', array_search($string, $arr, true));
+		assertType('(int|string)|false', array_search('foo', $arr, true));
+		assertType('(int|string)|false', array_search('foo', $arr));
+		assertType('(int|string)|false', array_search($string, $arr, true));
 	}
 
 	public function normalArrays(array $arr, string $string): void

diff --git a/tests/PHPStan/Analyser/data/key-of-generic.php b/tests/PHPStan/Analyser/data/key-of-generic.php
@@ -33,7 +33,7 @@ function test(
 ): void {
 	assertType("'j'|'k'|null", $result->getKey());
 	assertType('0|1|null', $listResult->getKey());
-	assertType('int|string|null', $mixedResult->getKey());
+	assertType('(int|string)|null', $mixedResult->getKey());
 	assertType('string|null', $stringKeyResult->getKey());
 	assertType('int|null', $intKeyResult->getKey());
 	assertType('null', $emptyResult->getKey());

diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
@@ -322,31 +322,23 @@ public function testCallMethods(): void
 				791,
 			],
 			[
-				'Parameter #1 $i of method Test\CheckDefaultArrayKeys::doBar() expects int, int|stdClass|string given.',
+				'Parameter #1 $i of method Test\CheckDefaultArrayKeys::doBar() expects int, (int|string)|stdClass given.',
 				797,
 			],
 			[
-				'Parameter #1 $str of method Test\CheckDefaultArrayKeys::doBaz() expects string, int|stdClass|string given.',
+				'Parameter #1 $str of method Test\CheckDefaultArrayKeys::doBaz() expects string, (int|string)|stdClass given.',
 				798,
 			],
 			[
-				'Parameter #1 $intOrString of method Test\CheckDefaultArrayKeys::doLorem() expects int|string, int|stdClass|string given.',
+				'Parameter #1 $intOrString of method Test\CheckDefaultArrayKeys::doLorem() expects int|string, (int|string)|stdClass given.',
 				799,
 			],
 			[
-				'Parameter #1 $stdOrInt of method Test\CheckDefaultArrayKeys::doIpsum() expects int|stdClass, int|stdClass|string given.', // should not expect this
-				800,
-			],
-			[
-				'Parameter #1 $stdOrString of method Test\CheckDefaultArrayKeys::doDolor() expects stdClass|string, int|stdClass|string given.', // should not expect this
-				801,
-			],
-			[
-				'Parameter #1 $dateOrString of method Test\CheckDefaultArrayKeys::doSit() expects DateTimeImmutable|string, int|stdClass|string given.',
+				'Parameter #1 $dateOrString of method Test\CheckDefaultArrayKeys::doSit() expects DateTimeImmutable|string, (int|string)|stdClass given.',
 				802,
 			],
 			[
-				'Parameter #1 $std of method Test\CheckDefaultArrayKeys::doAmet() expects stdClass, int|stdClass|string given.',
+				'Parameter #1 $std of method Test\CheckDefaultArrayKeys::doAmet() expects stdClass, (int|string)|stdClass given.',
 				803,
 			],
 			[
@@ -693,31 +685,23 @@ public function testCallMethodsOnThisOnly(): void
 				791,
 			],
 			[
-				'Parameter #1 $i of method Test\CheckDefaultArrayKeys::doBar() expects int, int|stdClass|string given.',
+				'Parameter #1 $i of method Test\CheckDefaultArrayKeys::doBar() expects int, (int|string)|stdClass given.',
 				797,
 			],
 			[
-				'Parameter #1 $str of method Test\CheckDefaultArrayKeys::doBaz() expects string, int|stdClass|string given.',
+				'Parameter #1 $str of method Test\CheckDefaultArrayKeys::doBaz() expects string, (int|string)|stdClass given.',
 				798,
 			],
 			[
-				'Parameter #1 $intOrString of method Test\CheckDefaultArrayKeys::doLorem() expects int|string, int|stdClass|string given.',
+				'Parameter #1 $intOrString of method Test\CheckDefaultArrayKeys::doLorem() expects int|string, (int|string)|stdClass given.',
 				799,
 			],
 			[
-				'Parameter #1 $stdOrInt of method Test\CheckDefaultArrayKeys::doIpsum() expects int|stdClass, int|stdClass|string given.', // should not expect this
-				800,
-			],
-			[
-				'Parameter #1 $stdOrString of method Test\CheckDefaultArrayKeys::doDolor() expects stdClass|string, int|stdClass|string given.', // should not expect this
-				801,
-			],
-			[
-				'Parameter #1 $dateOrString of method Test\CheckDefaultArrayKeys::doSit() expects DateTimeImmutable|string, int|stdClass|string given.',
+				'Parameter #1 $dateOrString of method Test\CheckDefaultArrayKeys::doSit() expects DateTimeImmutable|string, (int|string)|stdClass given.',
 				802,
 			],
 			[
-				'Parameter #1 $std of method Test\CheckDefaultArrayKeys::doAmet() expects stdClass, int|stdClass|string given.',
+				'Parameter #1 $std of method Test\CheckDefaultArrayKeys::doAmet() expects stdClass, (int|string)|stdClass given.',
 				803,
 			],
 			[

diff --git a/tests/PHPStan/Type/TypeCombinatorTest.php b/tests/PHPStan/Type/TypeCombinatorTest.php
@@ -821,23 +821,31 @@ public function dataUnion(): iterable
 					new BenevolentUnionType([new IntegerType(), new StringType()]),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
 			],
 			[
 				[
 					new UnionType([new StringType(), new FloatType()]),
 					new BenevolentUnionType([new IntegerType(), new StringType()]),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
 			],
 			[
 				[
 					new UnionType([new IntegerType(), new FloatType()]),
 					new BenevolentUnionType([new IntegerType(), new StringType()]),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
+			],
+			[
+				[
+					new BenevolentUnionType([new FloatType(), new IntegerType()]),
+					new NullType(),
+				],
+				UnionType::class,
+				'(float|int)|null',
 			],
 			[
 				[
@@ -1362,15 +1370,15 @@ public function dataUnion(): iterable
 					new FloatType(),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
 			],
 			[
 				[
 					new BenevolentUnionType([new IntegerType(), new StringType()]),
 					new UnionType([new IntegerType(), new StringType(), new FloatType()]),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
 			],
 			[
 				[
@@ -1386,7 +1394,7 @@ public function dataUnion(): iterable
 					new UnionType([new ConstantIntegerType(1), new ConstantIntegerType(2), new FloatType()]),
 				],
 				UnionType::class,
-				'float|int|string',
+				'(int|string)|float',
 			],
 			[
 				[