Improve TypeCombinator::reduceArrays() perf with retained type comple…

…teness
phpstan · Sep 22, 2022 · c5729cc · c5729cc
1 parent 210c35e
commit c5729cc
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 58 deletions.
diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php
@@ -594,7 +594,7 @@ private static function processArrayTypes(array $arrayTypes, array $accessoryTyp
 		}
 		if (count($arrayTypes) === 1) {
 			return [
-				self::intersect(...self::optimizeConstantArrays($arrayTypes), ...$accessoryTypes),
+				self::intersect(...$arrayTypes, ...$accessoryTypes),
 			];
 		}
 
@@ -636,96 +636,79 @@ private static function processArrayTypes(array $arrayTypes, array $accessoryTyp
 			return [
 				self::intersect(new ArrayType(
 					self::union(...$keyTypesForGeneralArray),
-					self::union(...self::optimizeConstantArrays($valueTypesForGeneralArray)),
+					self::union(...$valueTypesForGeneralArray),
 				), ...$accessoryTypes),
 			];
 		}
 
-		$reducedArrayTypes = self::reduceArrays($arrayTypes);
-
 		return array_map(
 			static fn (Type $arrayType) => self::intersect($arrayType, ...$accessoryTypes),
-			self::optimizeConstantArrays($reducedArrayTypes),
+			self::reduceArrays($arrayTypes),
 		);
 	}
 
 	/**
-	 * @param Type[] $types
+	 * @param Type[] $constantArrays
 	 * @return Type[]
 	 */
-	private static function optimizeConstantArrays(array $types): array
+	private static function reduceArrays(array $constantArrays): array
 	{
-		$constantArrayValuesCount = self::countConstantArrayValueTypes($types);
-
-		if ($constantArrayValuesCount > ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {
-			$results = [];
-			foreach ($types as $type) {
-				$results[] = TypeTraverser::map($type, static function (Type $type, callable $traverse): Type {
-					if ($type instanceof ConstantArrayType) {
-						return $type->generalize(GeneralizePrecision::moreSpecific());
-					}
+		$newArrays = [];
+		$arraysToProcess = [];
+		$emptyArray = null;
+		foreach ($constantArrays as $constantArray) {
+			if (!$constantArray instanceof ConstantArrayType) {
+				$newArrays[] = $constantArray;
+				continue;
+			}
 
-					return $traverse($type);
-				});
+			if ($constantArray->isEmpty()) {
+				$emptyArray = $constantArray;
+				continue;
 			}
 
-			return $results;
+			$arraysToProcess[] = $constantArray;
 		}
 
-		return $types;
-	}
+		if ($emptyArray !== null) {
+			$newArrays[] = $emptyArray;
+		}
 
-	/**
-	 * @param Type[] $types
-	 */
-	private static function countConstantArrayValueTypes(array $types): int
-	{
-		$constantArrayValuesCount = 0;
-		foreach ($types as $type) {
-			if ($type instanceof ConstantArrayType) {
-				$constantArrayValuesCount += count($type->getValueTypes());
+		$arraysToProcessPerKey = [];
+		foreach ($arraysToProcess as $i => $arrayToProcess) {
+			foreach ($arrayToProcess->getKeyTypes() as $keyType) {
+				$arraysToProcessPerKey[$keyType->getValue()][] = $i;
 			}
+		}
 
-			TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$constantArrayValuesCount): Type {
-				if ($type instanceof ConstantArrayType) {
-					$constantArrayValuesCount += count($type->getValueTypes());
-				}
+		$eligibleCombinations = [];
 
-				return $traverse($type);
-			});
+		foreach ($arraysToProcessPerKey as $arrays) {
+			for ($i = 0, $arraysCount = count($arrays); $i < $arraysCount - 1; $i++) {
+				for ($j = $i + 1; $j < $arraysCount; $j++) {
+					$eligibleCombinations[$arrays[$i]][$arrays[$j]] = $arrays[$j];
+				}
+			}
 		}
-		return $constantArrayValuesCount;
-	}
 
-	/**
-	 * @param Type[] $constantArrays
-	 * @return Type[]
-	 */
-	private static function reduceArrays(array $constantArrays): array
-	{
-		$newArrays = [];
-		$arraysToProcess = [];
-		foreach ($constantArrays as $constantArray) {
-			if (!$constantArray instanceof ConstantArrayType) {
-				$newArrays[] = $constantArray;
+		foreach ($eligibleCombinations as $i => $other) {
+			if (!array_key_exists($i, $arraysToProcess)) {
 				continue;
 			}
 
-			$arraysToProcess[] = $constantArray;
-		}
+			foreach ($other as $j) {
+				if (!array_key_exists($j, $arraysToProcess)) {
+					continue;
+				}
 
-		for ($i = 0, $arraysToProcessCount = count($arraysToProcess); $i < $arraysToProcessCount; $i++) {
-			for ($j = $i + 1; $j < $arraysToProcessCount; $j++) {
 				if ($arraysToProcess[$j]->isKeysSupersetOf($arraysToProcess[$i])) {
 					$arraysToProcess[$j] = $arraysToProcess[$j]->mergeWith($arraysToProcess[$i]);
-					array_splice($arraysToProcess, $i--, 1);
-					$arraysToProcessCount--;
+					unset($arraysToProcess[$i]);
 					continue 2;
 
 				} elseif ($arraysToProcess[$i]->isKeysSupersetOf($arraysToProcess[$j])) {
 					$arraysToProcess[$i] = $arraysToProcess[$i]->mergeWith($arraysToProcess[$j]);
-					array_splice($arraysToProcess, $j--, 1);
-					$arraysToProcessCount--;
+					unset($arraysToProcess[$j]);
 					continue 1;
 				}
 			}

diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php
@@ -931,7 +931,19 @@ public function testBug7581(): void
 	public function testBug7903(): void
 	{
 		$errors = $this->runAnalyse(__DIR__ . '/data/bug-7903.php');
-		$this->assertNoErrors($errors);
+		$this->assertCount(6, $errors);
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[0]->getMessage());
+		$this->assertSame(212, $errors[0]->getLine());
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[1]->getMessage());
+		$this->assertSame(213, $errors[1]->getLine());
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[2]->getMessage());
+		$this->assertSame(214, $errors[2]->getLine());
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[3]->getMessage());
+		$this->assertSame(215, $errors[3]->getLine());
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[4]->getMessage());
+		$this->assertSame(229, $errors[4]->getLine());
+		$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[5]->getMessage());
+		$this->assertSame(230, $errors[5]->getLine());
 	}
 
 	public function testBug7901(): void