Skip to content

Commit 24de99b

Browse files
committed
Fix #12222
1 parent 0da93b4 commit 24de99b

File tree

5 files changed

+156
-3
lines changed

5 files changed

+156
-3
lines changed

CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,10 @@ Fixes typically involve `ConstantArrayType`, `TypeSpecifier` (for narrowing afte
280280

281281
The degradation loses specific key information. To preserve it, `getArrayType()` tracks `HasOffsetValueType` accessories for non-optional keys from constant array spreads with string keys. After building, these are intersected with the degraded result. When a non-constant spread appears later that could overwrite tracked keys (its key type is a supertype of the tracked offsets), those entries are invalidated. This ensures correct handling of PHP's spread ordering semantics where later spreads override earlier ones for same-named string keys.
282282

283+
### Nullsafe operator and ensureShallowNonNullability / revertNonNullability
284+
285+
`NodeScopeResolver` handles `NullsafeMethodCall` and `NullsafePropertyFetch` by temporarily removing null from the variable's type (`ensureShallowNonNullability`), processing the inner expression, then restoring the original nullable type (`revertNonNullability`). When the expression is an `ArrayDimFetch` (e.g. `$arr['key']?->method()`), `specifyExpressionType` recursively narrows the parent array type via `TypeCombinator::intersect` with `HasOffsetValueType`. This intersection only narrows and cannot widen, so `revertNonNullability` fails to restore the parent array's offset type. The fix is to also save and restore the parent expression's type in `ensureShallowNonNullability`. Without this, subsequent uses of the same nullsafe call are falsely reported as "Using nullsafe method call on non-nullable type" because the parent array retains the narrowed (non-null) offset type.
286+
283287
### Loop analysis: foreach, for, while
284288

285289
Loops are a frequent source of false positives because PHPStan must reason about types across iterations:

src/Analyser/NodeScopeResolver.php

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@
214214
use function array_pop;
215215
use function array_reverse;
216216
use function array_slice;
217+
use function array_unshift;
217218
use function array_values;
218219
use function base64_decode;
219220
use function count;
@@ -2462,6 +2463,29 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
24622463
}
24632464

24642465
$nativeType = $scope->getNativeType($exprToSpecify);
2466+
2467+
$specifiedExpressions = [
2468+
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty),
2469+
];
2470+
2471+
// When narrowing an ArrayDimFetch, specifyExpressionType also recursively
2472+
// narrows the parent array's offset type via intersection with HasOffsetValueType.
2473+
// To properly revert this, we must also save and restore the parent expression's type.
2474+
if ($exprToSpecify instanceof Expr\ArrayDimFetch && $exprToSpecify->dim !== null) {
2475+
$parentExpr = $exprToSpecify->var;
2476+
$parentCertainty = TrinaryLogic::createYes();
2477+
$hasParentExpressionType = $originalScope->hasExpressionType($parentExpr);
2478+
if (!$hasParentExpressionType->no()) {
2479+
$parentCertainty = $hasParentExpressionType;
2480+
}
2481+
array_unshift($specifiedExpressions, new EnsuredNonNullabilityResultExpression(
2482+
$parentExpr,
2483+
$scope->getType($parentExpr),
2484+
$scope->getNativeType($parentExpr),
2485+
$parentCertainty,
2486+
));
2487+
}
2488+
24652489
$scope = $scope->specifyExpressionType(
24662490
$exprToSpecify,
24672491
$exprTypeWithoutNull,
@@ -2471,9 +2495,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
24712495

24722496
return new EnsuredNonNullabilityResult(
24732497
$scope,
2474-
[
2475-
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty),
2476-
],
2498+
$specifiedExpressions,
24772499
);
24782500
}
24792501

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12222;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
enum ContractStatus: string
8+
{
9+
case ACTIVE = 'A';
10+
case BEING_TERMINATED = 'B';
11+
case TERMINATED = 'C';
12+
13+
public function isActive(): bool
14+
{
15+
return $this === self::ACTIVE;
16+
}
17+
18+
public function isBeingTerminated(): bool
19+
{
20+
return $this === self::BEING_TERMINATED;
21+
}
22+
23+
public function isTerminated(): bool
24+
{
25+
return $this === self::TERMINATED;
26+
}
27+
}
28+
29+
/**
30+
* @phpstan-type Contract array{
31+
* reference: string,
32+
* status: null|ContractStatus,
33+
* startDate: string,
34+
* isActive: bool,
35+
* isBeingTerminated: bool,
36+
* isTerminated: bool
37+
* }
38+
*/
39+
class DataProcessor
40+
{
41+
/**
42+
* @param mixed[] $data
43+
* @return Contract
44+
*/
45+
public function process(array $data): array
46+
{
47+
/** @var Contract $contract */
48+
$contract = [
49+
'reference' => $data['reference'],
50+
'status' => '' !== $data['status'] ? ContractStatus::from($data['status']) : null,
51+
'startDate' => $data['startDate'],
52+
];
53+
54+
assertType('Bug12222\ContractStatus|null', $contract['status']);
55+
$contract['isActive'] = $contract['status']?->isActive();
56+
assertType('Bug12222\ContractStatus|null', $contract['status']);
57+
$contract['isBeingTerminated'] = $contract['status']?->isBeingTerminated();
58+
assertType('Bug12222\ContractStatus|null', $contract['status']);
59+
$contract['isTerminated'] = $contract['status']?->isTerminated();
60+
61+
return $contract;
62+
}
63+
}

tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,10 @@ public function testBug8523c(): void
6464
$this->analyse([__DIR__ . '/data/bug-8523c.php'], []);
6565
}
6666

67+
#[RequiresPhp('>= 8.1')]
68+
public function testBug12222(): void
69+
{
70+
$this->analyse([__DIR__ . '/data/bug-12222.php'], []);
71+
}
72+
6773
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12222;
4+
5+
enum ContractStatus: string
6+
{
7+
case ACTIVE = 'A';
8+
case BEING_TERMINATED = 'B';
9+
case TERMINATED = 'C';
10+
11+
public function isActive(): bool
12+
{
13+
return $this === self::ACTIVE;
14+
}
15+
16+
public function isBeingTerminated(): bool
17+
{
18+
return $this === self::BEING_TERMINATED;
19+
}
20+
21+
public function isTerminated(): bool
22+
{
23+
return $this === self::TERMINATED;
24+
}
25+
}
26+
27+
/**
28+
* @phpstan-type Contract array{
29+
* reference: string,
30+
* status: null|ContractStatus,
31+
* startDate: string,
32+
* isActive: bool,
33+
* isBeingTerminated: bool,
34+
* isTerminated: bool
35+
* }
36+
*/
37+
class DataProcessor
38+
{
39+
/**
40+
* @param mixed[] $data
41+
* @return Contract
42+
*/
43+
public function process(array $data): array
44+
{
45+
/** @var Contract $contract */
46+
$contract = [
47+
'reference' => $data['reference'],
48+
'status' => '' !== $data['status'] ? ContractStatus::from($data['status']) : null,
49+
'startDate' => $data['startDate'],
50+
];
51+
52+
$contract['isActive'] = $contract['status']?->isActive();
53+
$contract['isBeingTerminated'] = $contract['status']?->isBeingTerminated();
54+
$contract['isTerminated'] = $contract['status']?->isTerminated();
55+
56+
return $contract;
57+
}
58+
}

0 commit comments

Comments
 (0)