Suppress undefined static property error when property_exists() guard is present#5544
Conversation
096ab28 to
6b1a093
Compare
VincentLanglet
left a comment
There was a problem hiding this comment.
Does the same issue exists for method_exists and/or for non-static property/method ?
|
Both Summary: The reviewer asked whether the same issue exists for
The bug was unique to |
|
|
||
| class Foo { | ||
| public static function test(): void { | ||
| if (property_exists(static::class, 'default')) { |
There was a problem hiding this comment.
we should also test non-static property fetches, like in https://phpstan.org/r/20ab67ef-ef8b-4f24-8783-1ff18cfe7168
There was a problem hiding this comment.
Done. Added non-static (instance) property fetch test cases as requested by staabm:
bug-2861.php: AddedInstancePropertyAccessclass with$this->defaultand$obj->defaultreads guarded byproperty_exists()bug-2861-assign.php: AddedtestInstance()andtestInstanceObj()methods with$this->default = 'value'and$obj->default = 'value'writes guarded byproperty_exists()AccessPropertiesRuleTest: AddedtestBug2861()to verify instance property reads don't trigger errorsAccessPropertiesInAssignRuleTest: AddedtestBug2861()to verify instance property writes don't trigger errors
All 12131 tests pass and make phpstan reports no errors.
| ); | ||
| } | ||
|
|
||
| if (!$objectType->isObject()->yes()) { |
There was a problem hiding this comment.
this if-statement is not covered by a test
There was a problem hiding this comment.
Pushed successfully. Here's a summary of what was done:
Addressing staabm's review comments:
-
Non-static property fetch tests — The remote already had concurrent changes adding
InstancePropertyAccessclass tests (with$thisand$objpatterns) to bothbug-2861.phpandbug-2861-assign.php. I resolved the rebase conflicts keeping those additions. -
Uncovered
!$objectType->isObject()->yes()branch — Addedtests/PHPStan/Analyser/nsrt/bug-2861.phpwithassertTypetests that exercise theobject|stringnarrowing path inPropertyExistsTypeSpecifyingExtension. This covers the branch where the first argument toproperty_exists()is neither purely string nor purely object, verifying it narrows toclass-string|(object&hasProperty(foo)).
…rd is present - Handle class-string first argument in `PropertyExistsTypeSpecifyingExtension` by marking the `property_exists()` FuncCall expression as `true` in scope, instead of trying to intersect `HasPropertyType` with the class-string type (which produces `never` due to ObjectTypeTrait incompatibility) - Add a virtual `property_exists()` check in `AccessStaticPropertiesCheck` before reporting "Access to an undefined static property" — constructs a `property_exists(ClassName::class, 'propName')` FuncCall and checks if the scope evaluates it as `true` - Covers `static::$prop`, `self::$prop`, `ClassName::$prop`, and expression-based `$className::$prop` access patterns - Also works for the assign context via `AccessStaticPropertiesInAssignRule` which shares the same `AccessStaticPropertiesCheck` - Updated phpstan-baseline.neon to reflect one fewer `instanceof ConstantStringType`
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…owing - Add instance property access tests (property_exists($this, 'default')) to bug-2861.php and bug-2861-assign.php test data files - Add corresponding test methods in AccessPropertiesRuleTest and AccessPropertiesInAssignRuleTest - Add nsrt test covering the !$objectType->isObject()->yes() branch in PropertyExistsTypeSpecifyingExtension for object|string first argument Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7155dbd to
42ffe26
Compare
|
hmm somehow the error
did not got fixed with this PR |
Do you mind opening an issue. I think it's because we have a |
Summary
When
property_exists()is called with a class-string first argument (e.g.property_exists(static::class, 'default')), the subsequent static property accessstatic::$defaultwas reported as "Access to an undefined static property" even though the existence check guarded it. This fix teaches PHPStan to recognize that guard.Changes
src/Type/Php/PropertyExistsTypeSpecifyingExtension.php: When the first argument toproperty_exists()is a string type (class-string or constant class name), mark theproperty_exists()FuncCall expression itself asConstantBooleanType(true)in scope. Previously, class-string arguments caused the extension to return emptySpecifiedTypes, providing no narrowing. The approach of intersectingHasPropertyTypewith class-string was not viable becauseHasPropertyTypeusesObjectTypeTrait, making it incompatible with string types inTypeCombinator::intersect()(producingnever). This matches the approach already used for non-constant property names.src/Rules/Properties/AccessStaticPropertiesCheck.php: Before reporting "Access to an undefined static property", construct a virtualproperty_exists(ClassName::class, 'propName')FuncCall and check if the scope evaluates it astrue. ForName-based class references (static,self,Foo), constructsClassConstFetch(Name, 'class')as the first argument. For expression-based references ($className::$prop), uses the expression directly.phpstan-baseline.neon: Updated expected count forinstanceof ConstantStringTypeinPropertyExistsTypeSpecifyingExtension.phpfrom 2 to 1 (removed the old$objectType instanceof ConstantStringTypecheck).Analogous cases probed
AccessStaticPropertiesInAssignRule): Uses the sameAccessStaticPropertiesCheck, so automatically covered. Added regression test.$className::$propafterproperty_exists($className, 'prop')): Works because the expression key matches. Added test case.$this->propafterproperty_exists($this, 'prop')): Already handled by existing code inAccessPropertiesCheck(lines 201-209 for dynamic names, andHasPropertyTypenarrowing for static names).HasMethodTypevsHasPropertyTypewith class-string: Investigated whymethod_exists()correctly narrows class-string types whileproperty_exists()didn't —HasMethodType::isSuperTypeOfcheckshasMethod()which class-strings support (for static method calls), butHasPropertyType::isSuperTypeOfcheckshasInstanceProperty()/hasStaticProperty()which class-strings don't support. This is why the FuncCall-marking approach was chosen over type intersection.Root cause
PropertyExistsTypeSpecifyingExtensiondid not handle class-string first arguments. When the first argument was aConstantStringType(which includes class-string types), the extension returned emptySpecifiedTypes, so no type narrowing occurred. Meanwhile,AccessStaticPropertiesCheckhad no mechanism to detect that aproperty_exists()guard was present before reporting undefined static properties — unlikeAccessPropertiesCheckwhich already had a virtualproperty_exists()check for dynamic instance property names.Test
tests/PHPStan/Rules/Properties/data/bug-2861.php— Regression test with trait usingproperty_exists(static::class, 'default')andproperty_exists(self::class, 'default')guards, plus expression-based$className::$propaccess. Expects no errors.tests/PHPStan/Rules/Properties/data/bug-2861-assign.php— Assignment context regression test withstatic::$default = 'value'afterproperty_exists()guard. Expects no errors.Fixes phpstan/phpstan#2861