Skip to content

Commit 032c128

Browse files
phpstan-botclaude
andcommitted
Answer isSubTypeOf() against a single value from the FiniteTypeSet too
`finiteTypeSetContainedIn()` only ever compared two unions, so a union of values asked about one value still walked its members. That is the shape `TypeCombinator::doRemove()` opens with: `$typeToRemove->isSuperTypeOf($fromType)` lands on `UnionType::isSubTypeOf()` through the `CompoundType` delegation, once per removal, over the whole union. A single value is a one-member set, so the map answers it: the union is under that value only if it has nothing else in it, and disjoint from it if the key is missing. `FiniteTypeSet::containedInKey()` is `containedIn()` for that case, and the helper now treats a non-union other type as the set holding just that value. `isAcceptedBy()` shares the helper and keeps its yes-only guard. As it happens no constant value accepts another one - not a numeric string an int, in either direction - so the guard cannot change an answer today; it stays because that is a fact about the current value types rather than something the map guarantees. Measured on the reproducer from phpstan/phpstan#15004, a `1|2|...|N` parameter narrowed by a chain of N `!==` clauses, `analyse -l 8` wall clock: | N | before the identity map | at HEAD | with this commit | | --- | --- | --- | --- | | 50 | 1.30 s | 1.20 s | 1.20 s | | 100 | 3.11 s | 2.70 s | 2.41 s | | 200 | 15.02 s | 12.92 s | 10.42 s | | 300 | 46.27 s | 39.57 s | 30.95 s | This is a constant factor, not the fix that issue wants: counting calls at N=100 shows 79674 `doRemove()`s costing 0.92 s, split 0.40 s `isSubTypeOf()` and 0.47 s `tryRemove()`, and both are O(n) per call against an O(n) chain of O(n) removals. Keying takes the first of the two down to a lookup; the growth stays ~O(N^2.7), which only stopping the removals themselves would change. The reference-implementation matrix now runs `isSubTypeOf()` and `isAcceptedBy()` over the 17x18 union/non-union pairs as well as the union pairs, and asserts that no member attaches a reason to an answer the map now gives on its behalf. Mutating the fix afterwards fails 399 cases (a miss answering yes), 1 (never answering yes), 92 (dropping the completeness gate) and 36 (keying an unkeyable other type). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4c0234d commit 032c128

3 files changed

Lines changed: 147 additions & 10 deletions

File tree

src/Type/FiniteTypeSet.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,29 @@ public function containedIn(self $other): TrinaryLogic
264264
return TrinaryLogic::createMaybe();
265265
}
266266

267+
/**
268+
* containedIn() against the one-member set holding just $key.
269+
*
270+
* Yes only when this set holds nothing besides that value, no when it does not hold it
271+
* at all, maybe in between - the same three answers as containedIn(), which is what a
272+
* single value is being compared as here.
273+
*
274+
* Only keyed members are compared - call isComplete() first when the answer has to
275+
* hold for the whole union.
276+
*/
277+
public function containedInKey(string $key): TrinaryLogic
278+
{
279+
if (!$this->has($key)) {
280+
return TrinaryLogic::createNo();
281+
}
282+
283+
if (count($this->members) === 1) {
284+
return TrinaryLogic::createYes();
285+
}
286+
287+
return TrinaryLogic::createMaybe();
288+
}
289+
267290
/**
268291
* Whether a constant string member might also be a class-string.
269292
*

src/Type/UnionType.php

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -394,23 +394,48 @@ public function isAcceptedBy(Type $acceptingType, bool $strictTypes): AcceptsRes
394394
}
395395

396396
/**
397-
* Whether the other union holds every member of this one, or null when the question
398-
* cannot be settled from the identity maps alone.
397+
* Whether $otherType holds every member of this one, or null when the question cannot
398+
* be settled from the identity maps alone.
399+
*
400+
* $otherType is compared as a set of values: a union through its own map, a single
401+
* finite value as the one-member set holding it.
399402
*
400403
* $yesOnly skips the completeness requirement on the other union: a member missing from
401404
* its map only rules out the yes answer, which is all the caller is after.
402405
*/
403406
private function finiteTypeSetContainedIn(Type $otherType, bool $yesOnly): ?TrinaryLogic
404407
{
405-
if (!$otherType instanceof self || $otherType instanceof TemplateType) {
406-
return null;
407-
}
408-
409408
$finiteTypeSet = $this->getFiniteTypeSet();
410409
if ($finiteTypeSet === null || !$finiteTypeSet->isComplete()) {
411410
return null;
412411
}
413412

413+
if (!$otherType instanceof self || $otherType instanceof TemplateType) {
414+
// A single value covers a member iff they are the same value, and no member
415+
// stands for a value it is not keyed by - so one lookup answers for every member
416+
// at once. This is the shape TypeCombinator::remove() takes to ask whether
417+
// removing a value empties a union of values, and the only way a non-union other
418+
// type reaches this helper at all.
419+
$key = FiniteTypeSet::key($otherType);
420+
if ($key === null) {
421+
return null;
422+
}
423+
424+
$containment = $finiteTypeSet->containedInKey($key);
425+
426+
// No constant value accepts another one - not even a numeric string an int, in
427+
// either direction - so today accepts() agrees with isSuperTypeOf() here and this
428+
// guard never changes an answer. It is kept because that is a fact about the
429+
// current value types rather than something the identity map guarantees: a value
430+
// the map does not hold being accepted anyway is exactly what accepts() is
431+
// allowed to do, and all isAcceptedBy() wants from the map is the yes.
432+
if (!$containment->yes() && $yesOnly) {
433+
return null;
434+
}
435+
436+
return $containment;
437+
}
438+
414439
$otherFiniteTypeSet = $otherType->getFiniteTypeSet();
415440
if ($otherFiniteTypeSet === null) {
416441
return null;

tests/PHPStan/Type/FiniteTypeSetTest.php

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,22 @@ public function testAcceptsUnion(UnionType $type, UnionType $otherType): void
154154
$this->assertAccepts($type, $otherType);
155155
}
156156

157-
private function assertAccepts(UnionType $type, Type $otherType): void
157+
/**
158+
* Whether the map alone settles a comparison of $type against $otherType - the whole
159+
* union is keyed, and $otherType is one value to look up in it.
160+
*/
161+
private static function answersFromTheMap(UnionType $type, Type $otherType): bool
158162
{
159163
$finiteTypeSet = $type->getFiniteTypeSet();
160-
$answersPerKind = FiniteTypeSet::key($otherType) !== null
164+
165+
return FiniteTypeSet::key($otherType) !== null
161166
&& $finiteTypeSet !== null
162167
&& $finiteTypeSet->isComplete();
168+
}
169+
170+
private function assertAccepts(UnionType $type, Type $otherType): void
171+
{
172+
$answersPerKind = self::answersFromTheMap($type, $otherType);
163173

164174
foreach ([true, false] as $strictTypes) {
165175
$this->assertSame(
@@ -198,17 +208,35 @@ public function testTryRemove(UnionType $type, Type $otherType): void
198208
}
199209

200210
#[DataProvider('dataUnionPairs')]
201-
public function testIsSubTypeOf(UnionType $type, UnionType $otherType): void
211+
#[DataProvider('dataUnionAndOtherType')]
212+
public function testIsSubTypeOf(UnionType $type, Type $otherType): void
202213
{
203214
$this->assertSame(
204215
self::referenceIsSubTypeOf($type, $otherType)->describe(),
205216
$type->isSubTypeOf($otherType)->result->describe(),
206217
sprintf('%s -> isSubTypeOf(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())),
207218
);
219+
220+
if (!self::answersFromTheMap($type, $otherType)) {
221+
return;
222+
}
223+
224+
// One key lookup stands in for asking every member, which is only invisible as long
225+
// as none of them attaches a reason to its answer - reasons are per member.
226+
foreach ($type->getTypes() as $innerType) {
227+
$result = $otherType->isSuperTypeOf($innerType);
228+
$this->assertSame(
229+
[],
230+
$result->reasons,
231+
sprintf('%s -> isSuperTypeOf(%s)', $otherType->describe(VerbosityLevel::precise()), $innerType->describe(VerbosityLevel::precise())),
232+
);
233+
$this->assertSame([], $result->lazyReasons);
234+
}
208235
}
209236

210237
#[DataProvider('dataUnionPairs')]
211-
public function testIsAcceptedBy(UnionType $type, UnionType $otherType): void
238+
#[DataProvider('dataUnionAndOtherType')]
239+
public function testIsAcceptedBy(UnionType $type, Type $otherType): void
212240
{
213241
foreach ([true, false] as $strictTypes) {
214242
$this->assertSame(
@@ -484,6 +512,67 @@ public function testContainedIn(array $types, array $otherTypes, TrinaryLogic $e
484512
$this->assertSame($expected->describe(), $set->containedIn($otherSet)->describe());
485513
}
486514

515+
/**
516+
* @return Iterator<string, array{list<Type>, Type, TrinaryLogic}>
517+
*/
518+
public static function dataContainedInKey(): Iterator
519+
{
520+
// the only way a set is wholly under one value is by being that one value
521+
yield 'the only member' => [[new ConstantStringType('a')], new ConstantStringType('a'), TrinaryLogic::createYes()];
522+
yield 'one of two members' => [
523+
[new ConstantStringType('a'), new ConstantStringType('b')],
524+
new ConstantStringType('a'),
525+
TrinaryLogic::createMaybe(),
526+
];
527+
yield 'the last of many members' => [
528+
[new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c')],
529+
new ConstantStringType('c'),
530+
TrinaryLogic::createMaybe(),
531+
];
532+
yield 'not held' => [
533+
[new ConstantStringType('a'), new ConstantStringType('b')],
534+
new ConstantStringType('z'),
535+
TrinaryLogic::createNo(),
536+
];
537+
yield 'not held by a single-member set' => [
538+
[new ConstantStringType('a')],
539+
new ConstantStringType('z'),
540+
TrinaryLogic::createNo(),
541+
];
542+
// the key carries the kind, so the same value of another kind is not held either
543+
yield 'the same value of another kind' => [
544+
[new ConstantIntegerType(1)],
545+
new ConstantStringType('1'),
546+
TrinaryLogic::createNo(),
547+
];
548+
yield 'one of two kinds' => [
549+
[new ConstantStringType('a'), new NullType()],
550+
new NullType(),
551+
TrinaryLogic::createMaybe(),
552+
];
553+
}
554+
555+
/**
556+
* @param list<Type> $types
557+
*/
558+
#[DataProvider('dataContainedInKey')]
559+
public function testContainedInKey(array $types, Type $value, TrinaryLogic $expected): void
560+
{
561+
$set = FiniteTypeSet::create($types);
562+
$this->assertNotNull($set);
563+
564+
$key = FiniteTypeSet::key($value);
565+
$this->assertNotNull($key);
566+
567+
$this->assertSame($expected->describe(), $set->containedInKey($key)->describe());
568+
569+
// the same answer containedIn() gives against the set of just that one value, which
570+
// is what a single value is being compared as
571+
$singleMemberSet = FiniteTypeSet::create([$value]);
572+
$this->assertNotNull($singleMemberSet);
573+
$this->assertSame($expected->describe(), $set->containedIn($singleMemberSet)->describe());
574+
}
575+
487576
public function testUnkeyableMembersDoNotDefeatTheSet(): void
488577
{
489578
$set = (new UnionType([

0 commit comments

Comments
 (0)