-
-
Notifications
You must be signed in to change notification settings - Fork 724
feat(linter): implement fixer for jsx_curly_brace_presence #13005
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
feat(linter): implement fixer for jsx_curly_brace_presence #13005
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
47cb5e5 to
6f152d0
Compare
CodSpeed Instrumentation Performance ReportMerging #13005 will not alter performanceComparing Summary
Footnotes |
|
thanks for this, the direction looks good, but CI is failing. |
Didn't setup clippy on my dev env yet. Will rebase and fix failing CI tonight. |
6f152d0 to
2be0314
Compare
2be0314 to
2d6b909
Compare
camc314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, it's great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a fixer for the jsx_curly_brace_presence ESLint rule, enabling automatic correction of curly brace issues in JSX. The implementation adds comprehensive fix suggestions for both unnecessary and missing curly braces scenarios.
Key changes:
- Added fixer functionality to automatically correct curly brace presence issues
- Implemented complex text node handling for multiline content with HTML entities
- Enhanced diagnostic messages with helpful fix suggestions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
crates/oxc_linter/src/rules/react/jsx_curly_brace_presence.rs |
Main implementation adding fixer logic, helper functions for text processing, and comprehensive test cases |
crates/oxc_linter/src/snapshots/react_jsx_curly_brace_presence.snap |
Updated snapshot with new help messages from the fixer implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| build_missing_curly_fix_context_for_part(span, string_value, 0) | ||
| .iter() | ||
| .copied() | ||
| .collect::<std::vec::Vec<_>>() |
Copilot
AI
Aug 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using std::vec::Vec instead of oxc_allocator::Vec may not be optimal in this codebase. Consider using the project's allocator-based Vec for consistency and potential performance benefits.
| .collect::<std::vec::Vec<_>>() | |
| string_value.match_indices('\n').map(|(i, _)| i).collect::<Vec<_>>(); | |
| let fix_contexts = if line_matches.is_empty() { | |
| build_missing_curly_fix_context_for_part(span, string_value, 0) | |
| .iter() | |
| .copied() | |
| .collect::<Vec<_>>() |
This is my first rust code, so an expert review might be appreciated. Especially the
report_missing_curly_for_text_nodemethod could use a good look. I am not sure I made the right choices when it comes to performance as I miss a lot of the nuance rust offers.