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: enable CSS linter and CSS formatter #2790

Merged
merged 6 commits into from May 17, 2024
Merged

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented May 9, 2024

Summary

This PR enables CSS parsing and CSS formatting.

Important

Internally, we decided that linter and formatting are opt-in and disabled by default. Why? Because it's possible that parser, formatter and linter aren't quite stable yet, and it's hightly possible that there are bugs.

Enabling this in Biome comes with new options across the configuration. Mainly now, each language section can enable or disable the linter. This was required because, until now, we haven't officially had lint rules for languages other than JSON, and that single lint rule still needs some work, that's why it isn't documented yet.

The PR adds:

  • javascript.linter.enabled
  • css.linter.enabled
  • json.linter.enabled

The relative CLI options are also added.

Also, since we are officially enabling the CSS formatter to the people with this PR, I took the chance to remove inde_size from the options. It doesn't make sense to expose an option that is already deprecated.

Technical changes

Most of the changes are around configuration and overrides. It's mostly boilerplate and copying code from other implementations.

One thing of relevance though, is the creation of the type AnalyzerOptions when we do linting. So far, we were using a standalone function to do that, however, that's not the right way not.

Our infrastructure allows to generate generic functions at triat level that return options for a specifict language, for example format_options. These are function meant to receive options from global settings (biome.json and CLI) and the LSP (client options). I did create a new function called analyzer_options, and how each language implements that. The implementation of each function is a copy-paste of their former functions, but adapted to use the proper parameters.

Test Plan

I created new test cases inside a new file called handle_css_files.rs. I find using the "cases" concept better than grouping stuff by commands. More files, sure, but they are smaller and focused on certain things.

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter A-Formatter Area: formatter L-CSS Language: CSS labels May 9, 2024
@github-actions github-actions bot added A-CLI Area: CLI A-Parser Area: parser and removed A-Linter Area: linter labels May 17, 2024
Copy link

codspeed-hq bot commented May 17, 2024

CodSpeed Performance Report

Merging #2790 will not alter performance

Comparing chore/css-analyzer (4ed0034) with chore/css-analyzer (ef9364a)

Summary

✅ 99 untouched benchmarks

@ematipico ematipico marked this pull request as ready for review May 17, 2024 12:25
@ematipico ematipico requested review from a team May 17, 2024 12:36
@github-actions github-actions bot added the A-Changelog Area: changelog label May 17, 2024
Copy link
Contributor

@denbezrukov denbezrukov left a comment

Choose a reason for hiding this comment

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

Do I understand correctly that we don't even parse a file if we disable linting and formatting?

CHANGELOG.md Outdated Show resolved Hide resolved
@ematipico
Copy link
Member Author

Do I understand correctly that we don't even parse a file if we disable linting and formatting?

Exactly, we skip it

Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
@ematipico ematipico requested a review from Sec-ant May 17, 2024 15:50
@ematipico ematipico merged commit 2fb7d63 into main May 17, 2024
14 checks passed
@ematipico ematipico deleted the chore/css-analyzer branch May 17, 2024 16:08
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-Formatter Area: formatter A-Parser Area: parser A-Project Area: project L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants