php:S1481; False Positive for unused local variables when references are used

Rules and Languages Report False-positive / False-negative...
1

Hi, we are currently running a self-hosted SonarQube for our PHP 8.3 project but receive some issues when using function calls in combination with variables passend by reference to functions. In these cases SonarLint and SonarQube shows an issue for the inner variable “php:S1481: Unused local variables should be removed” although it is used in the following code.

  • Language: PHP 8.3
  • Rule: php:S1481 “Unused local variables should be removed”
  • SonarQube Developer Edition v10.7

Example with 1 issue:
Line 7: Remove this unused "$someVariable" local variable.

class SomeClassWithAnIssue
{
    public function doSomething()
    {
        $someVariable = 'some value';

        $this->task("Reading File", function () use (&$someVariable) {
            $someVariable = 'some other value';
        });

        $this->task("Some task name", function () use ($someVariable) {
            $this->doSomethingMore($someVariable);
        });
    }

    private function doSomethingMore($someVariable)
    {
        echo $someVariable . "\n";
    }
}

The variable $someVariable is used in the following and should not trigger an issue. The issue arises when the variable is passed to another function like above, and does not occure, when it is used like in the following example:

Example with no issues:

class SomeClassWithoutAnyIssues
{
    public function doSomething()
    {
        $someVariable = 'some value';

        $this->task("Reading File", function () use (&$someVariable) {
            $someVariable = 'some other value';
        });

        $this->doSomethingMore($someVariable);
    }

    private function doSomethingMore($someVariable)
    {
        echo $someVariable . "\n";
    }
}

Therefore i think this is an false-positive (or Bug) in the detection.

Thanks.

4

Hi Jannik,

First of all, thank you very much for the quality of your feedback and for providing some examples!

You’re right; this is indeed a false positive. As you probably know, closures have their own scopes, and this is an edge case we don’t handle:

  • In the first example, S1481 raises only on line 7 because it has no read usage in the closure or directly in the above scope (i.e., the scope of the doSomething method). The read usage is nested in the second closure scope, and we currently don’t detect it. We detect it from the function scope, though, which is why it doesn’t raise on line 5.

  • In the second example, it doesn’t raise on line 7 because there is a direct read usage in the doSomething scope on line 11, and we properly detect this case.

I created a ticket to improve our detection logic for this rule; you can track the progress here: Jira

If you find any other false positive, please let us know! Thanks for your investment in our products :smile: