Readonly properties cannot be unset() #3827
Conversation
|
This pull request has been marked as ready for review. |
staabm
force-pushed
the
unset-readonly
branch
2 times, most recently
from
3ee6c73 to
06e46c2
Compare
staabm
force-pushed
the
unset-readonly
branch
from
06e46c2 to
b390edc
Compare
staabm
force-pushed
the
unset-readonly
branch
from
b390edc to
aa238e2
Compare
|
adjusted per feedback. thank you. |
| } elseif ( | ||
| $node instanceof Node\Expr\PropertyFetch | ||
| && $node->name instanceof Node\Identifier | ||
| ) { |
There was a problem hiding this comment.
We have better ways of getting a property reflection from PropertyFetch. Two actually:
- PropertyReflectionFinder. FoundPropertyReflection will even tell you if it's a native one or not.
Scope::getPropertyReflection()
There was a problem hiding this comment.
ahh nice.
I was already searching how to know about property hooks so I can implement the error for unsetting a hooked property.
will be a followup PR
There was a problem hiding this comment.
I wrote about property hooks here phpstan/phpstan#12337
src/Rules/Variables/UnsetRule.php
Outdated
| } | ||
|
|
||
| if ($propertyReflection->isReadOnly() || $propertyReflection->isReadOnlyByPhpDoc()) { | ||
| $type = $scope->getType($node->var); |
There was a problem hiding this comment.
You don't need to do this, just ask for property declaringClass getDisplayName.
src/Rules/Variables/UnsetRule.php
Outdated
|
|
||
| return RuleErrorBuilder::message( | ||
| sprintf( | ||
| 'Cannot unset %s property %s of %s.', |
There was a problem hiding this comment.
Usually we do Foo::$name format.
%s of %s sounds like an ancient text: Yennefer of Vengerberg 😀
clxmstaab
force-pushed
the
unset-readonly
branch
from
31eb5d6 to
71ff1f6
Compare
|
Perfect, thank you! |
closes phpstan/phpstan#12421