From 6f9986a1347dac041c42c1dc569f9cfc62d422de Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 28 Jul 2026 22:14:47 +0200 Subject: [PATCH 1/3] Resolve isset/empty/?? chains from handler-built descriptors Rules\IssetCheck was a hand-maintained mirror of MutatingScope::issetCheck's chain walk (variable / offset / property / leaf), re-walking the AST and re-resolving types and property reflections with its own drifting copy of the logic. The chain-link handlers (Variable / ArrayDimFetch / PropertyFetch / StaticPropertyFetch) now attach an IssetabilityDescriptor to their ExpressionResult, built from the child results they already have in hand. IssetHandler, EmptyHandler, CoalesceHandler and AssignOpHandler (??=) emit virtual nodes carrying the subject ExpressionResults; IssetRule, EmptyRule and NullCoalesceRule listen on those nodes, and Rules\IssetCheck folds the resolved links of an IssetabilityResolution instead of walking the AST. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7 --- .../ExprHandler/ArrayDimFetchHandler.php | 2 + src/Analyser/ExprHandler/AssignOpHandler.php | 19 ++ src/Analyser/ExprHandler/CoalesceHandler.php | 3 + src/Analyser/ExprHandler/EmptyHandler.php | 3 + src/Analyser/ExprHandler/IssetHandler.php | 5 + .../ExprHandler/PropertyFetchHandler.php | 3 + .../StaticPropertyFetchHandler.php | 4 + src/Analyser/ExprHandler/VariableHandler.php | 2 + src/Analyser/ExpressionResult.php | 27 ++ src/Analyser/ExpressionResultFactory.php | 1 + src/Analyser/IssetabilityDescriptor.php | 163 ++++++++++ src/Analyser/IssetabilityLinkInfo.php | 307 ++++++++++++++++++ src/Analyser/IssetabilityResolution.php | 154 +++++++++ src/Node/CoalesceExpressionNode.php | 59 ++++ src/Node/EmptyExpressionNode.php | 45 +++ src/Node/IssetExpressionNode.php | 51 +++ src/Rules/IssetCheck.php | 227 ++++++------- src/Rules/Variables/EmptyRule.php | 7 +- src/Rules/Variables/IssetRule.php | 9 +- src/Rules/Variables/NullCoalesceRule.php | 66 ++-- .../PHPStan/Rules/Variables/EmptyRuleTest.php | 2 - .../PHPStan/Rules/Variables/IssetRuleTest.php | 2 - .../Rules/Variables/NullCoalesceRuleTest.php | 2 - 23 files changed, 988 insertions(+), 175 deletions(-) create mode 100644 src/Analyser/IssetabilityDescriptor.php create mode 100644 src/Analyser/IssetabilityLinkInfo.php create mode 100644 src/Analyser/IssetabilityResolution.php create mode 100644 src/Node/CoalesceExpressionNode.php create mode 100644 src/Node/EmptyExpressionNode.php create mode 100644 src/Node/IssetExpressionNode.php diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index b52587be917..210e1318d6d 100644 --- a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php +++ b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php @@ -15,6 +15,7 @@ use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ExprHandler\Helper\NullsafeShortCircuitingHelper; +use PHPStan\Analyser\IssetabilityDescriptor; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -125,6 +126,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex throwPoints: $throwPoints, impurePoints: $impurePoints, containsNullsafe: $varResult->containsNullsafe(), + issetabilityDescriptor: IssetabilityDescriptor::offset($varResult, $dimResult), ); } diff --git a/src/Analyser/ExprHandler/AssignOpHandler.php b/src/Analyser/ExprHandler/AssignOpHandler.php index fb25464e4b5..5484969f1b7 100644 --- a/src/Analyser/ExprHandler/AssignOpHandler.php +++ b/src/Analyser/ExprHandler/AssignOpHandler.php @@ -15,14 +15,17 @@ use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ExprHandler\Helper\ImplicitToStringCallHelper; +use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; +use PHPStan\Analyser\NoopNodeCallback; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Node\CoalesceExpressionNode; use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantIntegerType; @@ -45,6 +48,7 @@ public function __construct( private InitializerExprTypeResolver $initializerExprTypeResolver, private ImplicitToStringCallHelper $implicitToStringCallHelper, private ExpressionResultFactory $expressionResultFactory, + private NonNullabilityHelper $nonNullabilityHelper, ) { } @@ -57,6 +61,17 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; + $condResult = null; + if ($expr instanceof Expr\AssignOp\Coalesce) { + // `$lvalue ??= ...` is `$lvalue = $lvalue ?? ...`: price the left side + // once as a read (mirroring CoalesceHandler's left-side processing) so + // its result carries the isset descriptor for NullCoalesceRule. The + // NoopNodeCallback avoids duplicate reports and the read's scope is + // discarded: processAssignVar() walks the target itself. + $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $expr->var); + $condScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->var); + $condResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $condScope, $storage, new NoopNodeCallback(), $context->enterDeep()); + } $assignResult = $this->assignHandler->processAssignVar( $nodeScopeResolver, $scope, @@ -114,6 +129,10 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $impurePoints = array_merge($impurePoints, $toStringResult->getImpurePoints()); } + if ($expr instanceof Expr\AssignOp\Coalesce) { + $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new CoalesceExpressionNode($expr, $condResult, 'on left side of ??='), $beforeScope, $storage, $context); + } + return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, diff --git a/src/Analyser/ExprHandler/CoalesceHandler.php b/src/Analyser/ExprHandler/CoalesceHandler.php index d7b0fa53571..c0a54c5bee3 100644 --- a/src/Analyser/ExprHandler/CoalesceHandler.php +++ b/src/Analyser/ExprHandler/CoalesceHandler.php @@ -18,6 +18,7 @@ use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Node\CoalesceExpressionNode; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\NeverType; @@ -135,6 +136,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $scope->filterByTruthyValue(new Expr\Isset_([$expr->left]))->mergeWith($rightResult->getScope()); } + $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new CoalesceExpressionNode($expr, $condResult, 'on left side of ??'), $beforeScope, $storage, $context); + return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, diff --git a/src/Analyser/ExprHandler/EmptyHandler.php b/src/Analyser/ExprHandler/EmptyHandler.php index 54e9c397fa1..45e30427624 100644 --- a/src/Analyser/ExprHandler/EmptyHandler.php +++ b/src/Analyser/ExprHandler/EmptyHandler.php @@ -19,6 +19,7 @@ use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Node\EmptyExpressionNode; use PHPStan\ShouldNotHappenException; use PHPStan\Type\BooleanType; use PHPStan\Type\Constant\ConstantBooleanType; @@ -95,6 +96,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $this->nonNullabilityHelper->revertNonNullability($scope, $nonNullabilityResult->getSpecifiedExpressions()); $scope = $nodeScopeResolver->lookForUnsetAllowedUndefinedExpressions($scope, $expr->expr); + $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new EmptyExpressionNode($expr, $exprResult), $beforeScope, $storage, $context); + return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, diff --git a/src/Analyser/ExprHandler/IssetHandler.php b/src/Analyser/ExprHandler/IssetHandler.php index 9fc6c135a1c..62d9abadf04 100644 --- a/src/Analyser/ExprHandler/IssetHandler.php +++ b/src/Analyser/ExprHandler/IssetHandler.php @@ -29,6 +29,7 @@ use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Node\IssetExpr; +use PHPStan\Node\IssetExpressionNode; use PHPStan\Rules\Arrays\AllowedArrayKeysTypes; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Accessory\HasOffsetType; @@ -352,10 +353,12 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $impurePoints = []; $nonNullabilityResults = []; $isAlwaysTerminating = false; + $varResults = []; foreach ($expr->vars as $var) { $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $var); $scope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); $varResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $varResults[] = $varResult; $scope = $varResult->getScope(); $hasYield = $hasYield || $varResult->hasYield(); $throwPoints = array_merge($throwPoints, $varResult->getThrowPoints()); @@ -388,6 +391,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $this->nonNullabilityHelper->revertNonNullability($scope, $nonNullabilityResult->getSpecifiedExpressions()); } + $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new IssetExpressionNode($expr, $varResults), $beforeScope, $storage, $context); + return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, diff --git a/src/Analyser/ExprHandler/PropertyFetchHandler.php b/src/Analyser/ExprHandler/PropertyFetchHandler.php index 805b2b190d5..95f1e1c72d9 100644 --- a/src/Analyser/ExprHandler/PropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/PropertyFetchHandler.php @@ -14,6 +14,7 @@ use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ExprHandler\Helper\NullsafeShortCircuitingHelper; use PHPStan\Analyser\InternalThrowPoint; +use PHPStan\Analyser\IssetabilityDescriptor; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\Scope; @@ -22,6 +23,7 @@ use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Php\PhpVersion; +use PHPStan\Rules\Properties\FoundPropertyReflection; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\ErrorType; use PHPStan\Type\MixedType; @@ -95,6 +97,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex throwPoints: $throwPoints, impurePoints: $impurePoints, containsNullsafe: $varResult->containsNullsafe(), + issetabilityDescriptor: IssetabilityDescriptor::property($varResult, fn (MutatingScope $s): ?FoundPropertyReflection => $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $s), $expr), ); } diff --git a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php index dd15b5a6eb4..0d964e7fafc 100644 --- a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php @@ -16,6 +16,7 @@ use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ExprHandler\Helper\NullsafeShortCircuitingHelper; use PHPStan\Analyser\ImpurePoint; +use PHPStan\Analyser\IssetabilityDescriptor; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\Scope; @@ -23,6 +24,7 @@ use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Rules\Properties\FoundPropertyReflection; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\ErrorType; use PHPStan\Type\MixedType; @@ -67,6 +69,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ]; $isAlwaysTerminating = false; $containsNullsafe = false; + $classResult = null; if ($expr->class instanceof Expr) { $classResult = $nodeScopeResolver->processExprNode($stmt, $expr->class, $scope, $storage, $nodeCallback, $context->enterDeep()); $hasYield = $classResult->hasYield(); @@ -94,6 +97,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex throwPoints: $throwPoints, impurePoints: $impurePoints, containsNullsafe: $containsNullsafe, + issetabilityDescriptor: IssetabilityDescriptor::property($classResult, fn (MutatingScope $s): ?FoundPropertyReflection => $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $s), $expr), ); } diff --git a/src/Analyser/ExprHandler/VariableHandler.php b/src/Analyser/ExprHandler/VariableHandler.php index eff0cee6c9a..c34c281ffad 100644 --- a/src/Analyser/ExprHandler/VariableHandler.php +++ b/src/Analyser/ExprHandler/VariableHandler.php @@ -13,6 +13,7 @@ use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ImpurePoint; +use PHPStan\Analyser\IssetabilityDescriptor; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\Scope; @@ -103,6 +104,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $isAlwaysTerminating, $throwPoints, $impurePoints, + issetabilityDescriptor: is_string($expr->name) ? IssetabilityDescriptor::variable($expr->name) : null, truthyScopeCallback: static fn (): MutatingScope => $scope->filterByTruthyValue($expr), falseyScopeCallback: static fn (): MutatingScope => $scope->filterByFalseyValue($expr), ); diff --git a/src/Analyser/ExpressionResult.php b/src/Analyser/ExpressionResult.php index 91a801351b7..3a937d61903 100644 --- a/src/Analyser/ExpressionResult.php +++ b/src/Analyser/ExpressionResult.php @@ -35,6 +35,7 @@ public function __construct( private array $throwPoints, private array $impurePoints, private bool $containsNullsafe = false, + private ?IssetabilityDescriptor $issetabilityDescriptor = null, ?callable $truthyScopeCallback = null, ?callable $falseyScopeCallback = null, ) @@ -69,6 +70,32 @@ public function containsNullsafe(): bool return $this->containsNullsafe; } + /** + * The isset/empty/?? view of this expression evaluated at the given + * scope: folds the chain descriptor, or builds a leaf resolution from the + * expression's own type when it is not a chain link (e.g. a method-call-rooted + * base like $this->getFoo()['x']). $useNativeTypes selects native vs phpdoc. + */ + public function getIssetabilityResolution(MutatingScope $scope, bool $useNativeTypes): IssetabilityResolution + { + if ($this->issetabilityDescriptor !== null) { + return $this->issetabilityDescriptor->resolve($scope, $useNativeTypes, $this->expr); + } + + $type = $this->getTypeOnScope($scope, $useNativeTypes); + + return new IssetabilityResolution( + IssetabilityLinkInfo::leaf($type, $this->expr, $this->expr instanceof Expr\NullsafePropertyFetch), + null, + ); + } + + /** Prices this result's expression on the given scope in the requested flavour. */ + public function getTypeOnScope(MutatingScope $scope, bool $useNativeTypes): Type + { + return $useNativeTypes ? $scope->getNativeType($this->expr) : $scope->getType($this->expr); + } + /** * @return InternalThrowPoint[] */ diff --git a/src/Analyser/ExpressionResultFactory.php b/src/Analyser/ExpressionResultFactory.php index 8044067a87f..e49481036da 100644 --- a/src/Analyser/ExpressionResultFactory.php +++ b/src/Analyser/ExpressionResultFactory.php @@ -22,6 +22,7 @@ public function create( array $throwPoints, array $impurePoints, bool $containsNullsafe = false, + ?IssetabilityDescriptor $issetabilityDescriptor = null, ?callable $truthyScopeCallback = null, ?callable $falseyScopeCallback = null, ): ExpressionResult; diff --git a/src/Analyser/IssetabilityDescriptor.php b/src/Analyser/IssetabilityDescriptor.php new file mode 100644 index 00000000000..3ac913dc4a2 --- /dev/null +++ b/src/Analyser/IssetabilityDescriptor.php @@ -0,0 +1,163 @@ +kind === self::KIND_VARIABLE) { + $variableName = $this->variableName; + if ($variableName === null) { + throw new ShouldNotHappenException(); + } + + $hasVariable = $scope->hasVariableType($variableName); + $valueType = $hasVariable->yes() + ? ($useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain()->getVariableType($variableName) : $scope->getVariableType($variableName)) + : new NeverType(); + + return new IssetabilityResolution(IssetabilityLinkInfo::variable($variableName, $hasVariable, $valueType), null); + } + + if ($this->kind === self::KIND_OFFSET) { + $varResult = $this->varResult; + $dimResult = $this->dimResult; + if ($varResult === null || $dimResult === null) { + throw new ShouldNotHappenException(); + } + + $varType = $varResult->getTypeOnScope($scope, $useNativeTypes); + $dimType = $dimResult->getTypeOnScope($scope, $useNativeTypes); + $hasOffsetValue = $varType->hasOffsetValueType($dimType); + $valueType = $hasOffsetValue->no() ? new NeverType() : $varType->getOffsetValueType($dimType); + + return new IssetabilityResolution( + IssetabilityLinkInfo::offset( + $varType->isOffsetAccessible(), + $hasOffsetValue, + $scope->hasExpressionType($expr)->yes(), + $varType, + $dimType, + $valueType, + ), + $varResult->getIssetabilityResolution($scope, $useNativeTypes), + ); + } + + $reflectionResolver = $this->reflectionResolver; + $propertyFetch = $this->propertyFetch; + if ($reflectionResolver === null || $propertyFetch === null) { + throw new ShouldNotHappenException(); + } + + $inner = $this->innerResult !== null ? $this->innerResult->getIssetabilityResolution($scope, $useNativeTypes) : null; + + $propertyReflection = $reflectionResolver($scope); + if ($propertyReflection === null) { + return new IssetabilityResolution( + IssetabilityLinkInfo::property(null, $propertyFetch, false, false, TrinaryLogic::createNo(), new NeverType(), new NeverType(), false, false, false, false, false, false, false, false), + $inner, + ); + } + + $hasNativeType = $propertyReflection->hasNativeType(); + $nativeReflection = $propertyReflection->getNativeReflection(); + $initializedThisProperty = $propertyFetch instanceof PropertyFetch + && $propertyFetch->name instanceof Identifier + && $propertyFetch->var instanceof Variable + && $propertyFetch->var->name === 'this' + && $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes(); + + return new IssetabilityResolution( + IssetabilityLinkInfo::property( + $propertyReflection, + $propertyFetch, + $propertyReflection->isNative(), + $hasNativeType, + $propertyReflection->isVirtual(), + $propertyReflection->getWritableType(), + $hasNativeType ? $propertyReflection->getNativeType() : new NeverType(), + $scope->hasExpressionType($propertyFetch)->yes(), + isset($scope->getConditionalExpressions()[$scope->getNodeKey($propertyFetch)]), + $initializedThisProperty, + $nativeReflection !== null, + $nativeReflection !== null && $nativeReflection->isPromoted(), + $nativeReflection !== null && $nativeReflection->isReadOnly(), + $nativeReflection !== null && $nativeReflection->isHooked(), + $nativeReflection !== null && $nativeReflection->getNativeReflection()->hasDefaultValue(), + ), + $inner, + ); + } + +} diff --git a/src/Analyser/IssetabilityLinkInfo.php b/src/Analyser/IssetabilityLinkInfo.php new file mode 100644 index 00000000000..a59416d6b99 --- /dev/null +++ b/src/Analyser/IssetabilityLinkInfo.php @@ -0,0 +1,307 @@ +kind === self::KIND_VARIABLE; + } + + public function isOffset(): bool + { + return $this->kind === self::KIND_OFFSET; + } + + public function isProperty(): bool + { + return $this->kind === self::KIND_PROPERTY; + } + + public function getVariableName(): string + { + if ($this->variableName === null) { + throw new ShouldNotHappenException(); + } + + return $this->variableName; + } + + public function getHasVariable(): TrinaryLogic + { + if ($this->hasVariable === null) { + throw new ShouldNotHappenException(); + } + + return $this->hasVariable; + } + + /** The type the operator's callback inspects: variable type, offset value type, property writable type, or leaf type. */ + public function getValueType(): Type + { + if ($this->valueType === null) { + throw new ShouldNotHappenException(); + } + + return $this->valueType; + } + + public function getIsOffsetAccessible(): TrinaryLogic + { + if ($this->isOffsetAccessible === null) { + throw new ShouldNotHappenException(); + } + + return $this->isOffsetAccessible; + } + + public function getHasOffsetValue(): TrinaryLogic + { + if ($this->hasOffsetValue === null) { + throw new ShouldNotHappenException(); + } + + return $this->hasOffsetValue; + } + + public function hasExpressionTypeOfExpr(): bool + { + return $this->hasExpressionTypeOfExpr; + } + + public function getVarType(): Type + { + if ($this->varType === null) { + throw new ShouldNotHappenException(); + } + + return $this->varType; + } + + public function getDimType(): Type + { + if ($this->dimType === null) { + throw new ShouldNotHappenException(); + } + + return $this->dimType; + } + + public function getPropertyReflection(): ?FoundPropertyReflection + { + return $this->propertyReflection; + } + + /** + * @return Expr\PropertyFetch|Expr\StaticPropertyFetch + */ + public function getPropertyFetch(): Expr + { + if (!$this->propertyFetch instanceof Expr\PropertyFetch && !$this->propertyFetch instanceof Expr\StaticPropertyFetch) { + throw new ShouldNotHappenException(); + } + + return $this->propertyFetch; + } + + public function isReflectionNative(): bool + { + return $this->reflectionNative; + } + + public function hasNativeType(): bool + { + return $this->hasNativeType; + } + + public function isVirtual(): TrinaryLogic + { + if ($this->isVirtual === null) { + throw new ShouldNotHappenException(); + } + + return $this->isVirtual; + } + + public function getNativeType(): Type + { + if ($this->nativeType === null) { + throw new ShouldNotHappenException(); + } + + return $this->nativeType; + } + + public function hasExpressionTypeOfFetch(): bool + { + return $this->hasExpressionTypeOfFetch; + } + + /** + * Whether the scope holds conditional-expression entries about the fetch. + * Such entries exist only when the fetch was narrowed in an evaluated + * condition - and evaluating a condition READS the fetch, which would have + * thrown on an uninitialized typed property. A typed-property read + * witnesses initialization. + */ + public function hasConditionalExpressionsOfFetch(): bool + { + return $this->hasConditionalExpressionsOfFetch; + } + + public function isInitializedThisProperty(): bool + { + return $this->initializedThisProperty; + } + + public function nativeReflectionExists(): bool + { + return $this->nativeReflectionExists; + } + + public function nativeIsPromoted(): bool + { + return $this->nativeIsPromoted; + } + + public function nativeIsReadOnly(): bool + { + return $this->nativeIsReadOnly; + } + + public function nativeIsHooked(): bool + { + return $this->nativeIsHooked; + } + + public function nativeHasDefaultValue(): bool + { + return $this->nativeHasDefaultValue; + } + + public function getLeafExpr(): Expr + { + if ($this->leafExpr === null) { + throw new ShouldNotHappenException(); + } + + return $this->leafExpr; + } + + public function leafIsNullsafePropertyFetch(): bool + { + return $this->leafIsNullsafePropertyFetch; + } + +} diff --git a/src/Analyser/IssetabilityResolution.php b/src/Analyser/IssetabilityResolution.php new file mode 100644 index 00000000000..9695f977ecd --- /dev/null +++ b/src/Analyser/IssetabilityResolution.php @@ -0,0 +1,154 @@ +link; + } + + public function getInner(): ?IssetabilityResolution + { + return $this->inner; + } + + /** + * Whether isset() of the whole chain holds: null = maybe (resolves to bool), + * true/false = the typeCallback's verdict on the leaf type threaded outward + * over the chain's set-ness. Mirrors the former MutatingScope::issetCheck(). + * + * @param callable(Type): ?bool $typeCallback + */ + public function isSet(callable $typeCallback, ?bool $result = null): ?bool + { + $link = $this->link; + + if ($link->isVariable()) { + $hasVariable = $link->getHasVariable(); + if ($hasVariable->maybe()) { + return null; + } + + if ($result === null) { + if ($hasVariable->yes()) { + if ($link->getVariableName() === '_SESSION') { + return null; + } + + return $typeCallback($link->getValueType()); + } + + return false; + } + + return $result; + } + + if ($link->isOffset()) { + if (!$link->getIsOffsetAccessible()->yes()) { + return $result ?? ($this->inner !== null ? $this->inner->isSetUndefined() : null); + } + + $hasOffsetValue = $link->getHasOffsetValue(); + if ($hasOffsetValue->no()) { + return false; + } + + // If offset cannot be null, store this verdict and see if one of the earlier + // offsets is. E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR + // b OR c might be null. + if ($hasOffsetValue->yes()) { + $result = $typeCallback($link->getValueType()); + + if ($result !== null) { + return $this->inner !== null ? $this->inner->isSet($typeCallback, $result) : $result; + } + } + + // Has offset, it is nullable + return null; + } + + if ($link->isProperty()) { + if ($link->getPropertyReflection() === null || !$link->isReflectionNative()) { + return $this->inner !== null ? $this->inner->isSetUndefined() : null; + } + + if ( + $link->hasNativeType() + && !$link->isVirtual()->yes() + && !$link->hasExpressionTypeOfFetch() + && !$link->hasConditionalExpressionsOfFetch() + && !$link->nativeHasDefaultValue() + && (!$link->nativeReflectionExists() || !$link->nativeIsPromoted() || (!$link->nativeIsReadOnly() && !$link->nativeIsHooked())) + ) { + return $this->inner !== null ? $this->inner->isSetUndefined() : null; + } + + if ($result !== null) { + return $this->inner !== null ? $this->inner->isSet($typeCallback, $result) : $result; + } + + $result = $typeCallback($link->getValueType()); + if ($result !== null && $this->inner !== null) { + return $this->inner->isSet($typeCallback, $result); + } + + return $result; + } + + // leaf + return $result ?? $typeCallback($link->getValueType()); + } + + private function isSetUndefined(): ?bool + { + $link = $this->link; + + if ($link->isVariable()) { + if (!$link->getHasVariable()->no()) { + return null; + } + + return false; + } + + if ($link->isOffset()) { + if (!$link->getIsOffsetAccessible()->yes()) { + return $this->inner !== null ? $this->inner->isSetUndefined() : null; + } + + if (!$link->getHasOffsetValue()->no()) { + return $this->inner !== null ? $this->inner->isSetUndefined() : null; + } + + return false; + } + + if ($link->isProperty()) { + return $this->inner !== null ? $this->inner->isSetUndefined() : null; + } + + return null; + } + +} diff --git a/src/Node/CoalesceExpressionNode.php b/src/Node/CoalesceExpressionNode.php new file mode 100644 index 00000000000..e8653bbc426 --- /dev/null +++ b/src/Node/CoalesceExpressionNode.php @@ -0,0 +1,59 @@ +getAttributes()); + } + + public function getOriginalExpr(): Expr + { + return $this->originalExpr; + } + + public function getSubjectResult(): ExpressionResult + { + return $this->subjectResult; + } + + public function getOperatorDescription(): string + { + return $this->operatorDescription; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_CoalesceExpressionNode'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Node/EmptyExpressionNode.php b/src/Node/EmptyExpressionNode.php new file mode 100644 index 00000000000..e6685101a76 --- /dev/null +++ b/src/Node/EmptyExpressionNode.php @@ -0,0 +1,45 @@ +getAttributes()); + } + + public function getExprResult(): ExpressionResult + { + return $this->exprResult; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_EmptyExpressionNode'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Node/IssetExpressionNode.php b/src/Node/IssetExpressionNode.php new file mode 100644 index 00000000000..6c8bc870198 --- /dev/null +++ b/src/Node/IssetExpressionNode.php @@ -0,0 +1,51 @@ +getAttributes()); + } + + /** + * @return ExpressionResult[] + */ + public function getVarResults(): array + { + return $this->varResults; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_IssetExpressionNode'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 35195e7ea52..eff4ed98840 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -2,22 +2,27 @@ namespace PHPStan\Rules; -use PhpParser\Node; -use PhpParser\Node\Expr; +use PhpParser\Node\Expr\NullsafePropertyFetch; +use PhpParser\Node\Identifier; +use PHPStan\Analyser\ExpressionResult; +use PHPStan\Analyser\IssetabilityResolution; +use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; -use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Rules\Properties\PropertyDescriptor; -use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; -use function is_string; use function sprintf; use function str_starts_with; /** + * Renders the isset/empty/?? "does it make sense" errors from the single + * IssetabilityResolution the engine already computed. The chain is walked and + * resolved once (IssetabilityDescriptor::resolve); this only projects the + * resolved facts into messages - it never re-walks the AST nor re-resolves types. + * * @phpstan-type ErrorIdentifier = 'empty'|'isset'|'nullCoalesce' */ #[AutowiredService] @@ -26,7 +31,6 @@ final class IssetCheck public function __construct( private PropertyDescriptor $propertyDescriptor, - private PropertyReflectionFinder $propertyReflectionFinder, #[AutowiredParameter] private bool $checkAdvancedIsset, #[AutowiredParameter] @@ -39,26 +43,40 @@ public function __construct( * @param ErrorIdentifier $identifier * @param callable(Type): ?string $typeMessageCallback */ - public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError + public function check(ExpressionResult $exprResult, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback): ?IdentifierRuleError { - // mirrored in PHPStan\Analyser\MutatingScope::issetCheck() - if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { - $hasVariable = $scope->hasVariableType($expr->name); + $mutatingScope = $scope->toMutatingScope(); + $resolution = $exprResult->getIssetabilityResolution($mutatingScope, !$this->treatPhpDocTypesAsCertain); + + return $this->doCheck($resolution, $mutatingScope, $operatorDescription, $identifier, $typeMessageCallback, null); + } + + /** + * @param ErrorIdentifier $identifier + * @param callable(Type): ?string $typeMessageCallback + */ + private function doCheck(IssetabilityResolution $resolution, MutatingScope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error): ?IdentifierRuleError + { + $link = $resolution->getLink(); + $inner = $resolution->getInner(); + + if ($link->isVariable()) { + $hasVariable = $link->getHasVariable(); if ($hasVariable->maybe()) { return null; } if ($error === null) { if ($hasVariable->yes()) { - if ($expr->name === '_SESSION') { + if ($link->getVariableName() === '_SESSION') { return null; } - $type = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr) : $scope->getScopeNativeType($expr); + $type = $link->getValueType(); if (!$type instanceof NeverType) { return $this->generateError( $type, - sprintf('Variable $%s %s always exists and', $expr->name, $operatorDescription), + sprintf('Variable $%s %s always exists and', $link->getVariableName(), $operatorDescription), $typeMessageCallback, $identifier, 'variable', @@ -66,24 +84,22 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str } } - return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription)) + return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $link->getVariableName(), $operatorDescription)) ->identifier(sprintf('%s.variable', $identifier)) ->build(); } return $error; - } elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) { - $type = $this->treatPhpDocTypesAsCertain - ? $scope->getScopeType($expr->var) - : $scope->getScopeNativeType($expr->var); - if (!$type->isOffsetAccessible()->yes()) { - return $error ?? $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); + } + + if ($link->isOffset()) { + $type = $link->getVarType(); + if (!$link->getIsOffsetAccessible()->yes()) { + return $error ?? $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } - $dimType = $this->treatPhpDocTypesAsCertain - ? $scope->getScopeType($expr->dim) - : $scope->getScopeNativeType($expr->dim); - $hasOffsetValue = $type->hasOffsetValueType($dimType); + $dimType = $link->getDimType(); + $hasOffsetValue = $link->getHasOffsetValue(); if ($hasOffsetValue->no()) { if (!$this->checkAdvancedIsset) { return null; @@ -101,12 +117,12 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str // If offset cannot be null, store this error message and see if one of the earlier offsets is. // E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null. - if ($hasOffsetValue->yes() || $scope->hasExpressionType($expr)->yes()) { + if ($hasOffsetValue->yes() || $link->hasExpressionTypeOfExpr()) { if (!$this->checkAdvancedIsset) { return null; } - $error ??= $this->generateError($type->getOffsetValueType($dimType), sprintf( + $error ??= $this->generateError($link->getValueType(), sprintf( 'Offset %s on %s %s always exists and', $dimType->describe(VerbosityLevel::value()), $type->describe(VerbosityLevel::value()), @@ -114,56 +130,27 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str ), $typeMessageCallback, $identifier, 'offset'); if ($error !== null) { - return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); + return $inner !== null ? $this->doCheck($inner, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error) : $error; } } // Has offset, it is nullable return null; + } - } elseif ($expr instanceof Node\Expr\PropertyFetch || $expr instanceof Node\Expr\StaticPropertyFetch) { - - $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $scope->toMutatingScope()); - - if ($propertyReflection === null) { - if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); - } - - if ($expr->class instanceof Expr) { - return $this->checkUndefined($expr->class, $scope, $operatorDescription, $identifier); - } - - return null; - } - - if (!$propertyReflection->isNative()) { - if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); - } - - if ($expr->class instanceof Expr) { - return $this->checkUndefined($expr->class, $scope, $operatorDescription, $identifier); - } + if ($link->isProperty()) { + $reflection = $link->getPropertyReflection(); + $propertyFetch = $link->getPropertyFetch(); - return null; + if ($reflection === null || !$link->isReflectionNative()) { + return $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } - if ($propertyReflection->hasNativeType() && !$propertyReflection->isVirtual()->yes()) { - if ( - $expr instanceof Node\Expr\PropertyFetch - && $expr->name instanceof Node\Identifier - && $expr->var instanceof Expr\Variable - && $expr->var->name === 'this' - && $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes() - ) { + if ($link->hasNativeType() && !$link->isVirtual()->yes()) { + if ($link->isInitializedThisProperty()) { return $this->generateError( - $propertyReflection->getNativeType(), - sprintf( - '%s %s', - $this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr), - $operatorDescription, - ), + $link->getNativeType(), + sprintf('%s %s', $this->propertyDescriptor->describeProperty($reflection, $scope, $propertyFetch), $operatorDescription), static function (Type $type) use ($typeMessageCallback): ?string { $originalMessage = $typeMessageCallback($type); if ($originalMessage === null) { @@ -181,64 +168,43 @@ static function (Type $type) use ($typeMessageCallback): ?string { ); } - if (!$scope->hasExpressionType($expr)->yes()) { - $nativeReflection = $propertyReflection->getNativeReflection(); - if ( - $nativeReflection !== null - && !$nativeReflection->getNativeReflection()->hasDefaultValue() - && (!$nativeReflection->isPromoted() || (!$nativeReflection->isReadOnly() && !$nativeReflection->isHooked())) - ) { - return null; - } + if ( + !$link->hasExpressionTypeOfFetch() + && $link->nativeReflectionExists() + && !$link->nativeHasDefaultValue() + && (!$link->nativeIsPromoted() || (!$link->nativeIsReadOnly() && !$link->nativeIsHooked())) + ) { + return null; } } - $propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr); - $propertyType = $propertyReflection->getWritableType(); + $propertyDescription = $this->propertyDescriptor->describeProperty($reflection, $scope, $propertyFetch); + $propertyType = $reflection->getWritableType(); if ($error !== null) { - if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); - } - - if ($expr->class instanceof Expr) { - return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); - } - - return $error; + return $inner !== null + ? $this->doCheck($inner, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error) + : $error; } if (!$this->checkAdvancedIsset) { - if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); - } - - if ($expr->class instanceof Expr) { - return $this->checkUndefined($expr->class, $scope, $operatorDescription, $identifier); - } - - return null; + return $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } $error = $this->generateError( - $propertyReflection->getWritableType(), + $propertyType, sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription), $typeMessageCallback, $identifier, 'property', ); - if ($error !== null) { - if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); - } - - if ($expr->class instanceof Expr) { - return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); - } + if ($error !== null && $inner !== null) { + return $this->doCheck($inner, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); } return $error; } + // leaf - an arbitrary base expression that is not a chain link if ($error !== null) { return $error; } @@ -248,7 +214,7 @@ static function (Type $type) use ($typeMessageCallback): ?string { } $error = $this->generateError( - $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr) : $scope->getScopeNativeType($expr), + $link->getValueType(), sprintf('Expression %s', $operatorDescription), $typeMessageCallback, $identifier, @@ -258,9 +224,10 @@ static function (Type $type) use ($typeMessageCallback): ?string { return $error; } - if ($expr instanceof Expr\NullsafePropertyFetch) { - if ($expr->name instanceof Node\Identifier) { - return RuleErrorBuilder::message(sprintf('Using nullsafe property access "?->%s" %s is unnecessary. Use -> instead.', $expr->name->name, $operatorDescription)) + if ($link->leafIsNullsafePropertyFetch()) { + $leafExpr = $link->getLeafExpr(); + if ($leafExpr instanceof NullsafePropertyFetch && $leafExpr->name instanceof Identifier) { + return RuleErrorBuilder::message(sprintf('Using nullsafe property access "?->%s" %s is unnecessary. Use -> instead.', $leafExpr->name->name, $operatorDescription)) ->identifier('nullsafe.neverNull') ->build(); } @@ -273,50 +240,46 @@ static function (Type $type) use ($typeMessageCallback): ?string { return null; } - /** - * @param ErrorIdentifier $identifier - */ - private function checkUndefined(Expr $expr, Scope $scope, string $operatorDescription, string $identifier): ?IdentifierRuleError + private function checkUndefinedInner(?IssetabilityResolution $resolution, MutatingScope $scope, string $operatorDescription, string $identifier): ?IdentifierRuleError { - if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { - $hasVariable = $scope->hasVariableType($expr->name); - if (!$hasVariable->no()) { + if ($resolution === null) { + return null; + } + + $link = $resolution->getLink(); + $inner = $resolution->getInner(); + + if ($link->isVariable()) { + if (!$link->getHasVariable()->no()) { return null; } - return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription)) + return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $link->getVariableName(), $operatorDescription)) ->identifier(sprintf('%s.variable', $identifier)) ->build(); } - if ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) { - $type = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->var) : $scope->getScopeNativeType($expr->var); - $dimType = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->dim) : $scope->getScopeNativeType($expr->dim); - $hasOffsetValue = $type->hasOffsetValueType($dimType); - if (!$type->isOffsetAccessible()->yes()) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); + if ($link->isOffset()) { + if (!$link->getIsOffsetAccessible()->yes()) { + return $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } - if (!$hasOffsetValue->no()) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); + if (!$link->getHasOffsetValue()->no()) { + return $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } return RuleErrorBuilder::message( sprintf( 'Offset %s on %s %s does not exist.', - $dimType->describe(VerbosityLevel::value()), - $type->describe(VerbosityLevel::value()), + $link->getDimType()->describe(VerbosityLevel::value()), + $link->getVarType()->describe(VerbosityLevel::value()), $operatorDescription, ), )->identifier(sprintf('%s.offset', $identifier))->build(); } - if ($expr instanceof Expr\PropertyFetch) { - return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); - } - - if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) { - return $this->checkUndefined($expr->class, $scope, $operatorDescription, $identifier); + if ($link->isProperty()) { + return $this->checkUndefinedInner($inner, $scope, $operatorDescription, $identifier); } return null; diff --git a/src/Rules/Variables/EmptyRule.php b/src/Rules/Variables/EmptyRule.php index d1656a3be27..b884cebddc3 100644 --- a/src/Rules/Variables/EmptyRule.php +++ b/src/Rules/Variables/EmptyRule.php @@ -5,12 +5,13 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\Node\EmptyExpressionNode; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; use PHPStan\Type\Type; /** - * @implements Rule + * @implements Rule */ #[RegisteredRule(level: 1)] final class EmptyRule implements Rule @@ -22,12 +23,12 @@ public function __construct(private IssetCheck $issetCheck) public function getNodeType(): string { - return Node\Expr\Empty_::class; + return EmptyExpressionNode::class; } public function processNode(Node $node, Scope $scope): array { - $error = $this->issetCheck->check($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string { + $error = $this->issetCheck->check($node->getExprResult(), $scope, 'in empty()', 'empty', static function (Type $type): ?string { $isNull = $type->isNull(); if ($isNull->maybe()) { return null; diff --git a/src/Rules/Variables/IssetRule.php b/src/Rules/Variables/IssetRule.php index 839241a169b..cd9300c2dc5 100644 --- a/src/Rules/Variables/IssetRule.php +++ b/src/Rules/Variables/IssetRule.php @@ -5,12 +5,13 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\Node\IssetExpressionNode; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; use PHPStan\Type\Type; /** - * @implements Rule + * @implements Rule */ #[RegisteredRule(level: 1)] final class IssetRule implements Rule @@ -22,14 +23,14 @@ public function __construct(private IssetCheck $issetCheck) public function getNodeType(): string { - return Node\Expr\Isset_::class; + return IssetExpressionNode::class; } public function processNode(Node $node, Scope $scope): array { $messages = []; - foreach ($node->vars as $var) { - $error = $this->issetCheck->check($var, $scope, 'in isset()', 'isset', static function (Type $type): ?string { + foreach ($node->getVarResults() as $varResult) { + $error = $this->issetCheck->check($varResult, $scope, 'in isset()', 'isset', static function (Type $type): ?string { $isNull = $type->isNull(); if ($isNull->maybe()) { return null; diff --git a/src/Rules/Variables/NullCoalesceRule.php b/src/Rules/Variables/NullCoalesceRule.php index 6250b59b906..409ae4e431d 100644 --- a/src/Rules/Variables/NullCoalesceRule.php +++ b/src/Rules/Variables/NullCoalesceRule.php @@ -6,6 +6,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\Node\CoalesceExpressionNode; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; @@ -14,7 +15,7 @@ use function sprintf; /** - * @implements Rule + * @implements Rule */ #[RegisteredRule(level: 1)] final class NullCoalesceRule implements Rule @@ -30,42 +31,35 @@ public function __construct( public function getNodeType(): string { - return Node\Expr::class; + return CoalesceExpressionNode::class; } public function processNode(Node $node, Scope $scope): array { - $typeMessageCallback = static function (Type $type): ?string { - $isNull = $type->isNull(); - if ($isNull->maybe()) { - return null; - } - - if ($isNull->yes()) { - return 'is always null'; - } - - return 'is not nullable'; - }; - - if ($node instanceof Node\Expr\BinaryOp\Coalesce) { - $left = $node->left; - $right = $node->right; - $operator = '??'; - } elseif ($node instanceof Node\Expr\AssignOp\Coalesce) { - $left = $node->var; - $right = $node->expr; - $operator = '??='; - } else { - return []; - } + $error = $this->issetCheck->check( + $node->getSubjectResult(), + $scope, + $node->getOperatorDescription(), + 'nullCoalesce', + static function (Type $type): ?string { + $isNull = $type->isNull(); + if ($isNull->maybe()) { + return null; + } + + if ($isNull->yes()) { + return 'is always null'; + } + + return 'is not nullable'; + }, + ); - $error = $this->issetCheck->check($left, $scope, sprintf('on left side of %s', $operator), 'nullCoalesce', $typeMessageCallback); if ($error !== null) { return [$error]; } - $unnecessaryError = $this->checkUnnecessaryNullCoalesce($left, $right, $operator, $scope); + $unnecessaryError = $this->checkUnnecessaryNullCoalesce($node, $scope); if ($unnecessaryError !== null) { return [$unnecessaryError]; } @@ -73,12 +67,23 @@ public function processNode(Node $node, Scope $scope): array return []; } - private function checkUnnecessaryNullCoalesce(Node\Expr $left, Node\Expr $right, string $operator, Scope $scope): ?IdentifierRuleError + private function checkUnnecessaryNullCoalesce(CoalesceExpressionNode $node, Scope $scope): ?IdentifierRuleError { if (!$this->unnecessaryNullCoalesce) { return null; } + $originalExpr = $node->getOriginalExpr(); + if ($originalExpr instanceof Node\Expr\BinaryOp\Coalesce) { + $right = $originalExpr->right; + $operator = '??'; + } elseif ($originalExpr instanceof Node\Expr\AssignOp\Coalesce) { + $right = $originalExpr->expr; + $operator = '??='; + } else { + return null; + } + if (!$scope->getType($right)->isNull()->yes()) { return null; } @@ -86,7 +91,8 @@ private function checkUnnecessaryNullCoalesce(Node\Expr $left, Node\Expr $right, // The coalesce only changes the result when the left side is undefined. // If the left side is always set, `?? null` (or `??= null`) never changes // anything, so the whole coalesce is redundant. - if ($scope->toMutatingScope()->issetCheck($left, static fn (): bool => true) !== true) { + $resolution = $node->getSubjectResult()->getIssetabilityResolution($scope->toMutatingScope(), false); + if ($resolution->isSet(static fn (): bool => true) !== true) { return null; } diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 582fdeb1076..079b032e8ee 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -4,7 +4,6 @@ use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; -use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\DataProvider; @@ -22,7 +21,6 @@ protected function getRule(): Rule { return new EmptyRule(new IssetCheck( new PropertyDescriptor(), - new PropertyReflectionFinder(), true, $this->treatPhpDocTypesAsCertain, )); diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 7e83c53117b..af8f1542e04 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -4,7 +4,6 @@ use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; -use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -21,7 +20,6 @@ protected function getRule(): Rule { return new IssetRule(new IssetCheck( new PropertyDescriptor(), - new PropertyReflectionFinder(), true, $this->treatPhpDocTypesAsCertain, )); diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index fc6c577822d..17d1dd85949 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -4,7 +4,6 @@ use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; -use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -20,7 +19,6 @@ protected function getRule(): Rule { return new NullCoalesceRule(new IssetCheck( new PropertyDescriptor(), - new PropertyReflectionFinder(), true, $this->shouldTreatPhpDocTypesAsCertain(), ), true); From 0e8d8b3b2579dcf5c5651a3e335f8a79643066e0 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 30 Jul 2026 10:59:51 +0200 Subject: [PATCH 2/3] Split processAssignVar into prepareTarget and applyWrite, compose the ??= read processAssignVar owned the whole assignment timeline - target sub-expression walk, value evaluation, write - so callers had to inject the value evaluation as a processExprCallback closure invoked once per target-shape branch. The invocation point is the same in every branch: after the target's sub-expressions are walked, before the write - PHP's own evaluation order. Splitting at that boundary gives the caller the timeline: prepareTarget() walks the target into a PreparedAssignTarget, the caller processes the assigned value inline on its scope, and applyWrite() performs the write. prepareTarget() takes an AssignTargetWalkMode; in the coalesce mode it also prices the whole target as a read with isset() semantics, composed from the just-walked child results through the chain-link handlers' new composeResult() methods (the pure tails of their processExpr, which routes through the same method) - so AssignOpHandler's ??= subject result for CoalesceExpressionNode no longer needs the separate noop-callback walk of the target chain. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7 --- src/Analyser/AssignTargetWalkMode.php | 64 ++ .../ExprHandler/ArrayDimFetchHandler.php | 24 +- src/Analyser/ExprHandler/AssignHandler.php | 736 ++++++++++++------ src/Analyser/ExprHandler/AssignOpHandler.php | 93 ++- .../ExprHandler/PropertyFetchHandler.php | 19 +- .../StaticPropertyFetchHandler.php | 30 +- src/Analyser/ExprHandler/VariableHandler.php | 20 +- src/Analyser/NodeScopeResolver.php | 19 +- src/Analyser/PreparedAssignTarget.php | 239 ++++++ 9 files changed, 951 insertions(+), 293 deletions(-) create mode 100644 src/Analyser/AssignTargetWalkMode.php create mode 100644 src/Analyser/PreparedAssignTarget.php diff --git a/src/Analyser/AssignTargetWalkMode.php b/src/Analyser/AssignTargetWalkMode.php new file mode 100644 index 00000000000..36d677a6f5e --- /dev/null +++ b/src/Analyser/AssignTargetWalkMode.php @@ -0,0 +1,64 @@ +enterExpressionAssign; + } + + public function producesTargetReadResult(): bool + { + return $this->producesTargetReadResult; + } + + public function issetSemanticsForRead(): bool + { + return $this->issetSemanticsForRead; + } + +} diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index 210e1318d6d..9e0bb3021f6 100644 --- a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php +++ b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php @@ -85,8 +85,27 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $beforeScope = $scope; if ($expr->dim === null) { $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); - $scope = $varResult->getScope(); + return $this->composeResult($nodeScopeResolver, $stmt, $expr, null, $varResult, $storage, $context, $beforeScope); + } + + $dimResult = $nodeScopeResolver->processExprNode($stmt, $expr->dim, $scope, $storage, $nodeCallback, $context->enterDeep()); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $dimResult->getScope(), $storage, $nodeCallback, $context->enterDeep()); + + return $this->composeResult($nodeScopeResolver, $stmt, $expr, $dimResult, $varResult, $storage, $context, $beforeScope); + } + + /** + * Builds the offset read's ExpressionResult from the already-walked + * dimension and receiver results - the chain is not re-walked (only the + * ArrayAccess offsetGet simulation runs, over synthetic nodes). + * processExpr() routes through this; AssignHandler::prepareTarget() calls it + * to price a read-modify-write target from the write walk's child results. + */ + public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, ArrayDimFetch $expr, ?ExpressionResult $dimResult, ExpressionResult $varResult, ExpressionResultStorage $storage, ExpressionContext $context, MutatingScope $beforeScope): ExpressionResult + { + $scope = $varResult->getScope(); + if ($expr->dim === null || $dimResult === null) { return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, @@ -99,11 +118,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ); } - $dimResult = $nodeScopeResolver->processExprNode($stmt, $expr->dim, $scope, $storage, $nodeCallback, $context->enterDeep()); - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $dimResult->getScope(), $storage, $nodeCallback, $context->enterDeep()); $throwPoints = array_merge($dimResult->getThrowPoints(), $varResult->getThrowPoints()); $impurePoints = array_merge($dimResult->getImpurePoints(), $varResult->getImpurePoints()); - $scope = $varResult->getScope(); $varType = $varResult->getType(); if (!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) { diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index f5b30e63bbe..4a1868716a6 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -3,7 +3,6 @@ namespace PHPStan\Analyser\ExprHandler; use ArrayAccess; -use Closure; use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Expr\ArrayDimFetch; @@ -22,6 +21,7 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; use PhpParser\Node\Stmt; +use PHPStan\Analyser\AssignTargetWalkMode; use PHPStan\Analyser\ConditionalExpressionHolder; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; @@ -29,11 +29,13 @@ use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExpressionTypeHolder; use PHPStan\Analyser\ExprHandler; +use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper; use PHPStan\Analyser\ImpurePoint; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; +use PHPStan\Analyser\PreparedAssignTarget; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -98,6 +100,11 @@ public function __construct( private MatchHandler $matchHandler, private ExpressionResultFactory $expressionResultFactory, private PropertyReflectionFinder $propertyReflectionFinder, + private NonNullabilityHelper $nonNullabilityHelper, + private VariableHandler $variableHandler, + private ArrayDimFetchHandler $arrayDimFetchHandler, + private PropertyFetchHandler $propertyFetchHandler, + private StaticPropertyFetchHandler $staticPropertyFetchHandler, ) { } @@ -293,7 +300,7 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $result = $this->processAssignVar( + $target = $this->prepareTarget( $nodeScopeResolver, $scope, $storage, @@ -302,49 +309,55 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $expr->expr, $nodeCallback, $context, - function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $storage, $nodeScopeResolver): ExpressionResult { - $beforeScope = $scope; - $impurePoints = []; - if ($expr instanceof AssignRef) { - $referencedExpr = $expr->expr; - while ($referencedExpr instanceof ArrayDimFetch) { - $referencedExpr = $referencedExpr->var; - } + AssignTargetWalkMode::assign(), + ); - if ($referencedExpr instanceof PropertyFetch || $referencedExpr instanceof StaticPropertyFetch) { - $impurePoints[] = new ImpurePoint( - $scope, - $expr, - 'propertyAssignByRef', - 'property assignment by reference', - false, - ); - } + $valueBeforeScope = $target->getScope(); + $valueScope = $valueBeforeScope; + $valueImpurePoints = []; + if ($expr instanceof AssignRef) { + $referencedExpr = $expr->expr; + while ($referencedExpr instanceof ArrayDimFetch) { + $referencedExpr = $referencedExpr->var; + } - $scope = $scope->enterExpressionAssign($expr->expr); - } + if ($referencedExpr instanceof PropertyFetch || $referencedExpr instanceof StaticPropertyFetch) { + $valueImpurePoints[] = new ImpurePoint( + $valueScope, + $expr, + 'propertyAssignByRef', + 'property assignment by reference', + false, + ); + } - if ($expr->var instanceof Variable && is_string($expr->var->name)) { - $context = $context->enterRightSideAssign( - $expr->var->name, - $expr->expr, - ); - } + $valueScope = $valueScope->enterExpressionAssign($expr->expr); + } - $result = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); - $hasYield = $result->hasYield(); - $throwPoints = $result->getThrowPoints(); - $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); - $isAlwaysTerminating = $result->isAlwaysTerminating(); - $scope = $result->getScope(); + $valueContext = $context; + if ($expr->var instanceof Variable && is_string($expr->var->name)) { + $valueContext = $valueContext->enterRightSideAssign( + $expr->var->name, + $expr->expr, + ); + } - if ($expr instanceof AssignRef) { - $scope = $scope->exitExpressionAssign($expr->expr); - } + $assignedExprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext->enterDeep()); + $valueImpurePoints = array_merge($valueImpurePoints, $assignedExprResult->getImpurePoints()); + $valueScope = $assignedExprResult->getScope(); + + if ($expr instanceof AssignRef) { + $valueScope = $valueScope->exitExpressionAssign($expr->expr); + } - return $this->expressionResultFactory->create($scope, $beforeScope, $expr->expr, $hasYield, $isAlwaysTerminating, $throwPoints, $impurePoints); - }, - true, + $result = $this->applyWrite( + $nodeScopeResolver, + $target, + $this->expressionResultFactory->create($valueScope, $valueBeforeScope, $expr->expr, $assignedExprResult->hasYield(), $assignedExprResult->isAlwaysTerminating(), $assignedExprResult->getThrowPoints(), $valueImpurePoints), + $stmt, + $storage, + $nodeCallback, + $context, ); $scope = $result->getScope(); @@ -396,10 +409,15 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto } /** + * The pre-value half of an assignment: walks the target's sub-expressions + * (root, dimensions, receiver, dynamic name) in PHP's evaluation order and + * captures everything applyWrite() needs into a PreparedAssignTarget. The + * caller processes the assigned value on PreparedAssignTarget::getScope() + * between the two calls. + * * @param callable(Node $node, Scope $scope): void $nodeCallback - * @param Closure(MutatingScope $scope): ExpressionResult $processExprCallback */ - public function processAssignVar( + public function prepareTarget( NodeScopeResolver $nodeScopeResolver, MutatingScope $scope, ExpressionResultStorage $storage, @@ -408,10 +426,11 @@ public function processAssignVar( Expr $assignedExpr, callable $nodeCallback, ExpressionContext $context, - Closure $processExprCallback, - bool $enterExpressionAssign, - ): ExpressionResult + AssignTargetWalkMode $mode, + ): PreparedAssignTarget { + $enterExpressionAssign = $mode->enterExpressionAssign(); + $targetReadResult = null; $beforeScope = $scope; $nodeScopeResolver->storeExpressionResult($storage, $var, $this->expressionResultFactory->create( $scope, @@ -429,7 +448,408 @@ public function processAssignVar( $isAlwaysTerminating = false; $isAssignOp = $assignedExpr instanceof Expr\AssignOp && !$enterExpressionAssign; if ($var instanceof Variable) { - $result = $processExprCallback($scope); + if ($mode->producesTargetReadResult()) { + // `$lvalue OP= ...` reads the old value of `$lvalue`; the write walk + // processes a Variable target only as an assignment target, never as + // a read. The read is composed here without a walk - for ??= with + // isset() semantics (mirroring CoalesceHandler's left-side + // processing, with the isset descriptor - bug-13623). + $readScope = $scope; + if ($mode->issetSemanticsForRead()) { + $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $var); + $readScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); + } + if (is_string($var->name)) { + $targetReadResult = $this->variableHandler->composeResult($var, null, $readScope); + } else { + // a dynamic-name target: the name expression is only walked later + // by the write flow, so the read prices it here first + $targetReadResult = $nodeScopeResolver->processExprNode($stmt, $var, $readScope, $storage, new NoopNodeCallback(), $context->enterDeep()); + } + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_VARIABLE, + $var, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + targetReadResult: $targetReadResult, + ); + } + + if ($var instanceof ArrayDimFetch) { + $dimFetchStack = []; + $originalVar = $var; + $assignedPropertyExpr = $assignedExpr; + while ($var instanceof ArrayDimFetch) { + $varForSetOffsetValue = $var->var; + if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) { + $varForSetOffsetValue = new TypeExpr($this->getOriginalPropertyType($varForSetOffsetValue, $scope)); + } + + if ( + $var === $originalVar + && $var->dim !== null + && $scope->hasExpressionType($var)->yes() + ) { + $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( + $varForSetOffsetValue, + $var->dim, + $assignedPropertyExpr, + ); + } else { + $assignedPropertyExpr = new SetOffsetValueTypeExpr( + $varForSetOffsetValue, + $var->dim, + $assignedPropertyExpr, + ); + } + $dimFetchStack[] = $var; + $var = $var->var; + } + + // 1. eval root expr + // The root is read to obtain the container that receives the offset write, so a + // property root must resolve to its readable type (not its writable one) even + // though it sits on the left-hand side of the assignment. + if ($enterExpressionAssign) { + $scope = $scope->enterExpressionAssign($var, false); + } + $result = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $rootReadResult = $result; + $hasYield = $result->hasYield(); + $throwPoints = $result->getThrowPoints(); + $impurePoints = $result->getImpurePoints(); + $isAlwaysTerminating = $result->isAlwaysTerminating(); + $scope = $result->getScope(); + if ($enterExpressionAssign) { + $scope = $scope->exitExpressionAssign($var); + } + + // 2. eval dimensions + $offsetTypes = []; + $offsetNativeTypes = []; + $dimResults = []; + $dimFetchStack = array_reverse($dimFetchStack); + $lastDimKey = array_key_last($dimFetchStack); + foreach ($dimFetchStack as $key => $dimFetch) { + $dimExpr = $dimFetch->dim; + + // Callback was already called for last dim at the beginning of the method. + if ($key !== $lastDimKey) { + $nodeScopeResolver->callNodeCallback($nodeCallback, $dimFetch, $enterExpressionAssign ? $scope->enterExpressionAssign($dimFetch) : $scope, $storage); + } + + if ($dimExpr === null) { + $dimResults[$key] = null; + $offsetTypes[] = [null, $dimFetch]; + $offsetNativeTypes[] = [null, $dimFetch]; + $nodeScopeResolver->storeExpressionResult($storage, $dimFetch, $this->expressionResultFactory->create( + $scope, + beforeScope: $scope, + expr: $dimFetch, + hasYield: false, + isAlwaysTerminating: false, + throwPoints: [], + impurePoints: [], + )); + + } else { + if ($enterExpressionAssign) { + $scope->enterExpressionAssign($dimExpr); + } + $nodeScopeResolver->storeExpressionResult($storage, $dimFetch, $this->expressionResultFactory->create( + $scope, + beforeScope: $scope, + expr: $dimFetch, + hasYield: false, + isAlwaysTerminating: false, + throwPoints: [], + impurePoints: [], + )); + $result = $nodeScopeResolver->processExprNode($stmt, $dimExpr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $dimResults[$key] = $result; + $offsetTypes[] = [$result->getType(), $dimFetch]; + $offsetNativeTypes[] = [$result->getNativeType(), $dimFetch]; + $hasYield = $hasYield || $result->hasYield(); + $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); + $scope = $result->getScope(); + + if ($enterExpressionAssign) { + $scope = $scope->exitExpressionAssign($dimExpr); + } + } + } + + if ($mode->issetSemanticsForRead()) { + // `$lvalue ??= ...` reads the chain with isset() semantics. The root + // and dimensions were just walked, so each chain link's read is + // composed from their results - no re-walk - and carries the isset + // descriptor (bug-13623). + $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $originalVar); + $readScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $originalVar); + $levelReadResult = $rootReadResult; + foreach ($dimFetchStack as $key => $dimFetch) { + $levelReadResult = $this->arrayDimFetchHandler->composeResult($nodeScopeResolver, $stmt, $dimFetch, $dimResults[$key], $levelReadResult, $storage, $context, $readScope); + } + $targetReadResult = $levelReadResult; + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_ARRAY_DIM_FETCH, + $originalVar, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + rootVar: $var, + dimFetchStack: $dimFetchStack, + assignedPropertyExpr: $assignedPropertyExpr, + offsetTypes: $offsetTypes, + offsetNativeTypes: $offsetNativeTypes, + targetReadResult: $targetReadResult, + ); + } + + if ($var instanceof PropertyFetch) { + $scopeBeforeVar = $scope; + $objectResult = $nodeScopeResolver->processExprNode($stmt, $var->var, $scope, $storage, $nodeCallback, $context); + $hasYield = $objectResult->hasYield(); + $throwPoints = $objectResult->getThrowPoints(); + $impurePoints = $objectResult->getImpurePoints(); + $isAlwaysTerminating = $objectResult->isAlwaysTerminating(); + $scope = $objectResult->getScope(); + + $propertyName = null; + $propertyNameResult = null; + if ($var->name instanceof Node\Identifier) { + $propertyName = $var->name->name; + } else { + $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $hasYield = $hasYield || $propertyNameResult->hasYield(); + $throwPoints = array_merge($throwPoints, $propertyNameResult->getThrowPoints()); + $impurePoints = array_merge($impurePoints, $propertyNameResult->getImpurePoints()); + $isAlwaysTerminating = $isAlwaysTerminating || $propertyNameResult->isAlwaysTerminating(); + $scope = $propertyNameResult->getScope(); + } + + if ($mode->issetSemanticsForRead()) { + // `$lvalue ??= ...` reads the property with isset() semantics: the + // read is composed from the just-walked receiver and name results - + // no re-walk - and carries the isset descriptor (bug-13623). + $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $var); + $readScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); + $targetReadResult = $this->propertyFetchHandler->composeResult($nodeScopeResolver, $var, $objectResult, $propertyNameResult, $scopeBeforeVar, $readScope); + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_PROPERTY_FETCH, + $var, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + propertyName: $propertyName, + targetReadResult: $targetReadResult, + ); + } + + if ($var instanceof Expr\StaticPropertyFetch) { + $classResult = null; + if ($var->class instanceof Node\Name) { + $propertyHolderType = $scope->resolveTypeByName($var->class); + } else { + $classResult = $nodeScopeResolver->processExprNode($stmt, $var->class, $scope, $storage, $nodeCallback, $context); + $propertyHolderType = $scope->getType($var->class); + } + + $propertyName = null; + $propertyNameResult = null; + if ($var->name instanceof Node\Identifier) { + $propertyName = $var->name->name; + } else { + $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $hasYield = $propertyNameResult->hasYield(); + $throwPoints = $propertyNameResult->getThrowPoints(); + $impurePoints = $propertyNameResult->getImpurePoints(); + $isAlwaysTerminating = $propertyNameResult->isAlwaysTerminating(); + $scope = $propertyNameResult->getScope(); + } + + if ($mode->issetSemanticsForRead()) { + // Same as the PropertyFetch branch above: the ??= read is composed + // from the just-walked class/name results on the isset-semantics + // scope - no re-walk. + $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $var); + $readScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); + $targetReadResult = $this->staticPropertyFetchHandler->composeResult($var, $classResult, $propertyNameResult, $readScope); + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_STATIC_PROPERTY_FETCH, + $var, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + propertyName: $propertyName, + propertyHolderType: $propertyHolderType, + targetReadResult: $targetReadResult, + ); + } + + if ($var instanceof List_) { + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_LIST, + $var, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + ); + } + + if ($var instanceof ExistingArrayDimFetch) { + $originalVar = $var; + $dimFetchStack = []; + $assignedPropertyExpr = $assignedExpr; + while ($var instanceof ExistingArrayDimFetch) { + $varForSetOffsetValue = $var->getVar(); + if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) { + $varForSetOffsetValue = new TypeExpr($this->getOriginalPropertyType($varForSetOffsetValue, $scope)); + } + $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( + $varForSetOffsetValue, + $var->getDim(), + $assignedPropertyExpr, + ); + $dimFetchStack[] = $var; + $var = $var->getVar(); + } + + // the chain is usually a clone of AST nodes already processed elsewhere + // (see Unset_ handling) - process it with a noop callback so that + // results for its nodes are stored without invoking rules twice + $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); + + $offsetTypes = []; + $offsetNativeTypes = []; + foreach (array_reverse($dimFetchStack) as $dimFetch) { + $dimExpr = $dimFetch->getDim(); + $nodeScopeResolver->processExprNode($stmt, $dimExpr, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); + $offsetTypes[] = [$scope->getType($dimExpr), $dimFetch]; + $offsetNativeTypes[] = [$scope->getNativeType($dimExpr), $dimFetch]; + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_EXISTING_ARRAY_DIM_FETCH, + $originalVar, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + rootVar: $var, + assignedPropertyExpr: $assignedPropertyExpr, + existingOffsetTypes: $offsetTypes, + existingOffsetNativeTypes: $offsetNativeTypes, + ); + } + + $varResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context); + $hasYield = $varResult->hasYield(); + $throwPoints = array_merge($throwPoints, $varResult->getThrowPoints()); + $impurePoints = array_merge($impurePoints, $varResult->getImpurePoints()); + $isAlwaysTerminating = $varResult->isAlwaysTerminating(); + $scope = $varResult->getScope(); + + if ($mode->producesTargetReadResult()) { + // a synthetic op=/??= target: the walk above already priced the target + // as a read - its result is the read + $targetReadResult = $varResult; + } + + return new PreparedAssignTarget( + PreparedAssignTarget::KIND_FALLBACK, + $var, + $assignedExpr, + $beforeScope, + $scope, + $enterExpressionAssign, + $isAssignOp, + $hasYield, + $throwPoints, + $impurePoints, + $isAlwaysTerminating, + targetReadResult: $targetReadResult, + ); + } + + /** + * The post-value half of an assignment: performs the write and its + * bookkeeping (narrowing, conditional expressions, node callbacks) for a + * target walked by prepareTarget(), consuming the caller-processed value + * result. + * + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + public function applyWrite( + NodeScopeResolver $nodeScopeResolver, + PreparedAssignTarget $target, + ExpressionResult $valueResult, + Node\Stmt $stmt, + ExpressionResultStorage $storage, + callable $nodeCallback, + ExpressionContext $context, + ): ExpressionResult + { + $kind = $target->getKind(); + $var = $target->getVar(); + $assignedExpr = $target->getAssignedExpr(); + $beforeScope = $target->getBeforeScope(); + $scope = $target->getScope(); + $enterExpressionAssign = $target->enterExpressionAssign(); + $isAssignOp = $target->isAssignOp(); + $hasYield = $target->hasYield(); + $throwPoints = $target->getThrowPoints(); + $impurePoints = $target->getImpurePoints(); + $isAlwaysTerminating = $target->isAlwaysTerminating(); + if ($kind === PreparedAssignTarget::KIND_VARIABLE) { + if (!$var instanceof Variable) { + throw new ShouldNotHappenException(); + } + $result = $valueResult; $hasYield = $result->hasYield(); $throwPoints = $result->getThrowPoints(); $impurePoints = $result->getImpurePoints(); @@ -544,112 +964,22 @@ public function processAssignVar( $isAlwaysTerminating = $isAlwaysTerminating || $nameExprResult->isAlwaysTerminating(); $scope = $nameExprResult->getScope(); } - } elseif ($var instanceof ArrayDimFetch) { - $dimFetchStack = []; - $originalVar = $var; - $assignedPropertyExpr = $assignedExpr; - while ($var instanceof ArrayDimFetch) { - $varForSetOffsetValue = $var->var; - if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) { - $varForSetOffsetValue = new TypeExpr($this->getOriginalPropertyType($varForSetOffsetValue, $scope)); - } - - if ( - $var === $originalVar - && $var->dim !== null - && $scope->hasExpressionType($var)->yes() - ) { - $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( - $varForSetOffsetValue, - $var->dim, - $assignedPropertyExpr, - ); - } else { - $assignedPropertyExpr = new SetOffsetValueTypeExpr( - $varForSetOffsetValue, - $var->dim, - $assignedPropertyExpr, - ); - } - $dimFetchStack[] = $var; - $var = $var->var; - } - - // 1. eval root expr - // The root is read to obtain the container that receives the offset write, so a - // property root must resolve to its readable type (not its writable one) even - // though it sits on the left-hand side of the assignment. - if ($enterExpressionAssign) { - $scope = $scope->enterExpressionAssign($var, false); - } - $result = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context->enterDeep()); - $hasYield = $result->hasYield(); - $throwPoints = $result->getThrowPoints(); - $impurePoints = $result->getImpurePoints(); - $isAlwaysTerminating = $result->isAlwaysTerminating(); - $scope = $result->getScope(); - if ($enterExpressionAssign) { - $scope = $scope->exitExpressionAssign($var); - } - - // 2. eval dimensions - $offsetTypes = []; - $offsetNativeTypes = []; - $dimFetchStack = array_reverse($dimFetchStack); - $lastDimKey = array_key_last($dimFetchStack); - foreach ($dimFetchStack as $key => $dimFetch) { - $dimExpr = $dimFetch->dim; - - // Callback was already called for last dim at the beginning of the method. - if ($key !== $lastDimKey) { - $nodeScopeResolver->callNodeCallback($nodeCallback, $dimFetch, $enterExpressionAssign ? $scope->enterExpressionAssign($dimFetch) : $scope, $storage); - } - - if ($dimExpr === null) { - $offsetTypes[] = [null, $dimFetch]; - $offsetNativeTypes[] = [null, $dimFetch]; - $nodeScopeResolver->storeExpressionResult($storage, $dimFetch, $this->expressionResultFactory->create( - $scope, - beforeScope: $scope, - expr: $dimFetch, - hasYield: false, - isAlwaysTerminating: false, - throwPoints: [], - impurePoints: [], - )); - - } else { - if ($enterExpressionAssign) { - $scope->enterExpressionAssign($dimExpr); - } - $nodeScopeResolver->storeExpressionResult($storage, $dimFetch, $this->expressionResultFactory->create( - $scope, - beforeScope: $scope, - expr: $dimFetch, - hasYield: false, - isAlwaysTerminating: false, - throwPoints: [], - impurePoints: [], - )); - $result = $nodeScopeResolver->processExprNode($stmt, $dimExpr, $scope, $storage, $nodeCallback, $context->enterDeep()); - $offsetTypes[] = [$result->getType(), $dimFetch]; - $offsetNativeTypes[] = [$result->getNativeType(), $dimFetch]; - $hasYield = $hasYield || $result->hasYield(); - $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); - $scope = $result->getScope(); - - if ($enterExpressionAssign) { - $scope = $scope->exitExpressionAssign($dimExpr); - } - } + } elseif ($kind === PreparedAssignTarget::KIND_ARRAY_DIM_FETCH) { + if (!$var instanceof ArrayDimFetch) { + throw new ShouldNotHappenException(); } - + $originalVar = $var; + $var = $target->getRootVar(); + $dimFetchStack = $target->getDimFetchStack(); + $assignedPropertyExpr = $target->getAssignedPropertyExpr(); + $offsetTypes = $target->getOffsetTypes(); + $offsetNativeTypes = $target->getOffsetNativeTypes(); $valueToWrite = $scope->getType($assignedExpr); $nativeValueToWrite = $scope->getNativeType($assignedExpr); $scopeBeforeAssignEval = $scope; // 3. eval assigned expr - $result = $processExprCallback($scope); + $result = $valueResult; $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); @@ -753,28 +1083,13 @@ public function processAssignVar( $context, )->getThrowPoints()); } - } elseif ($var instanceof PropertyFetch) { - $objectResult = $nodeScopeResolver->processExprNode($stmt, $var->var, $scope, $storage, $nodeCallback, $context); - $hasYield = $objectResult->hasYield(); - $throwPoints = $objectResult->getThrowPoints(); - $impurePoints = $objectResult->getImpurePoints(); - $isAlwaysTerminating = $objectResult->isAlwaysTerminating(); - $scope = $objectResult->getScope(); - - $propertyName = null; - if ($var->name instanceof Node\Identifier) { - $propertyName = $var->name->name; - } else { - $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); - $hasYield = $hasYield || $propertyNameResult->hasYield(); - $throwPoints = array_merge($throwPoints, $propertyNameResult->getThrowPoints()); - $impurePoints = array_merge($impurePoints, $propertyNameResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $propertyNameResult->isAlwaysTerminating(); - $scope = $propertyNameResult->getScope(); + } elseif ($kind === PreparedAssignTarget::KIND_PROPERTY_FETCH) { + if (!$var instanceof PropertyFetch) { + throw new ShouldNotHappenException(); } - + $propertyName = $target->getPropertyName(); $scopeBeforeAssignEval = $scope; - $result = $processExprCallback($scope); + $result = $valueResult; $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); @@ -865,28 +1180,14 @@ public function processAssignVar( } } - } elseif ($var instanceof Expr\StaticPropertyFetch) { - if ($var->class instanceof Node\Name) { - $propertyHolderType = $scope->resolveTypeByName($var->class); - } else { - $nodeScopeResolver->processExprNode($stmt, $var->class, $scope, $storage, $nodeCallback, $context); - $propertyHolderType = $scope->getType($var->class); - } - - $propertyName = null; - if ($var->name instanceof Node\Identifier) { - $propertyName = $var->name->name; - } else { - $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); - $hasYield = $propertyNameResult->hasYield(); - $throwPoints = $propertyNameResult->getThrowPoints(); - $impurePoints = $propertyNameResult->getImpurePoints(); - $isAlwaysTerminating = $propertyNameResult->isAlwaysTerminating(); - $scope = $propertyNameResult->getScope(); + } elseif ($kind === PreparedAssignTarget::KIND_STATIC_PROPERTY_FETCH) { + if (!$var instanceof Expr\StaticPropertyFetch) { + throw new ShouldNotHappenException(); } - + $propertyHolderType = $target->getPropertyHolderType(); + $propertyName = $target->getPropertyName(); $scopeBeforeAssignEval = $scope; - $result = $processExprCallback($scope); + $result = $valueResult; $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); @@ -930,8 +1231,11 @@ public function processAssignVar( $nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage); $scope = $scope->assignExpression($var, $assignedExprType, $scope->getNativeType($assignedExpr)); } - } elseif ($var instanceof List_) { - $result = $processExprCallback($scope); + } elseif ($kind === PreparedAssignTarget::KIND_LIST) { + if (!$var instanceof List_) { + throw new ShouldNotHappenException(); + } + $result = $valueResult; $hasYield = $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); @@ -963,7 +1267,7 @@ public function processAssignVar( $dimExpr = $arrayItem->key; } $getOffsetValueTypeExpr = new TypeExpr($scope->getType($assignedExpr)->getOffsetValueType($scope->getType($dimExpr))); - $result = $this->processAssignVar( + $itemTarget = $this->prepareTarget( $nodeScopeResolver, $scope, $storage, @@ -972,8 +1276,16 @@ public function processAssignVar( $getOffsetValueTypeExpr, $nodeCallback, $context, - fn (MutatingScope $scope): ExpressionResult => $this->expressionResultFactory->create($scope, beforeScope: $scope, expr: $getOffsetValueTypeExpr, hasYield: false, isAlwaysTerminating: false, throwPoints: [], impurePoints: []), - $enterExpressionAssign, + $enterExpressionAssign ? AssignTargetWalkMode::assign() : AssignTargetWalkMode::virtualAssign(), + ); + $result = $this->applyWrite( + $nodeScopeResolver, + $itemTarget, + $this->expressionResultFactory->create($itemTarget->getScope(), beforeScope: $itemTarget->getScope(), expr: $getOffsetValueTypeExpr, hasYield: false, isAlwaysTerminating: false, throwPoints: [], impurePoints: []), + $stmt, + $storage, + $nodeCallback, + $context, ); $scope = $result->getScope(); $hasYield = $hasYield || $result->hasYield(); @@ -981,37 +1293,11 @@ public function processAssignVar( $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); $isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating(); } - } elseif ($var instanceof ExistingArrayDimFetch) { - $dimFetchStack = []; - $assignedPropertyExpr = $assignedExpr; - while ($var instanceof ExistingArrayDimFetch) { - $varForSetOffsetValue = $var->getVar(); - if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) { - $varForSetOffsetValue = new TypeExpr($this->getOriginalPropertyType($varForSetOffsetValue, $scope)); - } - $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( - $varForSetOffsetValue, - $var->getDim(), - $assignedPropertyExpr, - ); - $dimFetchStack[] = $var; - $var = $var->getVar(); - } - - // the chain is usually a clone of AST nodes already processed elsewhere - // (see Unset_ handling) - process it with a noop callback so that - // results for its nodes are stored without invoking rules twice - $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); - - $offsetTypes = []; - $offsetNativeTypes = []; - foreach (array_reverse($dimFetchStack) as $dimFetch) { - $dimExpr = $dimFetch->getDim(); - $nodeScopeResolver->processExprNode($stmt, $dimExpr, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); - $offsetTypes[] = [$scope->getType($dimExpr), $dimFetch]; - $offsetNativeTypes[] = [$scope->getNativeType($dimExpr), $dimFetch]; - } - + } elseif ($kind === PreparedAssignTarget::KIND_EXISTING_ARRAY_DIM_FETCH) { + $var = $target->getRootVar(); + $assignedPropertyExpr = $target->getAssignedPropertyExpr(); + $offsetTypes = $target->getExistingOffsetTypes(); + $offsetNativeTypes = $target->getExistingOffsetNativeTypes(); $valueToWrite = $scope->getType($assignedExpr); $nativeValueToWrite = $scope->getNativeType($assignedExpr); $varType = $scope->getType($var); @@ -1055,13 +1341,7 @@ public function processAssignVar( ); } } else { - $varResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context); - $hasYield = $varResult->hasYield(); - $throwPoints = array_merge($throwPoints, $varResult->getThrowPoints()); - $impurePoints = array_merge($impurePoints, $varResult->getImpurePoints()); - $isAlwaysTerminating = $varResult->isAlwaysTerminating(); - $scope = $varResult->getScope(); - $result = $processExprCallback($scope); + $result = $valueResult; $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); diff --git a/src/Analyser/ExprHandler/AssignOpHandler.php b/src/Analyser/ExprHandler/AssignOpHandler.php index 5484969f1b7..ad3c1c67afe 100644 --- a/src/Analyser/ExprHandler/AssignOpHandler.php +++ b/src/Analyser/ExprHandler/AssignOpHandler.php @@ -9,17 +9,16 @@ use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Name; use PhpParser\Node\Stmt; +use PHPStan\Analyser\AssignTargetWalkMode; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ExpressionResultFactory; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExprHandler; use PHPStan\Analyser\ExprHandler\Helper\ImplicitToStringCallHelper; -use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; -use PHPStan\Analyser\NoopNodeCallback; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -48,7 +47,6 @@ public function __construct( private InitializerExprTypeResolver $initializerExprTypeResolver, private ImplicitToStringCallHelper $implicitToStringCallHelper, private ExpressionResultFactory $expressionResultFactory, - private NonNullabilityHelper $nonNullabilityHelper, ) { } @@ -61,18 +59,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $condResult = null; - if ($expr instanceof Expr\AssignOp\Coalesce) { - // `$lvalue ??= ...` is `$lvalue = $lvalue ?? ...`: price the left side - // once as a read (mirroring CoalesceHandler's left-side processing) so - // its result carries the isset descriptor for NullCoalesceRule. The - // NoopNodeCallback avoids duplicate reports and the read's scope is - // discarded: processAssignVar() walks the target itself. - $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $expr->var); - $condScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->var); - $condResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $condScope, $storage, new NoopNodeCallback(), $context->enterDeep()); - } - $assignResult = $this->assignHandler->processAssignVar( + $target = $this->assignHandler->prepareTarget( $nodeScopeResolver, $scope, $storage, @@ -81,38 +68,48 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $expr, $nodeCallback, $context, - function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $storage, $nodeScopeResolver): ExpressionResult { - $originalScope = $scope; - if ($expr instanceof Expr\AssignOp\Coalesce) { - $scope = $scope->filterByFalseyValue( - new BinaryOp\NotIdentical($expr->var, new ConstFetch(new Name('null'))), - ); - - if ($expr->var instanceof Expr\Variable && is_string($expr->var->name)) { - $context = $context->enterRightSideAssign( - $expr->var->name, - $expr->expr, - ); - } - } - - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); - if ($expr instanceof Expr\AssignOp\Coalesce) { - $isAlwaysTerminating = $exprResult->isAlwaysTerminating() && $originalScope->getType($expr->var)->isNull()->yes(); - return $this->expressionResultFactory->create( - $exprResult->getScope()->mergeWith($originalScope), - $originalScope, - $expr->expr, - $exprResult->hasYield(), - $isAlwaysTerminating, - $exprResult->getThrowPoints(), - $exprResult->getImpurePoints(), - ); - } - - return $exprResult; - }, - $expr instanceof Expr\AssignOp\Coalesce, + $expr instanceof Expr\AssignOp\Coalesce ? AssignTargetWalkMode::coalesceReadModifyWrite() : AssignTargetWalkMode::readModifyWrite(), + ); + $condResult = $expr instanceof Expr\AssignOp\Coalesce ? $target->getTargetReadResult() : null; + + $valueBeforeScope = $target->getScope(); + $valueScope = $valueBeforeScope; + $valueContext = $context; + if ($expr instanceof Expr\AssignOp\Coalesce) { + $valueScope = $valueScope->filterByFalseyValue( + new BinaryOp\NotIdentical($expr->var, new ConstFetch(new Name('null'))), + ); + + if ($expr->var instanceof Expr\Variable && is_string($expr->var->name)) { + $valueContext = $valueContext->enterRightSideAssign( + $expr->var->name, + $expr->expr, + ); + } + } + + $valueResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext->enterDeep()); + if ($expr instanceof Expr\AssignOp\Coalesce) { + $isAlwaysTerminatingCoalesce = $valueResult->isAlwaysTerminating() && $valueBeforeScope->getType($expr->var)->isNull()->yes(); + $valueResult = $this->expressionResultFactory->create( + $valueResult->getScope()->mergeWith($valueBeforeScope), + $valueBeforeScope, + $expr->expr, + $valueResult->hasYield(), + $isAlwaysTerminatingCoalesce, + $valueResult->getThrowPoints(), + $valueResult->getImpurePoints(), + ); + } + + $assignResult = $this->assignHandler->applyWrite( + $nodeScopeResolver, + $target, + $valueResult, + $stmt, + $storage, + $nodeCallback, + $context, ); $scope = $assignResult->getScope(); $throwPoints = $assignResult->getThrowPoints(); @@ -129,7 +126,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $impurePoints = array_merge($impurePoints, $toStringResult->getImpurePoints()); } - if ($expr instanceof Expr\AssignOp\Coalesce) { + if ($condResult !== null) { $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new CoalesceExpressionNode($expr, $condResult, 'on left side of ??='), $beforeScope, $storage, $context); } diff --git a/src/Analyser/ExprHandler/PropertyFetchHandler.php b/src/Analyser/ExprHandler/PropertyFetchHandler.php index 95f1e1c72d9..a5a97c2c21c 100644 --- a/src/Analyser/ExprHandler/PropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/PropertyFetchHandler.php @@ -58,6 +58,22 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $beforeScope = $scope; $scopeBeforeVar = $scope; $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $nameResult = null; + if (!$expr->name instanceof Identifier) { + $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $varResult->getScope(), $storage, $nodeCallback, $context->enterDeep()); + } + + return $this->composeResult($nodeScopeResolver, $expr, $varResult, $nameResult, $scopeBeforeVar, $beforeScope); + } + + /** + * Builds the property read's ExpressionResult from the already-walked + * receiver and name results - the fetch is not re-walked. processExpr() + * routes through this; AssignHandler::prepareTarget() calls it to price a + * read-modify-write target from the write walk's child results. + */ + public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetch $expr, ExpressionResult $varResult, ?ExpressionResult $nameResult, MutatingScope $scopeBeforeVar, MutatingScope $beforeScope): ExpressionResult + { $hasYield = $varResult->hasYield(); $throwPoints = $varResult->getThrowPoints(); $impurePoints = $varResult->getImpurePoints(); @@ -76,8 +92,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } } } - } else { - $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); + } elseif ($nameResult !== null) { $hasYield = $hasYield || $nameResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $nameResult->getImpurePoints()); diff --git a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php index 0d964e7fafc..3283f6fdadf 100644 --- a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php @@ -56,6 +56,29 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; + $classResult = null; + if ($expr->class instanceof Expr) { + $classResult = $nodeScopeResolver->processExprNode($stmt, $expr->class, $scope, $storage, $nodeCallback, $context->enterDeep()); + $scope = $classResult->getScope(); + } + $nameResult = null; + if (!$expr->name instanceof VarLikeIdentifier) { + $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); + } + + return $this->composeResult($expr, $classResult, $nameResult, $beforeScope); + } + + /** + * Builds the static property read's ExpressionResult from the + * already-walked class and name results - the fetch is not re-walked. + * processExpr() routes through this; AssignHandler::prepareTarget() calls + * it to price a read-modify-write target from the write walk's child + * results. + */ + public function composeResult(StaticPropertyFetch $expr, ?ExpressionResult $classResult, ?ExpressionResult $nameResult, MutatingScope $beforeScope): ExpressionResult + { + $scope = $beforeScope; $hasYield = false; $throwPoints = []; $impurePoints = [ @@ -69,9 +92,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ]; $isAlwaysTerminating = false; $containsNullsafe = false; - $classResult = null; - if ($expr->class instanceof Expr) { - $classResult = $nodeScopeResolver->processExprNode($stmt, $expr->class, $scope, $storage, $nodeCallback, $context->enterDeep()); + if ($classResult !== null) { $hasYield = $classResult->hasYield(); $throwPoints = $classResult->getThrowPoints(); $impurePoints = $classResult->getImpurePoints(); @@ -79,8 +100,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $classResult->getScope(); $containsNullsafe = $classResult->containsNullsafe(); } - if (!$expr->name instanceof VarLikeIdentifier) { - $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); + if ($nameResult !== null) { $hasYield = $hasYield || $nameResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $nameResult->getImpurePoints()); diff --git a/src/Analyser/ExprHandler/VariableHandler.php b/src/Analyser/ExprHandler/VariableHandler.php index c34c281ffad..8cdfdd7b387 100644 --- a/src/Analyser/ExprHandler/VariableHandler.php +++ b/src/Analyser/ExprHandler/VariableHandler.php @@ -80,6 +80,23 @@ public function resolveType(MutatingScope $scope, Expr $expr): Type public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; + $nameResult = null; + if (!is_string($expr->name)) { + $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); + } + + return $this->composeResult($expr, $nameResult, $beforeScope); + } + + /** + * Builds the variable read's ExpressionResult from an already-walked state - + * no node processing happens here. processExpr() routes through this after + * walking a dynamic name; AssignHandler::prepareTarget() calls it to price a + * read-modify-write target without re-walking it. + */ + public function composeResult(Variable $expr, ?ExpressionResult $nameResult, MutatingScope $beforeScope): ExpressionResult + { + $scope = $beforeScope; $hasYield = false; $throwPoints = []; $impurePoints = []; @@ -88,8 +105,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if (in_array($expr->name, Scope::SUPERGLOBAL_VARIABLES, true)) { $impurePoints[] = new ImpurePoint($scope, $expr, 'superglobal', 'access to superglobal variable', true); } - } else { - $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); + } elseif ($nameResult !== null) { $hasYield = $nameResult->hasYield(); $throwPoints = $nameResult->getThrowPoints(); $impurePoints = $nameResult->getImpurePoints(); diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 9df08b89aeb..dfd51eac1f5 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4044,17 +4044,28 @@ private function getParameterOutExtensionsType(CallLike $callLike, $calleeReflec */ public function processVirtualAssign(MutatingScope $scope, ExpressionResultStorage $storage, Node\Stmt $stmt, Expr $var, Expr $assignedExpr, callable $nodeCallback): ExpressionResult { - return $this->container->getByType(AssignHandler::class)->processAssignVar( + $assignHandler = $this->container->getByType(AssignHandler::class); + $virtualAssignNodeCallback = new VirtualAssignNodeCallback($nodeCallback); + $target = $assignHandler->prepareTarget( $this, $scope, $storage, $stmt, $var, $assignedExpr, - new VirtualAssignNodeCallback($nodeCallback), + $virtualAssignNodeCallback, + ExpressionContext::createDeep(), + AssignTargetWalkMode::virtualAssign(), + ); + + return $assignHandler->applyWrite( + $this, + $target, + $this->expressionResultFactory->create($target->getScope(), beforeScope: $target->getScope(), expr: $assignedExpr, hasYield: false, isAlwaysTerminating: false, throwPoints: [], impurePoints: []), + $stmt, + $storage, + $virtualAssignNodeCallback, ExpressionContext::createDeep(), - fn (MutatingScope $scope): ExpressionResult => $this->expressionResultFactory->create($scope, beforeScope: $scope, expr: $assignedExpr, hasYield: false, isAlwaysTerminating: false, throwPoints: [], impurePoints: []), - false, ); } diff --git a/src/Analyser/PreparedAssignTarget.php b/src/Analyser/PreparedAssignTarget.php new file mode 100644 index 00000000000..2b294aa5cdb --- /dev/null +++ b/src/Analyser/PreparedAssignTarget.php @@ -0,0 +1,239 @@ +|null $dimFetchStack + * @param non-empty-list|null $offsetTypes + * @param non-empty-list|null $offsetNativeTypes + * @param list|null $existingOffsetTypes + * @param list|null $existingOffsetNativeTypes + */ + public function __construct( + private string $kind, + private Expr $var, + private Expr $assignedExpr, + private MutatingScope $beforeScope, + private MutatingScope $scope, + private bool $enterExpressionAssign, + private bool $isAssignOp, + private bool $hasYield, + private array $throwPoints, + private array $impurePoints, + private bool $isAlwaysTerminating, + private ?Expr $rootVar = null, + private ?array $dimFetchStack = null, + private ?Expr $assignedPropertyExpr = null, + private ?array $offsetTypes = null, + private ?array $offsetNativeTypes = null, + private ?array $existingOffsetTypes = null, + private ?array $existingOffsetNativeTypes = null, + private ?string $propertyName = null, + private ?Type $propertyHolderType = null, + private ?ExpressionResult $targetReadResult = null, + ) + { + } + + /** + * @return self::KIND_* + */ + public function getKind(): string + { + return $this->kind; + } + + public function getVar(): Expr + { + return $this->var; + } + + public function getAssignedExpr(): Expr + { + return $this->assignedExpr; + } + + public function getBeforeScope(): MutatingScope + { + return $this->beforeScope; + } + + /** The scope the caller processes the assigned value on. */ + public function getScope(): MutatingScope + { + return $this->scope; + } + + public function enterExpressionAssign(): bool + { + return $this->enterExpressionAssign; + } + + public function isAssignOp(): bool + { + return $this->isAssignOp; + } + + public function hasYield(): bool + { + return $this->hasYield; + } + + /** + * @return InternalThrowPoint[] + */ + public function getThrowPoints(): array + { + return $this->throwPoints; + } + + /** + * @return ImpurePoint[] + */ + public function getImpurePoints(): array + { + return $this->impurePoints; + } + + public function isAlwaysTerminating(): bool + { + return $this->isAlwaysTerminating; + } + + public function getRootVar(): Expr + { + if ($this->rootVar === null) { + throw new ShouldNotHappenException(); + } + + return $this->rootVar; + } + + /** + * @return non-empty-list + */ + public function getDimFetchStack(): array + { + if ($this->dimFetchStack === null) { + throw new ShouldNotHappenException(); + } + + return $this->dimFetchStack; + } + + public function getAssignedPropertyExpr(): Expr + { + if ($this->assignedPropertyExpr === null) { + throw new ShouldNotHappenException(); + } + + return $this->assignedPropertyExpr; + } + + /** + * @return non-empty-list + */ + public function getOffsetTypes(): array + { + if ($this->offsetTypes === null) { + throw new ShouldNotHappenException(); + } + + return $this->offsetTypes; + } + + /** + * @return non-empty-list + */ + public function getOffsetNativeTypes(): array + { + if ($this->offsetNativeTypes === null) { + throw new ShouldNotHappenException(); + } + + return $this->offsetNativeTypes; + } + + /** + * @return list + */ + public function getExistingOffsetTypes(): array + { + if ($this->existingOffsetTypes === null) { + throw new ShouldNotHappenException(); + } + + return $this->existingOffsetTypes; + } + + /** + * @return list + */ + public function getExistingOffsetNativeTypes(): array + { + if ($this->existingOffsetNativeTypes === null) { + throw new ShouldNotHappenException(); + } + + return $this->existingOffsetNativeTypes; + } + + public function getPropertyName(): ?string + { + return $this->propertyName; + } + + public function getPropertyHolderType(): Type + { + if ($this->propertyHolderType === null) { + throw new ShouldNotHappenException(); + } + + return $this->propertyHolderType; + } + + /** + * The whole target priced as a read - produced only in the + * read-modify-write walk modes. For `$lvalue ??= ...` the read carries the + * isset descriptor, composed from the walked chain (bug-13623). + */ + public function getTargetReadResult(): ExpressionResult + { + if ($this->targetReadResult === null) { + throw new ShouldNotHappenException(); + } + + return $this->targetReadResult; + } + +} From 130c7a8bc895bc4a8ccc366169c9f349b13d07af Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 30 Jul 2026 11:11:49 +0200 Subject: [PATCH 3/3] Walk the dynamic variable name once, drop the vestigial clone walks Two leftover noop-callback walks over real AST: - A dynamic-name read-modify-write target (`$$name op= ...`) priced the whole variable through the walk machinery because the name expression was only walked later by the write flow. PHP evaluates the name before reading the old value, so prepareTarget() now walks it once - with the real node callback, at its actual evaluation point - composes the read from the result, and threads it on the PreparedAssignTarget; the write flow consumes it instead of walking the name again after the value. - The ExistingArrayDimFetch branch walked the cloned chain so its nodes had stored results, but the composition below prices the clones directly through the scope - the walks had no observable effect here. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7 --- src/Analyser/ExprHandler/AssignHandler.php | 34 +++++++++++++--------- src/Analyser/PreparedAssignTarget.php | 10 +++++++ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 4a1868716a6..d65abb00565 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -448,24 +448,29 @@ public function prepareTarget( $isAlwaysTerminating = false; $isAssignOp = $assignedExpr instanceof Expr\AssignOp && !$enterExpressionAssign; if ($var instanceof Variable) { + $variableNameResult = null; if ($mode->producesTargetReadResult()) { // `$lvalue OP= ...` reads the old value of `$lvalue`; the write walk // processes a Variable target only as an assignment target, never as // a read. The read is composed here without a walk - for ??= with // isset() semantics (mirroring CoalesceHandler's left-side // processing, with the isset descriptor - bug-13623). + if (!is_string($var->name)) { + // `$$name OP= ...` evaluates the name before reading the old + // value: walk it once here, the write flow consumes the result + $variableNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $hasYield = $variableNameResult->hasYield(); + $throwPoints = $variableNameResult->getThrowPoints(); + $impurePoints = $variableNameResult->getImpurePoints(); + $isAlwaysTerminating = $variableNameResult->isAlwaysTerminating(); + $scope = $variableNameResult->getScope(); + } $readScope = $scope; if ($mode->issetSemanticsForRead()) { $nonNullabilityResult = $this->nonNullabilityHelper->ensureNonNullability($scope, $var); $readScope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); } - if (is_string($var->name)) { - $targetReadResult = $this->variableHandler->composeResult($var, null, $readScope); - } else { - // a dynamic-name target: the name expression is only walked later - // by the write flow, so the read prices it here first - $targetReadResult = $nodeScopeResolver->processExprNode($stmt, $var, $readScope, $storage, new NoopNodeCallback(), $context->enterDeep()); - } + $targetReadResult = $this->variableHandler->composeResult($var, $variableNameResult, $readScope); } return new PreparedAssignTarget( @@ -481,6 +486,7 @@ public function prepareTarget( $impurePoints, $isAlwaysTerminating, targetReadResult: $targetReadResult, + variableNameResult: $variableNameResult, ); } @@ -754,16 +760,13 @@ public function prepareTarget( $var = $var->getVar(); } - // the chain is usually a clone of AST nodes already processed elsewhere - // (see Unset_ handling) - process it with a noop callback so that - // results for its nodes are stored without invoking rules twice - $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); - + // the chain is a clone of AST nodes already processed elsewhere (see + // Unset_ handling) - the types below price the clones directly, no + // walk is needed $offsetTypes = []; $offsetNativeTypes = []; foreach (array_reverse($dimFetchStack) as $dimFetch) { $dimExpr = $dimFetch->getDim(); - $nodeScopeResolver->processExprNode($stmt, $dimExpr, $scope, $storage, new NoopNodeCallback(), $context->enterDeep()); $offsetTypes[] = [$scope->getType($dimExpr), $dimFetch]; $offsetNativeTypes[] = [$scope->getNativeType($dimExpr), $dimFetch]; } @@ -956,7 +959,10 @@ public function applyWrite( if ($assignedExpr instanceof Expr\Array_) { $scope = $this->processArrayByRefItems($scope, $var->name, $assignedExpr, new Variable($var->name)); } - } else { + } elseif ($target->getVariableNameResult() === null) { + // a plain assignment does not read the target, so the dynamic name + // is walked here; read-modify-write targets walked it in + // prepareTarget() and already carry its state $nameExprResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); $hasYield = $hasYield || $nameExprResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameExprResult->getThrowPoints()); diff --git a/src/Analyser/PreparedAssignTarget.php b/src/Analyser/PreparedAssignTarget.php index 2b294aa5cdb..6d119f23d69 100644 --- a/src/Analyser/PreparedAssignTarget.php +++ b/src/Analyser/PreparedAssignTarget.php @@ -61,6 +61,7 @@ public function __construct( private ?string $propertyName = null, private ?Type $propertyHolderType = null, private ?ExpressionResult $targetReadResult = null, + private ?ExpressionResult $variableNameResult = null, ) { } @@ -236,4 +237,13 @@ public function getTargetReadResult(): ExpressionResult return $this->targetReadResult; } + /** + * A dynamic variable name (`$$name`) already walked by prepareTarget() - + * read-modify-write targets evaluate the name before reading the old value. + */ + public function getVariableNameResult(): ?ExpressionResult + { + return $this->variableNameResult; + } + }