Skip to content

Commit

Permalink
fix(linter): allow eslintrc to add rule when overriding (#1984)
Browse files Browse the repository at this point in the history
Fix a bug introduced in #1966.
Ideally, the rules in eslintrc should be merged into final rules as
described:

> The rules will start with the categories we apply, and then merge all
the configurations stated in the rules field.
>
> For example, if we begin with -D correctness with 80 rules, then
>
> "no-empty-file": "off" will remove the rule, yielding 79 rules
> "no-empty": "error" (restriction) will add the rule, yield 81 rules
> ""no-empty": ["error", { "allowEmptyCatch": true }]` add the rule's
configuration

However, the implementation did not include the newly added rules in the
eslintrc. As a test case and example, I added a new fixture to
`crates/oxc_cli/fixtures/no_undef`. No warn or deny will be found
without this PR.

This is my first Rust PR ever. Any nitpicking suggestions are welcome.
Thanks! 😊
  • Loading branch information
fi3ework authored Jan 11, 2024
1 parent c5887bc commit e0da12a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
5 changes: 5 additions & 0 deletions crates/oxc_cli/fixtures/no_undef/eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"no-undef": "warn"
}
}
1 change: 1 addition & 0 deletions crates/oxc_cli/fixtures/no_undef/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log()
9 changes: 9 additions & 0 deletions crates/oxc_cli/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,15 @@ mod test {
assert_eq!(result.number_of_errors, 0);
}

#[test]
fn eslintrc_no_undef() {
let args = &["-c", "fixtures/no_undef/eslintrc.json", "fixtures/no_undef/test.js"];
let result = test(args);
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_warnings, 1);
assert_eq!(result.number_of_errors, 0);
}

#[test]
fn no_empty_allow_empty_catch() {
let args = &[
Expand Down
23 changes: 17 additions & 6 deletions crates/oxc_linter/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,20 @@ impl ESLintConfig {
})
}

pub fn override_rules(&self, rules_to_override: &mut FxHashSet<RuleEnum>) {
pub fn override_rules(
&self,
rules_to_override: &mut FxHashSet<RuleEnum>,
all_rules: &[RuleEnum],
) {
let mut rules_to_replace = vec![];
let mut rules_to_remove = vec![];
for rule in rules_to_override.iter() {
let plugin_name = rule.plugin_name();
let rule_name = rule.name();
if let Some(rule_to_configure) =
self.rules.iter().find(|r| r.plugin_name == plugin_name && r.rule_name == rule_name)

for rule_to_configure in &self.rules {
let (plugin_name, rule_name) =
(&rule_to_configure.plugin_name, &rule_to_configure.rule_name);
if let Some(rule) = rules_to_override
.iter()
.find(|r| r.plugin_name() == plugin_name && r.name() == rule_name)
{
match rule_to_configure.severity {
AllowWarnDeny::Warn | AllowWarnDeny::Deny => {
Expand All @@ -85,8 +91,13 @@ impl ESLintConfig {
rules_to_remove.push(rule.clone());
}
}
} else if let Some(rule) =
all_rules.iter().find(|r| r.plugin_name() == plugin_name && r.name() == rule_name)
{
rules_to_replace.push(rule.read_json(rule_to_configure.config.clone()));
}
}

for rule in rules_to_remove {
rules_to_override.remove(&rule);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl LintOptions {
}

if let Some(config) = &config {
config.override_rules(&mut rules);
config.override_rules(&mut rules, &all_rules);
}

let mut rules = rules.into_iter().collect::<Vec<_>>();
Expand Down

0 comments on commit e0da12a

Please sign in to comment.