False positive detection of "unused variable" when assigned by reference.

[rooflack]

Hello!
I stumbled upon a weird "unused variable" warning that I feel is a false positive, so I thought I'd report this here.

First things first, the env I'm working on is phpcs v3.6.0 (stable), with phpcs-variable-analysis v 2.11.0.

The case is the following: I'm declaring a variable, that I conditionnallly assign to different array elements. The assignment is done by reference, because I actually need to manipulate those elements further down. Anyway. Long story short, if I do this I get an "Unused variable" warning message, even though the variable is used.

Here are some examples that all trigger the same message.

• The first one is close to the actual code I'm writing.
• The second one uses an else statement, just to make sure the variable will ever get assigned only once.
• For the third I tried to simplify as much as possible to get rid of potential external side effects.

function somefunc($arr) {
  $var = [];
  if (isset($arr['foo'])) {
    $var = &$arr['foo'];
  }
  elseif (isset($arr['bar'])) {
    $var = &$arr['bar'];
  }

  foreach ($var as $key => $value) {
    $var[$key . '_plus_one'] = $value + 1;
  }
  return $arr;
}

function somefunc($arr) {
  if (isset($arr['foo'])) {
    $var = &$arr['foo'];
  }
  elseif (isset($arr['bar'])) {
    $var = &$arr['bar'];
  }
  else {
    $var = [];
  }

  foreach ($var as $key => $value) {
    $var[$key . '_plus_one'] = $value + 1;
  }
  return $arr;
}

private function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default;
  if ($choice) {
    $var = &$arr1;
  }

  foreach ($var as $key => $value) {
    $var[$key . '_plus_one'] = $value + 1;
  }
}

And here are two pieces of code that do not trigger the message. They are both based on the last one above.

• The first one has the conditional assignment removed entirely.
• The second one has the conditional assignment done by value and not by reference.
  (Obviously, both of these don't produce the expected result when run, so they're not valid workarounds.)

private function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default;

  foreach ($var as $key => $value) {
    $var[$key . '_plus_one'] = $value + 1;
  }
}

private function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default;
  if ($choice) {
    $var = $arr1;
  }

  foreach ($var as $key => $value) {
    $var[$key . '_plus_one'] = $value + 1;
  }
}

So it looks to me like this is the conditional re-assignment that makes the message pop up. I don't know the standards very well so it's possible that this pattern goes against some rule I don't know of, but "unused variable" sounds pretty misleading to me.

[sirbrillig]

Thanks for the detailed report! That does sound like a bug.

I haven't looked at the detection logic yet, but here's what I think is going on. Here's an even simpler version of the above code:

function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default; // ✅ Unused variable $var. (VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable)
  $var = &$arr1;
  echo $var;
}

But as you said, if we remove the second assignment, the warning goes away. I think what's happening is that the warning is trying to tell you: that first assignment is not being used because there's a second assignment that overwrites it. That's probably a valid warning.

Then when you put that second assignment into a conditional block, that's when the bug manifests, since the condition might not be true.

function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default; // ❌ Unused variable $var. (VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable)

  if ($choice) { // this might not be true, so the above assignment might actually be used, and we must assume it's a valid assignment
    $var = &$arr1;
  }

  echo $var;
}

I suspect that we need to be a little more intelligent about that particular warning if there's a condition involved.

[rooflack]

Hi. Thanks for the very quick response!

I also thought the case you describe, with the double, unconditional assignment, might have had something to do with this.
But what's funny is that, if you remove the ampersand for "by reference" on the second assignment, the warning goes away, even though the first assignment is definitely useless (in the case of an unconditional second assignment, I mean).

Here's a little snippet for clarity:

function somefunc($choice, &$arr1, &$arr_default) {
  $var = &$arr_default;
  $var = $arr1; // Same as your example, but without the ampersand.
  echo $var;
}

The above does not trigger any warning, yet I feel like it should (at least, if the system is indeed trying to signal that the first assignment is not being used.

It may just be a different symptom of the same bug, I'm not sure.