Skip to content

Commit 408f6e7

Browse files
ondrejmirtesondrejmirtes
committed
Merge branch 2.2.x into 2.3.x
# Conflicts: # src/Analyser/ExprHandler/AssignHandler.php
2 parents 05c338b + a88f398 commit 408f6e7

14 files changed

Lines changed: 628 additions & 58 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 252 additions & 12 deletions
Large diffs are not rendered by default.

‎src/Analyser/RuleErrorTransformer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ private function fix(string $fixingFile, array $fileNodes, Node $originalNode, c
188188
}
189189
}
190190

191-
if (!$replacingVisitor->isFound() || $indent === null) {
191+
if (!$replacingVisitor->isFound()) {
192192
return null;
193193
}
194194

‎src/Analyser/StmtHandler/WhileHandler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public function processStmt(
111111
$bodyScope = $backEdgeScope;
112112
// the candidate to replace the final walk when this pass's
113113
// entry turns out to be the fixpoint
114-
if ($condRecording instanceof RecordingNodeCallback && $bodyRecording instanceof RecordingNodeCallback) {
114+
if ($condRecording instanceof RecordingNodeCallback) {
115115
$replayCondRecording = $condRecording;
116116
$replayBodyRecording = $bodyRecording;
117117
$replayPassStorage = $storage;

‎src/Type/ObjectType.php‎

Lines changed: 88 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
use Traversable;
5959
use function array_key_exists;
6060
use function array_map;
61+
use function array_merge;
6162
use function array_values;
6263
use function count;
6364
use function get_class;
@@ -124,6 +125,14 @@ class ObjectType implements TypeWithClassName, SubtractableType
124125

125126
private ?string $cachedDescription = null;
126127

128+
/**
129+
* The class name as the reflection spells it - the properly cased name, or the
130+
* readable `class@anonymous...` form. Resolving it takes two ReflectionProvider
131+
* round-trips and both the type-only and the value level need it, the latter on
132+
* every operation RecursionGuard guards.
133+
*/
134+
private ?string $cachedPreciseName = null;
135+
127136
/**
128137
* The reflection resolved on demand by getClassReflection(), kept apart from the one
129138
* handed to the constructor. The constructor's reflection is part of the type's value
@@ -742,13 +751,21 @@ private function checkSubclassAcceptability(string $thatClass): AcceptsResult
742751

743752
public function describe(VerbosityLevel $level): string
744753
{
754+
if ($this->cachedPreciseName !== null && ($level->isValue() || $level->isTypeOnly())) {
755+
return $this->cachedPreciseName;
756+
}
757+
745758
$preciseNameCallback = function (): string {
759+
if ($this->cachedPreciseName !== null) {
760+
return $this->cachedPreciseName;
761+
}
762+
746763
$reflectionProvider = ReflectionProviderStaticAccessor::getInstance();
747764
if (!$reflectionProvider->hasClass($this->className)) {
748-
return $this->className;
765+
return $this->cachedPreciseName = $this->className;
749766
}
750767

751-
return $reflectionProvider->getClassName($this->className);
768+
return $this->cachedPreciseName = $reflectionProvider->getClassName($this->className);
752769
};
753770

754771
$preciseWithSubtracted = fn (): string => $this->className . $this->describeSubtractedType($this->subtractedType, $level);
@@ -1756,11 +1773,24 @@ public function isInstanceOf(string $className): TrinaryLogic
17561773

17571774
public function subtract(Type $type): Type
17581775
{
1759-
if ($this->subtractedType !== null) {
1760-
$type = TypeCombinator::union($this->subtractedType, $type);
1776+
if ($this->subtractedType === null) {
1777+
return $this->changeSubtractedType($type);
1778+
}
1779+
1780+
// A sealed hierarchy rebuilds its subtraction from the flattened parts
1781+
// below, so normalising the union of the old and the new subtracted type
1782+
// first is wasted work - and quadratic when a removal peels the subtypes
1783+
// off one at a time, as removing a whole enum from its own type does.
1784+
// Only the path that keeps the subtracted type as it is needs the union.
1785+
$matched = $this->matchAllowedSubTypes(array_merge(
1786+
TypeUtils::flattenTypes($this->subtractedType),
1787+
TypeUtils::flattenTypes($type),
1788+
));
1789+
if ($matched !== null) {
1790+
return $matched;
17611791
}
17621792

1763-
return $this->changeSubtractedType($type);
1793+
return $this->changeSubtractedType(TypeCombinator::union($this->subtractedType, $type));
17641794
}
17651795

17661796
public function getTypeWithoutSubtractedType(): Type
@@ -1788,55 +1818,70 @@ public function withoutFinalByKeywordOverride(): Type
17881818
public function changeSubtractedType(?Type $subtractedType): Type
17891819
{
17901820
if ($subtractedType !== null) {
1791-
$classReflection = $this->getClassReflection();
1792-
$allowedSubTypes = $classReflection !== null ? $classReflection->getAllowedSubTypes() : null;
1793-
if ($allowedSubTypes !== null) {
1794-
$preciseVerbosity = VerbosityLevel::precise();
1795-
1796-
$originalAllowedSubTypes = $allowedSubTypes;
1797-
$subtractedSubTypes = [];
1798-
1799-
$subtractedTypes = TypeUtils::flattenTypes($subtractedType);
1800-
foreach ($subtractedTypes as $subType) {
1801-
foreach ($allowedSubTypes as $key => $allowedSubType) {
1802-
if ($subType->equals($allowedSubType)) {
1803-
$description = $allowedSubType->describe($preciseVerbosity);
1804-
$subtractedSubTypes[$description] = $subType;
1805-
unset($allowedSubTypes[$key]);
1806-
continue 2;
1807-
}
1808-
}
1821+
$matched = $this->matchAllowedSubTypes(TypeUtils::flattenTypes($subtractedType));
1822+
if ($matched !== null) {
1823+
return $matched;
1824+
}
1825+
}
18091826

1810-
return new self($this->className, $subtractedType);
1811-
}
1827+
if ($this->subtractedType === null && $subtractedType === null) {
1828+
return $this;
1829+
}
18121830

1813-
if (count($allowedSubTypes) === 1) {
1814-
return array_values($allowedSubTypes)[0];
1815-
}
1831+
return new self($this->className, $subtractedType);
1832+
}
18161833

1817-
$subtractedSubTypes = array_values($subtractedSubTypes);
1818-
$subtractedSubTypesCount = count($subtractedSubTypes);
1819-
if ($subtractedSubTypesCount === count($originalAllowedSubTypes)) {
1820-
return new NeverType();
1821-
}
1834+
/**
1835+
* Rebuilds the type from the subtracted members of its sealed hierarchy.
1836+
*
1837+
* Returns null when the class has no allowed subtypes, or when a subtracted
1838+
* type is not one of them - the caller then keeps the subtracted type whole.
1839+
*
1840+
* @param array<Type> $subtractedTypes flattened, may repeat
1841+
*/
1842+
private function matchAllowedSubTypes(array $subtractedTypes): ?Type
1843+
{
1844+
$classReflection = $this->getClassReflection();
1845+
$allowedSubTypes = $classReflection !== null ? $classReflection->getAllowedSubTypes() : null;
1846+
if ($allowedSubTypes === null) {
1847+
return null;
1848+
}
18221849

1823-
if ($subtractedSubTypesCount === 0) {
1824-
return new self($this->className);
1825-
}
1850+
$allowedSubTypesCount = count($allowedSubTypes);
1851+
$subtractedSubTypes = [];
18261852

1827-
if ($subtractedSubTypesCount === 1) {
1828-
return new self($this->className, $subtractedSubTypes[0]);
1853+
foreach ($subtractedTypes as $subType) {
1854+
foreach ($allowedSubTypes as $key => $allowedSubType) {
1855+
if ($subType->equals($allowedSubType)) {
1856+
// An allowed subtype is dropped as it matches, so no two matches
1857+
// can be the same one and the matches need no keying.
1858+
$subtractedSubTypes[] = $subType;
1859+
unset($allowedSubTypes[$key]);
1860+
continue 2;
18291861
}
1830-
1831-
return new self($this->className, new UnionType($subtractedSubTypes));
18321862
}
1863+
1864+
return null;
18331865
}
18341866

1835-
if ($this->subtractedType === null && $subtractedType === null) {
1836-
return $this;
1867+
if (count($allowedSubTypes) === 1) {
1868+
return array_values($allowedSubTypes)[0];
18371869
}
18381870

1839-
return new self($this->className, $subtractedType);
1871+
$subtractedSubTypesCount = count($subtractedSubTypes);
1872+
if ($subtractedSubTypesCount === $allowedSubTypesCount) {
1873+
return new NeverType();
1874+
}
1875+
1876+
if ($subtractedSubTypesCount === 0) {
1877+
return new self($this->className);
1878+
}
1879+
1880+
if ($subtractedSubTypesCount === 1) {
1881+
return new self($this->className, $subtractedSubTypes[0]);
1882+
}
1883+
1884+
return new self($this->className, new UnionType($subtractedSubTypes));
18401885
}
18411886

18421887
public function getSubtractedType(): ?Type

‎src/Type/TypeCombinator.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,54 @@ public static function remove(Type $fromType, Type $typeToRemove): Type
9898
return self::doRemove($fromType, $typeToRemove);
9999
}
100100

101+
/**
102+
* Whether removing the union's members all at once gives what removing them
103+
* one at a time would: true when the subtraction is a set difference, so no
104+
* order of removals can sharpen the result.
105+
*
106+
* Everywhere else the members have to come off one at a time - removing one
107+
* can narrow the type so that removing the next achieves more.
108+
*/
109+
private static function canRemoveUnionAtOnce(Type $fromType): bool
110+
{
111+
// A sealed hierarchy - an enum, or a hierarchy an
112+
// AllowedSubTypesClassReflectionExtension seals - subtracts by dropping
113+
// allowed subtypes, and which ones are dropped does not depend on order.
114+
$classReflections = $fromType->getObjectClassReflections();
115+
if (count($classReflections) === 1 && $classReflections[0]->getAllowedSubTypes() !== null) {
116+
return true;
117+
}
118+
119+
// A union removes the whole thing from each of its members, and each of
120+
// those removals weighs this same question again - so a member that needs
121+
// the removals one at a time still gets them that way.
122+
return $fromType instanceof UnionType;
123+
}
124+
101125
/** @internal Delegated to from TypeCombinatorCache, which the native extension shadows to memoize it. */
102126
public static function doRemove(Type $fromType, Type $typeToRemove): Type
103127
{
104128
if ($typeToRemove instanceof UnionType) {
129+
// Removing the members one at a time is what keeps the result precise:
130+
// removing one can narrow the type so that removing the next achieves
131+
// more. A sealed hierarchy is the exception - there the subtraction is
132+
// a set difference over the allowed subtypes, so the order the members
133+
// come off cannot matter, and peeling them would rebuild the whole
134+
// subtraction once per member (removing an enum from its own type then
135+
// costs O(cases^2)).
136+
if (self::canRemoveUnionAtOnce($fromType)) {
137+
$removed = $fromType->tryRemove($typeToRemove);
138+
if ($removed !== null) {
139+
return $removed;
140+
}
141+
}
142+
105143
foreach ($typeToRemove->getTypes() as $unionTypeToRemove) {
106144
$fromType = self::remove($fromType, $unionTypeToRemove);
145+
if ($fromType instanceof NeverType) {
146+
// there is nothing left to remove from
147+
break;
148+
}
107149
}
108150
return $fromType;
109151
}

‎src/Type/UnionType.php‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
use function array_diff_assoc;
4040
use function array_fill_keys;
4141
use function array_intersect;
42+
use function array_key_exists;
4243
use function array_keys;
4344
use function array_map;
4445
use function array_merge;
@@ -1567,6 +1568,50 @@ public function tryRemove(Type $typeToRemove): ?Type
15671568

15681569
return new UnionType($remainingTypes);
15691570
}
1571+
1572+
// Every member of this union is a distinct value, and so is every member
1573+
// of the removed one: the result is their set difference, which no order
1574+
// of removals can sharpen. Taking it in one pass keeps removing a union
1575+
// of values linear instead of re-deriving this union once per value.
1576+
if ($typeToRemove instanceof self) {
1577+
$keysToRemove = [];
1578+
foreach ($typeToRemove->getTypes() as $innerTypeToRemove) {
1579+
$innerKey = FiniteTypeSet::key($innerTypeToRemove);
1580+
if ($innerKey === null) {
1581+
$keysToRemove = null;
1582+
break;
1583+
}
1584+
1585+
$keysToRemove[$innerKey] = true;
1586+
}
1587+
1588+
if ($keysToRemove !== null) {
1589+
$remainingTypes = [];
1590+
$removedAny = false;
1591+
foreach ($finiteTypeSet->getMembers() as $memberKey => $member) {
1592+
if (array_key_exists($memberKey, $keysToRemove)) {
1593+
$removedAny = true;
1594+
continue;
1595+
}
1596+
1597+
$remainingTypes[] = $member;
1598+
}
1599+
1600+
if (!$removedAny) {
1601+
return null;
1602+
}
1603+
1604+
if (count($remainingTypes) === 0) {
1605+
return new NeverType();
1606+
}
1607+
1608+
if (count($remainingTypes) === 1) {
1609+
return $remainingTypes[0];
1610+
}
1611+
1612+
return new UnionType($remainingTypes);
1613+
}
1614+
}
15701615
}
15711616

15721617
$innerTypes = [];
@@ -1617,7 +1662,11 @@ public function getFiniteTypes(): array
16171662
$types = $this->notBenevolentPickFromTypes(static fn (Type $type) => $type->getFiniteTypes());
16181663
$uniquedTypes = [];
16191664
foreach ($types as $type) {
1620-
$uniquedTypes[$type->describe(VerbosityLevel::cache())] = $type;
1665+
// These are finite types, so FiniteTypeSet keys them by the value they
1666+
// stand for - which is what the uniquing wants and what describe() was
1667+
// standing in for. Floats have no such key (equals() does not agree with
1668+
// value identity for them) and keep describing themselves.
1669+
$uniquedTypes[FiniteTypeSet::key($type) ?? $type->describe(VerbosityLevel::cache())] = $type;
16211670
}
16221671

16231672
if (count($uniquedTypes) > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) {

‎tests/PHPStan/Analyser/nsrt/bug-12620.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12620;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function (): void {
8+
$data = [];
9+
if(isset($GLOBALS['data']) && is_string($GLOBALS['data'])){
10+
$data = (array)json_decode($GLOBALS['data']);
11+
}
12+
13+
$isA = isset($data['a']) && $data['a'] !== '';
14+
$isB = isset($data['b']) && $data['b'] !== '';
15+
16+
17+
18+
if($isA){
19+
assertType("non-empty-array<mixed>&hasOffset('a')", $data);
20+
assertType("mixed~(''|null)", $data['a']);
21+
var_dump($data['a']);
22+
}
23+
if($isB){
24+
assertType("non-empty-array<mixed>&hasOffset('b')", $data);
25+
assertType("mixed~(''|null)", $data['b']);
26+
var_dump($data['b']);
27+
}
28+
};
29+
30+
// order-swapped variant: the historical bug was order-dependent
31+
function (): void {
32+
$data = [];
33+
if(isset($GLOBALS['data']) && is_string($GLOBALS['data'])){
34+
$data = (array)json_decode($GLOBALS['data']);
35+
}
36+
37+
$isB = isset($data['b']) && $data['b'] !== '';
38+
$isA = isset($data['a']) && $data['a'] !== '';
39+
40+
41+
if($isA){
42+
assertType("non-empty-array<mixed>&hasOffset('a')", $data);
43+
assertType("mixed~(''|null)", $data['a']);
44+
var_dump($data['a']);
45+
}
46+
if($isB){
47+
assertType("non-empty-array<mixed>&hasOffset('b')", $data);
48+
assertType("mixed~(''|null)", $data['b']);
49+
var_dump($data['b']);
50+
}
51+
};

0 commit comments

Comments
 (0)