Skip to content

Commit

Permalink
feat(linter): allow appending plugins in override (#7379)
Browse files Browse the repository at this point in the history 
follow up to #6896

for improved compatibility with ESLint, this tries to match the behavior of plugin overrides so that plugins can be enabled for certain paths. this does not allow disabling plugins.
  • Loading branch information
@camchenry
camchenry committed Nov 21, 2024
1 parent ad44cfa commit 8cebdc8
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 49 deletions.
79 changes: 44 additions & 35 deletions crates/oxc_linter/src/config/flat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,7 @@ impl ConfigStore {

/// NOTE: this function must not borrow any entries from `self.cache` or DashMap will deadlock.
fn apply_overrides(&self, override_ids: &[OverrideId]) -> ResolvedLinterState {
let plugins = self
.overrides
.iter()
.rev()
.find_map(|cfg| cfg.plugins)
.unwrap_or(self.base.config.plugins);
let mut plugins = self.base.config.plugins;

let all_rules = RULES
.iter()
Expand All @@ -135,10 +130,14 @@ impl ConfigStore {

let overrides = override_ids.iter().map(|id| &self.overrides[*id]);
for override_config in overrides {
if override_config.rules.is_empty() {
continue;
if !override_config.rules.is_empty() {
override_config.rules.override_rules(&mut rules, &all_rules);
}

// Append the override's plugins to the base list of enabled plugins.
if let Some(override_plugins) = override_config.plugins {
plugins |= override_plugins;
}
override_config.rules.override_rules(&mut rules, &all_rules);
}

let rules = rules.into_iter().collect::<Vec<_>>();
Expand All @@ -158,7 +157,10 @@ impl ConfigStore {
#[cfg(test)]
mod test {
use super::{ConfigStore, OxlintOverrides};
use crate::{config::LintConfig, AllowWarnDeny, LintPlugins, RuleEnum, RuleWithSeverity};
use crate::{
config::{LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings},
AllowWarnDeny, LintPlugins, RuleEnum, RuleWithSeverity,
};

macro_rules! from_json {
($json:tt) => {
Expand All @@ -171,11 +173,6 @@ mod test {
RuleWithSeverity::new(RuleEnum::NoExplicitAny(Default::default()), AllowWarnDeny::Warn)
}

#[allow(clippy::default_trait_access)]
fn no_cycle() -> RuleWithSeverity {
RuleWithSeverity::new(RuleEnum::NoCycle(Default::default()), AllowWarnDeny::Warn)
}

/// an empty ruleset is a no-op
#[test]
fn test_no_rules() {
Expand Down Expand Up @@ -219,26 +216,6 @@ mod test {
);
}

/// removing plugins strips rules from those plugins, even if no rules are
/// added/removed explicitly
#[test]
fn test_no_rules_and_remove_plugins() {
let base_rules = vec![no_cycle()];
let overrides = from_json!([{
"files": ["*.test.{ts,tsx}"],
"plugins": ["jest"],
"rules": {}
}]);
let config = LintConfig {
plugins: LintPlugins::default() | LintPlugins::IMPORT,
..LintConfig::default()
};
let store = ConfigStore::new(base_rules, config, overrides);

assert_eq!(store.resolve("App.tsx".as_ref()).rules.len(), 1);
assert_eq!(store.resolve("App.test.tsx".as_ref()).rules.len(), 0);
}

#[test]
fn test_remove_rule() {
let base_rules = vec![no_explicit_any()];
Expand Down Expand Up @@ -300,4 +277,36 @@ mod test {
assert_eq!(src_app.len(), 1);
assert_eq!(src_app[0].severity, AllowWarnDeny::Deny);
}

#[test]
fn test_add_plugins() {
let base_config = LintConfig {
plugins: LintPlugins::IMPORT,
env: OxlintEnv::default(),
settings: OxlintSettings::default(),
globals: OxlintGlobals::default(),
};
let overrides = from_json!([{
"files": ["*.jsx", "*.tsx"],
"plugins": ["react"],
}, {
"files": ["*.ts", "*.tsx"],
"plugins": ["typescript"],
}]);

let store = ConfigStore::new(vec![], base_config, overrides);
assert_eq!(store.base.config.plugins, LintPlugins::IMPORT);

let app = store.resolve("other.mjs".as_ref()).config;
assert_eq!(app.plugins, LintPlugins::IMPORT);

let app = store.resolve("App.jsx".as_ref()).config;
assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::REACT);

let app = store.resolve("App.ts".as_ref()).config;
assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::TYPESCRIPT);

let app = store.resolve("App.tsx".as_ref()).config;
assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::REACT | LintPlugins::TYPESCRIPT);
}
}
28 changes: 28 additions & 0 deletions crates/oxc_linter/src/config/overrides.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,31 @@ impl JsonSchema for GlobSet {
gen.subschema_for::<Vec<String>>()
}
}

mod test {
#[test]
fn test_parsing_plugins() {
use super::*;
use serde_json::{from_value, json};

let config: OxlintOverride = from_value(json!({
"files": ["*.tsx"],
}))
.unwrap();
assert_eq!(config.plugins, None);

let config: OxlintOverride = from_value(json!({
"files": ["*.tsx"],
"plugins": [],
}))
.unwrap();
assert_eq!(config.plugins, Some(LintPlugins::empty()));

let config: OxlintOverride = from_value(json!({
"files": ["*.tsx"],
"plugins": ["typescript", "react"],
}))
.unwrap();
assert_eq!(config.plugins, Some(LintPlugins::REACT | LintPlugins::TYPESCRIPT));
}
}
27 changes: 13 additions & 14 deletions crates/oxc_linter/src/config/plugins.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<S: AsRef<str>> FromIterator<S> for LintPlugins {
fn from_iter<T: IntoIterator<Item = S>>(iter: T) -> Self {
iter.into_iter()
.map(|plugin| plugin.as_ref().into())
.fold(LintPlugins::default(), LintPlugins::union)
.fold(LintPlugins::empty(), LintPlugins::union)
}
}

Expand Down Expand Up @@ -184,18 +184,17 @@ impl<'de> Deserialize<'de> for LintPlugins {
// serde_json::from_value provides a String. The former is
// used in almost all cases, but the latter is more
// convenient for test cases.
match seq.next_element::<&str>() {
match seq.next_element::<String>() {
Ok(Some(next)) => {
plugins |= next.into();
plugins |= next.as_str().into();
}
Ok(None) => break,
Err(_) => {
if let Some(next) = seq.next_element::<String>()? {
plugins |= next.as_str().into();
} else {
break;
Err(_) => match seq.next_element::<&str>() {
Ok(Some(next)) => {
plugins |= next.into();
}
}
Ok(None) | Err(_) => break,
},
};
}

Expand Down Expand Up @@ -316,7 +315,7 @@ impl LintPluginOptions {

impl<S: AsRef<str>> FromIterator<(S, bool)> for LintPluginOptions {
fn from_iter<I: IntoIterator<Item = (S, bool)>>(iter: I) -> Self {
let mut options = Self::default();
let mut options = Self::none();
for (s, enabled) in iter {
let flags = LintPlugins::from(s.as_ref());
match flags {
Expand Down Expand Up @@ -381,11 +380,11 @@ mod test {
fn test_collect_empty() {
let empty: &[&str] = &[];
let plugins: LintPluginOptions = empty.iter().copied().collect();
assert_eq!(plugins, LintPluginOptions::default());
assert_eq!(plugins, LintPluginOptions::none());

let empty: Vec<(String, bool)> = vec![];
let plugins: LintPluginOptions = empty.into_iter().collect();
assert_eq!(plugins, LintPluginOptions::default());
assert_eq!(plugins, LintPluginOptions::none());
}

#[test]
Expand All @@ -394,9 +393,9 @@ mod test {
let plugins: LintPluginOptions = enabled.into_iter().collect();
let expected = LintPluginOptions {
react: true,
unicorn: true,
unicorn: false,
typescript: true,
oxc: true,
oxc: false,
import: false,
jsdoc: false,
jest: true,
Expand Down

0 comments on commit 8cebdc8

Please sign in to comment.