Simplify accessory logic, remove logic that was extracted to other MR…

…, add some tests
phpstan · Oct 5, 2022 · 23b634e · 23b634e
1 parent f4f9414
commit 23b634e
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 95 deletions.
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -118,7 +118,6 @@
 use function array_key_exists;
 use function array_keys;
 use function array_map;
-use function array_merge;
 use function array_pop;
 use function array_slice;
 use function count;
@@ -4582,7 +4581,6 @@ private static function generalizeType(Type $a, Type $b): Type
 		$constantArrays = ['a' => [], 'b' => []];
 		$generalArrays = ['a' => [], 'b' => []];
 		$integerRanges = ['a' => [], 'b' => []];
-		$accessoryTypes = [];
 		$otherTypes = [];
 
 		foreach ([
@@ -4674,9 +4672,6 @@ private static function generalizeType(Type $a, Type $b): Type
 						TypeCombinator::union(self::generalizeType($constantArraysA->getIterableKeyType(), $constantArraysB->getIterableKeyType())),
 						TypeCombinator::union(self::generalizeType($constantArraysA->getIterableValueType(), $constantArraysB->getIterableValueType())),
 					);
-					if ($constantArraysA->isIterableAtLeastOnce()->yes() && $constantArraysB->isIterableAtLeastOnce()->yes()) {
-						$accessoryTypes[] = new NonEmptyArrayType();
-					}
 				}
 			}
 		} elseif (count($constantArrays['b']) > 0) {
@@ -4851,27 +4846,10 @@ private static function generalizeType(Type $a, Type $b): Type
 			$resultTypes[] = TypeCombinator::union(...$integerRanges['b']);
 		}
 
-		$commonTypeMaps = [];
-		foreach ([TypeUtils::getAccessoryTypes($a), TypeUtils::getAccessoryTypes($b)] as $listKey => $accessoryTypeList) {
-			foreach ($accessoryTypeList as $accessoryType) {
-				if ($accessoryType instanceof HasOffsetValueType) {
-					$commonTypeMaps[$listKey][sprintf('hasOffsetValue(%s)', $accessoryType->getOffsetType()->describe(VerbosityLevel::cache()))][] = $accessoryType;
-					continue;
-				}
-
-				$commonTypeMaps[$listKey][$accessoryType->describe(VerbosityLevel::cache())][] = $accessoryType;
-			}
-		}
-
-		if (count($commonTypeMaps) === 2) {
-			$accessoryTypes = array_merge(
-				$accessoryTypes,
-				array_map(
-					static fn (Type $type): Type => $type->generalize(GeneralizePrecision::moreSpecific()),
-					TypeCombinator::unionCommonTypeMaps($commonTypeMaps),
-				),
-			);
-		}
+		$accessoryTypes = array_map(
+			static fn (Type $type): Type => $type->generalize(GeneralizePrecision::moreSpecific()),
+			TypeUtils::getAccessoryTypes($a),
+		);
 
 		return TypeCombinator::intersect(
 			TypeCombinator::union(...$resultTypes, ...$otherTypes),

diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php
@@ -213,10 +213,28 @@ public static function union(Type ...$types): Type
 		/** @var ArrayType[] $arrayTypes */
 		$arrayTypes = $arrayTypes;
 
+		$commonArrayAccessoryTypesKeys = [];
+		if (count($arrayAccessoryTypes) > 1) {
+			$commonArrayAccessoryTypesKeys = array_keys(array_intersect_key(...$arrayAccessoryTypes));
+		} elseif (count($arrayAccessoryTypes) > 0) {
+			$commonArrayAccessoryTypesKeys = array_keys($arrayAccessoryTypes[0]);
+		}
+
+		$arrayAccessoryTypesToProcess = [];
+		foreach ($commonArrayAccessoryTypesKeys as $commonKey) {
+			$typesToUnion = [];
+			foreach ($arrayAccessoryTypes as $array) {
+				foreach ($array[$commonKey] as $arrayAccessoryType) {
+					$typesToUnion[] = $arrayAccessoryType;
+				}
+			}
+			$arrayAccessoryTypesToProcess[] = self::union(...$typesToUnion);
+		}
+
 		$types = array_values(
 			array_merge(
 				$types,
-				self::processArrayTypes($arrayTypes, self::unionCommonTypeMaps($arrayAccessoryTypes)),
+				self::processArrayTypes($arrayTypes, $arrayAccessoryTypesToProcess),
 			),
 		);
 		$typesCount = count($types);
@@ -345,34 +363,6 @@ public static function union(Type ...$types): Type
 		return new UnionType($types);
 	}
 
-	/**
-	 * @internal
-	 * @param list<array<string, list<Type>>> $commonTypeMaps
-	 * @return list<Type>
-	 */
-	public static function unionCommonTypeMaps(array $commonTypeMaps): array
-	{
-		$commonTypesKeys = [];
-		if (count($commonTypeMaps) > 1) {
-			$commonTypesKeys = array_keys(array_intersect_key(...$commonTypeMaps));
-		} elseif (count($commonTypeMaps) > 0) {
-			$commonTypesKeys = array_keys($commonTypeMaps[0]);
-		}
-
-		$types = [];
-		foreach ($commonTypesKeys as $commonKey) {
-			$typesToUnion = [];
-			foreach ($commonTypeMaps as $commonTypeMap) {
-				foreach ($commonTypeMap[$commonKey] as $commonType) {
-					$typesToUnion[] = $commonType;
-				}
-			}
-			$types[] = self::union(...$typesToUnion);
-		}
-
-		return $types;
-	}
-
 	/**
 	 * @return array{Type, null}|array{null, Type}|null
 	 */

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -1033,8 +1033,8 @@ public function dataFileAsserts(): iterable
 			yield from $this->gatherAssertTypes(__DIR__ . '/data/standalone-types.php');
 		}
 		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Arrays/data/bug-7954.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/data/scope-generalization.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8015.php');
-		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7996.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7993.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7141.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/cli-globals.php');

diff --git a/tests/PHPStan/Analyser/ScopeTest.php b/tests/PHPStan/Analyser/ScopeTest.php
@@ -139,7 +139,7 @@ public function dataGeneralize(): array
 					new ConstantIntegerType(1),
 					new ConstantIntegerType(1),
 				]),
-				'non-empty-array<literal-string&non-falsy-string, 1>',
+				'array<literal-string&non-falsy-string, 1>',
 			],
 			[
 				new ConstantArrayType([
@@ -154,7 +154,7 @@ public function dataGeneralize(): array
 					new ConstantIntegerType(1),
 					new ConstantIntegerType(2),
 				]),
-				'non-empty-array<literal-string&non-falsy-string, int<1, max>>',
+				'array<literal-string&non-falsy-string, int<1, max>>',
 			],
 			[
 				new UnionType([

diff --git a/tests/PHPStan/Analyser/data/bug-7996.php b/tests/PHPStan/Analyser/data/bug-7996.php
diff --git a/tests/PHPStan/Analyser/data/scope-generalization.php b/tests/PHPStan/Analyser/data/scope-generalization.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace ScopeGeneralization;
+
+use function PHPStan\Testing\assertType;
+
+function loopAddsAccessory(): void
+{
+	/** @var array<string> */
+	$foo = [];
+	for ($i = 0; $i < 3; $i++) {
+		array_push($foo, 'foo');
+	}
+	assertType('non-empty-array<string>', $foo);
+}
+
+function loopRemovesAccessory(): void
+{
+	/** @var non-empty-array<string> */
+	$foo = [];
+	for ($i = 0; $i < 3; $i++) {
+		array_pop($foo);
+	}
+	assertType('array<string>', $foo);
+}
+
+function closureRemovesAccessoryOfReferenceParameter(): void
+{
+	/** @var non-empty-array<string> */
+	$foo = [];
+	static function () use (&$foo) {
+		assertType('array<string>', $foo);
+		array_pop($foo);
+	};
+}
diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
@@ -746,10 +746,4 @@ public function testBug7904(): void
 		$this->analyse([__DIR__ . '/data/bug-7904.php'], []);
 	}
 
-	public function testBug7996(): void
-	{
-		$this->checkExplicitMixed = false;
-		$this->analyse([__DIR__ . '/../../Analyser/data/bug-7996.php'], []);
-	}
-
 }