Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False-positive for "unused" variable in try-catch #7613

Closed
zerkms opened this issue Feb 9, 2022 · 5 comments
Closed

False-positive for "unused" variable in try-catch #7613

zerkms opened this issue Feb 9, 2022 · 5 comments

Comments

@zerkms
Copy link
Contributor

zerkms commented Feb 9, 2022

https://psalm.dev/r/585b7b7e67

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/585b7b7e67
<?php

/**
 * @throws Exception
 */
function mayThrow(): void {}

$step = 'zero';

try {
    $step = 'one';
    mayThrow();
    
    $step = 'two';
    mayThrow();
} catch (Throwable) {
    echo $step;
}
Psalm output (using commit 9d8b6d6):

INFO: UnusedVariable - 11:5 - $step is never referenced or the value is not used

@weirdan
Copy link
Collaborator

weirdan commented Feb 9, 2022

I would expect $step = 'zero'; to be marked as unused.

@zerkms
Copy link
Contributor Author

zerkms commented Feb 9, 2022

That too: without zero $step is thought to be possibly undefined.

@AndrolGenhald
Copy link
Collaborator

AndrolGenhald commented Feb 9, 2022

I would expect $step = 'zero'; to be marked as unused.

I think Psalm just assumes it's possible that an exception is thrown immediately upon entering a try block, and makes no attempt to understand that $step = 'one' will always happen.

I might take a look at this tomorrow if no one else grabs it, I'm hoping it won't be too hard, but we'll see. I'm not sure if we still have a way to get at the $step = 'one' node to mark it as used by the time we analyze the catch. If we do it should be simple enough to mark every assignment in the try as used when a variable is referenced in the catch or finally.

AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Feb 13, 2022
@AndrolGenhald
Copy link
Collaborator

Took me longer than I'd hoped to get around to it, but I managed to find an alternate fix.

@orklah orklah closed this as completed in 7b1599d Feb 13, 2022
orklah added a commit that referenced this issue Feb 13, 2022
…le-in-try

Fix false positive for unused variable in try (fixes #7613).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants