Skip to content

Commit

Permalink
refactor(linter): move override_rule to OxlintRules (#5708)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonIsaac
DonIsaac committed Sep 17, 2024
1 parent a438743 commit 50834bc
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 99 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ mod test {
env::current_dir().unwrap().join("fixtures/eslint_config_vitest_replace.json");
let config = Oxlintrc::from_file(&fixture_path).unwrap();
let mut set = FxHashSet::default();
config.override_rules(&mut set, &RULES);
config.rules.override_rules(&mut set, &RULES);

let rule = set.into_iter().next().unwrap();
assert_eq!(rule.name(), "no-disabled-tests");
Expand Down
158 changes: 71 additions & 87 deletions crates/oxc_linter/src/config/oxlintrc.rs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
use std::path::Path;

use oxc_diagnostics::OxcDiagnostic;
use rustc_hash::FxHashSet;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use super::{env::OxlintEnv, globals::OxlintGlobals, rules::OxlintRules, settings::OxlintSettings};

use crate::{
rules::RuleEnum,
utils::{is_jest_rule_adapted_to_vitest, read_to_string},
AllowWarnDeny, RuleWithSeverity,
};
use crate::utils::read_to_string;

/// Oxlint Configuration File
///
Expand Down Expand Up @@ -93,88 +88,77 @@ impl Oxlintrc {
Ok(config)
}

#[allow(clippy::option_if_let_else)]
pub fn override_rules(
&self,
rules_for_override: &mut FxHashSet<RuleWithSeverity>,
all_rules: &[RuleEnum],
) {
use itertools::Itertools;
let mut rules_to_replace: Vec<RuleWithSeverity> = vec![];
let mut rules_to_remove: Vec<RuleWithSeverity> = vec![];
// #[allow(clippy::option_if_let_else)]
// pub fn override_rules(
// &self,
// rules_for_override: &mut FxHashSet<RuleWithSeverity>,
// all_rules: &[RuleEnum],
// ) {
// use itertools::Itertools;
// let mut rules_to_replace: Vec<RuleWithSeverity> = vec![];
// let mut rules_to_remove: Vec<RuleWithSeverity> = vec![];

// Rules can have the same name but different plugin names
let lookup = self.rules.iter().into_group_map_by(|r| r.rule_name.as_str());
// // Rules can have the same name but different plugin names
// let lookup = self.rules.iter().into_group_map_by(|r| r.rule_name.as_str());

for (name, rule_configs) in &lookup {
match rule_configs.len() {
0 => unreachable!(),
1 => {
let rule_config = &rule_configs[0];
let (rule_name, plugin_name) = transform_rule_and_plugin_name(
&rule_config.rule_name,
&rule_config.plugin_name,
);
let severity = rule_config.severity;
match severity {
AllowWarnDeny::Warn | AllowWarnDeny::Deny => {
if let Some(rule) = all_rules
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
let config = rule_config.config.clone().unwrap_or_default();
let rule = rule.read_json(config);
rules_to_replace.push(RuleWithSeverity::new(rule, severity));
}
}
AllowWarnDeny::Allow => {
if let Some(rule) = rules_for_override
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
let rule = rule.clone();
rules_to_remove.push(rule);
}
}
}
}
_ => {
// For overlapping rule names, use the "error" one
// "no-loss-of-precision": "off",
// "@typescript-eslint/no-loss-of-precision": "error"
if let Some(rule_config) =
rule_configs.iter().find(|r| r.severity.is_warn_deny())
{
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
let config = rule_config.config.clone().unwrap_or_default();
rules_to_replace
.push(RuleWithSeverity::new(rule.read_json(config), rule.severity));
}
} else if rule_configs.iter().all(|r| r.severity.is_allow()) {
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
rules_to_remove.push(rule.clone());
}
}
}
}
}

for rule in rules_to_remove {
rules_for_override.remove(&rule);
}
for rule in rules_to_replace {
rules_for_override.replace(rule);
}
}
}

fn transform_rule_and_plugin_name<'a>(
rule_name: &'a str,
plugin_name: &'a str,
) -> (&'a str, &'a str) {
if plugin_name == "vitest" && is_jest_rule_adapted_to_vitest(rule_name) {
return (rule_name, "jest");
}
// for (name, rule_configs) in &lookup {
// match rule_configs.len() {
// 0 => unreachable!(),
// 1 => {
// let rule_config = &rule_configs[0];
// let (rule_name, plugin_name) = transform_rule_and_plugin_name(
// &rule_config.rule_name,
// &rule_config.plugin_name,
// );
// let severity = rule_config.severity;
// match severity {
// AllowWarnDeny::Warn | AllowWarnDeny::Deny => {
// if let Some(rule) = all_rules
// .iter()
// .find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
// {
// let config = rule_config.config.clone().unwrap_or_default();
// let rule = rule.read_json(config);
// rules_to_replace.push(RuleWithSeverity::new(rule, severity));
// }
// }
// AllowWarnDeny::Allow => {
// if let Some(rule) = rules_for_override
// .iter()
// .find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
// {
// let rule = rule.clone();
// rules_to_remove.push(rule);
// }
// }
// }
// }
// _ => {
// // For overlapping rule names, use the "error" one
// // "no-loss-of-precision": "off",
// // "@typescript-eslint/no-loss-of-precision": "error"
// if let Some(rule_config) =
// rule_configs.iter().find(|r| r.severity.is_warn_deny())
// {
// if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
// let config = rule_config.config.clone().unwrap_or_default();
// rules_to_replace
// .push(RuleWithSeverity::new(rule.read_json(config), rule.severity));
// }
// } else if rule_configs.iter().all(|r| r.severity.is_allow()) {
// if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
// rules_to_remove.push(rule.clone());
// }
// }
// }
// }
// }

(rule_name, plugin_name)
// for rule in rules_to_remove {
// rules_for_override.remove(&rule);
// }
// for rule in rules_to_replace {
// rules_for_override.replace(rule);
// }
// }
}
117 changes: 107 additions & 10 deletions crates/oxc_linter/src/config/rules.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{borrow::Cow, fmt, ops::Deref};

use oxc_diagnostics::{Error, OxcDiagnostic};
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use schemars::{gen::SchemaGenerator, schema::Schema, JsonSchema};
use serde::{
de::{self, Deserializer, Visitor},
Expand All @@ -11,7 +11,8 @@ use serde::{

use crate::{
rules::{RuleEnum, RULES},
AllowWarnDeny,
utils::is_jest_rule_adapted_to_vitest,
AllowWarnDeny, RuleWithSeverity,
};

// TS type is `Record<string, RuleConf>`
Expand All @@ -30,6 +31,110 @@ pub struct ESLintRule {
pub config: Option<serde_json::Value>,
}

impl Deref for OxlintRules {
type Target = Vec<ESLintRule>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl IntoIterator for OxlintRules {
type Item = ESLintRule;
type IntoIter = <Vec<ESLintRule> as IntoIterator>::IntoIter;

fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}

impl OxlintRules {
#[allow(clippy::option_if_let_else)]
pub(crate) fn override_rules(
&self,
rules_for_override: &mut FxHashSet<RuleWithSeverity>,
all_rules: &[RuleEnum],
) {
use itertools::Itertools;
let mut rules_to_replace: Vec<RuleWithSeverity> = vec![];
let mut rules_to_remove: Vec<RuleWithSeverity> = vec![];

// Rules can have the same name but different plugin names
let lookup = self.iter().into_group_map_by(|r| r.rule_name.as_str());

for (name, rule_configs) in &lookup {
match rule_configs.len() {
0 => unreachable!(),
1 => {
let rule_config = &rule_configs[0];
let (rule_name, plugin_name) = transform_rule_and_plugin_name(
&rule_config.rule_name,
&rule_config.plugin_name,
);
let severity = rule_config.severity;
match severity {
AllowWarnDeny::Warn | AllowWarnDeny::Deny => {
if let Some(rule) = all_rules
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
let config = rule_config.config.clone().unwrap_or_default();
let rule = rule.read_json(config);
rules_to_replace.push(RuleWithSeverity::new(rule, severity));
}
}
AllowWarnDeny::Allow => {
if let Some(rule) = rules_for_override
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
let rule = rule.clone();
rules_to_remove.push(rule);
}
}
}
}
_ => {
// For overlapping rule names, use the "error" one
// "no-loss-of-precision": "off",
// "@typescript-eslint/no-loss-of-precision": "error"
if let Some(rule_config) =
rule_configs.iter().find(|r| r.severity.is_warn_deny())
{
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
let config = rule_config.config.clone().unwrap_or_default();
rules_to_replace
.push(RuleWithSeverity::new(rule.read_json(config), rule.severity));
}
} else if rule_configs.iter().all(|r| r.severity.is_allow()) {
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
rules_to_remove.push(rule.clone());
}
}
}
}
}

for rule in rules_to_remove {
rules_for_override.remove(&rule);
}
for rule in rules_to_replace {
rules_for_override.replace(rule);
}
}
}

fn transform_rule_and_plugin_name<'a>(
rule_name: &'a str,
plugin_name: &'a str,
) -> (&'a str, &'a str) {
if plugin_name == "vitest" && is_jest_rule_adapted_to_vitest(rule_name) {
return (rule_name, "jest");
}

(rule_name, plugin_name)
}

impl JsonSchema for OxlintRules {
fn schema_name() -> String {
"OxlintRules".to_owned()
Expand Down Expand Up @@ -189,14 +294,6 @@ fn parse_rule_value(
}
}

impl Deref for OxlintRules {
type Target = Vec<ESLintRule>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

fn failed_to_parse_rule_value(value: &str, err: &str) -> OxcDiagnostic {
OxcDiagnostic::error(format!("Failed to rule value {value:?} with error {err:?}"))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/options/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl OxlintOptions {
}

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

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

0 comments on commit 50834bc

Please sign in to comment.