From 88e5338e3469fedd5115677969502567ce98c7a9 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:46:14 +0000 Subject: [PATCH 1/8] Answer `UnionType` comparisons from an identity-keyed `FiniteTypeSet` instead of scanning every member * Add `PHPStan\Type\FiniteTypeSet`: a union's members indexed by value identity - null, constant scalars other than floats, and bare enum cases. Members that cannot be keyed are kept aside so one object type next to fifty constant strings does not defeat the lookup. * `UnionType` builds the set lazily once per instance and answers `isSuperTypeOf()`, `isSubTypeOf()`, `accepts()`, `isAcceptedBy()`, `equals()` and `tryRemove()` from it, turning O(n*m) member comparisons into O(n+m) and single-value lookups into O(1). * `accepts()` cannot read a negative answer off the map (scalar coercion is not value identity), so for a value the union does not hold it consults one member per kind instead of all of them - members of a kind answer `accepts()` alike for a value none of them holds. * `TypeCombinator::finiteUnionMembers()` now uses the same cached set instead of keying the union again on every `intersect()`, keeping its class-string bail via `FiniteTypeSet::hasClassStringMember()`. * Same treatment for every finite kind, not just constant strings: constant ints, bools, null and enum cases share the keying. Floats stay out - `equals()` does not agree with value identity for them (-0.0 === 0.0, NAN !== NAN). * `SkipTestsWithRequiresPhpAttributeRule` checked for `PHP_VERSION_ID` on the left of the comparison only after demanding an inverse operator, so a test method starting with an `if` on any other binary operator crashed the analysis. Ask about `PHP_VERSION_ID` first. * New `FiniteTypeSetTest` compares every one of those operations against a reference implementation of the member-by-member loop, over a matrix of union and query types. `tests/bench/data/big-constant-string-union.php` covers it in the benchmark suite. --- .../SkipTestsWithRequiresPhpAttributeRule.php | 18 +- src/Type/FiniteTypeSet.php | 271 ++++++++++ src/Type/TypeCombinator.php | 59 +-- src/Type/UnionType.php | 153 +++++- tests/PHPStan/Type/FiniteTypeSetTest.php | 465 ++++++++++++++++++ .../bench/data/big-constant-string-union.php | 233 +++++++++ 6 files changed, 1143 insertions(+), 56 deletions(-) create mode 100644 src/Type/FiniteTypeSet.php create mode 100644 tests/PHPStan/Type/FiniteTypeSetTest.php create mode 100644 tests/bench/data/big-constant-string-union.php diff --git a/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php b/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php index 05de1553e22..12f24f79882 100644 --- a/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php +++ b/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php @@ -52,6 +52,16 @@ public function processNode(Node $node, Scope $scope): array return []; } + // Before asking for an inverse: anything that is not a PHP_VERSION_ID comparison is + // none of this rule's business, and there is no inverse to name for it. + if (!$firstStmt->cond->left instanceof Node\Expr\ConstFetch || $firstStmt->cond->left->name->toString() !== 'PHP_VERSION_ID') { + return []; + } + + if (!$firstStmt->cond->right instanceof Node\Scalar\Int_) { + return []; + } + switch (get_class($firstStmt->cond)) { case Node\Expr\BinaryOp\SmallerOrEqual::class: $inverseBinaryOpSigil = '>'; @@ -75,14 +85,6 @@ public function processNode(Node $node, Scope $scope): array throw new ShouldNotHappenException('No inverse comparison specified for ' . get_class($firstStmt->cond)); } - if (!$firstStmt->cond->left instanceof Node\Expr\ConstFetch || $firstStmt->cond->left->name->toString() !== 'PHP_VERSION_ID') { - return []; - } - - if (!$firstStmt->cond->right instanceof Node\Scalar\Int_) { - return []; - } - if (count($firstStmt->stmts) !== 1) { return []; } diff --git a/src/Type/FiniteTypeSet.php b/src/Type/FiniteTypeSet.php new file mode 100644 index 00000000000..73801503737 --- /dev/null +++ b/src/Type/FiniteTypeSet.php @@ -0,0 +1,271 @@ + $members + * @param array $membersByKind + * @param list $others + */ + private function __construct(private array $members, private array $membersByKind, private array $others) + { + } + + /** + * Returns null when none of the types is a finite value - there is nothing to look up + * then, and the caller would only pay for building an empty map. + * + * Two types standing for the same value are not merged: the second one goes to $others + * so that the set never claims a union has fewer members than it does. + * + * @param list $types + */ + public static function create(array $types): ?self + { + $members = []; + $membersByKind = []; + $others = []; + foreach ($types as $type) { + $keyAndKind = self::keyAndKind($type); + if ($keyAndKind === null || array_key_exists($keyAndKind[0], $members)) { + $others[] = $type; + continue; + } + + [$key, $kind] = $keyAndKind; + $members[$key] = $type; + $membersByKind[$kind] ??= $type; + } + + if ($members === []) { + return null; + } + + return new self($members, $membersByKind, $others); + } + + /** + * Identity key of a single finite value: two types share a key iff they are equals(), + * and types with different keys are disjoint. + * + * Returns null for anything else. Floats are excluded because equals() does not agree + * with value identity for them (-0.0 === 0.0, NAN !== NAN). A type that merely contains + * a finite value - an intersection with an accessory type, a whole single-case enum, a + * conditional type resolving to a constant - is excluded by the equals() check: only a + * type that *is* the value can stand in for it. Template types are excluded outright, + * their comparison semantics are not value identity. + */ + public static function key(Type $type): ?string + { + return self::keyAndKind($type)[0] ?? null; + } + + /** + * The identity key together with the kind of value it is. + * + * Members of one kind are the same type class - and, for enum cases, of the same enum - + * so they answer accepts() identically for every value none of them holds. The kind is + * what makes one member stand in for all its siblings there. + * + * @return array{string, string}|null + */ + private static function keyAndKind(Type $type): ?array + { + if ($type instanceof TemplateType) { + return null; + } + + // Only a bare case is safe to key by class + case name: for anything else - + // $this & Enum::C, a whole single-case enum, an enum subtracted to one case - + // EnumCaseObjectType::equals() is false because it requires an EnumCaseObjectType, + // which makes instanceof exactly the question being asked here. Type::getEnumCases() + // would answer it too, but only by resolving a ClassReflection - and a key has to be + // derivable from the type alone, on every comparison, without reflection. + // Key by class + case name, the identity equals() compares (describe() would also + // fold in a subtracted type, which equals() ignores). + if ($type instanceof EnumCaseObjectType) { // @phpstan-ignore phpstanApi.instanceofType + $kind = self::ENUM_CASE_KEY_PREFIX . $type->getClassName(); + + return [$kind . '::' . $type->getEnumCaseName(), $kind]; + } + + if (!$type->isConstantScalarValue()->yes()) { + return null; + } + + $scalarTypes = $type->getConstantScalarTypes(); + if (count($scalarTypes) !== 1 || !$scalarTypes[0]->equals($type)) { + return null; + } + + $value = $scalarTypes[0]->getValue(); + if ($value === null) { + return [self::NULL_KEY, self::NULL_KEY]; + } + if (is_int($value)) { + return [self::INTEGER_KEY_PREFIX . $value, self::INTEGER_KEY_PREFIX]; + } + if (is_bool($value)) { + return [self::BOOLEAN_KEY_PREFIX . ($value ? '1' : '0'), self::BOOLEAN_KEY_PREFIX]; + } + if (is_string($value)) { + return [self::STRING_KEY_PREFIX . $value, self::STRING_KEY_PREFIX]; + } + + return null; + } + + /** + * One member per kind other than $type's own, in the union's order. + * + * For a value the set does not hold, every member of $type's kind answers accepts() no, + * and the remaining members answer per kind - so or()-ing over these few is the same + * answer as or()-ing over all of them. + * + * @return list + */ + public function getRepresentativesOfOtherKinds(Type $type): array + { + $kind = self::keyAndKind($type)[1] ?? null; + $representatives = []; + foreach ($this->membersByKind as $memberKind => $member) { + if ($memberKind === $kind) { + continue; + } + + $representatives[] = $member; + } + + return $representatives; + } + + public function has(string $key): bool + { + return array_key_exists($key, $this->members); + } + + /** Whether every member of the union is keyed, so the map answers for the whole union. */ + public function isComplete(): bool + { + return $this->others === []; + } + + /** + * Members in the union's own order. + * + * @return array + */ + public function getMembers(): array + { + return $this->members; + } + + /** @return list */ + public function getOthers(): array + { + return $this->others; + } + + /** + * Yes when every keyed member is also in $other, no when none of them is. + * + * Only keyed members are compared - call isComplete() first when the answer has to + * hold for the whole union. + */ + public function containedIn(self $other): TrinaryLogic + { + $contained = 0; + foreach (array_keys($this->members) as $key) { + if (!$other->has($key)) { + continue; + } + + $contained++; + } + + if ($contained === count($this->members)) { + return TrinaryLogic::createYes(); + } + + if ($contained === 0) { + return TrinaryLogic::createNo(); + } + + return TrinaryLogic::createMaybe(); + } + + /** + * Whether a constant string member might also be a class-string. + * + * The class-string flag is part of a constant string's representation but not of its + * value, so operations that pick a member to hand back - as opposed to merely comparing + * values - cannot treat two same-valued constant strings as interchangeable. Answering + * this costs a reflection lookup per string member, so it is computed on demand: only + * combining operations ask. + */ + public function hasClassStringMember(): bool + { + if ($this->hasClassStringMember !== null) { + return $this->hasClassStringMember; + } + + $this->hasClassStringMember = false; + foreach ($this->members as $member) { + if (!$member->isString()->yes()) { + continue; + } + if ($member->isClassString()->no()) { + continue; + } + + $this->hasClassStringMember = true; + break; + } + + return $this->hasClassStringMember; + } + +} diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php index 16445c1e6d6..1c6b04d0d60 100644 --- a/src/Type/TypeCombinator.php +++ b/src/Type/TypeCombinator.php @@ -43,9 +43,7 @@ use function get_class; use function implode; use function in_array; -use function is_bool; use function is_int; -use function is_string; use function sprintf; use function usort; use const PHP_INT_MAX; @@ -1581,60 +1579,27 @@ private static function intersectFiniteUnions(UnionType $a, UnionType $b): ?Type } /** - * Keys a union's members by identity for the finite-union fast path in intersect(). + * The union's members keyed by identity, or null when the fast path does not apply. * - * Handles constant scalars and enum cases: each stands for one concrete value, so two - * members are interchangeable iff they share a key and are otherwise disjoint. Returns - * null if any member is not such a value. Class-string constant strings are excluded - * (the class-string flag is not captured by the value) and floats are excluded (-0.0 / - * NAN comparison quirks). Enum cases are keyed by class + case name, the identity - * EnumCaseObjectType::equals() compares. + * FiniteTypeSet does the keying and the union caches it; on top of that, intersect() + * hands one of the two members back instead of only comparing them, so a constant string + * that is also a class-string is not interchangeable with a same-valued one that is not - + * the class-string flag would be lost. Such unions go the slow way. * * @return array|null */ private static function finiteUnionMembers(UnionType $union): ?array { - $members = []; - foreach ($union->getTypes() as $member) { - $enumCase = $member->getEnumCaseObject(); - if ($member->isNull()->yes()) { - $key = 'null'; - } elseif ($enumCase !== null) { - // getEnumCaseObject() also returns the case for a refined member - an - // intersection like $this & Enum::C, a whole single-case enum, or an enum - // subtracted to one case - none of which are a bare EnumCaseObjectType. - // Only a bare case is safe to key by class + case name; for the rest, - // EnumCaseObjectType::equals() is false (it requires an EnumCaseObjectType), - // so bail to the slow path rather than collapse the refinement. - if (!$enumCase->equals($member)) { - return null; - } - - // Key by class + case name, the identity EnumCaseObjectType::equals() compares - // (describe() would also fold in a subtracted type, which equals() ignores). - $key = 'enum:' . $enumCase->getClassName() . '::' . $enumCase->getEnumCaseName(); - } else { - $values = $member->getConstantScalarValues(); - if (count($values) !== 1) { - return null; - } - - $value = $values[0]; - if (is_int($value)) { - $key = 'i:' . $value; - } elseif (is_bool($value)) { - $key = $value ? 'b:1' : 'b:0'; - } elseif (is_string($value) && $member->isClassString()->no()) { - $key = 's:' . $value; - } else { - return null; - } - } + $finiteTypeSet = $union->getFiniteTypeSet(); + if ($finiteTypeSet === null || !$finiteTypeSet->isComplete()) { + return null; + } - $members[$key] = $member; + if ($finiteTypeSet->hasClassStringMember()) { + return null; } - return $members; + return $finiteTypeSet->getMembers(); } public static function intersect(Type ...$types): Type diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index 011cdec9389..f07aab71384 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -74,6 +74,12 @@ class UnionType implements CompoundType /** @var array */ private array $cachedDescriptions = []; + /** + * Identity-keyed view of $types, built on first use. False once it is known that the + * members cannot be keyed at all, so a union of objects pays for the attempt only once. + */ + private FiniteTypeSet|false|null $finiteTypeSet = null; + /** * @api * @param list $types @@ -138,6 +144,17 @@ public function isNormalized(): bool return $this->normalized; } + /** @internal */ + public function getFiniteTypeSet(): ?FiniteTypeSet + { + $finiteTypeSet = $this->finiteTypeSet ??= FiniteTypeSet::create($this->types) ?? false; + if ($finiteTypeSet === false) { + return null; + } + + return $finiteTypeSet; + } + /** * @return list */ @@ -200,6 +217,41 @@ public function getConstantStrings(): array public function accepts(Type $type, bool $strictTypes): AcceptsResult { + $finiteTypeSet = $this->getFiniteTypeSet(); + if ($finiteTypeSet !== null) { + $key = FiniteTypeSet::key($type); + if ($key !== null && $finiteTypeSet->has($key)) { + return AcceptsResult::createYes(); + } + + if ($finiteTypeSet->isComplete()) { + if ($key !== null) { + // A value the union does not hold can still be accepted through scalar + // coercion, so unlike isSuperTypeOf() the answer is not simply no - but it + // is the same for every member of a kind, so one member of each is enough. + // None of the branches below apply to a value: it is not iterable, not + // compound, and an enum case already is the union of its own cases. + $result = AcceptsResult::createNo(); + foreach ($finiteTypeSet->getRepresentativesOfOtherKinds($type) as $representative) { + $result = $result->or($representative->accepts($type, $strictTypes)); + } + + return $result; + } + + if ($type instanceof self && !$type instanceof TemplateType) { + $otherFiniteTypeSet = $type->getFiniteTypeSet(); + if ($otherFiniteTypeSet !== null && $otherFiniteTypeSet->isComplete()) { + // A member standing for a single value never accepts more than one of + // the other union's values, and the other union has at least two of + // them - so the member-by-member or() below cannot come out yes, and + // its result is discarded in favour of the compound answer anyway. + return $type->isAcceptedBy($this, $strictTypes); + } + } + } + } + if ($type instanceof IterableType) { return $this->accepts($type->toArrayOrTraversable(), $strictTypes); } @@ -276,8 +328,27 @@ public function isSuperTypeOf(Type $otherType): IsSuperTypeOfResult return $otherType->isSubTypeOf($this); } + $types = $this->types; + $finiteTypeSet = $this->getFiniteTypeSet(); + if ($finiteTypeSet !== null) { + $key = FiniteTypeSet::key($otherType); + if ($key !== null) { + if ($finiteTypeSet->has($key)) { + return IsSuperTypeOfResult::createYes(); + } + + // Every keyed member stands for a different value than $otherType, so all of + // them answer no - only the members that could not be keyed are left to ask. + if ($finiteTypeSet->isComplete()) { + return IsSuperTypeOfResult::createNo(); + } + + $types = $finiteTypeSet->getOthers(); + } + } + $results = []; - foreach ($this->types as $innerType) { + foreach ($types as $innerType) { $result = $innerType->isSuperTypeOf($otherType); if ($result->yes()) { return $result; @@ -298,14 +369,61 @@ public function isSuperTypeOf(Type $otherType): IsSuperTypeOfResult public function isSubTypeOf(Type $otherType): IsSuperTypeOfResult { + $containment = $this->finiteTypeSetContainedIn($otherType, false); + if ($containment !== null) { + if ($containment->maybe()) { + return IsSuperTypeOfResult::createMaybe(); + } + + return IsSuperTypeOfResult::createFromBoolean($containment->yes()); + } + return IsSuperTypeOfResult::extremeIdentity(...array_map(static fn (Type $innerType) => $otherType->isSuperTypeOf($innerType), $this->types)); } public function isAcceptedBy(Type $acceptingType, bool $strictTypes): AcceptsResult { + // Unlike isSubTypeOf() only the positive answer holds: a member $acceptingType does + // not hold can still be accepted through scalar coercion. + $containment = $this->finiteTypeSetContainedIn($acceptingType, true); + if ($containment !== null && $containment->yes()) { + return AcceptsResult::createYes(); + } + return AcceptsResult::extremeIdentity(...array_map(static fn (Type $innerType) => $acceptingType->accepts($innerType, $strictTypes), $this->types)); } + /** + * Whether the other union holds every member of this one, or null when the question + * cannot be settled from the identity maps alone. + * + * $yesOnly skips the completeness requirement on the other union: a member missing from + * its map only rules out the yes answer, which is all the caller is after. + */ + private function finiteTypeSetContainedIn(Type $otherType, bool $yesOnly): ?TrinaryLogic + { + if (!$otherType instanceof self || $otherType instanceof TemplateType) { + return null; + } + + $finiteTypeSet = $this->getFiniteTypeSet(); + if ($finiteTypeSet === null || !$finiteTypeSet->isComplete()) { + return null; + } + + $otherFiniteTypeSet = $otherType->getFiniteTypeSet(); + if ($otherFiniteTypeSet === null) { + return null; + } + + $containment = $finiteTypeSet->containedIn($otherFiniteTypeSet); + if (!$containment->yes() && ($yesOnly || !$otherFiniteTypeSet->isComplete())) { + return null; + } + + return $containment; + } + public function equals(Type $type): bool { if (!$type instanceof static) { @@ -316,6 +434,14 @@ public function equals(Type $type): bool return false; } + $finiteTypeSet = $this->getFiniteTypeSet(); + if ($finiteTypeSet !== null && $finiteTypeSet->isComplete()) { + $otherFiniteTypeSet = $type->getFiniteTypeSet(); + if ($otherFiniteTypeSet !== null && $otherFiniteTypeSet->isComplete()) { + return $finiteTypeSet->containedIn($otherFiniteTypeSet)->yes(); + } + } + $otherTypes = $type->types; foreach ($this->types as $innerType) { $match = false; @@ -1336,6 +1462,31 @@ public function traverseSimultaneously(Type $right, callable $cb): Type public function tryRemove(Type $typeToRemove): ?Type { + $finiteTypeSet = $this->getFiniteTypeSet(); + if ($finiteTypeSet !== null && $finiteTypeSet->isComplete()) { + $key = FiniteTypeSet::key($typeToRemove); + if ($key !== null) { + if (!$finiteTypeSet->has($key)) { + return null; + } + + $remainingTypes = []; + foreach ($finiteTypeSet->getMembers() as $memberKey => $member) { + if ($memberKey === $key) { + continue; + } + + $remainingTypes[] = $member; + } + + if (count($remainingTypes) === 1) { + return $remainingTypes[0]; + } + + return new UnionType($remainingTypes); + } + } + $innerTypes = []; $changed = false; foreach ($this->types as $innerType) { diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php new file mode 100644 index 00000000000..204a0b1f00a --- /dev/null +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -0,0 +1,465 @@ + + */ + private static function unions(): array + { + return [ + 'strings' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c')]), + 'strings superset' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c'), new ConstantStringType('d')]), + 'strings reordered' => new UnionType([new ConstantStringType('c'), new ConstantStringType('b'), new ConstantStringType('a')]), + 'strings disjoint' => new UnionType([new ConstantStringType('x'), new ConstantStringType('y')]), + 'strings and null' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new NullType()]), + 'integers' => new UnionType([new ConstantIntegerType(1), new ConstantIntegerType(2), new ConstantIntegerType(3)]), + 'booleans' => new UnionType([new ConstantBooleanType(true), new ConstantBooleanType(false)]), + 'enum cases' => new UnionType([ + new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), + ]), + 'enum cases superset' => new UnionType([ + new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'C'), + ]), + 'mixed kinds' => new UnionType([ + new ConstantStringType('a'), + new ConstantIntegerType(1), + new ConstantBooleanType(true), + new NullType(), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + ]), + 'strings and object' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new ObjectType('DateTimeImmutable')]), + 'strings and float' => new UnionType([new ConstantStringType('a'), new ConstantFloatType(1.0)]), + 'strings and class-string' => new UnionType([new ConstantStringType('a'), new ConstantStringType('DateTimeImmutable', true)]), + 'strings and general string' => new UnionType([new ConstantStringType('a'), new StringType()]), + 'string and integer' => new UnionType([new ConstantStringType('a'), new IntegerType()]), + 'benevolent strings' => new BenevolentUnionType([new ConstantStringType('a'), new ConstantStringType('b')]), + 'objects' => new UnionType([new ObjectType('DateTimeImmutable'), new ObjectType('DateTime')]), + ]; + } + + /** + * @return array + */ + private static function otherTypes(): array + { + return [ + 'string a' => new ConstantStringType('a'), + 'string d' => new ConstantStringType('d'), + 'string z' => new ConstantStringType('z'), + 'class-string' => new ConstantStringType('DateTimeImmutable', true), + 'numeric string' => new ConstantStringType('1'), + 'integer 1' => new ConstantIntegerType(1), + 'integer 9' => new ConstantIntegerType(9), + 'true' => new ConstantBooleanType(true), + 'float' => new ConstantFloatType(1.0), + 'null' => new NullType(), + 'enum case A' => new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + 'enum case F' => new EnumCaseObjectType(ManyCasesTestEnum::class, 'F'), + 'whole enum' => new ObjectType(ManyCasesTestEnum::class), + 'string' => new StringType(), + 'integer' => new IntegerType(), + 'object' => new ObjectType('DateTimeImmutable'), + 'mixed' => new MixedType(), + 'template' => TemplateTypeFactory::create( + TemplateTypeScope::createWithFunction('foo'), + 'T', + new StringType(), + TemplateTypeVariance::createInvariant(), + ), + ]; + } + + /** + * @return Iterator + */ + public static function dataUnionAndOtherType(): Iterator + { + foreach (self::unions() as $unionName => $union) { + foreach (self::otherTypes() as $otherName => $otherType) { + yield sprintf('%s <-> %s', $unionName, $otherName) => [$union, $otherType]; + } + } + } + + /** + * @return Iterator + */ + public static function dataUnionPairs(): Iterator + { + foreach (self::unions() as $unionName => $union) { + foreach (self::unions() as $otherName => $otherUnion) { + yield sprintf('%s <-> %s', $unionName, $otherName) => [$union, $otherUnion]; + } + } + } + + #[DataProvider('dataUnionAndOtherType')] + public function testIsSuperTypeOf(UnionType $type, Type $otherType): void + { + $this->assertSame( + self::referenceIsSuperTypeOf($type, $otherType)->describe(), + $type->isSuperTypeOf($otherType)->result->describe(), + sprintf('%s -> isSuperTypeOf(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + + #[DataProvider('dataUnionAndOtherType')] + public function testAccepts(UnionType $type, Type $otherType): void + { + $this->assertAccepts($type, $otherType); + } + + #[DataProvider('dataUnionPairs')] + public function testAcceptsUnion(UnionType $type, UnionType $otherType): void + { + $this->assertAccepts($type, $otherType); + } + + private function assertAccepts(UnionType $type, Type $otherType): void + { + $finiteTypeSet = $type->getFiniteTypeSet(); + $answersPerKind = FiniteTypeSet::key($otherType) !== null + && $finiteTypeSet !== null + && $finiteTypeSet->isComplete(); + + foreach ([true, false] as $strictTypes) { + $this->assertSame( + self::referenceAccepts($type, $otherType, $strictTypes)->describe(), + $type->accepts($otherType, $strictTypes)->result->describe(), + sprintf('%s -> accepts(%s, strictTypes: %s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise()), $strictTypes ? 'true' : 'false'), + ); + + if (!$answersPerKind) { + continue; + } + + // Letting one member of a kind answer for all of them is only invisible as long + // as none of them attaches a reason to its answer - reasons are per member. + foreach ($type->getTypes() as $innerType) { + $this->assertSame( + [], + $innerType->accepts($otherType, $strictTypes)->reasons, + sprintf('%s -> accepts(%s)', $innerType->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + } + } + + #[DataProvider('dataUnionAndOtherType')] + public function testTryRemove(UnionType $type, Type $otherType): void + { + $expected = self::referenceTryRemove($type, $otherType); + $actual = $type->tryRemove($otherType); + + $this->assertSame( + $expected === null ? null : $expected->describe(VerbosityLevel::precise()), + $actual === null ? null : $actual->describe(VerbosityLevel::precise()), + sprintf('%s -> tryRemove(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + + #[DataProvider('dataUnionPairs')] + public function testIsSubTypeOf(UnionType $type, UnionType $otherType): void + { + $this->assertSame( + self::referenceIsSubTypeOf($type, $otherType)->describe(), + $type->isSubTypeOf($otherType)->result->describe(), + sprintf('%s -> isSubTypeOf(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + + #[DataProvider('dataUnionPairs')] + public function testIsAcceptedBy(UnionType $type, UnionType $otherType): void + { + foreach ([true, false] as $strictTypes) { + $this->assertSame( + self::referenceIsAcceptedBy($type, $otherType, $strictTypes)->describe(), + $type->isAcceptedBy($otherType, $strictTypes)->result->describe(), + sprintf('%s -> isAcceptedBy(%s, strictTypes: %s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise()), $strictTypes ? 'true' : 'false'), + ); + } + } + + #[DataProvider('dataUnionPairs')] + public function testEquals(UnionType $type, UnionType $otherType): void + { + $this->assertSame( + self::referenceEquals($type, $otherType), + $type->equals($otherType), + sprintf('%s -> equals(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + + /** + * @return Iterator + */ + public static function dataKey(): Iterator + { + yield [new ConstantStringType('a'), 's:a']; + yield [new ConstantStringType('a', true), 's:a']; + yield [new ConstantIntegerType(1), 'i:1']; + yield [new ConstantBooleanType(true), 'b:1']; + yield [new ConstantBooleanType(false), 'b:0']; + yield [new NullType(), 'null']; + yield [new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), 'enum:PHPStan\Fixture\ManyCasesTestEnum::A']; + + // floats do not have value identity: -0.0 === 0.0 and NAN !== NAN + yield [new ConstantFloatType(1.0), null]; + yield [new StringType(), null]; + yield [new IntegerType(), null]; + yield [new MixedType(), null]; + yield [new ObjectType(ManyCasesTestEnum::class), null]; + // a type that merely contains a value is not the value + yield [new IntersectionType([new ConstantStringType('a'), new AccessoryNonEmptyStringType()]), null]; + yield [new UnionType([new ConstantStringType('a'), new ConstantStringType('b')]), null]; + yield [ + TemplateTypeFactory::create( + TemplateTypeScope::createWithFunction('foo'), + 'T', + new StringType(), + TemplateTypeVariance::createInvariant(), + ), + null, + ]; + } + + #[DataProvider('dataKey')] + public function testKey(Type $type, ?string $expectedKey): void + { + $this->assertSame($expectedKey, FiniteTypeSet::key($type)); + } + + /** + * Types sharing a key must be interchangeable, so they have to be equal - and equal + * types must never end up under different keys. + */ + #[DataProvider('dataUnionAndOtherType')] + public function testKeyAgreesWithEquals(UnionType $type, Type $otherType): void + { + $otherKey = FiniteTypeSet::key($otherType); + $equal = []; + $sameKey = []; + foreach ($type->getTypes() as $i => $innerType) { + $innerKey = FiniteTypeSet::key($innerType); + if ($innerKey === null || $otherKey === null) { + continue; + } + + $equal[$i] = $innerType->equals($otherType); + $sameKey[$i] = $innerKey === $otherKey; + } + + $this->assertSame( + $equal, + $sameKey, + sprintf('%s <-> %s', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), + ); + } + + public function testUnkeyableMembersDoNotDefeatTheSet(): void + { + $set = (new UnionType([ + new ConstantStringType('a'), + new ObjectType('DateTimeImmutable'), + new ConstantStringType('b'), + ]))->getFiniteTypeSet(); + + $this->assertNotNull($set); + $this->assertFalse($set->isComplete()); + $this->assertTrue($set->has('s:a')); + $this->assertTrue($set->has('s:b')); + $this->assertCount(1, $set->getOthers()); + } + + public function testUnionWithoutFiniteMembersHasNoSet(): void + { + $this->assertNull((new UnionType([new StringType(), new IntegerType()]))->getFiniteTypeSet()); + } + + /** + * Mirrors UnionType::isSuperTypeOf() as it looked before the identity map, for types + * that reach its member loop - which none of the delegating or late-resolvable ones in + * dataUnionAndOtherType() are. + */ + private static function referenceIsSuperTypeOf(UnionType $type, Type $otherType): TrinaryLogic + { + $results = []; + foreach ($type->getTypes() as $innerType) { + $result = $innerType->isSuperTypeOf($otherType); + if ($result->yes()) { + return $result->result; + } + $results[] = $result->result; + } + + return TrinaryLogic::createNo()->or(...$results); + } + + /** Mirrors UnionType::isSubTypeOf() as it looked before the identity map. */ + private static function referenceIsSubTypeOf(UnionType $type, Type $otherType): TrinaryLogic + { + return TrinaryLogic::extremeIdentity(...array_map( + static fn (Type $innerType): TrinaryLogic => $otherType->isSuperTypeOf($innerType)->result, + $type->getTypes(), + )); + } + + /** + * Mirrors UnionType::accepts() as it looked before the identity map, minus the branches + * for iterables, callables and intersections - none of the types in + * dataUnionAndOtherType() take them. + */ + private static function referenceAccepts(UnionType $type, Type $otherType, bool $strictTypes): TrinaryLogic + { + foreach (UnionType::EQUAL_UNION_CLASSES as $baseClass => $classes) { + if (!$otherType->equals(new ObjectType($baseClass))) { + continue; + } + + $union = TypeCombinator::union( + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes), + ); + if (self::referenceAccepts($type, $union, $strictTypes)->yes()) { + return TrinaryLogic::createYes(); + } + break; + } + + $result = TrinaryLogic::createNo(); + foreach ($type->getTypes() as $innerType) { + $result = $result->or($innerType->accepts($otherType, $strictTypes)->result); + } + if ($result->yes()) { + return $result; + } + + if ($otherType instanceof CompoundType && !$otherType instanceof TemplateType) { + return $otherType->isAcceptedBy($type, $strictTypes)->result; + } + + if ($otherType->isEnum()->yes() && !$type->isEnum()->no()) { + $enumCasesUnion = TypeCombinator::union(...$otherType->getEnumCases()); + if (!$otherType->equals($enumCasesUnion)) { + return self::referenceAccepts($type, $enumCasesUnion, $strictTypes); + } + } + + return $result; + } + + /** Mirrors UnionType::isAcceptedBy() as it looked before the identity map. */ + private static function referenceIsAcceptedBy(UnionType $type, Type $acceptingType, bool $strictTypes): TrinaryLogic + { + if ($type instanceof BenevolentUnionType) { + $result = TrinaryLogic::createNo(); + foreach ($type->getTypes() as $innerType) { + $result = $result->or($acceptingType->accepts($innerType, $strictTypes)->result); + } + + return $result; + } + + return TrinaryLogic::extremeIdentity(...array_map( + static fn (Type $innerType): TrinaryLogic => $acceptingType->accepts($innerType, $strictTypes)->result, + $type->getTypes(), + )); + } + + /** Mirrors UnionType::equals() as it looked before the identity map. */ + private static function referenceEquals(UnionType $type, UnionType $otherType): bool + { + $otherTypes = $otherType->getTypes(); + if (count($type->getTypes()) !== count($otherTypes)) { + return false; + } + + foreach ($type->getTypes() as $innerType) { + $match = false; + foreach ($otherTypes as $i => $otherInnerType) { + if (!$innerType->equals($otherInnerType)) { + continue; + } + + $match = true; + unset($otherTypes[$i]); + break; + } + + if (!$match) { + return false; + } + } + + return count($otherTypes) === 0; + } + + /** Mirrors UnionType::tryRemove() as it looked before the identity map. */ + private static function referenceTryRemove(UnionType $type, Type $typeToRemove): ?Type + { + $innerTypes = []; + $changed = false; + foreach ($type->getTypes() as $innerType) { + $removed = TypeCombinator::remove($innerType, $typeToRemove); + if (!$removed->equals($innerType)) { + $changed = true; + } + if ($removed instanceof NeverType) { + continue; + } + if ($removed instanceof UnionType) { + foreach ($removed->getTypes() as $removedInnerType) { + $innerTypes[] = $removedInnerType; + } + } else { + $innerTypes[] = $removed; + } + } + + if (!$changed) { + return null; + } + + if (count($innerTypes) === 0) { + return new NeverType(); + } + + if (count($innerTypes) === 1) { + return $innerTypes[0]; + } + + return new UnionType($innerTypes); + } + +} diff --git a/tests/bench/data/big-constant-string-union.php b/tests/bench/data/big-constant-string-union.php new file mode 100644 index 00000000000..cc50a9e0f3f --- /dev/null +++ b/tests/bench/data/big-constant-string-union.php @@ -0,0 +1,233 @@ + Date: Mon, 27 Jul 2026 23:00:17 +0200 Subject: [PATCH 2/8] Update FiniteTypeSetTest.php --- tests/PHPStan/Type/FiniteTypeSetTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php index 204a0b1f00a..7dea3b574a7 100644 --- a/tests/PHPStan/Type/FiniteTypeSetTest.php +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -17,6 +17,7 @@ use PHPStan\Type\Generic\TemplateTypeScope; use PHPStan\Type\Generic\TemplateTypeVariance; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\RequiresPhp; use function array_map; use function count; use function sprintf; @@ -27,6 +28,7 @@ * real answer against a reference implementation that spells out the member-by-member loop * UnionType used before the map existed. */ +#[RequiresPhp('^8.1')] class FiniteTypeSetTest extends PHPStanTestCase { From af38312b3c18377d930205a2def0f01cef9c8a07 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 28 Jul 2026 06:41:26 +0000 Subject: [PATCH 3/8] Key a `FiniteTypeSet` member from `get_class()` instead of asking the type A union of finite values is made of `ConstantStringType`, `ConstantIntegerType`, `ConstantBooleanType`, `NullType` and `ConstantFloatType`, and every comparison against such a union derives at least one key - so `key()` now answers for those five straight from `get_class()`. Matching the class exactly is what makes it safe: for these, `getConstantScalarTypes()` is `[$this]` and `equals()` is value identity, which is precisely what the general path checks before keying. Subclasses - template constant types among them - do not match and still walk it. That drops the three virtual calls and the array allocation the general path costs. The other allocation was `keyAndKind()`'s pair: the kind it carried is just the member's class - the property the kind stands for, that members of one kind answer `accepts()` identically, holds because they are the same class - so `kind()` derives it separately and `key()` returns a bare string. Only `create()` and `getRepresentativesOfOtherKinds()` ever ask for a kind. On 400-member unions (`UnionType` API directly, before -> after): | operation | before | after | | --- | --- | --- | | `key()` | 0.40 us | 0.12 us | | `isSuperTypeOf(single value)` | 0.65 us | 0.33 us | | `create()` | 175 us | 76 us | The set is only an optimization, so the guard is that it never disagrees with comparing member by member: disabling the shortcut leaves all 2698 cases of the existing matrix passing, which is what says it is a pure shortcut. Added on top: the shortcut classes are asserted to satisfy the checks it skips, a subclass is asserted to reach the general path and land on the same key, each kind is asserted to be represented once, and the matrix gained strings differing only in case - without those, case-folding the key went unnoticed. Co-Authored-By: Claude Opus 5 --- src/Type/FiniteTypeSet.php | 79 +++++++++++++------ tests/PHPStan/Type/FiniteTypeSetTest.php | 99 ++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 25 deletions(-) diff --git a/src/Type/FiniteTypeSet.php b/src/Type/FiniteTypeSet.php index 73801503737..d7828725073 100644 --- a/src/Type/FiniteTypeSet.php +++ b/src/Type/FiniteTypeSet.php @@ -3,11 +3,16 @@ namespace PHPStan\Type; use PHPStan\TrinaryLogic; +use PHPStan\Type\Constant\ConstantBooleanType; +use PHPStan\Type\Constant\ConstantFloatType; +use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\Enum\EnumCaseObjectType; use PHPStan\Type\Generic\TemplateType; use function array_key_exists; use function array_keys; use function count; +use function get_class; use function is_bool; use function is_int; use function is_string; @@ -69,15 +74,14 @@ public static function create(array $types): ?self $membersByKind = []; $others = []; foreach ($types as $type) { - $keyAndKind = self::keyAndKind($type); - if ($keyAndKind === null || array_key_exists($keyAndKind[0], $members)) { + $key = self::key($type); + if ($key === null || array_key_exists($key, $members)) { $others[] = $type; continue; } - [$key, $kind] = $keyAndKind; $members[$key] = $type; - $membersByKind[$kind] ??= $type; + $membersByKind[self::kind($type)] ??= $type; } if ($members === []) { @@ -100,20 +104,27 @@ public static function create(array $types): ?self */ public static function key(Type $type): ?string { - return self::keyAndKind($type)[0] ?? null; - } + // A union of finite values is made of these five classes, and every comparison against + // such a union derives at least one key - so the general path below, three virtual + // calls and an array allocation, is worth skipping for them. Matching the class + // exactly is what makes that safe: for these, getConstantScalarTypes() is [$this] and + // equals() is value identity, which is precisely what the general path checks. + // Subclasses - template constant types among them - do not match and fall through. + switch (get_class($type)) { + case ConstantStringType::class: + return self::STRING_KEY_PREFIX . $type->getValue(); + case ConstantIntegerType::class: + return self::INTEGER_KEY_PREFIX . $type->getValue(); + case ConstantBooleanType::class: + return self::BOOLEAN_KEY_PREFIX . ($type->getValue() ? '1' : '0'); + case NullType::class: + return self::NULL_KEY; + case ConstantFloatType::class: + // Excluded on purpose, see above - said here too so that a union of constant + // floats pays one get_class() per member instead of walking the general path. + return null; + } - /** - * The identity key together with the kind of value it is. - * - * Members of one kind are the same type class - and, for enum cases, of the same enum - - * so they answer accepts() identically for every value none of them holds. The kind is - * what makes one member stand in for all its siblings there. - * - * @return array{string, string}|null - */ - private static function keyAndKind(Type $type): ?array - { if ($type instanceof TemplateType) { return null; } @@ -127,9 +138,7 @@ private static function keyAndKind(Type $type): ?array // Key by class + case name, the identity equals() compares (describe() would also // fold in a subtracted type, which equals() ignores). if ($type instanceof EnumCaseObjectType) { // @phpstan-ignore phpstanApi.instanceofType - $kind = self::ENUM_CASE_KEY_PREFIX . $type->getClassName(); - - return [$kind . '::' . $type->getEnumCaseName(), $kind]; + return self::kind($type) . '::' . $type->getEnumCaseName(); } if (!$type->isConstantScalarValue()->yes()) { @@ -143,21 +152,41 @@ private static function keyAndKind(Type $type): ?array $value = $scalarTypes[0]->getValue(); if ($value === null) { - return [self::NULL_KEY, self::NULL_KEY]; + return self::NULL_KEY; } if (is_int($value)) { - return [self::INTEGER_KEY_PREFIX . $value, self::INTEGER_KEY_PREFIX]; + return self::INTEGER_KEY_PREFIX . $value; } if (is_bool($value)) { - return [self::BOOLEAN_KEY_PREFIX . ($value ? '1' : '0'), self::BOOLEAN_KEY_PREFIX]; + return self::BOOLEAN_KEY_PREFIX . ($value ? '1' : '0'); } if (is_string($value)) { - return [self::STRING_KEY_PREFIX . $value, self::STRING_KEY_PREFIX]; + return self::STRING_KEY_PREFIX . $value; } return null; } + /** + * The kind of value a type stands for. + * + * Members of one kind answer accepts() identically for every value none of them holds, + * which is what lets one of them stand in for all its siblings there. Being of the same + * class is enough for that - accepts() on a constant scalar only asks whether the other + * type equals it - except for enum cases, where the enum is part of the answer. + * + * Only meaningful for a type that key() keys; anything else gets a kind of its own, + * which merely costs it a representative. + */ + private static function kind(Type $type): string + { + if ($type instanceof EnumCaseObjectType) { // @phpstan-ignore phpstanApi.instanceofType + return self::ENUM_CASE_KEY_PREFIX . $type->getClassName(); + } + + return get_class($type); + } + /** * One member per kind other than $type's own, in the union's order. * @@ -169,7 +198,7 @@ private static function keyAndKind(Type $type): ?array */ public function getRepresentativesOfOtherKinds(Type $type): array { - $kind = self::keyAndKind($type)[1] ?? null; + $kind = self::kind($type); $representatives = []; foreach ($this->membersByKind as $memberKind => $member) { if ($memberKind === $kind) { diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php index 7dea3b574a7..8905ed230fc 100644 --- a/tests/PHPStan/Type/FiniteTypeSetTest.php +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Type; use Iterator; +use PHPStan\Fixture\AnotherTestEnum; use PHPStan\Fixture\ManyCasesTestEnum; use PHPStan\Testing\PHPStanTestCase; use PHPStan\TrinaryLogic; @@ -42,6 +43,7 @@ private static function unions(): array 'strings superset' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c'), new ConstantStringType('d')]), 'strings reordered' => new UnionType([new ConstantStringType('c'), new ConstantStringType('b'), new ConstantStringType('a')]), 'strings disjoint' => new UnionType([new ConstantStringType('x'), new ConstantStringType('y')]), + 'strings differing only in case' => new UnionType([new ConstantStringType('a'), new ConstantStringType('A')]), 'strings and null' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new NullType()]), 'integers' => new UnionType([new ConstantIntegerType(1), new ConstantIntegerType(2), new ConstantIntegerType(3)]), 'booleans' => new UnionType([new ConstantBooleanType(true), new ConstantBooleanType(false)]), @@ -78,6 +80,7 @@ private static function otherTypes(): array { return [ 'string a' => new ConstantStringType('a'), + 'string A' => new ConstantStringType('A'), 'string d' => new ConstantStringType('d'), 'string z' => new ConstantStringType('z'), 'class-string' => new ConstantStringType('DateTimeImmutable', true), @@ -231,7 +234,14 @@ public static function dataKey(): Iterator { yield [new ConstantStringType('a'), 's:a']; yield [new ConstantStringType('a', true), 's:a']; + // the value goes into the key verbatim - two strings that differ in any way at all + // stand for two different values + yield [new ConstantStringType('A'), 's:A']; + yield [new ConstantStringType(''), 's:']; + yield [new ConstantStringType('0'), 's:0']; yield [new ConstantIntegerType(1), 'i:1']; + yield [new ConstantIntegerType(-1), 'i:-1']; + yield [new ConstantIntegerType(0), 'i:0']; yield [new ConstantBooleanType(true), 'b:1']; yield [new ConstantBooleanType(false), 'b:0']; yield [new NullType(), 'null']; @@ -263,6 +273,56 @@ public function testKey(Type $type, ?string $expectedKey): void $this->assertSame($expectedKey, FiniteTypeSet::key($type)); } + /** + * @return Iterator + */ + public static function dataShortcutClass(): Iterator + { + yield [new ConstantStringType('a')]; + yield [new ConstantStringType('A')]; + yield [new ConstantStringType('DateTimeImmutable', true)]; + yield [new ConstantStringType('')]; + yield [new ConstantIntegerType(1)]; + yield [new ConstantIntegerType(-1)]; + yield [new ConstantIntegerType(0)]; + yield [new ConstantBooleanType(true)]; + yield [new ConstantBooleanType(false)]; + yield [new NullType()]; + } + + /** + * key() answers for these classes straight from get_class() instead of asking the type, + * which is only sound while the checks it skips would have passed. Assert them here so + * that a change to any of these classes fails loudly rather than silently keying a type + * that no longer stands for exactly one value. + */ + #[DataProvider('dataShortcutClass')] + public function testShortcutClassesSatisfyTheGeneralPath(Type $type): void + { + $this->assertTrue($type->isConstantScalarValue()->yes()); + $this->assertSame([$type], $type->getConstantScalarTypes()); + $this->assertTrue($type->equals($type)); + $this->assertNotNull(FiniteTypeSet::key($type)); + } + + /** + * The shortcut matches the class exactly, so a subclass has to reach the general path - + * and land on the very same key, or a union holding both would count them as two values. + */ + public function testSubclassOfAShortcutClassIsKeyedTheSameWay(): void + { + $subclass = new class ('a') extends ConstantStringType { + + }; + + $this->assertSame(FiniteTypeSet::key(new ConstantStringType('a')), FiniteTypeSet::key($subclass)); + + $set = (new UnionType([new ConstantStringType('a'), $subclass]))->getFiniteTypeSet(); + $this->assertNotNull($set); + $this->assertCount(1, $set->getMembers()); + $this->assertCount(1, $set->getOthers()); + } + /** * Types sharing a key must be interchangeable, so they have to be equal - and equal * types must never end up under different keys. @@ -290,6 +350,45 @@ public function testKeyAgreesWithEquals(UnionType $type, Type $otherType): void ); } + /** + * Members of one kind stand in for each other when the set is asked about a value it does + * not hold, so a kind must never span two classes - nor two enums, which answer for their + * own cases only. + */ + public function testEachKindIsRepresentedOnce(): void + { + $set = (new UnionType([ + new ConstantStringType('a'), + new ConstantStringType('b'), + new ConstantIntegerType(1), + new ConstantIntegerType(2), + new ConstantBooleanType(true), + new NullType(), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), + new EnumCaseObjectType(AnotherTestEnum::class, 'ONE'), + ]))->getFiniteTypeSet(); + + $this->assertNotNull($set); + + $describe = static fn (Type $type): string => $type->describe(VerbosityLevel::precise()); + + // the first member of every kind but the queried one, in the union's order + $this->assertSame( + ['1', 'true', 'null', 'PHPStan\Fixture\ManyCasesTestEnum::A', 'PHPStan\Fixture\AnotherTestEnum::ONE'], + array_map($describe, $set->getRepresentativesOfOtherKinds(new ConstantStringType('zzz'))), + ); + + // one enum does not answer for another's cases, so both are represented + $this->assertSame( + ["'a'", '1', 'true', 'null', 'PHPStan\Fixture\AnotherTestEnum::ONE'], + array_map($describe, $set->getRepresentativesOfOtherKinds(new EnumCaseObjectType(ManyCasesTestEnum::class, 'C'))), + ); + + // a type of no keyed kind is answered by every one of the six kinds + $this->assertCount(6, $set->getRepresentativesOfOtherKinds(new ObjectType('DateTimeImmutable'))); + } + public function testUnkeyableMembersDoNotDefeatTheSet(): void { $set = (new UnionType([ From d6cbc98fe6f38a8714c496446627d06dd811029d Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 28 Jul 2026 06:41:38 +0000 Subject: [PATCH 4/8] Compare two `FiniteTypeSet`s with one `array_diff_key()` `containedIn()` asked the other set about one member at a time, so a union-vs-union comparison still cost a PHP-level call per member. The keys are what both sets are indexed by, so the whole comparison is a single C-level hash join. It asks for what is missing rather than for what is shared because that makes the yes answer the cheap one - `array_diff_key()` collects nothing when every member is present - and yes is the answer all three callers are after (`isAcceptedBy()` and `equals()` want nothing else). The no/maybe split then comes from the same count, no second pass. On 400-member unions, `isSubTypeOf()` goes from 20.1 ms to 4.3 ms for identical unions and from 17.4 ms to 6.7 ms for disjoint ones (2000 iterations). Covered by the existing matrix: making `containedIn()` answer no where it should answer maybe fails 43 of its cases. Co-Authored-By: Claude Opus 5 --- src/Type/FiniteTypeSet.php | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/Type/FiniteTypeSet.php b/src/Type/FiniteTypeSet.php index d7828725073..a77325b2508 100644 --- a/src/Type/FiniteTypeSet.php +++ b/src/Type/FiniteTypeSet.php @@ -9,8 +9,8 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\Enum\EnumCaseObjectType; use PHPStan\Type\Generic\TemplateType; +use function array_diff_key; use function array_key_exists; -use function array_keys; use function count; use function get_class; use function is_bool; @@ -246,20 +246,18 @@ public function getOthers(): array */ public function containedIn(self $other): TrinaryLogic { - $contained = 0; - foreach (array_keys($this->members) as $key) { - if (!$other->has($key)) { - continue; - } - - $contained++; - } - - if ($contained === count($this->members)) { + // One array_diff_key() rather than a lookup per member: the keys are what both sets + // are indexed by, so the whole comparison is a single C-level hash join. Asking for + // what is missing rather than for what is shared makes the yes answer the cheap one - + // it is the one that costs nothing to collect, and the one all three callers are + // after (isAcceptedBy() and equals() want nothing else). + $missing = count(array_diff_key($this->members, $other->members)); + + if ($missing === 0) { return TrinaryLogic::createYes(); } - if ($contained === 0) { + if ($missing === count($this->members)) { return TrinaryLogic::createNo(); } From abed2abeff687f30d87c77e148ea4fdd6946d973 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 29 Jul 2026 07:56:39 +0000 Subject: [PATCH 5/8] Revert the `SkipTestsWithRequiresPhpAttributeRule` reordering The reordering was pulled in because an earlier draft of `FiniteTypeSetTest` skipped on a `PHP_VERSION_ID` guard whose condition the rule could not name an inverse for. The test uses `#[RequiresPhp]` now, so nothing in this PR needs the change - and the latent crash it fixes (a test method whose first statement is an `if` on a binary operator that is not a comparison against `PHP_VERSION_ID`) is unrelated to keying unions by value identity. It belongs in its own change, together with the test that would pin the reordered guard. Co-Authored-By: Claude Opus 5 --- .../SkipTestsWithRequiresPhpAttributeRule.php | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php b/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php index 12f24f79882..05de1553e22 100644 --- a/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php +++ b/build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php @@ -52,16 +52,6 @@ public function processNode(Node $node, Scope $scope): array return []; } - // Before asking for an inverse: anything that is not a PHP_VERSION_ID comparison is - // none of this rule's business, and there is no inverse to name for it. - if (!$firstStmt->cond->left instanceof Node\Expr\ConstFetch || $firstStmt->cond->left->name->toString() !== 'PHP_VERSION_ID') { - return []; - } - - if (!$firstStmt->cond->right instanceof Node\Scalar\Int_) { - return []; - } - switch (get_class($firstStmt->cond)) { case Node\Expr\BinaryOp\SmallerOrEqual::class: $inverseBinaryOpSigil = '>'; @@ -85,6 +75,14 @@ public function processNode(Node $node, Scope $scope): array throw new ShouldNotHappenException('No inverse comparison specified for ' . get_class($firstStmt->cond)); } + if (!$firstStmt->cond->left instanceof Node\Expr\ConstFetch || $firstStmt->cond->left->name->toString() !== 'PHP_VERSION_ID') { + return []; + } + + if (!$firstStmt->cond->right instanceof Node\Scalar\Int_) { + return []; + } + if (count($firstStmt->stmts) !== 1) { return []; } From 90b4a0a95c2dc476827689556b1aca798758f308 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 29 Jul 2026 07:57:14 +0000 Subject: [PATCH 6/8] Name the enum fixtures by string in `FiniteTypeSetTest` Self-analysis excludes `tests/PHPStan/Fixture/{ManyCases,Another}TestEnum.php` when the analysed PHP version is below 8.1 (build/enums.neon), so the `::class` references made PHPStan report 15 `class.notFound` errors against its own test suite on the PHP 8.0 and PHP 7.4 jobs. `UnionTypeTest` and `EnumCaseObjectTypeTest` already spell these fixtures out as strings for the same reason; do the same here and say why, so it does not get "cleaned up" back into an import. Co-Authored-By: Claude Opus 5 --- tests/PHPStan/Type/FiniteTypeSetTest.php | 36 +++++++++++++----------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php index 8905ed230fc..09eb5b410b5 100644 --- a/tests/PHPStan/Type/FiniteTypeSetTest.php +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -3,8 +3,6 @@ namespace PHPStan\Type; use Iterator; -use PHPStan\Fixture\AnotherTestEnum; -use PHPStan\Fixture\ManyCasesTestEnum; use PHPStan\Testing\PHPStanTestCase; use PHPStan\TrinaryLogic; use PHPStan\Type\Accessory\AccessoryNonEmptyStringType; @@ -28,6 +26,10 @@ * disagrees with comparing a union member by member. Every test here therefore compares the * real answer against a reference implementation that spells out the member-by-member loop * UnionType used before the map existed. + * + * The enum fixtures are named by string rather than by ::class, as UnionTypeTest does: + * self-analysis excludes those files below PHP 8.1, and a ::class would have PHPStan report + * them as unknown classes there. */ #[RequiresPhp('^8.1')] class FiniteTypeSetTest extends PHPStanTestCase @@ -48,20 +50,20 @@ private static function unions(): array 'integers' => new UnionType([new ConstantIntegerType(1), new ConstantIntegerType(2), new ConstantIntegerType(3)]), 'booleans' => new UnionType([new ConstantBooleanType(true), new ConstantBooleanType(false)]), 'enum cases' => new UnionType([ - new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'B'), ]), 'enum cases superset' => new UnionType([ - new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'C'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'B'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'C'), ]), 'mixed kinds' => new UnionType([ new ConstantStringType('a'), new ConstantIntegerType(1), new ConstantBooleanType(true), new NullType(), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), ]), 'strings and object' => new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new ObjectType('DateTimeImmutable')]), 'strings and float' => new UnionType([new ConstantStringType('a'), new ConstantFloatType(1.0)]), @@ -90,9 +92,9 @@ private static function otherTypes(): array 'true' => new ConstantBooleanType(true), 'float' => new ConstantFloatType(1.0), 'null' => new NullType(), - 'enum case A' => new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), - 'enum case F' => new EnumCaseObjectType(ManyCasesTestEnum::class, 'F'), - 'whole enum' => new ObjectType(ManyCasesTestEnum::class), + 'enum case A' => new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), + 'enum case F' => new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'F'), + 'whole enum' => new ObjectType('PHPStan\Fixture\ManyCasesTestEnum'), 'string' => new StringType(), 'integer' => new IntegerType(), 'object' => new ObjectType('DateTimeImmutable'), @@ -245,14 +247,14 @@ public static function dataKey(): Iterator yield [new ConstantBooleanType(true), 'b:1']; yield [new ConstantBooleanType(false), 'b:0']; yield [new NullType(), 'null']; - yield [new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), 'enum:PHPStan\Fixture\ManyCasesTestEnum::A']; + yield [new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), 'enum:PHPStan\Fixture\ManyCasesTestEnum::A']; // floats do not have value identity: -0.0 === 0.0 and NAN !== NAN yield [new ConstantFloatType(1.0), null]; yield [new StringType(), null]; yield [new IntegerType(), null]; yield [new MixedType(), null]; - yield [new ObjectType(ManyCasesTestEnum::class), null]; + yield [new ObjectType('PHPStan\Fixture\ManyCasesTestEnum'), null]; // a type that merely contains a value is not the value yield [new IntersectionType([new ConstantStringType('a'), new AccessoryNonEmptyStringType()]), null]; yield [new UnionType([new ConstantStringType('a'), new ConstantStringType('b')]), null]; @@ -364,9 +366,9 @@ public function testEachKindIsRepresentedOnce(): void new ConstantIntegerType(2), new ConstantBooleanType(true), new NullType(), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'A'), - new EnumCaseObjectType(ManyCasesTestEnum::class, 'B'), - new EnumCaseObjectType(AnotherTestEnum::class, 'ONE'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A'), + new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'B'), + new EnumCaseObjectType('PHPStan\Fixture\AnotherTestEnum', 'ONE'), ]))->getFiniteTypeSet(); $this->assertNotNull($set); @@ -382,7 +384,7 @@ public function testEachKindIsRepresentedOnce(): void // one enum does not answer for another's cases, so both are represented $this->assertSame( ["'a'", '1', 'true', 'null', 'PHPStan\Fixture\AnotherTestEnum::ONE'], - array_map($describe, $set->getRepresentativesOfOtherKinds(new EnumCaseObjectType(ManyCasesTestEnum::class, 'C'))), + array_map($describe, $set->getRepresentativesOfOtherKinds(new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'C'))), ); // a type of no keyed kind is answered by every one of the six kinds From 4c0234defa1852c6cc768f2f73ad568be3f17ff8 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 29 Jul 2026 08:00:22 +0000 Subject: [PATCH 7/8] Pin the `FiniteTypeSet` branches the reference matrix only reaches sideways The union-vs-reference matrix covers these paths, but only as a by-product of whichever fixtures happen to hit them, so a mutation can survive by being wrong in a spot no fixture exercises. Test them head on instead: * `containedIn()` at each of its three thresholds, including the two boundaries - none held (which must count *this* set's members, not the other's) and every member held with the other set larger, which pins the direction of the diff. * `getRepresentativesOfOtherKinds()` on a single-kind set, where the answer is the empty list - the case that says the queried kind is skipped. * the `isComplete()` gates: a miss in an incomplete set still consults the members that could not be keyed (`isSuperTypeOf()`, `accepts()`, `tryRemove()`), and two unions with identical maps but different unkeyed members are not equal, nor a subtype of, nor accepted by one another. * `hasClassStringMember()` for a value that names a class, for the explicit flag, and for a set with no string members at all - plus the cached second answer. Each of these kills a mutation that the matrix alone let through: keying `containedIn()`'s no threshold off the wrong set, swapping its `array_diff_key()` arguments, widening its yes comparison, dropping the kind check from the representative loop, dropping the `isClassString()` guard, and removing any of the four `isComplete()` gates. Co-Authored-By: Claude Opus 5 --- tests/PHPStan/Type/FiniteTypeSetTest.php | 165 +++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php index 09eb5b410b5..18e54ca7fa9 100644 --- a/tests/PHPStan/Type/FiniteTypeSetTest.php +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -391,6 +391,99 @@ public function testEachKindIsRepresentedOnce(): void $this->assertCount(6, $set->getRepresentativesOfOtherKinds(new ObjectType('DateTimeImmutable'))); } + /** + * A union of one kind has nobody to speak for a value it does not hold, so accepts() + * has nothing left to consult and the plain no stands. + */ + public function testTheOnlyKindRepresentsNoOtherKind(): void + { + $set = FiniteTypeSet::create([new ConstantStringType('a'), new ConstantStringType('b')]); + + $this->assertNotNull($set); + $this->assertSame([], $set->getRepresentativesOfOtherKinds(new ConstantStringType('zzz'))); + } + + /** + * @return Iterator, list, TrinaryLogic}> + */ + public static function dataContainedIn(): Iterator + { + yield 'same members' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('a'), new ConstantStringType('b')], + TrinaryLogic::createYes(), + ]; + yield 'same members, reordered' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('b'), new ConstantStringType('a')], + TrinaryLogic::createYes(), + ]; + yield 'proper subset' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c')], + TrinaryLogic::createYes(), + ]; + yield 'single member, held' => [ + [new ConstantStringType('a')], + [new ConstantStringType('a'), new ConstantStringType('b')], + TrinaryLogic::createYes(), + ]; + // the other way round: containment is not symmetric, one member is missing + yield 'proper superset' => [ + [new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c')], + [new ConstantStringType('a'), new ConstantStringType('b')], + TrinaryLogic::createMaybe(), + ]; + yield 'one of two held' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('b'), new ConstantStringType('c')], + TrinaryLogic::createMaybe(), + ]; + yield 'none held' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('x'), new ConstantStringType('y')], + TrinaryLogic::createNo(), + ]; + // none held, and the other set is the smaller one - what counts is how many of *this* + // set's members are missing, not how many the other set has + yield 'none held, smaller other set' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + [new ConstantStringType('x')], + TrinaryLogic::createNo(), + ]; + yield 'single member, not held' => [ + [new ConstantStringType('a')], + [new ConstantStringType('b')], + TrinaryLogic::createNo(), + ]; + // the keys carry the kind, so a value is never held by a member of another kind + yield 'same values of another kind' => [ + [new ConstantIntegerType(1), new ConstantIntegerType(0)], + [new ConstantStringType('1'), new ConstantBooleanType(false)], + TrinaryLogic::createNo(), + ]; + yield 'across kinds' => [ + [new ConstantStringType('a'), new ConstantIntegerType(1), new NullType()], + [new NullType(), new ConstantStringType('a'), new ConstantIntegerType(1)], + TrinaryLogic::createYes(), + ]; + } + + /** + * @param list $types + * @param list $otherTypes + */ + #[DataProvider('dataContainedIn')] + public function testContainedIn(array $types, array $otherTypes, TrinaryLogic $expected): void + { + $set = FiniteTypeSet::create($types); + $otherSet = FiniteTypeSet::create($otherTypes); + $this->assertNotNull($set); + $this->assertNotNull($otherSet); + + $this->assertSame($expected->describe(), $set->containedIn($otherSet)->describe()); + } + public function testUnkeyableMembersDoNotDefeatTheSet(): void { $set = (new UnionType([ @@ -406,6 +499,78 @@ public function testUnkeyableMembersDoNotDefeatTheSet(): void $this->assertCount(1, $set->getOthers()); } + /** + * An incomplete set speaks for its keyed members only, so a miss is not the union's + * answer - the members it could not key still have to be asked, and here each of them + * answers differently than the map would have. + */ + public function testMissInAnIncompleteSetStillConsultsTheOtherMembers(): void + { + $union = new UnionType([new ConstantStringType('a'), new ConstantStringType('b'), new StringType()]); + $set = $union->getFiniteTypeSet(); + $this->assertNotNull($set); + $this->assertFalse($set->isComplete()); + $this->assertFalse($set->has('s:z')); + + // 'z' is not in the map, but the general string member holds it + $this->assertSame('Yes', $union->isSuperTypeOf(new ConstantStringType('z'))->result->describe()); + $this->assertSame('Yes', $union->accepts(new ConstantStringType('z'), true)->result->describe()); + + // removing 'a' must not drop the member the map does not know about + $removed = $union->tryRemove(new ConstantStringType('a')); + $this->assertNotNull($removed); + $this->assertSame("'b'|string", $removed->describe(VerbosityLevel::precise())); + } + + /** + * Union-against-union answers need both sets to speak for their whole union: here the + * maps are identical while the unions are not, so reading the answer off them alone + * would call two different types the same. + */ + public function testUnionComparisonsRequireBothSetsToBeComplete(): void + { + $union = new UnionType([new ConstantStringType('a'), new ObjectType('DateTime')]); + $otherUnion = new UnionType([new ConstantStringType('a'), new ObjectType('DateTimeImmutable')]); + + $set = $union->getFiniteTypeSet(); + $otherSet = $otherUnion->getFiniteTypeSet(); + $this->assertNotNull($set); + $this->assertNotNull($otherSet); + $this->assertFalse($set->isComplete()); + $this->assertFalse($otherSet->isComplete()); + $this->assertSame('Yes', $set->containedIn($otherSet)->describe()); + + $this->assertFalse($union->equals($otherUnion)); + $this->assertSame('Maybe', $union->isSubTypeOf($otherUnion)->result->describe()); + $this->assertSame('Maybe', $union->isAcceptedBy($otherUnion, true)->result->describe()); + } + + /** + * @return Iterator, bool}> + */ + public static function dataHasClassStringMember(): Iterator + { + yield 'plain strings' => [[new ConstantStringType('a'), new ConstantStringType('b')], false]; + yield 'a value that names a class' => [[new ConstantStringType('a'), new ConstantStringType('DateTimeImmutable')], true]; + yield 'the class-string flag' => [[new ConstantStringType('a'), new ConstantStringType('Zzz', true)], true]; + // only string members can carry the flag, so no other kind is worth asking + yield 'no strings at all' => [[new ConstantIntegerType(1), new NullType()], false]; + } + + /** + * @param list $types + */ + #[DataProvider('dataHasClassStringMember')] + public function testHasClassStringMember(array $types, bool $expected): void + { + $set = FiniteTypeSet::create($types); + $this->assertNotNull($set); + + $this->assertSame($expected, $set->hasClassStringMember()); + // answered from the cache the second time round, with the same answer + $this->assertSame($expected, $set->hasClassStringMember()); + } + public function testUnionWithoutFiniteMembersHasNoSet(): void { $this->assertNull((new UnionType([new StringType(), new IntegerType()]))->getFiniteTypeSet()); From 032c1284aec6c43dc95d87e2b37c1d310bc27f47 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 29 Jul 2026 09:07:33 +0000 Subject: [PATCH 8/8] 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 --- src/Type/FiniteTypeSet.php | 23 ++++++ src/Type/UnionType.php | 37 +++++++-- tests/PHPStan/Type/FiniteTypeSetTest.php | 97 +++++++++++++++++++++++- 3 files changed, 147 insertions(+), 10 deletions(-) diff --git a/src/Type/FiniteTypeSet.php b/src/Type/FiniteTypeSet.php index a77325b2508..3ba2dd7af2a 100644 --- a/src/Type/FiniteTypeSet.php +++ b/src/Type/FiniteTypeSet.php @@ -264,6 +264,29 @@ public function containedIn(self $other): TrinaryLogic return TrinaryLogic::createMaybe(); } + /** + * containedIn() against the one-member set holding just $key. + * + * Yes only when this set holds nothing besides that value, no when it does not hold it + * at all, maybe in between - the same three answers as containedIn(), which is what a + * single value is being compared as here. + * + * Only keyed members are compared - call isComplete() first when the answer has to + * hold for the whole union. + */ + public function containedInKey(string $key): TrinaryLogic + { + if (!$this->has($key)) { + return TrinaryLogic::createNo(); + } + + if (count($this->members) === 1) { + return TrinaryLogic::createYes(); + } + + return TrinaryLogic::createMaybe(); + } + /** * Whether a constant string member might also be a class-string. * diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index f07aab71384..97d60a32ec7 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -394,23 +394,48 @@ public function isAcceptedBy(Type $acceptingType, bool $strictTypes): AcceptsRes } /** - * Whether the other union holds every member of this one, or null when the question - * cannot be settled from the identity maps alone. + * Whether $otherType holds every member of this one, or null when the question cannot + * be settled from the identity maps alone. + * + * $otherType is compared as a set of values: a union through its own map, a single + * finite value as the one-member set holding it. * * $yesOnly skips the completeness requirement on the other union: a member missing from * its map only rules out the yes answer, which is all the caller is after. */ private function finiteTypeSetContainedIn(Type $otherType, bool $yesOnly): ?TrinaryLogic { - if (!$otherType instanceof self || $otherType instanceof TemplateType) { - return null; - } - $finiteTypeSet = $this->getFiniteTypeSet(); if ($finiteTypeSet === null || !$finiteTypeSet->isComplete()) { return null; } + if (!$otherType instanceof self || $otherType instanceof TemplateType) { + // A single value covers a member iff they are the same value, and no member + // stands for a value it is not keyed by - so one lookup answers for every member + // at once. This is the shape TypeCombinator::remove() takes to ask whether + // removing a value empties a union of values, and the only way a non-union other + // type reaches this helper at all. + $key = FiniteTypeSet::key($otherType); + if ($key === null) { + return null; + } + + $containment = $finiteTypeSet->containedInKey($key); + + // No constant value accepts another one - not even a numeric string an int, in + // either direction - so today accepts() agrees with isSuperTypeOf() here and this + // guard never changes an answer. It is kept because that is a fact about the + // current value types rather than something the identity map guarantees: a value + // the map does not hold being accepted anyway is exactly what accepts() is + // allowed to do, and all isAcceptedBy() wants from the map is the yes. + if (!$containment->yes() && $yesOnly) { + return null; + } + + return $containment; + } + $otherFiniteTypeSet = $otherType->getFiniteTypeSet(); if ($otherFiniteTypeSet === null) { return null; diff --git a/tests/PHPStan/Type/FiniteTypeSetTest.php b/tests/PHPStan/Type/FiniteTypeSetTest.php index 18e54ca7fa9..149b2439d18 100644 --- a/tests/PHPStan/Type/FiniteTypeSetTest.php +++ b/tests/PHPStan/Type/FiniteTypeSetTest.php @@ -154,12 +154,22 @@ public function testAcceptsUnion(UnionType $type, UnionType $otherType): void $this->assertAccepts($type, $otherType); } - private function assertAccepts(UnionType $type, Type $otherType): void + /** + * Whether the map alone settles a comparison of $type against $otherType - the whole + * union is keyed, and $otherType is one value to look up in it. + */ + private static function answersFromTheMap(UnionType $type, Type $otherType): bool { $finiteTypeSet = $type->getFiniteTypeSet(); - $answersPerKind = FiniteTypeSet::key($otherType) !== null + + return FiniteTypeSet::key($otherType) !== null && $finiteTypeSet !== null && $finiteTypeSet->isComplete(); + } + + private function assertAccepts(UnionType $type, Type $otherType): void + { + $answersPerKind = self::answersFromTheMap($type, $otherType); foreach ([true, false] as $strictTypes) { $this->assertSame( @@ -198,17 +208,35 @@ public function testTryRemove(UnionType $type, Type $otherType): void } #[DataProvider('dataUnionPairs')] - public function testIsSubTypeOf(UnionType $type, UnionType $otherType): void + #[DataProvider('dataUnionAndOtherType')] + public function testIsSubTypeOf(UnionType $type, Type $otherType): void { $this->assertSame( self::referenceIsSubTypeOf($type, $otherType)->describe(), $type->isSubTypeOf($otherType)->result->describe(), sprintf('%s -> isSubTypeOf(%s)', $type->describe(VerbosityLevel::precise()), $otherType->describe(VerbosityLevel::precise())), ); + + if (!self::answersFromTheMap($type, $otherType)) { + return; + } + + // One key lookup stands in for asking every member, which is only invisible as long + // as none of them attaches a reason to its answer - reasons are per member. + foreach ($type->getTypes() as $innerType) { + $result = $otherType->isSuperTypeOf($innerType); + $this->assertSame( + [], + $result->reasons, + sprintf('%s -> isSuperTypeOf(%s)', $otherType->describe(VerbosityLevel::precise()), $innerType->describe(VerbosityLevel::precise())), + ); + $this->assertSame([], $result->lazyReasons); + } } #[DataProvider('dataUnionPairs')] - public function testIsAcceptedBy(UnionType $type, UnionType $otherType): void + #[DataProvider('dataUnionAndOtherType')] + public function testIsAcceptedBy(UnionType $type, Type $otherType): void { foreach ([true, false] as $strictTypes) { $this->assertSame( @@ -484,6 +512,67 @@ public function testContainedIn(array $types, array $otherTypes, TrinaryLogic $e $this->assertSame($expected->describe(), $set->containedIn($otherSet)->describe()); } + /** + * @return Iterator, Type, TrinaryLogic}> + */ + public static function dataContainedInKey(): Iterator + { + // the only way a set is wholly under one value is by being that one value + yield 'the only member' => [[new ConstantStringType('a')], new ConstantStringType('a'), TrinaryLogic::createYes()]; + yield 'one of two members' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + new ConstantStringType('a'), + TrinaryLogic::createMaybe(), + ]; + yield 'the last of many members' => [ + [new ConstantStringType('a'), new ConstantStringType('b'), new ConstantStringType('c')], + new ConstantStringType('c'), + TrinaryLogic::createMaybe(), + ]; + yield 'not held' => [ + [new ConstantStringType('a'), new ConstantStringType('b')], + new ConstantStringType('z'), + TrinaryLogic::createNo(), + ]; + yield 'not held by a single-member set' => [ + [new ConstantStringType('a')], + new ConstantStringType('z'), + TrinaryLogic::createNo(), + ]; + // the key carries the kind, so the same value of another kind is not held either + yield 'the same value of another kind' => [ + [new ConstantIntegerType(1)], + new ConstantStringType('1'), + TrinaryLogic::createNo(), + ]; + yield 'one of two kinds' => [ + [new ConstantStringType('a'), new NullType()], + new NullType(), + TrinaryLogic::createMaybe(), + ]; + } + + /** + * @param list $types + */ + #[DataProvider('dataContainedInKey')] + public function testContainedInKey(array $types, Type $value, TrinaryLogic $expected): void + { + $set = FiniteTypeSet::create($types); + $this->assertNotNull($set); + + $key = FiniteTypeSet::key($value); + $this->assertNotNull($key); + + $this->assertSame($expected->describe(), $set->containedInKey($key)->describe()); + + // the same answer containedIn() gives against the set of just that one value, which + // is what a single value is being compared as + $singleMemberSet = FiniteTypeSet::create([$value]); + $this->assertNotNull($singleMemberSet); + $this->assertSame($expected->describe(), $set->containedIn($singleMemberSet)->describe()); + } + public function testUnkeyableMembersDoNotDefeatTheSet(): void { $set = (new UnionType([