Skip to content

Normalize constant-string member access ($obj->{'n'}) to bareword form when printing expression keys#5916

Merged
staabm merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-cl3anbx
Jun 22, 2026
Merged

Normalize constant-string member access ($obj->{'n'}) to bareword form when printing expression keys#5916
staabm merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-cl3anbx

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

In PHP $obj->{'n'} and $obj->n access the exact same property (and likewise
$obj->{'n'}()$obj->n(), Foo::${'s'}Foo::$s), but PHPStan treated
them as two distinct expressions in some contexts. This produced false negatives:
type narrowing performed through one syntax was not seen through the other, and
dead code / always-false comparisons were not reported when the two syntaxes were
mixed.

The fix teaches PHPStan that these are the same expression by normalizing how the
member name is printed, which is what backs the scope's expression keys.

Changes

  • src/Node/Printer/Printer.php: override pObjectProperty() so that a
    curly-brace member name which is a constant string literal matching the PHP
    label grammar (/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/) is printed as
    the bareword name instead of {'...'}. Non-label strings (e.g. {'complex-name'},
    {'0'}) and dynamic names ({$name}, {'a' . $b}) keep the curly form.
  • tests/PHPStan/Analyser/nsrt/bug-14847.php: type-narrowing regression test.
  • tests/PHPStan/Rules/Comparison/data/bug-14847.php +
    StrictComparisonOfDifferentTypesRuleTest::testBug14847(): dead-code regression test.

pObjectProperty() is the single helper nikic's printer uses for property
fetches, nullsafe property fetches, static property fetches, and method / static
method calls, so the normalization covers that whole family at once.

Root cause

MutatingScope::getNodeKey() keys an expression's tracked type on the
pretty-printed form returned by ExprPrinter/Printer. The standard printer
renders $obj->{'n'} and $obj->n differently, so narrowing stored under the
key $obj->n was invisible when reading $obj->{'n'} (and vice versa). Printing
both as $obj->n makes the keys identical, so every key-based mechanism
(narrowing, identical.alwaysFalse/alwaysTrue dead-code detection, impurity
invalidation, remembered impure return values) now treats them as one member.

Analogous cases probed

  • Static property fetch (Foo::${'s'}Foo::$s) and nullsafe property
    fetch
    ($obj?->{'n'}$obj?->n): same root cause, fixed by the same change
    (they all go through pObjectProperty()). Covered by the nsrt test.
  • Method / static-method calls ($obj->{'check'}()$obj->check()): the
    expression-key part is fixed by this change. A deeper discrepancy remains where
    the method-call handlers branch on $expr->name instanceof Identifier and skip
    reflection (assertions, conditional return types, side-effect tracking) for a
    string name. I deliberately did not force these through reflection: the
    current "string-named call ⇒ treat as unknown call ⇒ invalidate the receiver"
    behavior is what tests/.../nsrt/bug-3831.php depends on, and rewriting the
    String_ name to an Identifier at the AST level regresses that side-effect
    analysis. Fixing method-call reflection for dynamic-but-constant names is a
    larger, separate concern.

Test

  • tests/PHPStan/Analyser/nsrt/bug-14847.php asserts narrowing flows in both
    directions for instance properties, static properties and nullsafe access. It
    fails before the fix (the {'n'} !== null$obj->n direction infers
    string|null) and passes after.
  • tests/PHPStan/Rules/Comparison/data/bug-14847.php checks that an always-false
    === null comparison is reported when the guard and the comparison use
    different syntaxes; the curly-then-bareword direction is missed before the fix.

Fixes phpstan/phpstan#14847

…orm 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.
@staabm staabm force-pushed the create-pull-request/patch-cl3anbx branch from 7d9e164 to f5b90f7 Compare June 22, 2026 13:13
@staabm staabm requested a review from VincentLanglet June 22, 2026 13:13
@staabm staabm merged commit 2c49a0c into phpstan:2.2.x Jun 22, 2026
670 of 673 checks passed
@staabm staabm deleted the create-pull-request/patch-cl3anbx branch June 22, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Teach phpstan about about equivalence of two types of object property access

3 participants