diff --git a/CLAUDE.md b/CLAUDE.md index 840b0377dd..10279d2f0d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -280,6 +280,10 @@ Fixes typically involve `ConstantArrayType`, `TypeSpecifier` (for narrowing afte 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. +### Nullsafe operator and ensureShallowNonNullability / revertNonNullability + +`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. + ### Loop analysis: foreach, for, while Loops are a frequent source of false positives because PHPStan must reason about types across iterations: diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 828e2685cf..8a807ea745 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -214,6 +214,7 @@ use function array_pop; use function array_reverse; use function array_slice; +use function array_unshift; use function array_values; use function base64_decode; use function count; @@ -2462,6 +2463,29 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin } $nativeType = $scope->getNativeType($exprToSpecify); + + $specifiedExpressions = [ + new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty), + ]; + + // When narrowing an ArrayDimFetch, specifyExpressionType also recursively + // narrows the parent array's offset type via intersection with HasOffsetValueType. + // To properly revert this, we must also save and restore the parent expression's type. + if ($exprToSpecify instanceof Expr\ArrayDimFetch && $exprToSpecify->dim !== null) { + $parentExpr = $exprToSpecify->var; + $parentCertainty = TrinaryLogic::createYes(); + $hasParentExpressionType = $originalScope->hasExpressionType($parentExpr); + if (!$hasParentExpressionType->no()) { + $parentCertainty = $hasParentExpressionType; + } + array_unshift($specifiedExpressions, new EnsuredNonNullabilityResultExpression( + $parentExpr, + $scope->getType($parentExpr), + $scope->getNativeType($parentExpr), + $parentCertainty, + )); + } + $scope = $scope->specifyExpressionType( $exprToSpecify, $exprTypeWithoutNull, @@ -2471,9 +2495,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin return new EnsuredNonNullabilityResult( $scope, - [ - new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty), - ], + $specifiedExpressions, ); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-12222.php b/tests/PHPStan/Analyser/nsrt/bug-12222.php new file mode 100644 index 0000000000..191b8d758a --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12222.php @@ -0,0 +1,63 @@ += 8.1 + +namespace Bug12222; + +use function PHPStan\Testing\assertType; + +enum ContractStatus: string +{ + case ACTIVE = 'A'; + case BEING_TERMINATED = 'B'; + case TERMINATED = 'C'; + + public function isActive(): bool + { + return $this === self::ACTIVE; + } + + public function isBeingTerminated(): bool + { + return $this === self::BEING_TERMINATED; + } + + public function isTerminated(): bool + { + return $this === self::TERMINATED; + } +} + +/** + * @phpstan-type Contract array{ + * reference: string, + * status: null|ContractStatus, + * startDate: string, + * isActive: bool, + * isBeingTerminated: bool, + * isTerminated: bool + * } + */ +class DataProcessor +{ + /** + * @param mixed[] $data + * @return Contract + */ + public function process(array $data): array + { + /** @var Contract $contract */ + $contract = [ + 'reference' => $data['reference'], + 'status' => '' !== $data['status'] ? ContractStatus::from($data['status']) : null, + 'startDate' => $data['startDate'], + ]; + + assertType('Bug12222\ContractStatus|null', $contract['status']); + $contract['isActive'] = $contract['status']?->isActive(); + assertType('Bug12222\ContractStatus|null', $contract['status']); + $contract['isBeingTerminated'] = $contract['status']?->isBeingTerminated(); + assertType('Bug12222\ContractStatus|null', $contract['status']); + $contract['isTerminated'] = $contract['status']?->isTerminated(); + + return $contract; + } +} diff --git a/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php index ff7b48186a..80696dfe6b 100644 --- a/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php @@ -64,4 +64,10 @@ public function testBug8523c(): void $this->analyse([__DIR__ . '/data/bug-8523c.php'], []); } + #[RequiresPhp('>= 8.1')] + public function testBug12222(): void + { + $this->analyse([__DIR__ . '/data/bug-12222.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-12222.php b/tests/PHPStan/Rules/Methods/data/bug-12222.php new file mode 100644 index 0000000000..5efb074b80 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12222.php @@ -0,0 +1,58 @@ += 8.1 + +namespace Bug12222; + +enum ContractStatus: string +{ + case ACTIVE = 'A'; + case BEING_TERMINATED = 'B'; + case TERMINATED = 'C'; + + public function isActive(): bool + { + return $this === self::ACTIVE; + } + + public function isBeingTerminated(): bool + { + return $this === self::BEING_TERMINATED; + } + + public function isTerminated(): bool + { + return $this === self::TERMINATED; + } +} + +/** + * @phpstan-type Contract array{ + * reference: string, + * status: null|ContractStatus, + * startDate: string, + * isActive: bool, + * isBeingTerminated: bool, + * isTerminated: bool + * } + */ +class DataProcessor +{ + /** + * @param mixed[] $data + * @return Contract + */ + public function process(array $data): array + { + /** @var Contract $contract */ + $contract = [ + 'reference' => $data['reference'], + 'status' => '' !== $data['status'] ? ContractStatus::from($data['status']) : null, + 'startDate' => $data['startDate'], + ]; + + $contract['isActive'] = $contract['status']?->isActive(); + $contract['isBeingTerminated'] = $contract['status']?->isBeingTerminated(); + $contract['isTerminated'] = $contract['status']?->isTerminated(); + + return $contract; + } +}