Fix #12280: Analysis depends on order of union type#5098
Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Closed
Fix #12280: Analysis depends on order of union type#5098phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- UnionTypePropertyReflection's isPublic()/isPrivate() use AND semantics, returning false when union members have different visibilities - canAccessClassMember then fell through to the "protected" branch with an order-dependent getDeclaringClass(), causing false positives - Fix canReadProperty/canWriteProperty in MutatingScope to check each inner property individually when given a UnionTypePropertyReflection - Added getProperties() getter to UnionTypePropertyReflection - New regression test in tests/PHPStan/Rules/Properties/data/bug-12280.php Closes phpstan/phpstan#12280
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When accessing a property on a union type where the members have different visibilities (e.g.,
A|BwhereA::$dateis public andB::$dateis private), PHPStan incorrectly reported "Access to protected property" depending on the order of types in the union.Changes
getProperties()getter tosrc/Reflection/Type/UnionTypePropertyReflection.phpto expose inner property reflectionscanReadProperty()andcanWriteProperty()insrc/Analyser/MutatingScope.phpto check each inner property's accessibility individually when given aUnionTypePropertyReflectiontests/PHPStan/Rules/Properties/data/bug-12280.phpandtests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.phpRoot cause
UnionTypePropertyReflection::isPublic()uses AND semantics across all inner properties — it returnstrueonly if ALL inner properties are public. Similarly forisPrivate(). When one property is public and another is private, both returnfalse, causingcanAccessClassMember()to fall through to the "protected" branch.Additionally,
getDeclaringClass()returns only the first property's declaring class, making the protected-access check order-dependent: withA|self, A's declaring class was checked (not related to current class B → error); withself|A, B's declaring class was checked (matches current scope → no error).The fix checks each inner property individually through
canReadProperty/canWriteProperty, which correctly handles mixed visibilities —A::$datepasses as public, andB::$datepasses as accessible from within B's scope.Test
Added
tests/PHPStan/Rules/Properties/data/bug-12280.phpwith two test methods:test1usesA|selforder andtest2usesself|Aorder, both accessing a property whereA::$dateis public andB::$dateis private from within B's static method. Both should produce no errors.Fixes phpstan/phpstan#12280