From b649a9569f507efad3fd8c43ed9bac7e22b1c8ee Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Fri, 13 Nov 2026 23:27:16 +0000 Subject: [PATCH] Read untracked fetch links of an isset chain from the asking scope's state The chain links of an isset()/empty()/?? subject are walked under NonNullabilityHelper's ensured non-nullability, and ensuring a nested offset non-null also narrows the fetches it is read from. Their stored results therefore carry null-stripped types. IssetabilityDescriptor read an untracked receiver link through that stored result, so from three links on ($a['r']['a']['b'], $this->data['a']['b']) the verdict saw a nullable intermediate offset as non-null: isset() and ?? were typed as always set, the right side of ?? became unreachable and its merge dropped null from the subject, and the isset/?? rules reported the offset as always existing. An untracked array dim fetch, property fetch or static property fetch link is now read from the asking scope's state, derived from its receiver's state without a walk. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d722-233f-76e4-a119-f3620177393b --- src/Analyser/IssetabilityDescriptor.php | 33 +++++++---- tests/PHPStan/Analyser/nsrt/bug-15722.php | 24 ++++++++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 7 +++ .../Rules/Variables/NullCoalesceRuleTest.php | 5 ++ .../Rules/Variables/data/bug-15722.php | 27 +++++++++ turbo-ext/src/IssetabilityDescriptor.cpp | 56 +++++++++++++++---- 6 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15722.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-15722.php diff --git a/src/Analyser/IssetabilityDescriptor.php b/src/Analyser/IssetabilityDescriptor.php index ad575e85daa..7cc9614d578 100644 --- a/src/Analyser/IssetabilityDescriptor.php +++ b/src/Analyser/IssetabilityDescriptor.php @@ -4,6 +4,7 @@ use Closure; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Expr\Variable; @@ -98,16 +99,28 @@ public function resolve(MutatingScope $scope, bool $useNativeTypes, Expr $expr, throw new ShouldNotHappenException(); } - // A rule-facing resolution re-prices untracked links on the asking - // scope: the stored results were priced under the coalesce walk's - // ensured non-nullability (NonNullabilityHelper), so their own types - // have null already stripped - folding the isset verdict from them - // would be circular. Engine-side folds keep the stored reads (their - // scopes track the ensured links; re-pricing there would walk). - $reprocessVar = $reprocessUntrackedLinks && !$scope->hasExpressionType($varResult->getExpr())->yes(); - $varType = $reprocessVar - ? ($useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain()->getNativeType($varResult->getExpr()) : $scope->getType($varResult->getExpr())) - : $varResult->getTypeOnScope($scope, $useNativeTypes); + // The stored results of the chain links were priced under the + // isset/empty/?? walk's ensured non-nullability (NonNullabilityHelper): + // a link deviced non-null also narrows the fetches it is read from, + // so their own types have null already stripped - folding the verdict + // from them would be circular. A fetch the asking scope does not track + // reads its state there instead, derived from its receiver's state + // without a walk. Any other untracked link is re-priced on the asking + // scope by a rule-facing resolution; engine-side folds keep its stored + // read (re-pricing there would walk). + $varExpr = $varResult->getExpr(); + $isVarTracked = $scope->hasExpressionType($varExpr)->yes(); + if ( + !$isVarTracked + && !$varResult->containsNullsafe() + && ($varExpr instanceof ArrayDimFetch || $varExpr instanceof PropertyFetch || $varExpr instanceof StaticPropertyFetch) + ) { + $varType = ($useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain() : $scope)->getStateType($varExpr); + } elseif ($reprocessUntrackedLinks && !$isVarTracked) { + $varType = $useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain()->getNativeType($varExpr) : $scope->getType($varExpr); + } else { + $varType = $varResult->getTypeOnScope($scope, $useNativeTypes); + } $dimType = $dimResult->getTypeOnScope($scope, $useNativeTypes); $hasOffsetValue = $varType->hasOffsetValueType($dimType); $valueType = $hasOffsetValue->no() ? new NeverType() : $varType->getOffsetValueType($dimType); diff --git a/tests/PHPStan/Analyser/nsrt/bug-15722.php b/tests/PHPStan/Analyser/nsrt/bug-15722.php new file mode 100644 index 00000000000..8aad0eb2b94 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15722.php @@ -0,0 +1,24 @@ +analyse([__DIR__ . '/data/bug-14421.php'], []); } + public function testBug15722(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-15722.php'], []); + } + public function testPregMatchOffsetCaptureWithoutTreatPhpDocTypesAsCertain(): void { $this->treatPhpDocTypesAsCertain = false; diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 05ba7627ce9..8df093703a5 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -716,6 +716,11 @@ public function testBug12780(): void $this->analyse([__DIR__ . '/data/bug-12780.php'], []); } + public function testBug15722(): void + { + $this->analyse([__DIR__ . '/data/bug-15722.php'], []); + } + #[RequiresPhp('>= 8.2.0')] public function testReadonlyPropertyOnNullableReceiver(): void { diff --git a/tests/PHPStan/Rules/Variables/data/bug-15722.php b/tests/PHPStan/Rules/Variables/data/bug-15722.php new file mode 100644 index 00000000000..96dcc589694 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-15722.php @@ -0,0 +1,27 @@ + null]; + + public function get(): ?int + { + return $this->data['a']['b'] ?? null; + } + + public function has(): bool + { + return isset($this->data['a']['b']); + } +} diff --git a/turbo-ext/src/IssetabilityDescriptor.cpp b/turbo-ext/src/IssetabilityDescriptor.cpp index ac0026a0fe1..72b6fc38182 100644 --- a/turbo-ext/src/IssetabilityDescriptor.cpp +++ b/turbo-ext/src/IssetabilityDescriptor.cpp @@ -72,6 +72,23 @@ zv::Val newNeverType() return true; } +/* $expr instanceof ArrayDimFetch || PropertyFetch || StaticPropertyFetch; + * false = pending exception */ +[[nodiscard]] bool isFetchLink(zval *expr, bool &out) +{ + out = false; + if (Z_TYPE_P(expr) != IS_OBJECT) return true; + for (int classIdx : { PT_CLASS_ARRAY_DIM_FETCH, PT_CLASS_PROPERTY_FETCH, PT_CLASS_STATIC_PROPERTY_FETCH }) { + zend_class_entry *ce = pt_class(classIdx); + if (UNEXPECTED(ce == NULL)) return false; + if (instanceof_function(Z_OBJCE_P(expr), ce)) { + out = true; + return true; + } + } + return true; +} + } // namespace namespace phpstanturbo { @@ -216,20 +233,35 @@ class IssetabilityDescriptor return zv::Val(); } - bool reprocessVar = false; - if (reprocessUntrackedLinks) { - zv::Val varExprHold; - zval *varExpr = pt_expression_result_expr(varResult, varExprHold); - if (UNEXPECTED(varExpr == NULL)) return zv::Val(); - zend_long isTracked = pt_mutating_scope_has_expression_type(Z_OBJ_P(scope), varExpr); - if (UNEXPECTED(isTracked < 0)) return zv::Val(); - reprocessVar = isTracked != PT_TRI_YES; + zv::Val varExprHold; + zval *varExpr = pt_expression_result_expr(varResult, varExprHold); + if (UNEXPECTED(varExpr == NULL)) return zv::Val(); + zend_long isVarTracked = pt_mutating_scope_has_expression_type(Z_OBJ_P(scope), varExpr); + if (UNEXPECTED(isVarTracked < 0)) return zv::Val(); + bool readsState = false; + if (isVarTracked != PT_TRI_YES) { + bool containsNullsafe; + if (UNEXPECTED(!pt_expression_result_contains_nullsafe(varResult, containsNullsafe))) return zv::Val(); + if (!containsNullsafe) { + bool isFetch; + if (UNEXPECTED(!isFetchLink(varExpr, isFetch))) return zv::Val(); + readsState = isFetch; + } } zv::Val varType; - if (reprocessVar) { - zv::Val varExprHold; - zval *varExpr = pt_expression_result_expr(varResult, varExprHold); - if (UNEXPECTED(varExpr == NULL)) return zv::Val(); + if (readsState) { + if (useNativeTypes) { + zv::Val nativeScope = pt_mutating_scope_do_not_treat_phpdoc_types_as_certain(Z_OBJ_P(scope)); + if (UNEXPECTED(nativeScope.isUndef())) return zv::Val(); + if (UNEXPECTED(Z_TYPE_P(nativeScope.raw()) != IS_OBJECT)) { + zend_throw_error(NULL, "Call to a member function getStateType() on %s", zend_zval_value_name(nativeScope.raw())); + return zv::Val(); + } + varType = pt_mutating_scope_get_state_type(Z_OBJ_P(nativeScope.raw()), Z_OBJ_P(varExpr)); + } else { + varType = pt_mutating_scope_get_state_type(Z_OBJ_P(scope), Z_OBJ_P(varExpr)); + } + } else if (reprocessUntrackedLinks && isVarTracked != PT_TRI_YES) { if (useNativeTypes) { zv::Val nativeScope = pt_mutating_scope_do_not_treat_phpdoc_types_as_certain(Z_OBJ_P(scope)); if (UNEXPECTED(nativeScope.isUndef())) return zv::Val();