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: noValueAtRule css lint rule #3293

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rishabh3112
Copy link

@rishabh3112 rishabh3112 commented Jun 26, 2024

Summary

Implements noValueAtRule lint rule for css modules

TODO:

  • Run this rule only when used inside css_modules (i.e. when enabled in parser options)
  • Auto fixers - in seperate PR

Test Plan

  • Add Snapshot tests for @value rule

Refers #3001

@github-actions github-actions bot added A-Project Area: project L-CSS Language: CSS A-Diagnostic Area: diagnostocis labels Jun 26, 2024
@denbezrukov
Copy link
Contributor

Sorry, I forgot to mention that it seems now it's impossible to get a parser option inside in a rule.
I'd like to propose ignoring Rule this rule only when used inside css_modules (i.e. when enabled in parser options)

@rishabh3112
Copy link
Author

rishabh3112 commented Jul 1, 2024

@denbezrukov I would like to pick up autofixing as part of seperate PR. It seems to be much more involved. Adding findings here for future reference:

@rishabh3112 rishabh3112 marked this pull request as ready for review July 1, 2024 03:04
@ematipico
Copy link
Member

If you want to land the code action in another PR, I would suggest to not close the original issue or opening a new one.

Copy link

codspeed-hq bot commented Jul 1, 2024

CodSpeed Performance Report

Merging #3293 will degrade performances by 6.93%

Comparing rishabh3112:feat/no-value-at-rule-lint-rule (c8f5b94) with main (40727ca)

Summary

❌ 1 regressions
✅ 107 untouched benchmarks

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

Benchmarks breakdown

Benchmark main rishabh3112:feat/no-value-at-rule-lint-rule Change
router_14177007973872119684.ts[uncached] 2.4 ms 2.6 ms -6.93%

@rishabh3112 rishabh3112 force-pushed the feat/no-value-at-rule-lint-rule branch from 4b5d5d9 to 7081ec2 Compare July 6, 2024 15:18
@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools A-LSP Area: language server protocol L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages A-Changelog Area: changelog labels Jul 6, 2024
@rishabh3112 rishabh3112 force-pushed the feat/no-value-at-rule-lint-rule branch from 7081ec2 to 8f4f8be Compare July 6, 2024 15:21
@github-actions github-actions bot removed A-Core Area: core A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools A-LSP Area: language server protocol A-Changelog Area: changelog labels Jul 6, 2024
@ematipico
Copy link
Member

@rishabh3112

A couple of things:

  • the categories are sorted alphabetically, probably this broke when merging the conflicts
  • always run just gen-lint
  • always run the tests with cargo t

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Project Area: project L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants