Phpinspectionsea: Precedence issues: new pattern

Created on 24 Jul 2017  路  5Comments  路  Source: kalessil/phpinspectionsea

If you write this code:

if ($value1 = function1()) {
  if ($value2 = function2($value1)) {
    code();
  }
}

EA suggests merging it into:

if ($value1 = function1() && $value2 = function2($value1)) {
  code();
}

But this is apparently not correct, because (as of 7.1.*) PHP will throw an undefined variable notice if you refer to a variable defined earlier in the same condition.

enhancement fixed

Most helpful comment

Update: never mind; the bug in the second block is that && binds more strongly than =, so without parentheses, it's interpreted as

$value1 = (
  function1() && $value2 = function2($value1)
);

rather than the intended form.

(Maybe this would be a useful case where to warn about operator precedence (like with $a && $b || $c).)

All 5 comments

I can't found this error (https://3v4l.org/QHCoB):

error_reporting(E_ALL);

function function1() { return 'Base'; }
function function2($n) { return $n . '-Modified'; }
function code($n) { echo $n . "\n"; }

if ($value1 = function1()) {
  if ($value2 = function2($value1)) {
    code($value2);
  }
}

if ($value1 = function1() && $value2 = function2($value1)) {
  code($value2);
}

Both are working. What error you are receiving, and what is the exact version of your PHP?

Update: never mind; the bug in the second block is that && binds more strongly than =, so without parentheses, it's interpreted as

$value1 = (
  function1() && $value2 = function2($value1)
);

rather than the intended form.

(Maybe this would be a useful case where to warn about operator precedence (like with $a && $b || $c).)

I think that it should be fixed by QF by putting the parentheses in this case, but I liked you suggestion about warning about precedences in cases like that. Can you open a new issue about that?

Reporting the precedence here totally makes sense, picking up as it seems to be a minor change.

Done. I'll also move the priorities check into suspicious binary operations inspection.

Was this page helpful?
0 / 5 - 0 ratings