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

feat(lint): add useConsistentCurlyBraces #3182

Merged

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented Jun 11, 2024

Summary

This PR adds the useConsistentCurlyBraces lint. It doesn't implement any of the options from the eslint rule for simplicity.

This implementation aims to follow the recommendations outlined in the eslint rule: react/jsx-curly-brace-presence

closes #2435

Test Plan

Added snapshot tests

cargo test -p biome_js_analyze use_consistent_curly_braces

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Jun 11, 2024
Copy link

codspeed-hq bot commented Jun 11, 2024

CodSpeed Performance Report

Merging #3182 will degrade performances by 9.36%

Comparing dyc3:06-02-feat_lint_add_usejsxcurlybraceconvention_ (ec663b9) with dyc3:06-02-feat_lint_add_usejsxcurlybraceconvention_ (7890fbe)

Summary

❌ 7 regressions
✅ 101 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark dyc3:06-02-feat_lint_add_usejsxcurlybraceconvention_ dyc3:06-02-feat_lint_add_usejsxcurlybraceconvention_ Change
js_analyzer[css_557271668126329798.js] 15.3 ms 16.6 ms -7.45%
js_analyzer[index_129807605716630866.js] 32.9 ms 35.6 ms -7.53%
js_analyzer[lint_11654203871763747189.ts] 25.9 ms 28.4 ms -8.76%
js_analyzer[parser_8282555732666328882.ts] 53 ms 57.3 ms -7.45%
js_analyzer[router_14177007973872119684.ts] 15.3 ms 16.8 ms -9.06%
js_analyzer[statement_17851180443367440905.ts] 47.7 ms 52.6 ms -9.36%
js_analyzer[typescript_4309201908630379962.ts] 76.2 ms 84 ms -9.32%

@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch from d81ef73 to 39f6256 Compare June 14, 2024 15:11
@github-actions github-actions bot added the A-CLI Area: CLI label Jun 14, 2024
@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch 3 times, most recently from fe22002 to 5309e5f Compare June 15, 2024 13:10
@dyc3 dyc3 marked this pull request as ready for review June 15, 2024 13:11
@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch from 5309e5f to a28ee0f Compare June 22, 2024 22:19
@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch 3 times, most recently from fdcc6c3 to 962f7a4 Compare June 29, 2024 22:44
@dyc3 dyc3 requested a review from chansuke June 29, 2024 22:44
@chansuke
Copy link
Member

Great work! Thank you :)

@dyc3 dyc3 changed the title feat(lint): add useJsxCurlyBraceConvention feat(lint): add useConsistentCurlyBraces Jul 2, 2024
@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch 3 times, most recently from 7b46bf8 to 5d6bcf6 Compare July 5, 2024 11:05
@dyc3
Copy link
Contributor Author

dyc3 commented Jul 5, 2024

Hit a small blocker with this merge conflict. Usually other rules getting added causes these merge conflicts, and running just gen-lint usually fixes it, but this time it's throwing cannot find macro rule_category in this scope. Would the easiest fix be to just redo the codegen on a fresh branch?

@dyc3 dyc3 requested a review from ematipico July 5, 2024 11:12
@ematipico
Copy link
Member

@dyc3 declare_rule has been renamed declare_lint_rule. That will fix everything

@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch from 5d6bcf6 to fd07089 Compare July 5, 2024 11:59
@dyc3 dyc3 force-pushed the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch from fd07089 to 7890fbe Compare July 6, 2024 00:18
@ematipico ematipico merged commit 67579a1 into biomejs:main Jul 15, 2024
10 of 11 checks passed
@Geczy
Copy link

Geczy commented Aug 9, 2024

when does this get released to production?

404
https://biomejs.dev/linter/rules/use-jsx-curly-brace-convention

@ematipico
Copy link
Member

ematipico commented Aug 9, 2024

when does this get released to production?

404
https://biomejs.dev/linter/rules/use-jsx-curly-brace-convention

Probably beginning of September. You can use the nightly channel for early releases

@dyc3 dyc3 deleted the 06-02-feat_lint_add_usejsxcurlybraceconvention_ branch September 4, 2024 16:41
@Conaclos Conaclos added the A-Changelog Area: changelog label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Implement lint/useJsxCurlyBraceConvention - react/jsx-curly-brace-presence
5 participants