sirbrillig / phpcs-variable-analysis

Find undefined and unused variables with the PHP Codesniffer static analysis tool.
Other
136 stars 14 forks source link

False-positive `UndefinedVariable` for static variable inside an anonymous function #277

Closed shvlv closed 2 years ago

shvlv commented 2 years ago

I stumbled upon the behavior when using the following code:

add_action('hook', static function (): void {
            static $providerId;
        });

It causes Variable $providerId is undefined. (VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable)

For my tests the minimum reproducible snippet was

$test = (function() {
            static $test;
        });

It seems round brackets do matter.

Thanks for maintaining a very useful library!

sirbrillig commented 2 years ago

It looks like this is happening because the code that detects if a variable is inside a list of function call arguments is incorrectly finding the static variable.

https://github.com/sirbrillig/phpcs-variable-analysis/blob/9f703905f61c2e75957af4daffc097f881116b0d/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php#L1337

sirbrillig commented 2 years ago

This should be fixed by https://github.com/sirbrillig/phpcs-variable-analysis/pull/279

However, I wanted to ask, @shvlv: in your examples, would you expect those static variables to produce no warnings at all?

For example,

add_action('hook', static function (): void {
            static $providerId;
        });

In this code, $providerId is never used, only declared, so I think this sniff should mark it as unused (but not undefined). Is there a reason why that would be wrong? I wanted to check before I closed this issue in case I'm mistaken.

shvlv commented 2 years ago

Thanks, @sirbrillig!

In this code, $providerId is never used, only declared, so I think this sniff should mark it as unused (but not undefined). Is there a reason why that would be wrong? I wanted to check before I closed this issue in case I'm mistaken.

Yes, there was a minimum reproducible example. So Unused warning would be pretty nice.

This should be fixed by #279

I've tested your code. It works nicely for $test = (function() { static $test;}); but snippet with add_action still triggers UndefinedVariable:

add_action('test', function () {
    static $providerId;
});
sirbrillig commented 2 years ago

Ohhh... thanks. I bet it's because in that case, $providerId is, lexically, inside of a function call's arguments, except that it's inside a closure and we don't account for that. I can fix it.