Skip to content

Fix phpstan/phpstan#7088: Accessing SimpleXMLElement nodes using braces gives mixed type#5406

Merged
VincentLanglet merged 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-2l111d3
Apr 7, 2026
Merged

Fix phpstan/phpstan#7088: Accessing SimpleXMLElement nodes using braces gives mixed type#5406
VincentLanglet merged 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-2l111d3

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

Accessing SimpleXMLElement properties using dynamic names ($prop->{$x}) was incorrectly resolved as mixed instead of SimpleXMLElement|null. Static property names ($prop->foo) and constant string expressions ($prop->{'foo-bar'}) already worked correctly.

Changes

  • Added fallback logic in src/Analyser/ExprHandler/PropertyFetchHandler.php for non-constant string property names: when the name is a string type but not a constant string, query the var type's property reflection to determine the type
  • New regression test in tests/PHPStan/Analyser/nsrt/bug-7088.php

Root cause

In PropertyFetchHandler::resolveType(), when the property name expression is not an Identifier and not a constant string, the code immediately returned MixedType. For classes like SimpleXMLElement that have a PropertiesClassReflectionExtension accepting any property name, we can still determine the correct type by querying the property reflection with a representative property name.

Test

Added tests/PHPStan/Analyser/nsrt/bug-7088.php that verifies all three property access patterns on SimpleXMLElement resolve to (SimpleXMLElement|null):

  • $prop->foo (static name)
  • $prop->{'foo-bar'} (constant string expression)
  • $prop->{$x} (dynamic variable name)

Fixes phpstan/phpstan#7088

…turns mixed

- When accessing SimpleXMLElement properties via $prop->{$x} with a non-constant
  string name, the type was incorrectly resolved as mixed instead of SimpleXMLElement|null
- Added fallback in PropertyFetchHandler::resolveType() to query the var type's
  property reflection for non-constant string property names
- New regression test in tests/PHPStan/Analyser/nsrt/bug-7088.php
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about passing null as propertyName rather than a custom constant __phpstan_dynamic_property

Edit: The method in Type interface requires a string

@VincentLanglet VincentLanglet force-pushed the create-pull-request/patch-2l111d3 branch from 9e33578 to 721d265 Compare April 6, 2026 09:22
@phpstan phpstan deleted a comment from phpstan-bot Apr 6, 2026
@VincentLanglet
Copy link
Copy Markdown
Contributor

VincentLanglet commented Apr 6, 2026

While using a hardcoded constant __phpstan_dynamic_property is not the best, I don't see another easy way without BC break (and lot of changes) just to have hasInstanceProperty() method accepting NULL.

I think it's good enough but we might prefer ask validation on ondrej. WDYT ?

@VincentLanglet VincentLanglet self-assigned this Apr 6, 2026
@VincentLanglet VincentLanglet requested a review from staabm April 6, 2026 09:39
Address review feedback: the magic string '__phpstan_dynamic_property'
is now a private constant with a docblock explaining its purpose.
It serves as a representative property name for dynamic property access
resolution ($obj->{$expr}), allowing PropertiesClassReflectionExtensions
that accept any property name to return the correct type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@VincentLanglet VincentLanglet merged commit 85b4f24 into phpstan:2.1.x Apr 7, 2026
654 of 656 checks passed
@VincentLanglet VincentLanglet deleted the create-pull-request/patch-2l111d3 branch April 7, 2026 08:32
phpstan-bot added a commit to phpstan-bot/phpstan-src that referenced this pull request Apr 7, 2026
…es gives mixed type (phpstan#5406)

Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

3 participants