-
-
Notifications
You must be signed in to change notification settings - Fork 540
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(cli/lint): add the --rule
option
#2860
Conversation
@@ -311,6 +311,7 @@ impl Session { | |||
path: biome_path.clone(), | |||
categories, | |||
max_diagnostics: u64::MAX, | |||
rule: None, |
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.
Here I set rule
to None
because I do not have access to this data. I am not sure about the consequences. Maybe it only affects the LSP. Since the CLI is the only one using the rule
option, it's probably OK.
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
3fec75f
to
cd94ba6
Compare
You use the following phrase inside the doc comments, but what does that mean? I believe it requires more explanation. The phrase is too opaque.
|
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.
A believe we can make this option more powerful by running rules that belong to a single group too.
249a08b
to
cc05d50
Compare
8990648
to
1a6661f
Compare
CodSpeed Performance ReportMerging #2860 will improve performances by 6.09%Comparing Summary
Benchmarks breakdown
|
1a6661f
to
bde3a10
Compare
fcbfe35
to
cde089f
Compare
--rule
option--rule
option
cde089f
to
4f7f019
Compare
4f7f019
to
2b9c194
Compare
@Conaclos you forgot to address
It's still somewhere inside the doc comments and in the description of the PR. Could you please make sure to follow up? I still find that phrase opaque. I truly don't understand what it means 😅 |
I made the following changes:
Then I changed to:
And so:
I am unsure how to address your concern |
What I don't understand is what "taking the configuration file into account" mean. It's very generic. I know you explained that in some comments, it would great to use that explanation here:
|
Summary
Fix #58
This PR adds a new option
--rule
on thebiome lint
command.The option allows executing a given rule on a set of files.
The configuration is taken into account.
Only the severity is overridden to its default, i.e.
error
for recommended rules andwarn
for other rules.This ensures that the rule is not
off
.The rule is executed even if
recommended
is disabled.However, the rule is not executed if the linter is disabled in the configuration.
To implement this feature, I introduced a new type
RuleCode
that references a rule with static string slices.This allows passing the rule
group/name
without cloning it.I modified
Rules::matches_diagnostic_code
to return static string slices. It acts as a static string interner.I also added methods to set the severity level of a given rule.
The argument of the rule option is passed as parameters in the different calls.
Test Plan
I added tests.