dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 12:19:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(linter): rule and generic filters do not re-configure existing rules (#6087)

Browse source 
Continuation of #6085.
This commit is contained in:
DonIsaac 2024-09-27 03:39:01 +00:00
parent c5cdb4c615
commit bd8f7867eb
1 changed files with 65 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

96
crates/oxc_linter/src/builder.rs
View file

@ -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);
}
}
},
@ -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
@ -403,6 +396,47 @@ mod test {
}
}
// change a rule already set to "warn" to "deny"
#[test]
fn test_filter_deny_single_enabled_rule_on_default() {
for filter_string in ["no-const-assign", "eslint/no-const-assign"] {
let builder = LinterBuilder::default();
let initial_rule_count = builder.rules.len();
let builder =
builder
.with_filters([LintFilter::new(AllowWarnDeny::Deny, filter_string).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);
}
}
// turn on a rule that isn't configured yet and set it to "warn"
// note that this is an eslint rule, a plugin that's already turned on.
#[test]
fn test_filter_warn_single_disabled_rule_on_default() {
for filter_string in ["no-console", "eslint/no-console"] {
let filter = LintFilter::new(AllowWarnDeny::Warn, filter_string).unwrap();
let builder = LinterBuilder::default();
// sanity check: not already turned on
assert!(!builder.rules.iter().any(|r| r.name() == "no-console"));
let builder = builder.with_filter(filter);
let no_console = builder
.rules
.iter()
.find(|r| r.plugin_name() == "eslint" && r.name() == "no-console")
.expect("Could not find eslint/no-console after configuring it to 'warn'");
assert_eq!(no_console.severity, AllowWarnDeny::Warn);
}
}
#[test]
fn test_filter_allow_all_then_warn() {
let builder =