What's the logic behind php:S1117?

There’s this rule about shadowing class members that to me doesn’t really make much sense in php.

In php, there’s no automatic scoping like in js or other languages; variables are always local and members must be accessed with $this, so it’s not easy to confuse the variable like the explanation says (you can’t be using the wrong variable).

The rule is so strict it wouldn’t even let me do a local copy of the variable to avoid writing $this-> all the time.

class SonarTest
{
    protected $user;

    function printUser(){
        $user = $this->user; //triggers warning

       //lots of logic using the value of $user
        echo $user;
    }
}

the most common variable shadowing I can think of in php is foreach variables.

this code doesn’t trigger the shadowing warning in the foreach (it happened to me several times, with different variable names; it’s also easy to make this error with nested loops)
it also doesn’t trigger the useless variable assignment php:S1854 on the first $i (but phpstorm correctly identifies it as useless)

    function testSonar(){
        
        $i = 4;
        
        foreach(range(1, 10) as $i){
            SomeClass::doSomething($i);
        }
        
        echo $i;
        
    }

Hi @pablo.videoslots,

Thanks for raising your concerns about the value of this rule for PHP. In fact, in other languages like Java, this rule is more meaningful and valuable. We will discuss in the team how to adapt the logic of the rule so that it is no longer so noisy in the PHP context.

Best,
Nils

1 Like