-
-
Notifications
You must be signed in to change notification settings - Fork 475
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
fix(lint/noUnreachable): correctly analyze nested try-finally
statements
#1856
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CodSpeed Performance ReportMerging #1856 will not alter performanceFalling back to comparing Summary
|
Thanks for your contribution! We should also evaluate if other rules that use the control flow graph need to be updated. |
Hi @Conaclos Thank you for the review! The fix is related to finally_fallthrough, and upon searching, I only found the I am not confident about the fix, could you please help check the PR description and see if the fix is heading in the right direction? Thanks! |
a804c2f
to
babc8ee
Compare
Sorry I missed your answer. Looking at the fix and the repro, this looks good to me :) |
babc8ee
to
e2811f7
Compare
finally
clause for nested try statementstry-finally
statements
try-finally
statementstry-finally
statements
Thanks! |
Summary
Closes #1827
The
noUnreachable
rule offers two approaches for analysisbiome/crates/biome_js_analyze/src/analyzers/correctness/no_unreachable.rs
Lines 69 to 73 in 5394928
The reported bug is caused by nested try statements, but it only occurs with
analyze_simple
.Let's consider the following example (assuming the analysis is driven by
analyze_simple
)mayThrow()
function throws an exception, then block 1 jumps to block 3mayThrow()
function doesn't throw an exception, then block 1 jumps to block 2.In the current implementation, we only handle the first scenario, but not the second. So we need to handle the second scenario.
Test Plan
Add a new test case