-
-
Notifications
You must be signed in to change notification settings - Fork 400
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)!: support plugins in oxlint config files #6088
base: main
Are you sure you want to change the base?
feat(linter)!: support plugins in oxlint config files #6088
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
CodSpeed Performance ReportMerging #6088 will degrade performances by 3.79%Comparing Summary
Benchmarks breakdown
|
d08cf2c
to
51b398d
Compare
It looks like some rules are not running that weren't before. This either means this PR fixed a bug that caused them not to run, or ...run() is now taking longer, so those spans are showing up? TBH I'm not quite sure what to make of this. |
b1a12f2
to
29c00c5
Compare
51b398d
to
27c99ab
Compare
29c00c5
to
bd8f786
Compare
27c99ab
to
8592ba8
Compare
8592ba8
to
d5a088e
Compare
d5a088e
to
b197bff
Compare
b197bff
to
2be7add
Compare
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.
I suggest manually test this setup on some of the oxlint-ecosystem-ci projects and see if the rule numbers match.
2be7add
to
dd0946f
Compare
dd0946f
to
8ea51af
Compare
We'll need to allow for category-based rule configuration in |
Waiting on #6120 to land before merging this. |
plugins
in oxlint config files are now supportedBreaking Changes
Before, config files would override rules set in CLI arguments. For example, running this command:
With this config file
Would result in a single rule,
no-const-assign
being turned on at an error level with all other rules disabled (i.e. set to "allow").Now, CLI arguments will override config files. That same command with the same config file will result with all rules being disabled.
Details
For a more in-depth explanation, assume we are running the below command using the
oxlintrc.json
file above:Before
Here was the config resolution process before this PR:
Before Steps
a. start with an empty list
[]
b.
("all", "allow")
->[]
c.
("correctness", "warn")
->[ <correctness rules> ]
d.
("all", "allow")
->[]
e.
("suspicious", "warn")
->[ <suspicious rules> ]
. This is the final rule set for this stepoxlintrc.json
. This is where things get a little funky, as mentioned in point 2 of what this PR does. At this point, all rules in the rules list are only from the CLI.a. If a rule is only set in the CLI and is not present in the config file, there's no effect
b. If a rule is in the config file but not the CLI, it gets inserted into the list.
c. If a rule is already in the list and in the config file
i. If the rule is only present once (e.g.
"no-loss-of-precision": "error"
), unconditionally override whatever was in the CLI with what was set in the config fileii. If the rule is present twice (e.g.
"no-loss-of-precision": "off", "@typescript-eslint/no-loss-of-precision": "error"
),a. if all rules in the config file are set to
allow
, then turn the rule offb. If one of them is
warn
ordeny
, then update the currently-set rule's config object, but leave its severity alone.So, for our example, the final rule set would be
[<all suspicious rules: "warn">, no-const-assign: "error"]
After
Rule filters were completely re-worked in a previous PR. Now, lint filters aren't kept on hand-only the rule list is.
After Steps
[<all correctness rules: "warn">]
)oxlintrc.json
first.a. If the rule is warn/deny exists in the list already, update its severity and config object. If it's not in the list, add it.
b. If the rule is set to allow, remove it from the list.
The list is now
[<all correctness rules except no-const-assign: "warn">, no-const-assign: "error"]
.[<suspicous rules: "warn">]