undefined variable errors after isset checks

[kkmuffme]

if ( !isset( $hello ) ) {
    return;
}
echo $hello;

results in an undefined variable error on both lines, but it should only give an error on the first line, since if the variable passes this check it is in fact not undefined.
Afair this was working correctly in earlier versions

[sirbrillig]

That's a good point. There's actually a few things to consider here.

Technically, when examining the code as-is without knowledge of the special meaning of isset (for example if it was some other function), both uses are undefined. However, isset (like empty) is a language feature that operates safely on undefined variables. So it's fair to not warn on the first use. This was reported first in #202 and was resolved by #204 There's tests to cover that case and I just experimented by running the above example through phpcs and I do not get a warning for the first line. Are you using 2.10.2?

The second use, however, is tricky, because what if the code read like this?

if ( ! isset( $hello ) ) {
    echo 'it is not set';
}
echo $hello;

In that case, the second use is sometimes undefined. We'd need to be able to recognize not just the isset, but also the return. Let's say we could do that, what about this case?

if ( ! isset( $hello ) ) {
    if ( somethingElse() ) {
      return;
    }
}
echo $hello;

How do we know if the second use of $hello is undefined in that case? Here's another one.

if ( ! isset( $hello ) ) {
  maybeThrowException(); // sometimes throws if other conditions are right
}
echo $hello;

It's difficult to determine if further uses of a variable are defined based on one call to isset or empty, so the sniff marks them all as undefined.

This is a little like how TypeScript requires that variables of type unknown pass through an explicit type guard before you can use them. You could do something similar with this sniff, if desired (I'm not saying this is a great pattern, just that it would work).

if ( ! isset( $hello ) ) {
    return;
}
$helloInput = $hello; // phpcs:ignore VariableAnalysis
// none of the following will generate a warning
echo $helloInput;
doSomethingWith($helloInput);
return $helloInput;

I think that to properly mark the second use as defined we'd need a fairly full-featured static analysis system to figure out if it is actually safe or not, unless you can think of a way to make that decision for the above examples.

[kkmuffme]

Good point, you are right.
I guess this can be closed, as it's out of scope of this plugin, which works great in many ways but it doesn't need to be able to do everything.