Skip to content

Commit 580f63b

Browse files
phpstan-botclaude
andcommitted
Leave callbacks collecting by spl_object_id() to TypeTraverser::map()
Keying collected values by the Type instance id only emulates collecting every occurrence, so TypeTraverserMapMemoizedRule now treats such a write as depending on the occurrences. ConditionalReturnTypeRuleHelper and GenericCallableRuleHelper go back to map(); the latter still collects the errors of every callable instead of keeping only the last one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 12306ca commit 580f63b

6 files changed

Lines changed: 101 additions & 23 deletions

File tree

‎build/PHPStan/Build/TypeTraverserMapMemoizedRule.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PhpParser\Node\Expr\CallLike;
1313
use PhpParser\Node\Expr\Closure;
1414
use PhpParser\Node\Expr\ConstFetch;
15+
use PhpParser\Node\Expr\FuncCall;
1516
use PhpParser\Node\Expr\PostDec;
1617
use PhpParser\Node\Expr\PostInc;
1718
use PhpParser\Node\Expr\PreDec;
@@ -52,6 +53,7 @@
5253
* the occurrences when it:
5354
* - writes to a property or declares a static variable,
5455
* - appends to, increments or otherwise accumulates into a variable captured by reference,
56+
* - writes to a variable captured by reference keyed by spl_object_id(), which collects every instance,
5557
* - assigns a non-constant value to a variable captured by reference,
5658
* - passes a variable captured by reference to a call,
5759
* - reads a variable captured by reference that it also writes, because the state
@@ -249,9 +251,12 @@ private function processWrite(
249251
{
250252
$root = $target;
251253
$appends = false;
254+
$keysByObjectId = false;
252255
while ($root instanceof ArrayDimFetch) {
253256
if ($root->dim === null) {
254257
$appends = true;
258+
} elseif ($this->isObjectIdCall($root->dim)) {
259+
$keysByObjectId = true;
255260
}
256261
$root = $root->var;
257262
}
@@ -270,6 +275,9 @@ private function processWrite(
270275

271276
if ($appends) {
272277
$reasons[] = sprintf('appends to $%s captured by reference', $name);
278+
} elseif ($keysByObjectId) {
279+
// collecting every instance by its id is collecting every occurrence, leave that to map()
280+
$reasons[] = sprintf('writes to $%s captured by reference by spl_object_id()', $name);
273281
} elseif ($target !== $root) {
274282
// writing or unsetting by key
275283
if ($node instanceof Assign || $node instanceof Unset_) {
@@ -298,6 +306,13 @@ private function getByRefVariableName(Expr $expr, array $byRefNames): ?string
298306
return $expr->name;
299307
}
300308

309+
private function isObjectIdCall(Expr $expr): bool
310+
{
311+
return $expr instanceof FuncCall
312+
&& $expr->name instanceof Name
313+
&& $expr->name->toLowerString() === 'spl_object_id';
314+
}
315+
301316
private function isConstant(Expr $expr): bool
302317
{
303318
return $expr instanceof Scalar || $expr instanceof ConstFetch;

‎src/Rules/PhpDoc/ConditionalReturnTypeRuleHelper.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use PHPStan\Type\VerbosityLevel;
1616
use function array_key_exists;
1717
use function count;
18-
use function spl_object_id;
1918
use function sprintf;
2019
use function substr;
2120

@@ -31,28 +30,28 @@ public function check(ExtendedParametersAcceptor $acceptor): array
3130
$conditionalTypes = [];
3231
$parametersByName = [];
3332
foreach ($acceptor->getParameters() as $parameter) {
34-
TypeTraverser::mapMemoized($parameter->getType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
33+
TypeTraverser::map($parameter->getType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
3534
if ($type instanceof ConditionalType || $type instanceof ConditionalTypeForParameter) {
36-
$conditionalTypes[spl_object_id($type)] = $type;
35+
$conditionalTypes[] = $type;
3736
}
3837

3938
return $traverse($type);
4039
});
4140

4241
if ($parameter->getOutType() !== null) {
43-
TypeTraverser::mapMemoized($parameter->getOutType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
42+
TypeTraverser::map($parameter->getOutType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
4443
if ($type instanceof ConditionalType || $type instanceof ConditionalTypeForParameter) {
45-
$conditionalTypes[spl_object_id($type)] = $type;
44+
$conditionalTypes[] = $type;
4645
}
4746

4847
return $traverse($type);
4948
});
5049
}
5150

5251
if ($parameter->getClosureThisType() !== null) {
53-
TypeTraverser::mapMemoized($parameter->getClosureThisType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
52+
TypeTraverser::map($parameter->getClosureThisType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
5453
if ($type instanceof ConditionalType || $type instanceof ConditionalTypeForParameter) {
55-
$conditionalTypes[spl_object_id($type)] = $type;
54+
$conditionalTypes[] = $type;
5655
}
5756

5857
return $traverse($type);
@@ -62,9 +61,9 @@ public function check(ExtendedParametersAcceptor $acceptor): array
6261
$parametersByName[$parameter->getName()] = $parameter;
6362
}
6463

65-
TypeTraverser::mapMemoized($acceptor->getReturnType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
64+
TypeTraverser::map($acceptor->getReturnType(), static function (Type $type, callable $traverse) use (&$conditionalTypes): Type {
6665
if ($type instanceof ConditionalType || $type instanceof ConditionalTypeForParameter) {
67-
$conditionalTypes[spl_object_id($type)] = $type;
66+
$conditionalTypes[] = $type;
6867
}
6968

7069
return $traverse($type);

‎src/Rules/PhpDoc/GenericCallableRuleHelper.php‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
use PHPStan\Type\TypeTraverser;
1818
use PHPStan\Type\VerbosityLevel;
1919
use function array_keys;
20-
use function array_merge;
21-
use function array_values;
22-
use function spl_object_id;
2320
use function sprintf;
2421

2522
#[AutowiredService]
@@ -47,10 +44,9 @@ public function check(
4744
?ClassReflection $classReflection,
4845
): array
4946
{
50-
/** @var array<int, list<IdentifierRuleError>> $errorsByCallable */
51-
$errorsByCallable = [];
47+
$allErrors = [];
5248

53-
TypeTraverser::mapMemoized($callableType, function (Type $type, callable $traverse) use (&$errorsByCallable, $node, $scope, $location, $functionName, $functionTemplateTags, $classReflection) {
49+
TypeTraverser::map($callableType, function (Type $type, callable $traverse) use (&$allErrors, $node, $scope, $location, $functionName, $functionTemplateTags, $classReflection) {
5450
if (!($type instanceof CallableType || $type instanceof ClosureType)) {
5551
return $traverse($type);
5652
}
@@ -117,12 +113,14 @@ public function check(
117113
}
118114
}
119115

120-
$errorsByCallable[spl_object_id($type)] = $errors;
116+
foreach ($errors as $error) {
117+
$allErrors[] = $error;
118+
}
121119

122120
return $traverse($type);
123121
});
124122

125-
return array_merge(...array_values($errorsByCallable));
123+
return $allErrors;
126124
}
127125

128126
}

‎tests/PHPStan/Build/TypeTraverserMapMemoizedRuleTest.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,31 @@ public function testRule(): void
2222
$this->analyse([__DIR__ . '/data/type-traverser-map-memoized.php'], [
2323
[
2424
'Callback of TypeTraverser::map() does not depend on where or how many times a Type instance occurs. Use TypeTraverser::mapMemoized() instead.',
25-
19,
25+
20,
2626
],
2727
[
2828
'Callback of TypeTraverser::map() does not depend on where or how many times a Type instance occurs. Use TypeTraverser::mapMemoized() instead.',
29-
30,
29+
31,
3030
],
3131
[
3232
'Callback of TypeTraverser::map() does not depend on where or how many times a Type instance occurs. Use TypeTraverser::mapMemoized() instead.',
33-
37,
33+
38,
3434
],
3535
[
3636
'Callback of TypeTraverser::map() does not depend on where or how many times a Type instance occurs. Use TypeTraverser::mapMemoized() instead.',
37-
53,
37+
54,
3838
],
3939
[
4040
'Callback of TypeTraverser::mapMemoized() appends to $types captured by reference, so it might depend on where or how many times a Type instance occurs. Use TypeTraverser::map() instead.',
41-
168,
41+
169,
4242
],
4343
[
4444
'Callback of TypeTraverser::mapMemoized() reads $found captured by reference that it also writes, so it might depend on where or how many times a Type instance occurs. Use TypeTraverser::map() instead.',
45-
180,
45+
181,
46+
],
47+
[
48+
'Callback of TypeTraverser::mapMemoized() writes to $types captured by reference by spl_object_id(), so it might depend on where or how many times a Type instance occurs. Use TypeTraverser::map() instead.',
49+
196,
4650
],
4751
]);
4852
}

‎tests/PHPStan/Build/data/type-traverser-map-memoized.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPStan\Type\Type;
88
use PHPStan\Type\TypeTraverser;
99
use function array_merge;
10+
use function spl_object_id;
1011

1112
class Foo
1213
{
@@ -186,6 +187,36 @@ public function memoizedFlagReadWithUsedResult(Type $type): Type
186187
});
187188
}
188189

190+
/**
191+
* @return array<int, Type>
192+
*/
193+
public function memoizedKeyedByObjectId(Type $type): array
194+
{
195+
$types = [];
196+
TypeTraverser::mapMemoized($type, static function (Type $type, callable $traverse) use (&$types): Type {
197+
$types[spl_object_id($type)] = $type;
198+
199+
return $traverse($type);
200+
});
201+
202+
return $types;
203+
}
204+
205+
/**
206+
* @return array<int, Type>
207+
*/
208+
public function keyedByObjectId(Type $type): array
209+
{
210+
$types = [];
211+
TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$types): Type {
212+
$types[spl_object_id($type)] = $type;
213+
214+
return $traverse($type);
215+
});
216+
217+
return $types;
218+
}
219+
189220
public function notAClosure(Type $type, callable $cb): Type
190221
{
191222
return TypeTraverser::map($type, $cb);

‎tests/PHPStan/Build/data/type-traverser-map-memoized.php.fixed‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use PHPStan\Type\IntegerType;
77
use PHPStan\Type\Type;
88
use PHPStan\Type\TypeTraverser;
99
use function array_merge;
10+
use function spl_object_id;
1011

1112
class Foo
1213
{
@@ -186,6 +187,36 @@ class Foo
186187
});
187188
}
188189

190+
/**
191+
* @return array<int, Type>
192+
*/
193+
public function memoizedKeyedByObjectId(Type $type): array
194+
{
195+
$types = [];
196+
TypeTraverser::mapMemoized($type, static function (Type $type, callable $traverse) use (&$types): Type {
197+
$types[spl_object_id($type)] = $type;
198+
199+
return $traverse($type);
200+
});
201+
202+
return $types;
203+
}
204+
205+
/**
206+
* @return array<int, Type>
207+
*/
208+
public function keyedByObjectId(Type $type): array
209+
{
210+
$types = [];
211+
TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$types): Type {
212+
$types[spl_object_id($type)] = $type;
213+
214+
return $traverse($type);
215+
});
216+
217+
return $types;
218+
}
219+
189220
public function notAClosure(Type $type, callable $cb): Type
190221
{
191222
return TypeTraverser::map($type, $cb);

0 commit comments

Comments
 (0)