Phpinspectionsea: False-positive for immediate variable override

[zerkms]

| Subject | Details |
| :------------- | :---------------------------------------------------------------------------- |
| Plugin | Php Inspections (EA Extended) v3.0.13 |
| Language level | PHP 7.2 |

Current behaviour

function a(): void
{
    try {
        $v = 0;
        $v = t();
    } finally {
        var_dump($v);
    }
}

function t(): int
{
    throw new \Exception('catch me if you can');
}

a();

It marks the second $v = t(); assignment as "is immediately overridden".

Expected behaviour

Ideally, it should not

Environment details

N/A

[funivan]

Can you explain why not? Why we need this line in the code $v=0?

[zerkms]

@funivan run that code with and without $v = 0;

[funivan]

Oh, I got it.
But you can rewrite tour code to this:

try {
  $v = t();
} finally {
  var_dump(0);
}

Behavior will be the same. Or i`m wrong?

[zerkms]

@funivan

the whole example can be rewritten to

var_dump(0);
throw new \Exception('catch me if you can');

My report here is not about "please help me rewrite that piece of code", it's about this very detection that produces a false positive.

[funivan]

Analyzing exception inside functions is not so easy. If t is a simple function without any nested function calls then this is probably possible. But if t contains another function call then we need to analyze each nested function to check if it can throw an exception. Maybe kalessil can do this)

IMHO: this is a bug, but do not know if it is fixable ツ.

[zerkms]

Yep, I'd say without checked exceptions it's impossible to determine reliably.

It looks in general if there is a function call on the next line - it may throw. Not sure about operations.

Can operators throw in php? What other types of expressions other than operators and function calls can throw?

Possibly - everything if one converts warnings and notices to exceptions.

[funivan]

We can get type error exception in php 7.2 =)

[zerkms]

Not by default for everything https://3v4l.org/rkp7g

[kalessil]

Looks like a false-positive in this case.

[zerkms]

Yep, and after thinking on it for some time now I agree with @funivan that in general one simply should just avoid writing code like that (it may be not always trivial but still)

[kalessil]

Yep try should contain only statements which you want to guard, so the initial zero assignment ideally should be outside of the try-statement.

[zerkms]

The problem is that irl the initialisation is also dynamic and may throw :-D

[kalessil]

I suspected it'll be not so easy ;)

[kalessil]

Fixed!

[zerkms]

@kalessil