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(lint): don't crash on malformed JSX #4939

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Jan 22, 2025

Summary

Fixes #4623 and resolves a TODO in cursor/node.rs.

The real fix is in no_comment_text.rs, where I replaced ctx.query().range().start() with ctx.query().value_token().ok()?.text_range().start(). The reason this works is that ctx.query().range() can return a range with a higher starting offset than ctx.query().value_token().ok()?.text_range(), due to trimming in the former. I am not really happy with this fix, since it seems like a very easy mistake to make and there's no type safety to protect against this. It's also not really intuitive, so I'm open to better suggestions.

Test Plan

Test case added.

@arendjr arendjr requested review from a team January 22, 2025 12:11
@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Jan 22, 2025
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link

codspeed-hq bot commented Jan 22, 2025

CodSpeed Performance Report

Merging #4939 will not alter performance

Comparing arendjr:fix-issue-4623 (55e106f) with next (c10068b)

Summary

✅ 95 untouched benchmarks

@arendjr arendjr merged commit 7dc32e6 into biomejs:next Jan 22, 2025
12 checks passed
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-Linter Area: linter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants