Detect override of deprecated property #90
Conversation
There was a problem hiding this comment.
- We shouldn't report this for private properties that are marked as
@deprecated. - We should also have the same rule for methods and constants.
- These new rules have to be registered in bleedingEdge only. Example here: https://github.com/phpstan/phpstan-doctrine/blob/62bd362b432fe29e175168689510ddd927b698f8/rules.neon#L27-L29
Khartir
force-pushed
the
overridden-property
branch
from
a5d3011
to
7ba6d2f
Compare
Khartir
force-pushed
the
overridden-property
branch
from
7ba6d2f
to
2226d83
Compare
|
|
||
| $parents = $class->getParents(); | ||
|
|
||
| $name = (string) $node->consts[0]->name; |
There was a problem hiding this comment.
You should go through all the constants, not just the first one.
|
|
||
| $class = $scope->getClassReflection(); | ||
|
|
||
| $parents = $class->getParents(); |
There was a problem hiding this comment.
You shouldn't go through all the parents. This would break the @deprecated / @not-deprecated behaviour. Please read about it and write a test: https://phpstan.org/writing-php-code/phpdocs-basics#deprecations
It's sufficient to ask for the property in the first parent class if there's one.
You should also look into interfaces.
|
|
||
| $class = $scope->getClassReflection(); | ||
|
|
||
| $parents = $class->getParents(); |
| return []; | ||
| } | ||
|
|
||
| return [sprintf( |
There was a problem hiding this comment.
Please use the more modern approach with RuleErrorBuilder instead of just returning a string.
Khartir
force-pushed
the
overridden-property
branch
from
2226d83
to
99cadb0
Compare
|
Hi, I can't review this because you need to rebase your commits on top of 1.1.x. Your branch includes commits from 1.2.x which is unwanted. |
Khartir
force-pushed
the
overridden-property
branch
from
5ec6592
to
daf5f98
Compare
|
Sorry about that, after it took me so long to get back to this, I just updated to the most recent branch. |
Khartir
force-pushed
the
overridden-property
branch
from
daf5f98
to
b924bc4
Compare
Closes #82