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

fix(core): don't build control flow graph inside bogus nodes #4963

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Jan 24, 2025

Summary

Fixes #4723 by adding a BogusVisitor to our control flow graph builder. The original panic was caused by a broken assumption due to the presence of a bogus statement between the if statement and its else clause. I think the safer alternative is to just skip bogus branches from analysis.

To aid with this, codegen now generate an Any*BogusNode for every language as well.

Test Plan

Added test case.

@arendjr arendjr requested review from a team January 24, 2025 13:49
@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages L-HTML Language: HTML L-Grit Language: GritQL labels Jan 24, 2025
@arendjr arendjr changed the title fix(core): don't build control flow graph from bogus nodes fix(core): don't build control flow graph inside bogus nodes Jan 24, 2025
Copy link

codspeed-hq bot commented Jan 24, 2025

CodSpeed Performance Report

Merging #4963 will not alter performance

Comparing arendjr:fix-bogus-analysis (a251c59) with next (ba26e90)

Summary

✅ 95 untouched benchmarks

@arendjr arendjr merged commit 44d3f81 into biomejs:next Jan 24, 2025
13 checks passed
unvalley pushed a commit to unvalley/biome that referenced this pull request Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Core Area: core A-Formatter Area: formatter A-Linter Area: linter A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS L-Grit Language: GritQL L-HTML Language: HTML L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant