Hooked properties cannot be both final and private #3830
Conversation
|
This pull request has been marked as ready for review. |
There was a problem hiding this comment.
Even non-hooked property cannot be final&private: https://3v4l.org/fAXFZ
Also please check the same error for methods and constants (it might already be implemented, not sure)
Also, please add an error that would report final properties on PHP < 8.4.
Also, please add error for final properties in interfaces.
Also, please add error for final hooks in abstract properties (in both class and interface): https://3v4l.org/hSO0C#v8.4.4
Also, abstract + final property together is not allowed.
|
memo to me: more cases to consider additional: follow discussion in php/php-src#17861 |
done
methods are already covered and tests exist for it constants are missing (separate PR? new rule similar to FinalPrivateMethodRule?)
done
should I add a new PropertyInInterfaceRule ?
done
php-src doesn't error on https://3v4l.org/2fSaq#v8.4.4 |
staabm
force-pushed
the
private-final
branch
from
321a6d9 to
52ddad6
Compare
There is already |
|
Reported bug to PHP php/php-src#17916 |
|
Please add an error for |
|
I think we are running now in a php-parser bug: nikic/PHP-Parser#1071. will look further |
|
It's fine if it's a parse error. It means we don't need to implement a custom rule to detect this invalid code. |
tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php
Outdated
Show resolved
Hide resolved
|
One more thing to report - final hook without a body in a class: https://phpstan.org/r/1d253f61-25a3-4654-8ef0-4f3c4268c770 |
I think thats already covered in https://github.com/phpstan/phpstan-src/pull/3830/files#diff-2678a88fc82b38e00cf1a626ec3234f0d0e78b202c1324abce258eadfc735e68R3-R10 |
clxmstaab
force-pushed
the
private-final
branch
from
e6e84f7 to
b1310f7
Compare
| return [ | ||
| RuleErrorBuilder::message('Property in interface cannot be explicitly abstract.') | ||
| ->nonIgnorable() | ||
| ->identifier('property.hookedStatic') |
There was a problem hiding this comment.
Still wrong? property.hookedStatic does not make sense for Property in interface cannot be explicitly abstract.?
| return [ | ||
| RuleErrorBuilder::message('Property in interface cannot be explicitly abstract.') | ||
| ->nonIgnorable() | ||
| ->identifier('property.hookedStatic') |
There was a problem hiding this comment.
Still wrong? property.hookedStatic does not make sense for Property in interface cannot be explicitly abstract.?
|
Perfect, thank you! |
|
Something you could do next: abstract + private (https://3v4l.org/es9gD) |
refs phpstan/phpstan#12336
see https://3v4l.org/5ZXOF