From 856b9a59eb0dc60840e24da004dfbfe19c04027d Mon Sep 17 00:00:00 2001 From: Boshen Date: Wed, 10 Jan 2024 15:44:53 +0800 Subject: [PATCH] feat(linter): support overriding oxlint rules by eslint config (#1966) Previously if .eslintrc.json contains ``` { "rules": { "no-empty": "off" } } ``` Then no rules will be enabled. --- This PR changes how we configure oxlint's rules. 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 --- crates/oxc_cli/src/lint/mod.rs | 2 +- crates/oxc_linter/src/config/mod.rs | 217 +++++++----------- ...oxc_linter__config__test__parse_rules.snap | 60 ++--- crates/oxc_linter/src/lib.rs | 10 +- crates/oxc_linter/src/options.rs | 21 +- 5 files changed, 125 insertions(+), 185 deletions(-) diff --git a/crates/oxc_cli/src/lint/mod.rs b/crates/oxc_cli/src/lint/mod.rs index be7c1cff507c8..fa85fe16e4927 100644 --- a/crates/oxc_cli/src/lint/mod.rs +++ b/crates/oxc_cli/src/lint/mod.rs @@ -320,7 +320,7 @@ mod test { let args = &["-c", "fixtures/eslintrc_off/eslintrc.json", "fixtures/eslintrc_off/test.js"]; let result = test(args); assert_eq!(result.number_of_files, 1); - assert_eq!(result.number_of_warnings, 0); + assert_eq!(result.number_of_warnings, 1); // triggered by no_empty_file assert_eq!(result.number_of_errors, 0); } diff --git a/crates/oxc_linter/src/config/mod.rs b/crates/oxc_linter/src/config/mod.rs index 9b6053502a483..306fcce1f543f 100644 --- a/crates/oxc_linter/src/config/mod.rs +++ b/crates/oxc_linter/src/config/mod.rs @@ -1,163 +1,119 @@ -use std::{collections::HashSet, path::PathBuf}; +use std::path::Path; pub mod errors; use oxc_diagnostics::{Error, FailedToOpenFileError, Report}; -use phf::{phf_map, Map}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use serde_json::Value; -use crate::{ - rules::{RuleEnum, RULES}, - AllowWarnDeny, JsxA11y, LintSettings, -}; +use crate::{rules::RuleEnum, AllowWarnDeny, JsxA11y, LintSettings}; use self::errors::{ - FailedToParseConfigError, FailedToParseConfigJsonError, FailedToParseConfigPropertyError, - FailedToParseRuleValueError, + FailedToParseConfigError, FailedToParseConfigJsonError, FailedToParseRuleValueError, }; pub struct ESLintConfig { - rules: std::vec::Vec, + rules: Vec, settings: LintSettings, } +#[derive(Debug)] +pub struct ESLintRuleConfig { + plugin_name: String, + rule_name: String, + severity: AllowWarnDeny, + config: Option, +} + impl ESLintConfig { - pub fn new(path: &PathBuf) -> Result { + pub fn new(path: &Path) -> Result { + let json = Self::read_json(path)?; + let rules = parse_rules(&json)?; + let settings = parse_settings_from_root(&json); + Ok(Self { rules, settings }) + } + + pub fn settings(self) -> LintSettings { + self.settings + } + + fn read_json(path: &Path) -> Result { let file = match std::fs::read_to_string(path) { Ok(file) => file, Err(e) => { return Err(FailedToParseConfigError(vec![Error::new(FailedToOpenFileError( - path.clone(), + path.to_path_buf(), e, ))]) .into()); } }; - let file = match serde_json::from_str::(&file) { - Ok(file) => file, - Err(e) => { - let guess = mime_guess::from_path(path); - let err = match guess.first() { - // syntax error - Some(mime) if mime.subtype() == "json" => e.to_string(), - Some(_) => "only json configuration is supported".to_string(), - None => { - format!( - "{e}, if the configuration is not a json file, please use json instead." - ) - } - }; - return Err(FailedToParseConfigError(vec![Error::new( - FailedToParseConfigJsonError(path.clone(), err), - )]) - .into()); - } - }; - - // See https://github.com/oxc-project/oxc/issues/1672 - let extends_hm: HashSet<&str> = HashSet::new(); - - let roles_hm = match parse_rules(&file) { - Ok(roles_hm) => roles_hm - .into_iter() - .map(|(plugin_name, rule_name, allow_warn_deny, config)| { - ((plugin_name, rule_name), (allow_warn_deny, config)) - }) - .collect::>(), - Err(e) => { - return Err(e); - } - }; - - let settings = parse_settings_from_root(&file); - - // `extends` provides the defaults - // `rules` provides the overrides - let rules = RULES.clone().into_iter().filter_map(|rule| { - // Check if the extends set is empty or contains the plugin name - let in_extends = extends_hm.contains(rule.plugin_name()); - - // Check if there's a custom rule that explicitly handles this rule - let (is_explicitly_handled, policy, config) = - if let Some((policy, config)) = roles_hm.get(&(rule.plugin_name(), rule.name())) { - // Return true for handling, and also whether it's enabled or not - (true, *policy, config) - } else { - // Not explicitly handled - (false, AllowWarnDeny::Allow, &None) - }; - - // The rule is included if it's in the extends set and not explicitly disabled, - // or if it's explicitly enabled - if (in_extends && !is_explicitly_handled) || policy.is_enabled() { - Some(rule.read_json(config.clone())) - } else { - None - } - }); - - Ok(Self { rules: rules.collect::>(), settings }) - } - - pub fn into_rules(mut self) -> Self { - self.rules.sort_unstable_by_key(RuleEnum::name); - self + serde_json::from_str::(&file).map_err(|err| { + let guess = mime_guess::from_path(path); + let err = match guess.first() { + // syntax error + Some(mime) if mime.subtype() == "json" => err.to_string(), + Some(_) => "only json configuration is supported".to_string(), + None => { + format!( + "{err}, if the configuration is not a json file, please use json instead." + ) + } + }; + FailedToParseConfigError(vec![Error::new(FailedToParseConfigJsonError( + path.to_path_buf(), + err, + ))]) + .into() + }) } - pub fn get_config(self) -> (std::vec::Vec, LintSettings) { - (self.rules, self.settings) + pub fn override_rules(&self, rules_to_override: &mut FxHashSet) { + 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) + { + match rule_to_configure.severity { + AllowWarnDeny::Warn | AllowWarnDeny::Deny => { + rules_to_replace.push(rule.read_json(rule_to_configure.config.clone())); + } + AllowWarnDeny::Allow => { + rules_to_remove.push(rule.clone()); + } + } + } + } + for rule in rules_to_remove { + rules_to_override.remove(&rule); + } + for rule in rules_to_replace { + rules_to_override.replace(rule); + } } } -#[allow(unused)] -fn parse_extends(root_json: &Value) -> Result>, Report> { - let Some(extends) = root_json.get("extends") else { - return Ok(None); - }; +fn parse_rules(root_json: &Value) -> Result, Error> { + let Value::Object(rules_object) = root_json else { return Ok(Vec::default()) }; - let extends_obj = match extends { - Value::Array(v) => v, - _ => { - return Err(FailedToParseConfigPropertyError("extends", "Expected an array.").into()); - } + let Some(Value::Object(rules_object)) = rules_object.get("rules") else { + return Ok(Vec::default()); }; - let extends_rule_groups = extends_obj - .iter() - .filter_map(|v| { - let v = match v { - Value::String(s) => s, - _ => return None, - }; - - if let Some(m) = EXTENDS_MAP.get(v.as_str()) { - return Some(*m); - } - - None - }) - .collect::>(); - - Ok(Some(extends_rule_groups)) -} - -#[allow(clippy::type_complexity)] -fn parse_rules( - root_json: &Value, -) -> Result)>, Error> { - let Value::Object(rules_object) = root_json else { return Ok(vec![]) }; - - let Some(Value::Object(rules_object)) = rules_object.get("rules") else { return Ok(vec![]) }; - rules_object - .iter() + .into_iter() .map(|(key, value)| { - let (plugin_name, name) = parse_rule_name(key); - - let (rule_severity, rule_config) = resolve_rule_value(value)?; - - Ok((plugin_name, name, rule_severity, rule_config)) + let (plugin_name, rule_name) = parse_rule_name(key); + let (severity, config) = resolve_rule_value(value)?; + Ok(ESLintRuleConfig { + plugin_name: plugin_name.to_string(), + rule_name: rule_name.to_string(), + severity, + config, + }) }) .collect::, Error>>() } @@ -198,15 +154,6 @@ pub fn parse_settings(setting_value: &Value) -> LintSettings { LintSettings::default() } -pub const EXTENDS_MAP: Map<&'static str, &'static str> = phf_map! { - "eslint:recommended" => "eslint", - "plugin:react/recommended" => "react", - "plugin:@typescript-eslint/recommended" => "typescript", - "plugin:react-hooks/recommended" => "react", - "plugin:unicorn/recommended" => "unicorn", - "plugin:jest/recommended" => "jest", -}; - fn parse_rule_name(name: &str) -> (&str, &str) { if let Some((category, name)) = name.split_once('/') { let category = category.trim_start_matches('@'); diff --git a/crates/oxc_linter/src/config/snapshots/oxc_linter__config__test__parse_rules.snap b/crates/oxc_linter/src/config/snapshots/oxc_linter__config__test__parse_rules.snap index 83f244a81ef6f..b8e3fd4b8e63a 100644 --- a/crates/oxc_linter/src/config/snapshots/oxc_linter__config__test__parse_rules.snap +++ b/crates/oxc_linter/src/config/snapshots/oxc_linter__config__test__parse_rules.snap @@ -3,17 +3,17 @@ source: crates/oxc_linter/src/config/mod.rs expression: rules --- [ - ( - "eslint", - "no-console", - Allow, - None, - ), - ( - "eslint", - "no-bitwise", - Deny, - Some( + ESLintRuleConfig { + plugin_name: "eslint", + rule_name: "no-console", + severity: Allow, + config: None, + }, + ESLintRuleConfig { + plugin_name: "eslint", + rule_name: "no-bitwise", + severity: Deny, + config: Some( Array [ Object { "allow": Array [ @@ -22,12 +22,12 @@ expression: rules }, ], ), - ), - ( - "eslint", - "eqeqeq", - Deny, - Some( + }, + ESLintRuleConfig { + plugin_name: "eslint", + rule_name: "eqeqeq", + severity: Deny, + config: Some( Array [ String("always"), Object { @@ -35,17 +35,17 @@ expression: rules }, ], ), - ), - ( - "typescript", - "ban-types", - Deny, - None, - ), - ( - "jsx_a11y", - "alt-text", - Warn, - None, - ), + }, + ESLintRuleConfig { + plugin_name: "typescript", + rule_name: "ban-types", + severity: Deny, + config: None, + }, + ESLintRuleConfig { + plugin_name: "jsx_a11y", + rule_name: "alt-text", + severity: Warn, + config: None, + }, ] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 40d92761b2ae8..284f6334fe094 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -18,7 +18,7 @@ mod rules; mod service; mod utils; -use std::{self, fs, io::Write, rc::Rc, time::Duration}; +use std::{io::Write, rc::Rc, time::Duration}; use oxc_diagnostics::Report; pub(crate) use oxc_semantic::AstNode; @@ -167,14 +167,6 @@ impl Linter { pub fn get_settings(&self) -> LintSettings { self.settings.clone() } - #[allow(unused)] - fn read_rules_configuration() -> Option> { - fs::read_to_string(".eslintrc.json") - .ok() - .and_then(|s| serde_json::from_str(&s).ok()) - .and_then(|v: serde_json::Value| v.get("rules").cloned()) - .and_then(|v| v.as_object().cloned()) - } pub fn print_rules(writer: &mut W) { let rules_by_category = RULES.iter().fold( diff --git a/crates/oxc_linter/src/options.rs b/crates/oxc_linter/src/options.rs index 4e0897baaf888..312de4c9bc32f 100644 --- a/crates/oxc_linter/src/options.rs +++ b/crates/oxc_linter/src/options.rs @@ -11,7 +11,7 @@ use crate::{ rules::RULES, LintSettings, RuleCategory, RuleEnum, }; -use oxc_diagnostics::{Error, Report}; +use oxc_diagnostics::Error; use rustc_hash::FxHashSet; use serde_json::{Number, Value}; @@ -153,15 +153,12 @@ const NEXTJS_PLUGIN_NAME: &str = "nextjs"; impl LintOptions { /// # Errors - /// Returns `Err` if there are any errors parsing the configuration file. - pub fn derive_rules_and_settings(&self) -> Result<(Vec, LintSettings), Report> { - let mut rules: FxHashSet = FxHashSet::default(); - - if let Some(path) = &self.config_path { - let (rules, settings) = ESLintConfig::new(path)?.into_rules().get_config(); - return Ok((rules, settings)); - } + /// + /// * Returns `Err` if there are any errors parsing the configuration file. + pub fn derive_rules_and_settings(&self) -> Result<(Vec, LintSettings), Error> { + let config = self.config_path.as_ref().map(|path| ESLintConfig::new(path)).transpose()?; + let mut rules: FxHashSet = FxHashSet::default(); let all_rules = self.get_filtered_rules(); for (allow_warn_deny, name_or_category) in &self.filter { @@ -201,10 +198,14 @@ impl LintOptions { } } + if let Some(config) = &config { + config.override_rules(&mut rules); + } + let mut rules = rules.into_iter().collect::>(); // for stable diagnostics output ordering rules.sort_unstable_by_key(RuleEnum::name); - Ok((rules, LintSettings::default())) + Ok((rules, config.map(ESLintConfig::settings).unwrap_or_default())) } // get final filtered rules by reading `self.jest_plugin` and `self.jsx_a11y_plugin`