Implement ProtectedMethodInFinalClassRule #68
Conversation
| { | ||
| } | ||
|
|
||
| protected function protectedMethod() |
There was a problem hiding this comment.
seems like there is a ECS style rule which reduces the visibility of the method and therefore destroys the test-case :)
There was a problem hiding this comment.
skipped with 4814f0e
|
I think this one is good to go. |
|
I think we could the same for final methods in non-final classes.. same rule? |
|
@TomasVotruba wdyt? |
|
Thinking again about it. Maybe it should even be a rector to reduce visibility instead? |
|
Hi, I just got in here, thank you for your patience. While this is useful rule, I think it's difference scope. Also as you said, already handled in Rector in "privatization" set that makes sure the visiblity of class elements is as low as possible. This package focuses only on reporting public class elements, nothing more. Also potential for own package, but I don't think it's worth as already handled in Rector/ECS :) |
protected methods in final classes are not very useful. private methods can be analyzed easier and phpstan covers more cases for them.
with this PR, we report a error for these unnecessary protected methods, because when these are reduced to private visibility e.g. phpstan private symbol analytics kicks in and might detect unused private methods.
the same is true for protected properties, and I plan to contribute another rule, in case we agree on this one.