diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index e02c18b0da..a7fb327bdd 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -21,6 +21,7 @@ use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; +use PhpParser\Node\VarLikeIdentifier; use PhpParser\NodeFinder; use PHPStan\Analyser\Traverser\TransformStaticTypeTraverser; use PHPStan\Collectors\Collector; @@ -38,6 +39,7 @@ use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; use PHPStan\Node\IssetExpr; use PHPStan\Node\Printer\ExprPrinter; +use PHPStan\Node\Printer\Printer; use PHPStan\Node\VirtualNode; use PHPStan\Parser\ArrayMapArgVisitor; use PHPStan\Parser\Parser; @@ -126,6 +128,7 @@ use function is_string; use function ltrim; use function md5; +use function preg_match; use function sprintf; use function str_contains; use function str_starts_with; @@ -921,7 +924,7 @@ public function getNodeKey(Expr $node): string return '$' . $node->name; } - $key = $this->exprPrinter->printExpr($node); + $key = $this->exprPrinter->printExpr($this->normalizeConstantMemberNames($node)); $attributes = $node->getAttributes(); if ( $node instanceof Node\FunctionLike @@ -942,6 +945,123 @@ public function getNodeKey(Expr $node): string return $key; } + /** + * Rewrites dynamic member names (`$obj->$name`, `$obj->{$name}()`, `Foo::${$name}`, ...) + * that resolve to a single constant string into their bareword form, so that they + * produce the same expression key as if the member had been accessed directly. + * Together with Printer::pObjectProperty (which normalizes constant-string literals) + * this makes `$obj->$n`, `$obj->{'n'}` and `$obj->n` interchangeable for type tracking. + */ + private function normalizeConstantMemberNames(Expr $node): Expr + { + if ($node instanceof PropertyFetch || $node instanceof Expr\NullsafePropertyFetch) { + $var = $this->normalizeConstantMemberNames($node->var); + $name = $node->name instanceof Expr ? $this->normalizeConstantMemberName($node->name) : $node->name; + if ($var === $node->var && $name === $node->name) { + return $node; + } + + return $node instanceof PropertyFetch + ? new PropertyFetch($var, $name) + : new Expr\NullsafePropertyFetch($var, $name); + } + + if ($node instanceof MethodCall || $node instanceof Expr\NullsafeMethodCall) { + $var = $this->normalizeConstantMemberNames($node->var); + $name = $node->name instanceof Expr ? $this->normalizeConstantMemberName($node->name) : $node->name; + if ($var === $node->var && $name === $node->name) { + return $node; + } + + return $node instanceof MethodCall + ? new MethodCall($var, $name, $node->args) + : new Expr\NullsafeMethodCall($var, $name, $node->args); + } + + if ($node instanceof Expr\StaticPropertyFetch && $node->name instanceof Expr) { + $name = $this->normalizeConstantMemberName($node->name); + if ($name === $node->name) { + return $node; + } + + return new Expr\StaticPropertyFetch($node->class, $name); + } + + if ($node instanceof Expr\StaticCall && $node->name instanceof Expr) { + $name = $this->normalizeConstantMemberName($node->name); + if ($name === $node->name) { + return $node; + } + + return new Expr\StaticCall($node->class, $name, $node->args); + } + + return $node; + } + + /** + * Returns the bareword (or quoted) name node when $nameExpr resolves to a single + * constant string, otherwise $nameExpr unchanged. The bareword is produced as a + * VarLikeIdentifier so it is accepted by every member-access node constructor. + */ + private function normalizeConstantMemberName(Expr $nameExpr): Expr|VarLikeIdentifier + { + $constantStrings = $this->getType($nameExpr)->getConstantStrings(); + if (count($constantStrings) !== 1) { + return $nameExpr; + } + + $value = $constantStrings[0]->getValue(); + if (preg_match(Printer::BAREWORD_NAME_REGEX, $value) !== 1) { + return new String_($value); + } + + return new VarLikeIdentifier($value); + } + + /** + * When a member is assigned through a dynamic name that resolves to a union of + * constant strings (`$obj->$name = ...` / `Foo::$$name = ...` with `$name` of type + * `'a'|'b'`), the write may have targeted any of those members, so any narrowing + * previously held for each of them is no longer valid. Returns one concrete member + * fetch per possible constant-string name so the caller can invalidate them. + * + * The single-constant case is already handled by getNodeKey() normalization (the + * key collapses to the bareword form, so invalidateExpression($expr) reaches it), + * and is intentionally excluded here. + * + * @return list + */ + private function constantMemberNameFetches(Expr $expr): array + { + if ($expr instanceof PropertyFetch || $expr instanceof Expr\StaticPropertyFetch) { + $nameExpr = $expr->name; + } else { + return []; + } + + if (!$nameExpr instanceof Expr) { + return []; + } + + $constantStrings = $this->getType($nameExpr)->getConstantStrings(); + if (count($constantStrings) < 2) { + return []; + } + + $fetches = []; + foreach ($constantStrings as $constantString) { + $name = preg_match(Printer::BAREWORD_NAME_REGEX, $constantString->getValue()) === 1 + ? new VarLikeIdentifier($constantString->getValue()) + : new String_($constantString->getValue()); + $fetches[] = $expr instanceof PropertyFetch + ? new PropertyFetch($expr->var, $name) + : new Expr\StaticPropertyFetch($expr->class, $name); + } + + return $fetches; + } + public function getClosureScopeCacheKey(): string { $parts = []; @@ -2923,6 +3043,10 @@ public function assignExpression(Expr $expr, Type $type, Type $nativeType): self $scope = $this->invalidateExpression($expr); } + foreach ($this->constantMemberNameFetches($expr) as $memberFetch) { + $scope = $scope->invalidateExpression($memberFetch); + } + return $scope->specifyExpressionType($expr, $type, $nativeType, TrinaryLogic::createYes()); } diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index 645ffb0de4..a435ec716f 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -44,6 +44,9 @@ final class Printer extends Standard { + /** Matches member names that can be written as a bareword (`$obj->name`) instead of `$obj->{'name'}`. */ + public const BAREWORD_NAME_REGEX = '/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/'; + /** * Normalize curly-brace member access with a constant string name to the * bareword form, so that e.g. `$obj->{'n'}` and `$obj->n` (or `$obj->{'n'}()` @@ -55,7 +58,7 @@ protected function pObjectProperty(Node $node): string { if ( $node instanceof String_ - && preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $node->value) === 1 + && preg_match(self::BAREWORD_NAME_REGEX, $node->value) === 1 ) { return $node->value; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-7851.php b/tests/PHPStan/Analyser/nsrt/bug-7851.php new file mode 100644 index 0000000000..589a2703e5 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-7851.php @@ -0,0 +1,141 @@ +v = 4; + assertType('4', $this->v); + $this->{'v'} = 5; + assertType('5', $this->v); + $n = 'v'; + $this->{$n} = 6; + assertType('6', $this->v); + $this->$n = 7; + assertType('7', $this->v); + assertType('7', $this->{'v'}); + assertType('7', $this->{$n}); + } + + public function testStatic(): void + { + self::$sv = 4; + assertType('4', self::$sv); + $s = 'sv'; + self::${$s} = 5; + assertType('5', self::$sv); + assertType('5', self::${'sv'}); + assertType('5', self::${$s}); + } + +} + +class WithUnionName +{ + + public int $a = 0; + + public int $b = 0; + + public static int $sa = 0; + + public static int $sb = 0; + + public function assignThroughUnionName(bool $c): void + { + $this->a = 3; + assertType('3', $this->a); + $name = $c ? 'a' : 'b'; + // the write may target either member, so neither keeps its narrowed type + $this->$name = 5; + assertType('int', $this->a); + assertType('int', $this->b); + } + + public function assignThroughUnionNameStatic(bool $c): void + { + self::$sa = 3; + assertType('3', self::$sa); + $name = $c ? 'sa' : 'sb'; + self::$$name = 5; + assertType('int', self::$sa); + assertType('int', self::$sb); + } + + public function readThroughUnionName(bool $c): void + { + $name = $c ? 'a' : 'b'; + assertType('int', $this->$name); + } + +} + +class WithNullable +{ + + public ?string $n = null; + + public static ?string $s = null; + + public function narrowInstance(): void + { + $name = 'n'; + if ($this->{$name} !== null) { + assertType('string', $this->n); + assertType('string', $this->{'n'}); + assertType('string', $this->{$name}); + } + } + + public function narrowStatic(): void + { + $name = 's'; + if (self::${$name} !== null) { + assertType('string', self::$s); + assertType('string', self::${'s'}); + } + } + +} + +class WithMethods +{ + + public function count(): int + { + return 0; + } + + public function getName(): ?string + { + return null; + } + + public function memberAccessViaConstantName(): void + { + $method = 'getName'; + // constant-string method calls resolve the same member as the bareword form + assertType('int', $this->{'count'}()); + assertType('string|null', $this->{$method}()); + } + + public function narrowMethodCall(): void + { + $method = 'getName'; + // narrowing a constant-string method call carries over to the bareword form + if ($this->{$method}() !== null) { + assertType('string', $this->getName()); + assertType('string', $this->{'getName'}()); + } + } + +}