Skip to content

Commit

Permalink
fix(linter): rule and generic filters do not re-configure existing rules
Browse files Browse the repository at this point in the history
Continuation of #6085.
  • Loading branch information
DonIsaac committed Sep 26, 2024
1 parent a439f40 commit 6d1f1e2
Showing 1 changed file with 44 additions and 31 deletions.
75 changes: 44 additions & 31 deletions crates/oxc_linter/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,44 +156,18 @@ impl LinterBuilder {

pub fn with_filter(mut self, filter: LintFilter) -> Self {
let (severity, filter) = filter.into();
let all_rules = self.cache.borrow();

match severity {
AllowWarnDeny::Deny | AllowWarnDeny::Warn => match filter {
LintFilterKind::Category(category) => {
let rules_to_configure = all_rules.iter().filter(|r| r.category() == category);
for rule in rules_to_configure {
if let Some(mut existing_rule) = self.rules.take(rule) {
existing_rule.severity = severity;
self.rules.insert(existing_rule);
} else {
self.rules.insert(RuleWithSeverity::new(rule.clone(), severity));
}
}
}
LintFilterKind::Rule(_, name) => {
self.rules.extend(
all_rules
.iter()
.filter(|rule| rule.name() == name)
.map(|rule| RuleWithSeverity::new(rule.clone(), severity)),
);
self.upsert_where(severity, |r| r.category() == category);
}
LintFilterKind::Rule(_, name) => self.upsert_where(severity, |r| r.name() == name),
LintFilterKind::Generic(name_or_category) => {
if name_or_category == "all" {
self.rules.extend(
all_rules
.iter()
.filter(|rule| rule.category() != RuleCategory::Nursery)
.map(|rule| RuleWithSeverity::new(rule.clone(), severity)),
);
self.upsert_where(severity, |r| r.category() != RuleCategory::Nursery);
} else {
self.rules.extend(
all_rules
.iter()
.filter(|rule| rule.name() == name_or_category)
.map(|rule| RuleWithSeverity::new(rule.clone(), severity)),
);
self.upsert_where(severity, |r| r.name() == name_or_category);
}
}
},
Expand All @@ -213,11 +187,30 @@ impl LinterBuilder {
}
},
}
drop(all_rules);

self
}

/// Warn/Deny a let of rules based on some predicate. Rules already in `self.rules` get
/// re-configured, while those that are not are added. Affects rules where `query` returns
/// `true`.
fn upsert_where<F>(&mut self, severity: AllowWarnDeny, query: F)
where
F: Fn(&&RuleEnum) -> bool,
{
let all_rules = self.cache.borrow();
// NOTE: we may want to warn users if they're configuring a rule that does not exist.
let rules_to_configure = all_rules.iter().filter(query);
for rule in rules_to_configure {
if let Some(mut existing_rule) = self.rules.take(rule) {
existing_rule.severity = severity;
self.rules.insert(existing_rule);
} else {
self.rules.insert(RuleWithSeverity::new(rule.clone(), severity));
}
}
}

#[must_use]
pub fn build(self) -> Linter {
// When a plugin gets disabled before build(), rules for that plugin aren't removed until
Expand Down Expand Up @@ -403,6 +396,26 @@ mod test {
}
}

#[test]
fn test_filter_deny_single_rule_on_default() {
let builder = LinterBuilder::default();
let initial_rule_count = builder.rules.len();

let builder =
builder.with_filters([
LintFilter::new(AllowWarnDeny::Deny, "eslint/no-const-assign").unwrap()
]);
let rule_count_after_deny = builder.rules.len();
assert_eq!(initial_rule_count, rule_count_after_deny, "Changing a single rule from warn to deny should not add a new one, just modify what's already there.");

let no_const_assign = builder
.rules
.iter()
.find(|r| r.plugin_name() == "eslint" && r.name() == "no-const-assign")
.expect("Could not find eslint/no-const-assign after configuring it to 'deny'");
assert_eq!(no_const_assign.severity, AllowWarnDeny::Deny);
}

#[test]
fn test_filter_allow_all_then_warn() {
let builder =
Expand Down

0 comments on commit 6d1f1e2

Please sign in to comment.