From 6705ac1bb69bf61d46f1b83cc3c722155a97852f Mon Sep 17 00:00:00 2001 From: schlndh Date: Wed, 1 May 2024 21:21:36 +0200 Subject: [PATCH] Preserve large arrays with same keys through union --- src/Type/Constant/ConstantArrayType.php | 2 +- src/Type/TypeCombinator.php | 45 ++++++-- .../Analyser/NodeScopeResolverTest.php | 3 + tests/PHPStan/Analyser/data/bug-10080.php | 76 +++++++++++++ tests/PHPStan/Analyser/data/bug-9397.php | 101 ++++++++++++++++++ .../data/preserve-large-constant-array.php | 67 ++++++++++++ 6 files changed, 284 insertions(+), 10 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-10080.php create mode 100644 tests/PHPStan/Analyser/data/bug-9397.php create mode 100644 tests/PHPStan/Analyser/data/preserve-large-constant-array.php diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 210cede622..1d520429bc 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -1557,7 +1557,7 @@ public function isKeysSupersetOf(self $otherArray): bool public function mergeWith(self $otherArray): self { - // only call this after verifying isKeysSupersetOf + // only call this after verifying isKeysSupersetOf, or if losing tagged unions is not an issue $valueTypes = $this->valueTypes; $optionalKeys = $this->optionalKeys; foreach ($this->keyTypes as $i => $keyType) { diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php index b4c52cd4cf..5159711e6f 100644 --- a/src/Type/TypeCombinator.php +++ b/src/Type/TypeCombinator.php @@ -643,7 +643,7 @@ private static function processArrayAccessoryTypes(array $arrayTypes): array } /** - * @param Type[] $arrayTypes + * @param list $arrayTypes * @return Type[] */ private static function processArrayTypes(array $arrayTypes): array @@ -669,9 +669,14 @@ private static function processArrayTypes(array $arrayTypes): array /** @var int|float $nextConstantKeyTypeIndex */ $nextConstantKeyTypeIndex = 1; + $constantArraysMap = array_map( + static fn (Type $t) => $t->getConstantArrays(), + $arrayTypes, + ); - foreach ($arrayTypes as $arrayType) { - $isConstantArray = $arrayType->isConstantArray()->yes(); + foreach ($arrayTypes as $arrayIdx => $arrayType) { + $constantArrays = $constantArraysMap[$arrayIdx]; + $isConstantArray = $constantArrays !== []; if (!$isConstantArray || !$arrayType->isIterableAtLeastOnce()->no()) { $filledArrays++; } @@ -708,6 +713,10 @@ private static function processArrayTypes(array $arrayTypes): array } if ($generalArrayOccurred && (!$overflowed || $filledArrays > 1)) { + $reducedArrayTypes = self::reduceArrays($arrayTypes, false); + if (count($reducedArrayTypes) === 1) { + return [self::intersect($reducedArrayTypes[0], ...$accessoryTypes)]; + } $scopes = []; $useTemplateArray = true; foreach ($arrayTypes as $arrayType) { @@ -740,7 +749,7 @@ private static function processArrayTypes(array $arrayTypes): array ]; } - $reducedArrayTypes = self::reduceArrays($arrayTypes); + $reducedArrayTypes = self::reduceArrays($arrayTypes, true); return array_map( static fn (Type $arrayType) => self::intersect($arrayType, ...$accessoryTypes), @@ -833,16 +842,21 @@ private static function countConstantArrayValueTypes(array $types): int } /** - * @param Type[] $constantArrays - * @return Type[] + * @param list $constantArrays + * @return list */ - private static function reduceArrays(array $constantArrays): array + private static function reduceArrays(array $constantArrays, bool $preserveTaggedUnions): array { $newArrays = []; $arraysToProcess = []; $emptyArray = null; foreach ($constantArrays as $constantArray) { if (!$constantArray->isConstantArray()->yes()) { + // This is an optimization for current use-case of $preserveTaggedUnions=false, where we need + // one constant array as a result, or we generalize the $constantArrays. + if (!$preserveTaggedUnions) { + return $constantArrays; + } $newArrays[] = $constantArray; continue; } @@ -888,7 +902,8 @@ private static function reduceArrays(array $constantArrays): array } if ( - $overlappingKeysCount === count($arraysToProcess[$i]->getKeyTypes()) + $preserveTaggedUnions + && $overlappingKeysCount === count($arraysToProcess[$i]->getKeyTypes()) && $arraysToProcess[$j]->isKeysSupersetOf($arraysToProcess[$i]) ) { $arraysToProcess[$j] = $arraysToProcess[$j]->mergeWith($arraysToProcess[$i]); @@ -897,13 +912,25 @@ private static function reduceArrays(array $constantArrays): array } if ( - $overlappingKeysCount === count($arraysToProcess[$j]->getKeyTypes()) + $preserveTaggedUnions + && $overlappingKeysCount === count($arraysToProcess[$j]->getKeyTypes()) && $arraysToProcess[$i]->isKeysSupersetOf($arraysToProcess[$j]) ) { $arraysToProcess[$i] = $arraysToProcess[$i]->mergeWith($arraysToProcess[$j]); unset($arraysToProcess[$j]); continue 1; } + + if ( + !$preserveTaggedUnions + // both arrays have same keys + && $overlappingKeysCount === count($arraysToProcess[$i]->getKeyTypes()) + && $overlappingKeysCount === count($arraysToProcess[$j]->getKeyTypes()) + ) { + $arraysToProcess[$j] = $arraysToProcess[$j]->mergeWith($arraysToProcess[$i]); + unset($arraysToProcess[$i]); + continue 2; + } } } diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 56f9d00d14..b52ebeef3a 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -1420,6 +1420,9 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/trigger-error-php7.php'); } + yield from $this->gatherAssertTypes(__DIR__ . '/data/preserve-large-constant-array.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9397.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10080.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/impure-error-log.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/falsy-isset.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/falsey-coalesce.php'); diff --git a/tests/PHPStan/Analyser/data/bug-10080.php b/tests/PHPStan/Analyser/data/bug-10080.php new file mode 100644 index 0000000000..1875d50dfd --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-10080.php @@ -0,0 +1,76 @@ + + * If the above type has 63 or more properties, the bug occurs + */ + private static function callable(): array { + return []; + } + + public function callsite(): void { + $result = self::callable(); + foreach ($result as $id => $p) { + assertType(Money::class, $p['foo1']); + assertType(Money::class . '|null', $p['foo2']); + assertType('string', $p['foo3']); + + $baseDeposit = $p['foo2'] ?? Money::zero(); + assertType(Money::class, $p['foo1']); + assertType(Money::class . '|null', $p['foo2']); + assertType('string', $p['foo3']); + } + } +} diff --git a/tests/PHPStan/Analyser/data/preserve-large-constant-array.php b/tests/PHPStan/Analyser/data/preserve-large-constant-array.php new file mode 100644 index 0000000000..a66425e3dd --- /dev/null +++ b/tests/PHPStan/Analyser/data/preserve-large-constant-array.php @@ -0,0 +1,67 @@ +