Skip to content

Commit e2d4e60

Browse files
ondrejmirtesclaude
andcommitted
Treat legacy-null and sealed-marker as equal in ConstantArrayType::equals()
`equals()` already documented that a null unsealed slot (legacy / pre-bleeding-edge, `isUnsealed()` = `Maybe`) and the `[NeverType, NeverType]` sealed marker (bleeding edge, `isUnsealed()` = `No`) both mean "no real extras" and should compare equal. But the implementation used `isUnsealed()->no()`, which is `false` for the null case and `true` for the marker — so a legacy-null shape and a marker-sealed shape of otherwise-identical structure compared *unequal*. This surfaced as `TypeToPhpDocNodeTest::testToPhpDocNode` failing only under old PHPUnit (PHP < 8.2): there the data provider runs before the test container enables bleeding edge, so the directly-constructed expected type gets a null slot while the round-trip-parsed type (built after bleeding edge is on) gets the marker. New PHPUnit runs data providers after container init, so both sides got the marker and the bug stayed hidden. Compare on `isUnsealed()->yes()` ("has real extras") instead, so null and marker are both treated as sealed and only genuine extras are compared. Added a timing-independent unit test that constructs both forms directly via `BleedingEdgeToggle`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e40cef4 commit e2d4e60

2 files changed

Lines changed: 38 additions & 8 deletions

File tree

src/Type/Constant/ConstantArrayType.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -903,17 +903,21 @@ public function equals(Type $type): bool
903903
return false;
904904
}
905905

906-
// Both `unsealed === null` and `unsealed === [explicitNever, explicitNever]`
907-
// mean "sealed", just from different code paths (pre-bleeding-edge vs.
908-
// fresh bleeding-edge builder). Treat them as equivalent here, only
909-
// comparing the actual extras when both sides have real ones.
910-
$thisIsSealed = $this->isUnsealed()->no();
911-
$otherIsSealed = $type->isUnsealed()->no();
912-
if ($thisIsSealed !== $otherIsSealed) {
906+
// Both `unsealed === null` (legacy / pre-bleeding-edge, where
907+
// `isUnsealed()` answers `Maybe`) and `unsealed === [explicitNever,
908+
// explicitNever]` (the fresh bleeding-edge sealed marker, where
909+
// `isUnsealed()` answers `No`) mean "no real extras". Treat them as
910+
// equivalent here — use `!isUnsealed()->yes()` rather than
911+
// `isUnsealed()->no()`, otherwise a legacy-null shape and a
912+
// marker-sealed shape compare unequal. Only compare the actual
913+
// extras when both sides genuinely have them.
914+
$thisHasExtras = $this->isUnsealed()->yes();
915+
$otherHasExtras = $type->isUnsealed()->yes();
916+
if ($thisHasExtras !== $otherHasExtras) {
913917
return false;
914918
}
915919

916-
if (!$thisIsSealed && $this->unsealed !== null && $type->unsealed !== null) {
920+
if ($thisHasExtras && $this->unsealed !== null && $type->unsealed !== null) {
917921
if (!$this->unsealed[0]->equals($type->unsealed[0])) {
918922
return false;
919923
}

tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,32 @@ public function testHasOffsetValueType(
15491549
);
15501550
}
15511551

1552+
public function testEqualsTreatsLegacyNullAndSealedMarkerAsEqual(): void
1553+
{
1554+
$bleedingEdgeBackup = BleedingEdgeToggle::isBleedingEdge();
1555+
1556+
try {
1557+
// Pre-bleeding-edge construction leaves the unsealed slot null
1558+
// (`isUnsealed()` answers `Maybe`).
1559+
BleedingEdgeToggle::setBleedingEdge(false);
1560+
$legacyNull = new ConstantArrayType([new ConstantStringType('a')], [new IntegerType()]);
1561+
1562+
// Bleeding-edge construction seeds the `[NeverType, NeverType]`
1563+
// sealed marker (`isUnsealed()` answers `No`).
1564+
BleedingEdgeToggle::setBleedingEdge(true);
1565+
$sealedMarker = new ConstantArrayType([new ConstantStringType('a')], [new IntegerType()]);
1566+
1567+
// Both represent the same sealed shape, so they must compare
1568+
// equal in both directions — this mismatch is what made the
1569+
// `TypeToPhpDocNode` round-trip fail under old PHPUnit (data
1570+
// providers run before the container enables bleeding edge).
1571+
$this->assertTrue($legacyNull->equals($sealedMarker), 'legacy-null should equal sealed-marker');
1572+
$this->assertTrue($sealedMarker->equals($legacyNull), 'sealed-marker should equal legacy-null');
1573+
} finally {
1574+
BleedingEdgeToggle::setBleedingEdge($bleedingEdgeBackup);
1575+
}
1576+
}
1577+
15521578
public function testSealedness(): void
15531579
{
15541580
$bleedingEdgeBackup = BleedingEdgeToggle::isBleedingEdge();

0 commit comments

Comments
 (0)