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(useSortedClasses): should suggest code fixes that match the JSX quote style of the formatter #4857

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

lucasweng
Copy link
Contributor

@lucasweng lucasweng commented Jan 8, 2025

Summary

  • Adds a preferred_jsx_quote to the analyzer. It is used only inside the JavaScript analyzer to ensure that code fixes match the JSX quote style specified in the formatter’s configuration.
  • Fixes useSortedClasses appears to not follow jsxQuoteStyle #4855 by updating the useSortedClasses rule to use this new logic.

Test Plan

  • I added a test case to useSortedClasses to check that code fixes follow the formatter’s configuration.

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Changelog Area: changelog labels Jan 8, 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.

Thank you for the PR. I suggested few changes

@@ -17,6 +17,7 @@ pub struct RuleContext<'a, R: Rule> {
file_path: &'a Path,
options: &'a R::Options,
preferred_quote: &'a PreferredQuote,
preferred_jsx_quote: Option<&'a PreferredQuote>,
Copy link
Member

Choose a reason for hiding this comment

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

Like preferred_quote, field (line 19) it shouldn't be a Option

Comment on lines 176 to 177
self.preferred_jsx_quote
.expect("preferred_jsx_quote should be provided")
Copy link
Member

Choose a reason for hiding this comment

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

When you remove the Option, you can remove expect

CHANGELOG.md Outdated
@@ -15,6 +15,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Analyzer

#### New features

- The analyzer now infers the correct JSX quote style from `javascript.formatter.jsxQuoteStyle`, so code fixes will use the same quote style as the formatter. Contributed by @lucasweng
Copy link
Member

Choose a reason for hiding this comment

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

That isn't true, though. In this PR you just fixed useSortedClasses, but there are many JSX-related rules that don't use this new feature yet. You're free to implement it in another PR if you want. As for now, we can remove this line

Copy link

codspeed-hq bot commented Jan 8, 2025

CodSpeed Performance Report

Merging #4857 will not alter performance

Comparing lucasweng:feat/analyzer-jsx-quote (2673601) with main (eedb22e)

Summary

✅ 95 untouched benchmarks

@lucasweng lucasweng changed the title feat(analyzer): infer JSX quote from formatter fix(useSortedClasses): should suggest code fixes that match the JSX quote style of the formatter Jan 8, 2025
@lucasweng
Copy link
Contributor Author

Thank you @ematipico! I’ve made some updates based on your comments:2673601

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.

Thank you!

@ematipico ematipico merged commit 4007147 into biomejs:main Jan 8, 2025
13 checks passed
@lucasweng lucasweng deleted the feat/analyzer-jsx-quote branch January 9, 2025 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog 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.

useSortedClasses appears to not follow jsxQuoteStyle
2 participants