Skip to content

Commit d148589

Browse files
jbboehrjbboehr
committed
Preserve sound key order for constant array merges
Optional keys and union operands can produce different insertion orders, while a constant array shape stores only one. Keep the shape only when its order covers every bounded variant, with shortcuts for stable integer-only and disjoint string-key cases. Use the generic fallback when order cannot be established. Cover native merge, keys, values, and flip outcomes, including both single-operand union orders and optional-variant limits.
1 parent a45b58b commit d148589

3 files changed

Lines changed: 381 additions & 2 deletions

File tree

‎src/ArrayMergeType.php‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class ArrayMergeType implements CompoundType, LateResolvableType
6464
/** @phpstan-ignore-next-line phpstanApi.trait */
6565
use NonGeneralizableTypeTrait;
6666

67+
private const KEY_ORDER_VARIANT_LIMIT = 64;
68+
6769
/**
6870
* @param non-empty-list<Type> $types
6971
*/
@@ -282,7 +284,15 @@ protected function getResult(): Type
282284
}
283285
}
284286

285-
return $builder->getArray();
287+
$result = $builder->getArray();
288+
$constantResults = $result->getConstantArrays();
289+
290+
if (count($constantResults) !== 1 || self::hasConsistentKeyOrder($types, $constantResults[0])) {
291+
return $result;
292+
}
293+
294+
// Use the generic fallback when order cannot be proven. Combining shapes
295+
// with TypeCombinator::union() can erase their different key orders.
286296
}
287297
}
288298

@@ -644,6 +654,99 @@ private static function getConstantArrayKeyTypes(Type $type): array
644654
return $keyTypes;
645655
}
646656

657+
/** @param non-empty-list<Type> $types */
658+
private static function hasDisjointStringKeys(array $types): bool
659+
{
660+
$seenKeys = [];
661+
662+
foreach ($types as $type) {
663+
$constantArrays = $type->getConstantArrays();
664+
if (count($constantArrays) !== 1) {
665+
return false;
666+
}
667+
668+
foreach ($constantArrays[0]->getKeyTypes() as $keyType) {
669+
$constantStrings = self::normalizeArrayMergeKeyType($keyType)->getConstantStrings();
670+
if (count($constantStrings) !== 1 || isset($seenKeys[$constantStrings[0]->getValue()])) {
671+
return false;
672+
}
673+
674+
$seenKeys[$constantStrings[0]->getValue()] = true;
675+
}
676+
}
677+
678+
return true;
679+
}
680+
681+
/** @param non-empty-list<Type> $types */
682+
private static function hasConsistentKeyOrder(array $types, ConstantArrayType $result): bool
683+
{
684+
$keyPositions = [];
685+
$hasStringKey = false;
686+
687+
foreach ($result->getKeyTypes() as $position => $keyType) {
688+
$keyPositions[$keyType->getValue()] = $position;
689+
$hasStringKey = $hasStringKey || $keyType->isString()->yes();
690+
}
691+
692+
// Integer-only merge results always have their keys in ascending order.
693+
if (!$hasStringKey) {
694+
return true;
695+
}
696+
697+
// Optional absence cannot reorder disjoint string keys from single shapes.
698+
if (self::hasDisjointStringKeys($types)) {
699+
return true;
700+
}
701+
702+
$mergedKeyArrays = [[]];
703+
704+
foreach ($types as $type) {
705+
$nextKeyArrays = [];
706+
707+
foreach ($type->getConstantArrays() as $constantArray) {
708+
// getAllArrays() samples rather than exhausts large optional shapes.
709+
// Bound both expansion and the Cartesian product before accepting an order.
710+
if (2 ** count($constantArray->getOptionalKeys()) > self::KEY_ORDER_VARIANT_LIMIT) {
711+
return false;
712+
}
713+
714+
foreach ($constantArray->getAllArrays() as $variant) {
715+
$keys = [];
716+
foreach ($variant->getKeyTypes() as $keyType) {
717+
$keys[$keyType->getValue()] = true;
718+
}
719+
720+
foreach ($mergedKeyArrays as $mergedKeys) {
721+
if (count($nextKeyArrays) >= self::KEY_ORDER_VARIANT_LIMIT) {
722+
return false;
723+
}
724+
725+
$nextKeyArrays[] = array_merge($mergedKeys, $keys);
726+
}
727+
}
728+
}
729+
730+
$mergedKeyArrays = $nextKeyArrays;
731+
}
732+
733+
// Every possible runtime key sequence must be a subsequence of the shape's
734+
// order. Offset containment alone cannot establish this for optional keys.
735+
foreach ($mergedKeyArrays as $keys) {
736+
$previousPosition = -1;
737+
foreach ($keys as $key => $_) {
738+
$position = $keyPositions[$key] ?? -1;
739+
if ($position <= $previousPosition) {
740+
return false;
741+
}
742+
743+
$previousPosition = $position;
744+
}
745+
}
746+
747+
return true;
748+
}
749+
647750
/**
648751
* @param callable(Type): Type $cb
649752
*/

‎tests/ArrayMergeKeyOrderTest.php‎

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
<?php
2+
/**
3+
* Copyright (c) anno Domini nostri Jesu Christi MMXXVI John Boehr & contributors
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU Affero General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU Affero General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Affero General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
declare(strict_types=1);
19+
20+
namespace jbboehr\PHPStan\ArrayMerge\Tests;
21+
22+
use jbboehr\PHPStan\ArrayMerge\ArrayMergeType;
23+
use PHPStan\PhpDoc\TypeStringResolver;
24+
use PHPStan\Testing\PHPStanTestCase;
25+
use PHPStan\Type\Constant\ConstantArrayType;
26+
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
27+
use PHPStan\Type\Constant\ConstantIntegerType;
28+
use PHPStan\Type\Constant\ConstantStringType;
29+
use PHPStan\Type\Type;
30+
use PHPStan\Type\VerbosityLevel;
31+
use PHPUnit\Framework\Attributes\DataProvider;
32+
use function array_flip;
33+
use function array_keys;
34+
use function array_merge;
35+
use function array_values;
36+
use function implode;
37+
use function is_int;
38+
use function sprintf;
39+
40+
final class ArrayMergeKeyOrderTest extends PHPStanTestCase
41+
{
42+
public static function getAdditionalConfigFiles(): array
43+
{
44+
return [__DIR__ . '/../extension.neon'];
45+
}
46+
47+
/** @return iterable<string, array{string, non-empty-list<array<int|string, int|string>>}> */
48+
public static function orderProvider(): iterable
49+
{
50+
yield 'absent optional string prefix' => [
51+
'array-merge<array{a?: 1}, array{b: 2, a: 3}>',
52+
[[], ['b' => 2, 'a' => 3]],
53+
];
54+
yield 'present optional string prefix' => [
55+
'array-merge<array{a?: 1}, array{b: 2, a: 3}>',
56+
[['a' => 1], ['b' => 2, 'a' => 3]],
57+
];
58+
yield 'duplicate flipped values' => [
59+
"array-merge<array{a?: 'x'}, array{b: 'x', a: 'x'}>",
60+
[[], ['b' => 'x', 'a' => 'x']],
61+
];
62+
yield 'only the second optional key is present' => [
63+
'array-merge<array{a?: 1, b?: 2}, array{a: 3, b: 4}>',
64+
[['b' => 2], ['a' => 3, 'b' => 4]],
65+
];
66+
yield 'union prefix' => [
67+
'array-merge<array{a: 1}|array{b: 2}, array{b: 3, a: 4}>',
68+
[['b' => 2], ['b' => 3, 'a' => 4]],
69+
];
70+
yield 'first order in a single union operand' => [
71+
'array-merge<array{a: 1, b: 2}|array{b: 2, a: 1}>',
72+
[['a' => 1, 'b' => 2]],
73+
];
74+
yield 'opposite order in a single union operand' => [
75+
'array-merge<array{a: 1, b: 2}|array{b: 2, a: 1}>',
76+
[['b' => 2, 'a' => 1]],
77+
];
78+
yield 'optional integer before a string and an appended integer' => [
79+
'array-merge<array{0?: 1, b: 2}, array{3}>',
80+
[['b' => 2], [3]],
81+
];
82+
yield 'optional integer inside a single mixed operand' => [
83+
'array-merge<array{0?: 1, b: 2, 1: 3}>',
84+
[['b' => 2, 1 => 3]],
85+
];
86+
87+
$optionalFields = [];
88+
$tailFields = [];
89+
$tail = [];
90+
for ($i = 0; $i < 12; $i++) {
91+
$optionalFields[] = sprintf('k%d?: %d', $i, $i);
92+
if ($i < 11) {
93+
$tailFields[] = sprintf('k%d: %d', $i, $i);
94+
$tail['k' . $i] = $i;
95+
}
96+
}
97+
98+
// The empty, first-only, last-only, and full optional subsets all agree.
99+
// A middle-only subset exposes the order change that sampling would miss.
100+
yield 'large optional shape must not rely on sampled subsets' => [
101+
sprintf(
102+
'array-merge<array{k11: 11}, array{%s}, array{%s}>',
103+
implode(', ', $optionalFields),
104+
implode(', ', $tailFields),
105+
),
106+
[['k11' => 11], ['k5' => 5], $tail],
107+
];
108+
}
109+
110+
/** @param non-empty-list<array<int|string, int|string>> $operands */
111+
#[DataProvider('orderProvider')]
112+
public function testDerivedArraysContainNativeOutcomes(string $phpDoc, array $operands): void
113+
{
114+
$resolver = self::getContainer()->getByType(TypeStringResolver::class);
115+
$merge = $resolver->resolve($phpDoc);
116+
$this->assertInstanceOf(ArrayMergeType::class, $merge);
117+
$result = $merge->resolve();
118+
119+
$this->assertDerivedArraysContainNativeOutcome($phpDoc, $result, $operands);
120+
}
121+
122+
/** @return iterable<string, array{string, array{list<array<array-key, int>>, list<array<array-key, int>>}}> */
123+
public static function completeBranchProvider(): iterable
124+
{
125+
yield 'cross-product of optional string keys' => [
126+
'array-merge<array{a?: 1, b?: 2}, array{b?: 3, c: 4, a?: 5}>',
127+
[
128+
[[], ['a' => 1], ['b' => 2], ['a' => 1, 'b' => 2]],
129+
[['c' => 4], ['b' => 3, 'c' => 4], ['c' => 4, 'a' => 5], ['b' => 3, 'c' => 4, 'a' => 5]],
130+
],
131+
];
132+
yield 'opposite union orders followed by an optional prefix' => [
133+
'array-merge<array{a: 1, b: 2}|array{b: 3, a: 4}, array{a?: 5, c: 6}>',
134+
[
135+
[['a' => 1, 'b' => 2], ['b' => 3, 'a' => 4]],
136+
[['c' => 6], ['a' => 5, 'c' => 6]],
137+
],
138+
];
139+
yield 'optional integer and string keys' => [
140+
'array-merge<array{0?: 1, a?: 2, 1?: 3}, array{b: 4, 0?: 5}>',
141+
[
142+
[
143+
[],
144+
[1],
145+
['a' => 2],
146+
[1, 'a' => 2],
147+
[1 => 3],
148+
[0 => 1, 1 => 3],
149+
['a' => 2, 1 => 3],
150+
[0 => 1, 'a' => 2, 1 => 3],
151+
],
152+
[['b' => 4], ['b' => 4, 0 => 5]],
153+
],
154+
];
155+
}
156+
157+
/** @param array{list<array<int|string, int>>, list<array<int|string, int>>} $operandBranches */
158+
#[DataProvider('completeBranchProvider')]
159+
public function testEverySmallOptionalAndUnionBranchIsSound(string $phpDoc, array $operandBranches): void
160+
{
161+
$resolver = self::getContainer()->getByType(TypeStringResolver::class);
162+
$merge = $resolver->resolve($phpDoc);
163+
$this->assertInstanceOf(ArrayMergeType::class, $merge);
164+
$result = $merge->resolve();
165+
166+
foreach ($operandBranches[0] as $first) {
167+
foreach ($operandBranches[1] as $second) {
168+
$this->assertDerivedArraysContainNativeOutcome($phpDoc, $result, [$first, $second]);
169+
}
170+
}
171+
}
172+
173+
/** @param non-empty-list<array<int|string, int|string>> $operands */
174+
private function assertDerivedArraysContainNativeOutcome(string $phpDoc, Type $result, array $operands): void
175+
{
176+
$native = array_merge(...$operands);
177+
178+
$outcomes = [
179+
'merge' => [$result, $native],
180+
'values' => [$result->getValuesArray(), array_values($native)],
181+
'keys' => [$result->getKeysArray(), array_keys($native)],
182+
'flip' => [$result->flipArray(), array_flip($native)],
183+
];
184+
185+
foreach ($outcomes as $operation => [$inferred, $runtime]) {
186+
$expected = self::runtimeArrayType($runtime);
187+
$this->assertTrue($inferred->isSuperTypeOf($expected)->yes(), sprintf(
188+
'%s of %s inferred %s, excluding native outcome %s.',
189+
$operation,
190+
$phpDoc,
191+
$inferred->describe(VerbosityLevel::precise()),
192+
$expected->describe(VerbosityLevel::precise()),
193+
));
194+
}
195+
}
196+
197+
public function testStableOrderRetainsShapePrecision(): void
198+
{
199+
$resolver = self::getContainer()->getByType(TypeStringResolver::class);
200+
201+
$scenarios = [
202+
'array-merge<array{a?: 1}, array{b: 2}>' => 'array{a?: 1, b: 2}',
203+
'array-merge<array{a: 1, b: 2}, array{b: 3, a: 4}>' => 'array{a: 4, b: 3}',
204+
'array-merge<array{a: 1}|array{b: 2}>' => 'array{a?: 1, b?: 2}',
205+
'array-merge<array{0?: 1}, array{2}>' => 'array{0: 1|2, 1?: 2}',
206+
];
207+
208+
foreach ($scenarios as $phpDoc => $expected) {
209+
$merge = $resolver->resolve($phpDoc);
210+
$this->assertInstanceOf(ArrayMergeType::class, $merge);
211+
$this->assertTrue($resolver->resolve($expected)->equals($merge->resolve()), $phpDoc);
212+
}
213+
}
214+
215+
public function testExactlySixtyFourOptionalVariantsRetainStableShapePrecision(): void
216+
{
217+
$resolver = self::getContainer()->getByType(TypeStringResolver::class);
218+
$phpDoc = 'array-merge<array{k0?: 0, k1?: 1, k2?: 2, k3?: 3, k4?: 4, k5?: 5}, array{k5: 6}>';
219+
$expected = 'array{k0?: 0, k1?: 1, k2?: 2, k3?: 3, k4?: 4, k5: 6}';
220+
$merge = $resolver->resolve($phpDoc);
221+
222+
$this->assertInstanceOf(ArrayMergeType::class, $merge);
223+
$this->assertTrue($resolver->resolve($expected)->equals($merge->resolve()), $phpDoc);
224+
}
225+
226+
public function testDisjointStringKeysRetainShapePrecisionBeyondOptionalVariantLimit(): void
227+
{
228+
$resolver = self::getContainer()->getByType(TypeStringResolver::class);
229+
$phpDoc = 'array-merge<array{a?: 1, b?: 2, c?: 3, d?: 4, e?: 5, f?: 6, g?: 7}, array{h: 8}>';
230+
$expected = 'array{a?: 1, b?: 2, c?: 3, d?: 4, e?: 5, f?: 6, g?: 7, h: 8}';
231+
$merge = $resolver->resolve($phpDoc);
232+
233+
$this->assertInstanceOf(ArrayMergeType::class, $merge);
234+
$result = $merge->resolve();
235+
$this->assertTrue($resolver->resolve($expected)->equals($result), $result->describe(VerbosityLevel::precise()));
236+
}
237+
238+
public function testRawNumericStringKeysAreReindexedInOrder(): void
239+
{
240+
$merge = new ArrayMergeType([
241+
new ConstantArrayType(
242+
[new ConstantStringType('7'), new ConstantStringType('b')],
243+
[new ConstantIntegerType(1), new ConstantIntegerType(2)],
244+
[0],
245+
[0],
246+
),
247+
new ConstantArrayType([new ConstantStringType('8')], [new ConstantIntegerType(3)]),
248+
]);
249+
250+
$this->assertDerivedArraysContainNativeOutcome(
251+
$merge->describe(VerbosityLevel::precise()),
252+
$merge->resolve(),
253+
[['b' => 2], [8 => 3]],
254+
);
255+
}
256+
257+
/** @param array<int|string, int|string> $values */
258+
private static function runtimeArrayType(array $values): Type
259+
{
260+
$builder = ConstantArrayTypeBuilder::createEmpty();
261+
262+
foreach ($values as $key => $value) {
263+
$builder->setOffsetValueType(
264+
is_int($key) ? new ConstantIntegerType($key) : new ConstantStringType($key),
265+
is_int($value) ? new ConstantIntegerType($value) : new ConstantStringType($value),
266+
);
267+
}
268+
269+
return $builder->getArray();
270+
}
271+
}

0 commit comments

Comments
 (0)