🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

Commit c53a7b8

Browse files
phpstan-botVincentLangletclaudestaabm
authored
Keep subtype-absorbed variables as conditional-expression targets when merging branches (#5876)
Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Markus Staab <markus.staab@redaxo.de>
1 parent f692c09 commit c53a7b8

11 files changed

Lines changed: 183 additions & 15 deletions

File tree

‎src/Analyser/ExprHandler/FuncCallHandler.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
336336
if (
337337
$functionReflection !== null
338338
&& $this->rememberPossiblyImpureFunctionValues
339-
&& $parametersAcceptor !== null
340339
&& $functionReflection->hasSideEffects()->maybe()
341340
&& !$functionReflection->isBuiltin()
342341
) {

‎src/Analyser/MutatingScope.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3787,6 +3787,13 @@ private function createConditionalExpressions(
37873787
): array
37883788
{
37893789
$newVariableTypes = $ourExpressionTypes;
3790+
3791+
// When our-branch type is a subtype of their-branch type, the union
3792+
// absorbs it (merged === their). Such a variable is a poor *guard* —
3793+
// asserting its our-branch type later wouldn't reliably select this
3794+
// branch — but it remains a valid conditional *target*, so only exclude
3795+
// it from guard selection instead of dropping it entirely.
3796+
$guardsToExclude = [];
37903797
foreach ($theirExpressionTypes as $exprString => $holder) {
37913798
if (!array_key_exists($exprString, $mergedExpressionTypes)) {
37923799
continue;
@@ -3804,7 +3811,7 @@ private function createConditionalExpressions(
38043811
continue;
38053812
}
38063813

3807-
unset($newVariableTypes[$exprString]);
3814+
$guardsToExclude[$exprString] = true;
38083815
}
38093816

38103817
$typeGuards = [];
@@ -3818,6 +3825,9 @@ private function createConditionalExpressions(
38183825
if (!$holder->getCertainty()->yes()) {
38193826
continue;
38203827
}
3828+
if (array_key_exists($exprString, $guardsToExclude)) {
3829+
continue;
3830+
}
38213831

38223832
if (
38233833
array_key_exists($exprString, $theirExpressionTypes)

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
112112

113113
if ($identifier->isClass()) {
114114
$fetchedClassNode = null;
115+
$fetchedFile = null;
115116
foreach ($files as $file) {
116117
$fetchedClassNodes = $this->fileNodesFetcher->fetchNodes($file)->getClassNodes();
117118

@@ -121,13 +122,10 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
121122

122123
/** @var FetchedNode<Node\Stmt\ClassLike> $fetchedClassNode */
123124
$fetchedClassNode = current($fetchedClassNodes[$identifierName]);
125+
$fetchedFile = $file;
124126
}
125127

126-
if ($fetchedClassNode === null) {
127-
return null;
128-
}
129-
130-
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier);
128+
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($fetchedFile, $identifier);
131129
$classReflection = $this->nodeToReflection($reflector, $fetchedClassNode);
132130
$this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache());
133131

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug11281;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
function hello2(string $values): void
10+
{
11+
$values = json_decode($values);
12+
$hasError = false;
13+
try {
14+
$values = array_map(static function ($item) {
15+
return Hello::fromObject($item);
16+
}, $values);
17+
assertType('array<Bug11281\Hello>', $values);
18+
} catch (\Throwable) {
19+
$hasError = true;
20+
}
21+
if (!$hasError) {
22+
// The successful try-branch proves $values is array<Hello>; the
23+
// pre-assignment mixed must not make the merged type collapse to mixed.
24+
assertType('array<Bug11281\Hello>', $values);
25+
}
26+
}
27+
28+
final class Hello
29+
{
30+
31+
public function __construct(public int $a)
32+
{
33+
}
34+
35+
public static function fromObject(\stdClass $object): self
36+
{
37+
return new self(...(array) $object);
38+
}
39+
40+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function testWithBooleans($data): void
4949

5050
if ($update) {
5151
assertType('10', $data);
52-
assertType('bool', $foo);
52+
assertType('true', $foo);
5353
} else {
5454
assertType('1|2|3', $data);
5555
assertType('bool', $foo);

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug7948;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class HelloWorld
8+
{
9+
/**
10+
* @param string|array<string, mixed> $name
11+
* @param mixed $value
12+
*/
13+
public function testMixed($name, $value): void
14+
{
15+
if (is_array($name)) {
16+
$value = null;
17+
}
18+
19+
if (is_array($name)) {
20+
assertType('null', $value);
21+
}
22+
}
23+
24+
/**
25+
* @param string|array<string, mixed> $name
26+
* @param int $value
27+
*/
28+
public function testInt($name, $value): void
29+
{
30+
if (is_array($name)) {
31+
$value = null;
32+
}
33+
34+
if (is_array($name)) {
35+
assertType('null', $value);
36+
}
37+
}
38+
39+
/**
40+
* Assigned value (5) is a subtype of the original type (int|string),
41+
* so the merged type absorbs it just like null|mixed does.
42+
*
43+
* @param string|array<string, mixed> $name
44+
* @param int|string $value
45+
*/
46+
public function testSubtype($name, $value): void
47+
{
48+
if (is_array($name)) {
49+
$value = 5;
50+
}
51+
52+
if (is_array($name)) {
53+
assertType('5', $value);
54+
}
55+
}
56+
57+
/**
58+
* @param string|array<string, mixed> $name
59+
* @param mixed $value
60+
*/
61+
public function testMixedNegated($name, $value): void
62+
{
63+
if (!is_array($name)) {
64+
$value = null;
65+
}
66+
67+
if (!is_array($name)) {
68+
assertType('null', $value);
69+
}
70+
}
71+
72+
/**
73+
* Boolean-flag guard variant: assigning null to a ?string absorbs into
74+
* string|null, so $cwd must re-narrow to null under the repeated flag check.
75+
*/
76+
public function testBooleanFlag(?string $cwd, bool $initialClone = false): void
77+
{
78+
if ($initialClone) {
79+
$cwd = null;
80+
}
81+
82+
if ($initialClone) {
83+
assertType('null', $cwd);
84+
}
85+
}
86+
}

‎tests/PHPStan/Analyser/nsrt/bug-8467b.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public function foo (?string $cwd, bool $initialClone = false): void {
1313

1414
if ($initialClone && isset($origCwd)) {
1515
assertType('string', $origCwd);
16-
assertType('string|null', $cwd); // could be null
16+
assertType('null', $cwd);
1717
}
1818
}
1919

‎tests/PHPStan/Analyser/nsrt/non-empty-string-str-containing-fns.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public function variants(string $s) {
112112
assertType('string', $s);
113113

114114
if (strpos($s, ':') === 5) {
115-
assertType('string', $s); // could be non-empty-string
115+
assertType('non-falsy-string', $s);
116116
}
117117
assertType('string', $s);
118118
if (strpos($s, ':') !== 5) {
@@ -164,7 +164,7 @@ public function variants(string $s) {
164164
assertType('string', $s);
165165

166166
if (mb_strpos($s, ':') === 5) {
167-
assertType('string', $s); // could be non-empty-string
167+
assertType('non-falsy-string', $s);
168168
}
169169
assertType('string', $s);
170170
if (mb_strpos($s, ':') !== 5) {

‎tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,4 +1302,12 @@ public function testBug2861(): void
13021302
$this->analyse([__DIR__ . '/data/bug-2861.php'], []);
13031303
}
13041304

1305+
public function testBug4905(): void
1306+
{
1307+
$this->checkThisOnly = false;
1308+
$this->checkUnionTypes = true;
1309+
$this->checkDynamicProperties = false;
1310+
$this->analyse([__DIR__ . '/data/bug-4905.php'], []);
1311+
}
1312+
13051313
}

‎tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,6 @@ public function testTypesAssignedToPropertiesExpressionNames(): void
172172
'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.',
173173
69,
174174
],
175-
[
176-
'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float.',
177-
83,
178-
],
179175
[
180176
'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float|int|string.',
181177
97,

0 commit comments

Comments
 (0)