From f5b90f7ad25802b98bda7cc84649c7675b04c82e Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Mon, 22 Jun 2026 10:13:37 +0000 Subject: [PATCH] Normalize constant-string member access (`$obj->{'n'}`) to bareword form when printing expression keys - Override `pObjectProperty()` in `PHPStan\Node\Printer\Printer` so that a curly-brace member name that is a constant string literal forming a valid PHP label (e.g. `$obj->{'n'}`, `$obj->{'n'}()`, `Foo::${'s'}`) is printed the same as its bareword equivalent (`$obj->n`, `$obj->n()`, `Foo::$s`). - Because `MutatingScope::getNodeKey()` keys expression types on this printed form, the two syntaxes now share an expression key. Type narrowing, dead-code detection, impurity invalidation and remembered values are therefore applied consistently regardless of which syntax is used to access the same member. - Covers property fetch, nullsafe property fetch, static property fetch, and method/static-method calls, since `pObjectProperty()` backs all of them. - Probed AST-level normalization (rewriting the `String_` name node to an `Identifier`) as a more aggressive alternative: rejected because it changes method-call side-effect analysis (e.g. `$this->{'increment'}()` is currently treated as an unknown call that invalidates `$this`, which bug-3831 relies on). The key-only fix keeps that behavior intact while fixing the reported narrowing and dead-code equivalence. --- src/Node/Printer/Printer.php | 23 +++++++++ tests/PHPStan/Analyser/nsrt/bug-14847.php | 51 +++++++++++++++++++ ...rictComparisonOfDifferentTypesRuleTest.php | 14 +++++ .../Rules/Comparison/data/bug-14847.php | 32 ++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14847.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-14847.php diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index 7409140e8c2..645ffb0de43 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -2,6 +2,9 @@ namespace PHPStan\Node\Printer; +use Override; +use PhpParser\Node; +use PhpParser\Node\Scalar\String_; use PhpParser\PrettyPrinter\Standard; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\BooleanAndNode; @@ -31,6 +34,7 @@ use PHPStan\Node\MethodCallableNode; use PHPStan\Node\StaticMethodCallableNode; use PHPStan\Type\VerbosityLevel; +use function preg_match; use function sprintf; /** @@ -40,6 +44,25 @@ final class Printer extends Standard { + /** + * 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'}()` + * and `$obj->n()`) produce identical expression keys and are treated as the + * same member by the analyser. + */ + #[Override] + 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 + ) { + return $node->value; + } + + return parent::pObjectProperty($node); + } + protected function pPHPStan_Node_TypeExpr(TypeExpr $expr): string // phpcs:ignore { return sprintf('__phpstanType(%s)', $expr->getExprType()->describe(VerbosityLevel::precise())); diff --git a/tests/PHPStan/Analyser/nsrt/bug-14847.php b/tests/PHPStan/Analyser/nsrt/bug-14847.php new file mode 100644 index 00000000000..6e24f79f6b0 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14847.php @@ -0,0 +1,51 @@ +n !== null) { + assertType('string', $obj->n); + assertType('string', $obj->{'n'}); + } +} + +function narrowFromCurly(Foo $obj): void +{ + if ($obj->{'n'} !== null) { + assertType('string', $obj->{'n'}); + assertType('string', $obj->n); + } +} + +function narrowStaticProperty(): void +{ + if (Foo::$s !== null) { + assertType('string', Foo::$s); + assertType('string', Foo::${'s'}); + } + + if (Foo::${'s'} !== null) { + assertType('string', Foo::${'s'}); + assertType('string', Foo::$s); + } +} + +function narrowNullsafe(?Foo $obj): void +{ + if ($obj?->n !== null) { + assertType('string', $obj->n); + assertType('string', $obj->{'n'}); + } +} diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 5122f366f8a..792d737e48f 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -1244,4 +1244,18 @@ public function testBug14791(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14791.php'], []); } + public function testBug14847(): void + { + $this->analyse([__DIR__ . '/data/bug-14847.php'], [ + [ + 'Strict comparison using === between string and null will always evaluate to false.', + 18, + ], + [ + 'Strict comparison using === between string and null will always evaluate to false.', + 29, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14847.php b/tests/PHPStan/Rules/Comparison/data/bug-14847.php new file mode 100644 index 00000000000..08a50bbcf5b --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14847.php @@ -0,0 +1,32 @@ +n === null) { + return; + } + + if ($obj->{'n'} === null) { + echo 'dead'; + } +} + +function g(Foo $obj): void +{ + if ($obj->{'n'} === null) { + return; + } + + if ($obj->n === null) { + echo 'dead'; + } +}