kalessil
Declaration access can be weaker: new pattern
Description (screenshot):
if the method is used in the class only (and in no interfaces) — warn that it should be private. If it's used in child only — warn that it should be protected
Example code:
<?php
/**
* Class test
*/
class TestMe
{
protected $x; // this property could be private
private function test1(): void {}
protected function test2(): void {} // this method could be private
public function doit(): void
{
$this->x;
$this->test1();
$this->test2();
}
}
I think that is very hard to predict that this class will never been extended by another class in some moment. Both by the own dev or by a third. So make $x private because of the current context will limit the usage of this property in a possible future child class.
I think that is very hard to predict that this class will never been extended by another class in some moment. Both by the own dev or by a third.
It is, but for me, the point of this software is not to predict what will be, but to work with the given situation. This tool shouldn't be used blindly, there has to be a developper who will have to judge whether a suggestion is valid or not.
On the other hand, for legacy project who need refactor, this is a HUGE help. Private methods are so much easier to change, if there is a good way to detect a method can be private, we should tend to it.
@rentalhost why we should use protected properties? We can use methods instead of properties. IMHO Properties should always be private =)
@funivan Well, imo, that really depends on use-case. Sure we can use methods instead of properties, but if it's something that is not being publicly exposed anyway, I prefer accessing the property directly.
Cases, where protected functions are part of the customization concept, e.g.:
class HelloBeforeAggegationAndCompositionTimes {
public function workflow() {
$this->step1();
...
$this->stepN();
}
protected function step1() {}
protected function stepN() {}
}
Sure but there are approaches where inheritance isn't wanted at all and composition is always preferred. For these cases such inspection would be helpful.
Ok, then I can deliver this as an optional behavior with setting to enable it.
For myself: public -> protected/private methods (in BC-friendly scenario) - containing class is not a trait nor interface - containing class extends a class or implements no-core interface - is not override nor implementation of a parent method - is not overridden in a child
Is this about LowerAccessLevelInspection / "[EA] Since the class is final, the member can be declared private." / "PHP Inspections > Architecture > Declaration access can be weaker" ?
I ran into an issue today, following the suggestion, making a protected method private. It turned out to be used from the parent class in a is_callable([$this, $method]) type of way. Making it private made it uncallable for the parent class' method.
