From 484c41e918c458cc0036882181bbd159db72c6d4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:00:36 -0700 Subject: [PATCH 01/31] docs(linter): Update DefaultRuleConfig deserialization for more complex config shapes. This will make it much easier to handle rules which accept tuples. I still need to fix a few things, though. --- crates/oxc_linter/src/rule.rs | 121 ++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 3220af9cc2d02..4ab976f5d10e9 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -80,7 +80,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// /// # Examples /// -/// ```ignore +/// ```rs /// impl Rule for MyRule { /// fn from_configuration(value: serde_json::Value) -> Self { /// let config = serde_json::from_value::>(value) @@ -89,6 +89,19 @@ pub trait Rule: Sized + Default + fmt::Debug { /// } /// } /// ``` +/// +/// For rules that take a tuple configuration object, e.g. `["foobar", { param: true, other_param: false }]`, you can also use this with a tuple struct: +/// ```rs +/// pub struct MyRuleWithTupleConfig(FirstParamType, SecondParamType); +/// +/// impl Rule for MyRuleWithTupleConfig { +/// fn from_configuration(value: serde_json::Value) -> Self { +/// serde_json::from_value::>(value) +/// .unwrap_or_default() +/// .into_inner(); +/// } +/// } +/// ``` #[derive(Debug, Clone)] pub struct DefaultRuleConfig(T); @@ -117,12 +130,32 @@ where let value = serde_json::Value::deserialize(deserializer)?; + // Even if it only has a single type parameter T, we still expect an array + // for ESLint-style rule configurations. if let serde_json::Value::Array(arr) = value { - let config = arr - .into_iter() - .next() - .and_then(|v| serde_json::from_value(v).ok()) - .unwrap_or_else(T::default); + // If the first element is an object, treat the array as the + // standard ESLint-style `[ configObject, ... ]` and parse the + // first element into `T`. This covers rules where the configuration is just an object. + if let Some(first_elem) = arr.get(0) { + if first_elem.is_object() { + let config = serde_json::from_value::(first_elem.clone()) + .unwrap_or_else(|_| T::default()); + return Ok(DefaultRuleConfig(config)); + } + } + + // Otherwise (e.g. tuple-like configs where the array represents + // the entire tuple-struct), try to deserialize the whole array + // into `T` first. If that fails, fall back to parsing only the + // first element as before. + if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { + return Ok(DefaultRuleConfig(t)); + } + let mut iter = arr.into_iter(); + + let config = + iter.next().and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + Ok(DefaultRuleConfig(config)) } else { Err(D::Error::custom("Expected array for rule configuration")) @@ -391,7 +424,9 @@ impl From for FixKind { #[cfg(test)] mod test { - use crate::{RuleMeta, RuleRunner}; + use rustc_hash::FxHashMap; + + use crate::{RuleMeta, RuleRunner, rule::DefaultRuleConfig}; use super::RuleCategory; @@ -490,6 +525,78 @@ mod test { ); } + #[test] + fn test_deserialize_default_rule_config_single() { + // single element present + let de: DefaultRuleConfig = serde_json::from_str("[123]").unwrap(); + assert_eq!(de.into_inner(), 123u32); + + // empty array should use defaults + let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); + assert_eq!(de.into_inner(), String::default()); + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + struct Obj { + foo: String, + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + struct Pair(u32, Obj); + + #[test] + fn test_deserialize_default_rule_config_tuple() { + // both elements present + let de: DefaultRuleConfig = + serde_json::from_str("[42, { \"foo\": \"abc\" }]").unwrap(); + assert_eq!(de.into_inner(), Pair(42u32, Obj { foo: "abc".to_string() })); + + // only first element present -> parsing the entire array into `Pair` + // will fail, so we fall back to the old behaviour and use T::default(). + let de: DefaultRuleConfig = serde_json::from_str("[10]").unwrap(); + assert_eq!(de.into_inner(), Pair::default()); + // empty array -> both default + let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); + assert_eq!(de.into_inner(), Pair(0u32, Obj::default())); + } + + #[test] + fn test_deserialize_default_rule_config_object_in_array() { + // Single-element array containing an object should parse into the object + // configuration (fallback behavior, not the "entire-array as T" path). + let de: DefaultRuleConfig = serde_json::from_str("[{ \"foo\": \"xyz\" }]").unwrap(); + assert_eq!(de.into_inner(), Obj { foo: "xyz".to_string() }); + + // Extra elements in the array should be ignored when parsing the object + let de: DefaultRuleConfig = + serde_json::from_str("[{ \"foo\": \"yyy\" }, 42]").unwrap(); + assert_eq!(de.into_inner(), Obj { foo: "yyy".to_string() }); + + // Empty array -> default + let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); + assert_eq!(de.into_inner(), Obj::default()); + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + struct ComplexConfig { + #[serde(default)] + foo: FxHashMap, + } + + #[test] + fn test_deserialize_default_rule_config_with_complex_shape() { + // A complex object shape for the rule config, like + // `[ { "foo": { "obj": "value" } } ]`. + let json = r#"[ { "foo": { "obj": "value" } } ]"#; + + let de: DefaultRuleConfig = + serde_json::from_str(json).unwrap(); + + let cfg = de.into_inner(); + let val = cfg.foo.get("obj").expect("obj key present"); + assert_eq!(val, &serde_json::Value::String("value".to_string())); + } + fn assert_rule_runs_on_node_types( rule: &R, node_types: &[oxc_ast::AstType], From f79c1aa52737e1cd9d839fa9b5afa35f69432ea3 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:17:58 -0700 Subject: [PATCH 02/31] refactor(linter): Refactor the DefaultRuleConfig implementation to simplify some things. This will hopefully also ensure that the most common case (a default config object for a given rule) is fast. But I still need to simplify the rest of the code here. --- crates/oxc_linter/src/rule.rs | 99 ++++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 24 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 4ab976f5d10e9..40af8a4d718ee 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -132,34 +132,55 @@ where // Even if it only has a single type parameter T, we still expect an array // for ESLint-style rule configurations. - if let serde_json::Value::Array(arr) = value { - // If the first element is an object, treat the array as the - // standard ESLint-style `[ configObject, ... ]` and parse the - // first element into `T`. This covers rules where the configuration is just an object. - if let Some(first_elem) = arr.get(0) { - if first_elem.is_object() { - let config = serde_json::from_value::(first_elem.clone()) - .unwrap_or_else(|_| T::default()); + let serde_json::Value::Array(arr) = value else { + return Err(D::Error::custom("Expected array for rule configuration")); + }; + + if arr.is_empty() { + return Ok(DefaultRuleConfig(T::default())); + } + + // If the first *and only* element is an object, parse that as + // the configuration object. + if arr.len() == 1 { + let first_elem = &arr[0]; + if first_elem.is_object() { + // 1) Try the simple case: parse the single object as `T`. + if let Ok(config) = serde_json::from_value::(first_elem.clone()) { return Ok(DefaultRuleConfig(config)); } - } - // Otherwise (e.g. tuple-like configs where the array represents - // the entire tuple-struct), try to deserialize the whole array - // into `T` first. If that fails, fall back to parsing only the - // first element as before. - if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { - return Ok(DefaultRuleConfig(t)); - } - let mut iter = arr.into_iter(); + // 2) Sometimes `T` is a tuple-struct of two object types and the + // config was given as `[ {..} ]`. Try faking a second empty + // object so the tuple deserializes correctly into + // `T( first_obj, Default::default() )` when the second type + // is structurally compatible with `{}`. + let empty_map = serde_json::Map::new(); + let second = serde_json::Value::Object(empty_map); + let arr_two = serde_json::Value::Array(vec![first_elem.clone(), second]); + if let Ok(config) = serde_json::from_value::(arr_two) { + return Ok(DefaultRuleConfig(config)); + } - let config = - iter.next().and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + // Fallback: parsing didn't succeed; return T::default() + return Ok(DefaultRuleConfig(T::default())); + } + } - Ok(DefaultRuleConfig(config)) - } else { - Err(D::Error::custom("Expected array for rule configuration")) + // Otherwise (e.g. tuple-like configs where the array represents + // the entire tuple-struct), try to deserialize the whole array + // into `T` first. If that fails, fall back to parsing only the + // first element as before. + if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { + return Ok(DefaultRuleConfig(t)); } + + let mut iter = arr.into_iter(); + + let config = + iter.next().and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + + Ok(DefaultRuleConfig(config)) } } @@ -538,6 +559,7 @@ mod test { #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] struct Obj { + #[serde(default)] foo: String, } @@ -589,14 +611,43 @@ mod test { // `[ { "foo": { "obj": "value" } } ]`. let json = r#"[ { "foo": { "obj": "value" } } ]"#; - let de: DefaultRuleConfig = - serde_json::from_str(json).unwrap(); + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); let cfg = de.into_inner(); let val = cfg.foo.get("obj").expect("obj key present"); assert_eq!(val, &serde_json::Value::String("value".to_string())); } + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + struct Obj2 { + #[serde(default)] + bar: bool, + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + struct ExampleTupleConfig(Obj, Obj2); + + #[test] + fn test_deserialize_default_rule_with_two_object_tuple() { + // Test a rule config that is a tuple of two objects. + let json = r#"[{ "foo": "fooval" }, { "bar": true }]"#; + + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + + let cfg = de.into_inner(); + assert_eq!(cfg, ExampleTupleConfig(Obj { foo: "fooval".to_string() }, Obj2 { bar: true })); + + // Ensure that we can pass just one value and it'll provide the default for the second. + let json = r#"[{ "foo": "onlyfooval" }]"#; + + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + let cfg = de.into_inner(); + assert_eq!( + cfg, + ExampleTupleConfig(Obj { foo: "onlyfooval".to_string() }, Obj2 { bar: false }) + ); + } + fn assert_rule_runs_on_node_types( rule: &R, node_types: &[oxc_ast::AstType], From 205409540b3fc59f6ad4a470763cde2774410794 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:25:52 -0700 Subject: [PATCH 03/31] Add more tests for the DefaultRuleConfig behavior. --- crates/oxc_linter/src/rule.rs | 45 +++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 40af8a4d718ee..c276fc189b6b7 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -551,6 +551,10 @@ mod test { // single element present let de: DefaultRuleConfig = serde_json::from_str("[123]").unwrap(); assert_eq!(de.into_inner(), 123u32); + let de: DefaultRuleConfig = serde_json::from_str("[true]").unwrap(); + assert_eq!(de.into_inner(), true); + let de: DefaultRuleConfig = serde_json::from_str("[false]").unwrap(); + assert_eq!(de.into_inner(), false); // empty array should use defaults let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); @@ -570,7 +574,7 @@ mod test { fn test_deserialize_default_rule_config_tuple() { // both elements present let de: DefaultRuleConfig = - serde_json::from_str("[42, { \"foo\": \"abc\" }]").unwrap(); + serde_json::from_str(r#"[42, { "foo": "abc" }]"#).unwrap(); assert_eq!(de.into_inner(), Pair(42u32, Obj { foo: "abc".to_string() })); // only first element present -> parsing the entire array into `Pair` @@ -586,12 +590,12 @@ mod test { fn test_deserialize_default_rule_config_object_in_array() { // Single-element array containing an object should parse into the object // configuration (fallback behavior, not the "entire-array as T" path). - let de: DefaultRuleConfig = serde_json::from_str("[{ \"foo\": \"xyz\" }]").unwrap(); + let de: DefaultRuleConfig = serde_json::from_str(r#"[{ "foo": "xyz" }]"#).unwrap(); assert_eq!(de.into_inner(), Obj { foo: "xyz".to_string() }); // Extra elements in the array should be ignored when parsing the object let de: DefaultRuleConfig = - serde_json::from_str("[{ \"foo\": \"yyy\" }, 42]").unwrap(); + serde_json::from_str(r#"[{ "foo": "yyy" }, 42]"#).unwrap(); assert_eq!(de.into_inner(), Obj { foo: "yyy".to_string() }); // Empty array -> default @@ -600,8 +604,8 @@ mod test { } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[serde(default)] struct ComplexConfig { - #[serde(default)] foo: FxHashMap, } @@ -619,12 +623,13 @@ mod test { } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[serde(default)] struct Obj2 { - #[serde(default)] bar: bool, } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[serde(default)] struct ExampleTupleConfig(Obj, Obj2); #[test] @@ -648,6 +653,36 @@ mod test { ); } + #[derive(serde::Deserialize, Debug, PartialEq, Eq)] + #[serde(default)] + struct ExampleObjConfig { + baz: String, + qux: bool, + } + + impl Default for ExampleObjConfig { + fn default() -> Self { + Self { baz: "defaultbaz".to_string(), qux: false } + } + } + + #[test] + fn test_deserialize_default_rule_with_object_with_multiple_fields() { + // Test a rule config that is a simple object with multiple fields. + let json = r#"[{ "baz": "fooval", "qux": true }]"#; + + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + + let cfg = de.into_inner(); + assert_eq!(cfg, ExampleObjConfig { baz: "fooval".to_string(), qux: true }); + + // Ensure that missing fields get their default values. + let json = r#"[{ "qux": true }]"#; + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + let cfg = de.into_inner(); + assert_eq!(cfg, ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }); + } + fn assert_rule_runs_on_node_types( rule: &R, node_types: &[oxc_ast::AstType], From 8c5917392b5bd46e1d0c20aa353fb43d7af733fe Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:38:23 -0700 Subject: [PATCH 04/31] Add more tests, simply code a bit hopefully. --- crates/oxc_linter/src/rule.rs | 122 +++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 33 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index c276fc189b6b7..8228d880e3ecc 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -136,51 +136,53 @@ where return Err(D::Error::custom("Expected array for rule configuration")); }; + // Empty array -> use T::default() if arr.is_empty() { return Ok(DefaultRuleConfig(T::default())); } - // If the first *and only* element is an object, parse that as - // the configuration object. - if arr.len() == 1 { - let first_elem = &arr[0]; - if first_elem.is_object() { - // 1) Try the simple case: parse the single object as `T`. - if let Ok(config) = serde_json::from_value::(first_elem.clone()) { + // Match on the array contents: + // - [arg] where arg is an object => try parsing `arg` as T; if that + // fails, attempt to parse [arg, {}] into T (helps tuple-of-objects) + // otherwise fall back to T::default(). + // - otherwise => try parsing the whole array as T (tuple form). If + // that fails, parse the first element into T or fall back to default. + match arr.as_slice() { + [first] if first.is_object() => { + // Try the simple object-as-T path first. + if let Ok(config) = serde_json::from_value::(first.clone()) { return Ok(DefaultRuleConfig(config)); } - // 2) Sometimes `T` is a tuple-struct of two object types and the - // config was given as `[ {..} ]`. Try faking a second empty - // object so the tuple deserializes correctly into - // `T( first_obj, Default::default() )` when the second type - // is structurally compatible with `{}`. - let empty_map = serde_json::Map::new(); - let second = serde_json::Value::Object(empty_map); - let arr_two = serde_json::Value::Array(vec![first_elem.clone(), second]); + // Attempt the tuple-of-objects case by appending an empty object + // (so `[obj]` -> `[obj, {}]`). If that deserializes to T, accept it. + let arr_two = serde_json::Value::Array(vec![ + first.clone(), + serde_json::Value::Object(serde_json::Map::new()), + ]); if let Ok(config) = serde_json::from_value::(arr_two) { return Ok(DefaultRuleConfig(config)); } - // Fallback: parsing didn't succeed; return T::default() - return Ok(DefaultRuleConfig(T::default())); + // Nothing worked; use T::default(). + Ok(DefaultRuleConfig(T::default())) } - } - - // Otherwise (e.g. tuple-like configs where the array represents - // the entire tuple-struct), try to deserialize the whole array - // into `T` first. If that fails, fall back to parsing only the - // first element as before. - if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { - return Ok(DefaultRuleConfig(t)); - } - - let mut iter = arr.into_iter(); + _ => { + // Try parsing the whole array into T (covers tuple-form configs). + if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { + return Ok(DefaultRuleConfig(t)); + } - let config = - iter.next().and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + // Otherwise parse only the first element or use default. + let config = arr + .into_iter() + .next() + .and_then(|v| serde_json::from_value(v).ok()) + .unwrap_or_else(T::default); - Ok(DefaultRuleConfig(config)) + Ok(DefaultRuleConfig(config)) + } + } } } @@ -594,8 +596,7 @@ mod test { assert_eq!(de.into_inner(), Obj { foo: "xyz".to_string() }); // Extra elements in the array should be ignored when parsing the object - let de: DefaultRuleConfig = - serde_json::from_str(r#"[{ "foo": "yyy" }, 42]"#).unwrap(); + let de: DefaultRuleConfig = serde_json::from_str(r#"[{ "foo": "yyy" }, 42]"#).unwrap(); assert_eq!(de.into_inner(), Obj { foo: "yyy".to_string() }); // Empty array -> default @@ -622,6 +623,61 @@ mod test { assert_eq!(val, &serde_json::Value::String("value".to_string())); } + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[serde(rename_all = "camelCase")] + enum EnumOptions { + #[default] + OptionA, + OptionB, + } + + #[test] + fn test_deserialize_default_rule_config_with_enum_config() { + // A basic enum config option. + let json = r#"["optionA"]"#; + + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + + let cfg = de.into_inner(); + assert_eq!(cfg, EnumOptions::OptionA); + } + + #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] + #[serde(default)] + struct TupleWithEnumAndObjectConfig { + option: EnumOptions, + extra: Obj, + } + + #[test] + fn test_deserialize_default_rule_config_with_enum_and_object() { + // A basic enum config option. + let json = r#"["optionA", { "foo": "bar" }]"#; + + let de: DefaultRuleConfig = + serde_json::from_str(json).unwrap(); + let cfg = de.into_inner(); + + assert_eq!( + cfg, + TupleWithEnumAndObjectConfig { + option: EnumOptions::OptionA, + extra: Obj { foo: "bar".to_string() } + } + ); + + // Ensure that we can pass just one value and it'll provide the default for the second. + let json = r#"["optionB"]"#; + let de: DefaultRuleConfig = + serde_json::from_str(json).unwrap(); + let cfg = de.into_inner(); + + assert_eq!( + cfg, + TupleWithEnumAndObjectConfig { option: EnumOptions::OptionB, extra: Obj::default() } + ); + } + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] #[serde(default)] struct Obj2 { From 986d18c0c2fdcc8a044e0989b425e030e6ca8e4d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:51:44 -0700 Subject: [PATCH 05/31] Updates for tests. --- crates/oxc_linter/src/rule.rs | 66 ++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 8228d880e3ecc..8ff5400c70dd8 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -136,7 +136,7 @@ where return Err(D::Error::custom("Expected array for rule configuration")); }; - // Empty array -> use T::default() + // Empty array -> use T::default() (fast-path; no clones) if arr.is_empty() { return Ok(DefaultRuleConfig(T::default())); } @@ -563,15 +563,28 @@ mod test { assert_eq!(de.into_inner(), String::default()); } - #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[derive(serde::Deserialize, Debug, PartialEq, Eq)] + #[serde(default)] struct Obj { - #[serde(default)] foo: String, } - #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + impl Default for Obj { + fn default() -> Self { + Self { foo: "defaultval".to_string() } + } + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq)] + #[serde(default)] struct Pair(u32, Obj); + impl Default for Pair { + fn default() -> Self { + Self(123u32, Obj::default()) + } + } + #[test] fn test_deserialize_default_rule_config_tuple() { // both elements present @@ -582,10 +595,11 @@ mod test { // only first element present -> parsing the entire array into `Pair` // will fail, so we fall back to the old behaviour and use T::default(). let de: DefaultRuleConfig = serde_json::from_str("[10]").unwrap(); - assert_eq!(de.into_inner(), Pair::default()); + assert_eq!(de.into_inner(), Pair(10u32, Obj { foo: "defaultval".to_string() })); + // empty array -> both default let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); - assert_eq!(de.into_inner(), Pair(0u32, Obj::default())); + assert_eq!(de.into_inner(), Pair(123u32, Obj { foo: "defaultval".to_string() })); } #[test] @@ -601,7 +615,7 @@ mod test { // Empty array -> default let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); - assert_eq!(de.into_inner(), Obj::default()); + assert_eq!(de.into_inner(), Obj { foo: "defaultval".to_string() }); } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] @@ -615,10 +629,9 @@ mod test { // A complex object shape for the rule config, like // `[ { "foo": { "obj": "value" } } ]`. let json = r#"[ { "foo": { "obj": "value" } } ]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); + let val = cfg.foo.get("obj").expect("obj key present"); assert_eq!(val, &serde_json::Value::String("value".to_string())); } @@ -635,10 +648,9 @@ mod test { fn test_deserialize_default_rule_config_with_enum_config() { // A basic enum config option. let json = r#"["optionA"]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); + assert_eq!(cfg, EnumOptions::OptionA); } @@ -653,13 +665,11 @@ mod test { fn test_deserialize_default_rule_config_with_enum_and_object() { // A basic enum config option. let json = r#"["optionA", { "foo": "bar" }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); assert_eq!( - cfg, + de.into_inner(), TupleWithEnumAndObjectConfig { option: EnumOptions::OptionA, extra: Obj { foo: "bar".to_string() } @@ -670,11 +680,13 @@ mod test { let json = r#"["optionB"]"#; let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); assert_eq!( - cfg, - TupleWithEnumAndObjectConfig { option: EnumOptions::OptionB, extra: Obj::default() } + de.into_inner(), + TupleWithEnumAndObjectConfig { + option: EnumOptions::OptionB, + extra: Obj { foo: "defaultval".to_string() } + } ); } @@ -692,19 +704,19 @@ mod test { fn test_deserialize_default_rule_with_two_object_tuple() { // Test a rule config that is a tuple of two objects. let json = r#"[{ "foo": "fooval" }, { "bar": true }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); - assert_eq!(cfg, ExampleTupleConfig(Obj { foo: "fooval".to_string() }, Obj2 { bar: true })); + assert_eq!( + de.into_inner(), + ExampleTupleConfig(Obj { foo: "fooval".to_string() }, Obj2 { bar: true }) + ); // Ensure that we can pass just one value and it'll provide the default for the second. let json = r#"[{ "foo": "onlyfooval" }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); + assert_eq!( - cfg, + de.into_inner(), ExampleTupleConfig(Obj { foo: "onlyfooval".to_string() }, Obj2 { bar: false }) ); } @@ -726,17 +738,15 @@ mod test { fn test_deserialize_default_rule_with_object_with_multiple_fields() { // Test a rule config that is a simple object with multiple fields. let json = r#"[{ "baz": "fooval", "qux": true }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); - assert_eq!(cfg, ExampleObjConfig { baz: "fooval".to_string(), qux: true }); + assert_eq!(de.into_inner(), ExampleObjConfig { baz: "fooval".to_string(), qux: true }); // Ensure that missing fields get their default values. let json = r#"[{ "qux": true }]"#; let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); - assert_eq!(cfg, ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }); + + assert_eq!(de.into_inner(), ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }); } fn assert_rule_runs_on_node_types( From 766f7c7568bac575ca3361650f827b57767bcd74 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:53:04 -0700 Subject: [PATCH 06/31] I'm sure this can be done better, but I've done my best trying to optimize it with this. --- crates/oxc_linter/src/rule.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 8ff5400c70dd8..32436f0254293 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -168,17 +168,15 @@ where Ok(DefaultRuleConfig(T::default())) } _ => { - // Try parsing the whole array into T (covers tuple-form configs). - if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr.clone())) { + let first = arr.get(0).cloned(); + + if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr)) { return Ok(DefaultRuleConfig(t)); } - // Otherwise parse only the first element or use default. - let config = arr - .into_iter() - .next() - .and_then(|v| serde_json::from_value(v).ok()) - .unwrap_or_else(T::default); + // Parsing the whole array failed; parse first element if present. + let config = + first.and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); Ok(DefaultRuleConfig(config)) } From 0e5ecc95d17dd75bae4d8ff19decf740dfd46c05 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:08:41 -0700 Subject: [PATCH 07/31] refactor(linter): Rewrite SortKeys config options to be a lot simpler now that we can do tuples in DefaultRuleConfig. This is a proof-of-concept to confirm that these updates to the DefaultRuleConfig tuple handling work as intended and simplify the code :) --- .../oxc_linter/src/rules/eslint/sort_keys.rs | 71 +++++-------------- 1 file changed, 17 insertions(+), 54 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/sort_keys.rs b/crates/oxc_linter/src/rules/eslint/sort_keys.rs index 22097dd7865c6..ad9007d87ea2d 100644 --- a/crates/oxc_linter/src/rules/eslint/sort_keys.rs +++ b/crates/oxc_linter/src/rules/eslint/sort_keys.rs @@ -8,14 +8,18 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize}; -use crate::{AstNode, context::LintContext, rule::Rule}; +use crate::{ + AstNode, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, +}; -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, Clone, Deserialize)] pub struct SortKeys(Box); -#[derive(Debug, Default, Clone, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Default, Clone, Eq, PartialEq, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] /// Sorting order for keys. Accepts "asc" for ascending or "desc" for descending. pub enum SortOrder { @@ -24,7 +28,7 @@ pub enum SortOrder { Asc, } -#[derive(Debug, Clone, JsonSchema)] +#[derive(Debug, Clone, JsonSchema, Deserialize)] #[serde(rename_all = "camelCase", default)] pub struct SortKeysOptions { /// Whether the sort comparison is case-sensitive (A < a when true). @@ -49,19 +53,10 @@ impl Default for SortKeysOptions { } } -#[derive(Debug, Default, Clone, JsonSchema)] +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] #[serde(default)] pub struct SortKeysConfig(SortOrder, SortKeysOptions); -impl SortKeys { - fn sort_order(&self) -> &SortOrder { - &(*self.0).0 - } - fn options(&self) -> &SortKeysOptions { - &(*self.0).1 - } -} - fn sort_properties_diagnostic(span: Span) -> OxcDiagnostic { OxcDiagnostic::warn("Object keys should be sorted").with_label(span) } @@ -102,50 +97,18 @@ declare_oxc_lint!( impl Rule for SortKeys { fn from_configuration(value: serde_json::Value) -> Self { - let Some(config_array) = value.as_array() else { - return Self::default(); - }; - - let sort_order = if config_array.is_empty() { - SortOrder::Asc - } else { - config_array[0].as_str().map_or(SortOrder::Asc, |s| match s { - "desc" => SortOrder::Desc, - _ => SortOrder::Asc, - }) - }; - - let config = if config_array.len() > 1 { - config_array[1].as_object().unwrap() - } else { - &serde_json::Map::new() - }; - - let case_sensitive = - config.get("caseSensitive").and_then(serde_json::Value::as_bool).unwrap_or(true); - let natural = config.get("natural").and_then(serde_json::Value::as_bool).unwrap_or(false); - let min_keys = config - .get("minKeys") - .and_then(serde_json::Value::as_u64) - .map_or(2, |n| n.try_into().unwrap_or(2)); - let allow_line_separated_groups = config - .get("allowLineSeparatedGroups") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - - Self(Box::new(SortKeysConfig( - sort_order, - SortKeysOptions { case_sensitive, natural, min_keys, allow_line_separated_groups }, - ))) + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::ObjectExpression(dec) = node.kind() { - let options = self.options(); + let SortKeysConfig(sort_order, options) = &*self.0; + if dec.properties.len() < options.min_keys { return; } - let sort_order = self.sort_order().clone(); let mut property_groups: Vec> = vec![vec![]]; @@ -194,7 +157,7 @@ impl Rule for SortKeys { alphanumeric_sort(group); } - if sort_order == SortOrder::Desc { + if sort_order == &SortOrder::Desc { group.reverse(); } } @@ -264,7 +227,7 @@ impl Rule for SortKeys { } else { alphanumeric_sort(&mut sorted_keys); } - if sort_order == SortOrder::Desc { + if sort_order == &SortOrder::Desc { sorted_keys.reverse(); } From 6b7ccaabd5929eaeb28dff8b3909a3399c4f1b82 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:08:50 -0700 Subject: [PATCH 08/31] fmt --- crates/oxc_linter/src/rules/eslint/sort_keys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/sort_keys.rs b/crates/oxc_linter/src/rules/eslint/sort_keys.rs index ad9007d87ea2d..84d48aae07922 100644 --- a/crates/oxc_linter/src/rules/eslint/sort_keys.rs +++ b/crates/oxc_linter/src/rules/eslint/sort_keys.rs @@ -8,7 +8,7 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; use schemars::JsonSchema; -use serde::{Deserialize}; +use serde::Deserialize; use crate::{ AstNode, From d0b25c0efa77c906e9907dbd9c799de066f00762 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:19:48 -0700 Subject: [PATCH 09/31] docs(linter): Provide correct config docs for eslint/no-inner-declarations rule. I'm pretty sure there's a bug in this rule. It doesn't seem like it should've been returning `None` for the `block_scoped_functions` value by default, the original ESLint rule returns "allow" by default. But I left the behavior alone here, we can address that later if we want to. --- .../src/rules/eslint/no_inner_declarations.rs | 57 +++++++------------ 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs index cb69e1abe1153..749a34f89b685 100644 --- a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs +++ b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs @@ -5,7 +5,11 @@ use oxc_span::Span; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::{AstNode, context::LintContext, rule::Rule}; +use crate::{ + AstNode, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, +}; fn no_inner_declarations_diagnostic(decl_type: &str, body: &str, span: Span) -> OxcDiagnostic { OxcDiagnostic::warn("Variable or `function` declarations are not allowed in nested blocks") @@ -15,17 +19,18 @@ fn no_inner_declarations_diagnostic(decl_type: &str, body: &str, span: Span) -> #[derive(Debug, Default, Clone, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase", default)] -pub struct NoInnerDeclarations { - /// Determines what type of declarations to check. - config: NoInnerDeclarationsConfig, +pub struct NoInnerDeclarations(NoInnerDeclarationsMode, NoInnerDeclarationsConfigObject); + +#[derive(Debug, Default, Clone, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", default)] +struct NoInnerDeclarationsConfigObject { /// Controls whether function declarations in nested blocks are allowed in strict mode (ES6+ behavior). - #[schemars(with = "BlockScopedFunctions")] block_scoped_functions: Option, } #[derive(Debug, Default, Clone, Copy, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] -enum NoInnerDeclarationsConfig { +enum NoInnerDeclarationsMode { /// Disallows function declarations in nested blocks. #[default] Functions, @@ -78,35 +83,18 @@ declare_oxc_lint!( impl Rule for NoInnerDeclarations { fn from_configuration(value: serde_json::Value) -> Self { - let config = value.get(0).and_then(serde_json::Value::as_str).map_or_else( - NoInnerDeclarationsConfig::default, - |value| match value { - "functions" => NoInnerDeclarationsConfig::Functions, - _ => NoInnerDeclarationsConfig::Both, - }, - ); - - let block_scoped_functions = if value.is_array() && !value.is_null() { - value - .get(1) - .and_then(|v| v.get("blockScopedFunctions")) - .and_then(serde_json::Value::as_str) - .map(|value| match value { - "disallow" => BlockScopedFunctions::Disallow, - _ => BlockScopedFunctions::Allow, - }) - .or(Some(BlockScopedFunctions::Allow)) - } else { - None - }; - - Self { config, block_scoped_functions } + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let NoInnerDeclarations(mode, NoInnerDeclarationsConfigObject { block_scoped_functions }) = + &self; + match node.kind() { AstKind::VariableDeclaration(decl) => { - if self.config == NoInnerDeclarationsConfig::Functions || !decl.kind.is_var() { + if mode == &NoInnerDeclarationsMode::Functions || !decl.kind.is_var() { return; } @@ -117,9 +105,8 @@ impl Rule for NoInnerDeclarations { return; } - if self.config == NoInnerDeclarationsConfig::Functions - && let Some(block_scoped_functions) = self.block_scoped_functions - && block_scoped_functions == BlockScopedFunctions::Allow + if mode == &NoInnerDeclarationsMode::Functions + && block_scoped_functions == &Some(BlockScopedFunctions::Allow) { let is_module = ctx.source_type().is_module(); let scope_id = node.scope_id(); @@ -200,8 +187,8 @@ fn test() { ("if (test) { var foo; }", None), ("if (test) { let x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 6 }, ("if (test) { const x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 6 }, - ("if (test) { using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module", }, - ("if (test) { await using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module", }, + ("if (test) { using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module" }, + ("if (test) { await using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module" }, ("function doSomething() { while (test) { var foo; } }", None), ("var foo;", Some(serde_json::json!(["both"]))), ("var foo = 42;", Some(serde_json::json!(["both"]))), From d32281eed5fae78e65cb36b2d944221acf94a1f7 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:44:51 -0700 Subject: [PATCH 10/31] Clean the code up a bit. I'm pretty sure the no_inner_declarations rule has a bug in it right now, so it failing here isn't really indicative of any problem with the . It's very odd to me that it was defaulting to None for the `block_scoped_functions` instead of `Allow`, I don't understand why that was done and it seems like there are behavioral differences vs. the original rule as a result. --- crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs index 749a34f89b685..3d222a9ab96bb 100644 --- a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs +++ b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs @@ -25,6 +25,7 @@ pub struct NoInnerDeclarations(NoInnerDeclarationsMode, NoInnerDeclarationsConfi #[serde(rename_all = "camelCase", default)] struct NoInnerDeclarationsConfigObject { /// Controls whether function declarations in nested blocks are allowed in strict mode (ES6+ behavior). + #[schemars(with = "BlockScopedFunctions")] block_scoped_functions: Option, } @@ -89,8 +90,7 @@ impl Rule for NoInnerDeclarations { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - let NoInnerDeclarations(mode, NoInnerDeclarationsConfigObject { block_scoped_functions }) = - &self; + let NoInnerDeclarations(mode, config) = &self; match node.kind() { AstKind::VariableDeclaration(decl) => { @@ -106,7 +106,7 @@ impl Rule for NoInnerDeclarations { } if mode == &NoInnerDeclarationsMode::Functions - && block_scoped_functions == &Some(BlockScopedFunctions::Allow) + && config.block_scoped_functions == Some(BlockScopedFunctions::Allow) { let is_module = ctx.source_type().is_module(); let scope_id = node.scope_id(); From bbfd138b8fd2dd1a699e4f4356e468081756a6a4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:00:09 -0700 Subject: [PATCH 11/31] Resolve lint warning and fix feedback. --- crates/oxc_linter/src/rule.rs | 2 +- crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 32436f0254293..8c2b8131c5bcf 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -98,7 +98,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// fn from_configuration(value: serde_json::Value) -> Self { /// serde_json::from_value::>(value) /// .unwrap_or_default() -/// .into_inner(); +/// .into_inner() /// } /// } /// ``` diff --git a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs index 3d222a9ab96bb..bd25c7465b963 100644 --- a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs +++ b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs @@ -90,10 +90,10 @@ impl Rule for NoInnerDeclarations { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - let NoInnerDeclarations(mode, config) = &self; - match node.kind() { AstKind::VariableDeclaration(decl) => { + let NoInnerDeclarations(mode, _config) = &self; + if mode == &NoInnerDeclarationsMode::Functions || !decl.kind.is_var() { return; } @@ -101,6 +101,8 @@ impl Rule for NoInnerDeclarations { check_rule(node, ctx); } AstKind::Function(func) => { + let NoInnerDeclarations(mode, config) = &self; + if !func.is_function_declaration() { return; } From 4eeccd316eff125367404fdb41e8feae83a980d8 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:01:33 -0700 Subject: [PATCH 12/31] Resolve feedback. --- crates/oxc_linter/src/rule.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 8c2b8131c5bcf..37ce0ebad5b16 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -80,7 +80,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// /// # Examples /// -/// ```rs +/// ```rust /// impl Rule for MyRule { /// fn from_configuration(value: serde_json::Value) -> Self { /// let config = serde_json::from_value::>(value) @@ -91,7 +91,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// ``` /// /// For rules that take a tuple configuration object, e.g. `["foobar", { param: true, other_param: false }]`, you can also use this with a tuple struct: -/// ```rs +/// ```rust /// pub struct MyRuleWithTupleConfig(FirstParamType, SecondParamType); /// /// impl Rule for MyRuleWithTupleConfig { @@ -591,7 +591,8 @@ mod test { assert_eq!(de.into_inner(), Pair(42u32, Obj { foo: "abc".to_string() })); // only first element present -> parsing the entire array into `Pair` - // will fail, so we fall back to the old behaviour and use T::default(). + // will fail, so we parse the first element. Since Pair has #[serde(default)], + // serde will use the default value for the missing second field. let de: DefaultRuleConfig = serde_json::from_str("[10]").unwrap(); assert_eq!(de.into_inner(), Pair(10u32, Obj { foo: "defaultval".to_string() })); From d71a82b3ee5a9af74f0ecd4f89538b2ef87d589a Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:08:49 -0700 Subject: [PATCH 13/31] Resolve lint warnings. --- crates/oxc_linter/src/rule.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 37ce0ebad5b16..4da9bafd295a1 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -168,7 +168,7 @@ where Ok(DefaultRuleConfig(T::default())) } _ => { - let first = arr.get(0).cloned(); + let first = arr.first().cloned(); if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr)) { return Ok(DefaultRuleConfig(t)); @@ -552,9 +552,9 @@ mod test { let de: DefaultRuleConfig = serde_json::from_str("[123]").unwrap(); assert_eq!(de.into_inner(), 123u32); let de: DefaultRuleConfig = serde_json::from_str("[true]").unwrap(); - assert_eq!(de.into_inner(), true); + assert!(de.into_inner()); let de: DefaultRuleConfig = serde_json::from_str("[false]").unwrap(); - assert_eq!(de.into_inner(), false); + assert!(!de.into_inner()); // empty array should use defaults let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); @@ -648,9 +648,8 @@ mod test { // A basic enum config option. let json = r#"["optionA"]"#; let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); - assert_eq!(cfg, EnumOptions::OptionA); + assert_eq!(de.into_inner(), EnumOptions::OptionA); } #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] From 96cf9aec775b9ce84995eaeaa4f1aeae142fe90a Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:10:15 -0700 Subject: [PATCH 14/31] doctests lol --- crates/oxc_linter/src/rule.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 4da9bafd295a1..b12514906aeae 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -80,7 +80,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// /// # Examples /// -/// ```rust +/// ```ignore /// impl Rule for MyRule { /// fn from_configuration(value: serde_json::Value) -> Self { /// let config = serde_json::from_value::>(value) @@ -91,7 +91,7 @@ pub trait Rule: Sized + Default + fmt::Debug { /// ``` /// /// For rules that take a tuple configuration object, e.g. `["foobar", { param: true, other_param: false }]`, you can also use this with a tuple struct: -/// ```rust +/// ```ignore /// pub struct MyRuleWithTupleConfig(FirstParamType, SecondParamType); /// /// impl Rule for MyRuleWithTupleConfig { From 5c771ef37c543cd5fae758e3435300985271a437 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:42:21 -0700 Subject: [PATCH 15/31] docs(linter): Update eslint/func-style rule to fix the config option docs. --- .../oxc_linter/src/rules/eslint/func_style.rs | 126 ++++++++---------- 1 file changed, 54 insertions(+), 72 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/func_style.rs b/crates/oxc_linter/src/rules/eslint/func_style.rs index 4594abf1a70d3..609d53b8a3ced 100644 --- a/crates/oxc_linter/src/rules/eslint/func_style.rs +++ b/crates/oxc_linter/src/rules/eslint/func_style.rs @@ -1,4 +1,8 @@ -use crate::{ast_util::nth_outermost_paren_parent, context::LintContext, rule::Rule}; +use crate::{ + ast_util::nth_outermost_paren_parent, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, +}; use oxc_ast::{AstKind, ast::FunctionType}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -9,8 +13,9 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; -fn func_style_diagnostic(span: Span, style: &str) -> OxcDiagnostic { - OxcDiagnostic::warn(format!("Expected a function {style}.")) +fn func_style_diagnostic(span: Span, style: &Style) -> OxcDiagnostic { + let style_str = if style == &Style::Declaration { "declaration" } else { "expression" }; + OxcDiagnostic::warn(format!("Expected a function {style_str}.")) .with_help("Enforce the consistent use of either `function` declarations or expressions assigned to variables") .with_label(span) } @@ -22,19 +27,6 @@ enum Style { Expression, Declaration, } -impl From<&str> for Style { - fn from(raw: &str) -> Self { - if raw == "declaration" { Self::Declaration } else { Self::Expression } - } -} -impl Style { - pub fn as_str(&self) -> &str { - match self { - Style::Expression => "expression", - Style::Declaration => "declaration", - } - } -} #[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] @@ -44,26 +36,25 @@ enum NamedExports { Expression, Declaration, } -impl From<&str> for NamedExports { - fn from(raw: &str) -> Self { - match raw { - "expression" => Self::Expression, - "declaration" => Self::Declaration, - _ => Self::Ignore, - } - } -} #[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase", default)] -pub struct FuncStyle { - /// The style to enforce. Either "expression" (default) or "declaration". - style: Style, +pub struct FuncStyle(Style, FuncStyleConfig); + +#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase", default)] +struct FuncStyleConfig { /// When true, arrow functions are allowed regardless of the style setting. allow_arrow_functions: bool, /// When true, functions with type annotations are allowed regardless of the style setting. allow_type_annotation: bool, /// Override the style specifically for named exports. Can be "expression", "declaration", or "ignore" (default). + overrides: Overrides, +} + +#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase", default)] +struct Overrides { named_exports: Option, } @@ -205,29 +196,15 @@ fn is_ancestor_export_name_decl<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) - impl Rule for FuncStyle { fn from_configuration(value: Value) -> Self { - let obj1 = value.get(0); - let obj2 = value.get(1); - - Self { - style: obj1.and_then(Value::as_str).map(Style::from).unwrap_or_default(), - allow_arrow_functions: obj2 - .and_then(|v| v.get("allowArrowFunctions")) - .and_then(Value::as_bool) - .unwrap_or(false), - allow_type_annotation: obj2 - .and_then(|v| v.get("allowTypeAnnotation")) - .and_then(Value::as_bool) - .unwrap_or(false), - named_exports: obj2 - .and_then(|v| v.get("overrides")) - .and_then(|v| v.get("namedExports")) - .and_then(Value::as_str) - .map(NamedExports::from), - } + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() } + fn run_once<'a>(&self, ctx: &LintContext) { + let FuncStyle(style, config) = &self; let semantic = ctx.semantic(); - let is_decl_style = self.style == Style::Declaration; + let is_decl_style = style == &Style::Declaration; // step 1 // We can iterate over ctx.nodes() and process FunctionDeclaration and FunctionExpression, @@ -262,52 +239,53 @@ impl Rule for FuncStyle { let should_diagnostic = match parent.kind() { AstKind::ExportDefaultDeclaration(_) => false, AstKind::ExportNamedDeclaration(_) => { - self.named_exports.is_none() + config.overrides.named_exports.is_none() } _ => true, }; if should_diagnostic { - ctx.diagnostic(func_style_diagnostic( - func.span, - self.style.as_str(), - )); + ctx.diagnostic(func_style_diagnostic(func.span, style)); } } - if self.named_exports == Some(NamedExports::Expression) + if config.overrides.named_exports == Some(NamedExports::Expression) && matches!(parent.kind(), AstKind::ExportNamedDeclaration(_)) { - ctx.diagnostic(func_style_diagnostic(func.span, "expression")); + ctx.diagnostic(func_style_diagnostic( + func.span, + &Style::Expression, + )); } } FunctionType::FunctionExpression => { let is_ancestor_export = is_ancestor_export_name_decl(node, ctx); if let AstKind::VariableDeclarator(decl) = parent.kind() { - let is_type_annotation = - self.allow_type_annotation && decl.id.type_annotation.is_some(); + let is_type_annotation = config.allow_type_annotation + && decl.id.type_annotation.is_some(); if is_type_annotation { continue; } if is_decl_style - && (self.named_exports.is_none() || !is_ancestor_export) + && (config.overrides.named_exports.is_none() + || !is_ancestor_export) { - ctx.diagnostic(func_style_diagnostic( - decl.span, - self.style.as_str(), - )); + ctx.diagnostic(func_style_diagnostic(decl.span, style)); } - if self.named_exports == Some(NamedExports::Declaration) + if config.overrides.named_exports == Some(NamedExports::Declaration) && is_ancestor_export { - ctx.diagnostic(func_style_diagnostic(decl.span, "declaration")); + ctx.diagnostic(func_style_diagnostic( + decl.span, + &Style::Declaration, + )); } } } _ => {} } } - AstKind::ThisExpression(_) | AstKind::Super(_) if !self.allow_arrow_functions => { + AstKind::ThisExpression(_) | AstKind::Super(_) if !config.allow_arrow_functions => { // We need to determine if the recent FunctionBody is an arrow function let arrow_func_ancestor = semantic .nodes() @@ -318,7 +296,7 @@ impl Rule for FuncStyle { arrow_func_ancestor_records.insert(ret.id()); } } - AstKind::ArrowFunctionExpression(_) if !self.allow_arrow_functions => { + AstKind::ArrowFunctionExpression(_) if !config.allow_arrow_functions => { arrow_func_nodes.push(node); } _ => {} @@ -332,17 +310,21 @@ impl Rule for FuncStyle { let parent = semantic.nodes().parent_node(node.id()); if let AstKind::VariableDeclarator(decl) = parent.kind() { let is_type_annotation = - self.allow_type_annotation && decl.id.type_annotation.is_some(); + config.allow_type_annotation && decl.id.type_annotation.is_some(); if is_type_annotation { continue; } let is_ancestor_export = is_ancestor_export_name_decl(node, ctx); - if is_decl_style && (self.named_exports.is_none() || !is_ancestor_export) { - ctx.diagnostic(func_style_diagnostic(decl.span, "declaration")); + if is_decl_style + && (config.overrides.named_exports.is_none() || !is_ancestor_export) + { + ctx.diagnostic(func_style_diagnostic(decl.span, &Style::Declaration)); } - if self.named_exports == Some(NamedExports::Declaration) && is_ancestor_export { - ctx.diagnostic(func_style_diagnostic(decl.span, "declaration")); + if config.overrides.named_exports == Some(NamedExports::Declaration) + && is_ancestor_export + { + ctx.diagnostic(func_style_diagnostic(decl.span, &Style::Declaration)); } } } @@ -598,7 +580,7 @@ fn test() { ( "export const expression: Fn = function () {}", Some( - serde_json::json!(["expression", { "allowTypeAnnotation": true, "overrides": { "namedExports": "declaration" } }]), + serde_json::json!(["expression", { "allowTypeAnnotation": true, "overrides": { "namedExports": "declaration" } }]), ), ), ( From 414a7f544b411cf64f6f87095bc245a9994761ee Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:53:44 -0700 Subject: [PATCH 16/31] docs(linter): Add auto-generated config option docs for `eslint/arrow-body-style` rule. --- .../src/rules/eslint/arrow_body_style.rs | 183 ++++++++---------- .../rule_configuration_documentation_test.rs | 1 - 2 files changed, 78 insertions(+), 106 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/arrow_body_style.rs b/crates/oxc_linter/src/rules/eslint/arrow_body_style.rs index b70a4fcc152f9..bac9c6dffddd5 100644 --- a/crates/oxc_linter/src/rules/eslint/arrow_body_style.rs +++ b/crates/oxc_linter/src/rules/eslint/arrow_body_style.rs @@ -1,3 +1,5 @@ +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; use serde_json::Value; use oxc_ast::{ @@ -13,7 +15,7 @@ use crate::{ AstNode, context::LintContext, fixer::{RuleFix, RuleFixer}, - rule::Rule, + rule::{DefaultRuleConfig, Rule}, }; fn arrow_body_style_diagnostic(span: Span, msg: &str) -> OxcDiagnostic { @@ -23,76 +25,14 @@ fn arrow_body_style_diagnostic(span: Span, msg: &str) -> OxcDiagnostic { const EXPECTED_BLOCK_MSG: &str = "Expected block statement surrounding arrow body."; const UNEXPECTED_BLOCK_SINGLE_MSG: &str = "Unexpected block statement surrounding arrow body; move the returned value immediately after the `=>`."; -#[derive(Debug, Default, PartialEq, Clone)] +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "kebab-case")] enum Mode { - #[default] - AsNeeded, - Always, - Never, -} - -impl Mode { - pub fn from(raw: &str) -> Self { - match raw { - "always" => Self::Always, - "never" => Self::Never, - _ => Self::AsNeeded, - } - } -} - -#[derive(Debug, Default, Clone)] -pub struct ArrowBodyStyle { - mode: Mode, - require_return_for_object_literal: bool, -} - -declare_oxc_lint!( - /// ### What it does - /// - /// This rule can enforce or disallow the use of braces around arrow function body. - /// Arrow functions can use either: - /// - a block body `() => { ... }` - /// - or a concise body `() => expression` with an implicit return. - /// - /// ### Why is this bad? - /// - /// Inconsistent use of block vs. concise bodies makes code harder to read. - /// Concise bodies are limited to a single expression, whose value is implicitly returned. - /// - /// ### Options - /// - /// First option: - /// - Type: `string` - /// - Enum: `"always"`, `"as-needed"`, `"never"` - /// - Default: `"as-needed"` - /// - /// Possible values: - /// * `never` enforces no braces around the function body (constrains arrow functions to the role of returning an expression) - /// * `always` enforces braces around the function body - /// * `as-needed` enforces no braces where they can be omitted (default) - /// - /// Second option: - /// - Type: `object` - /// - Properties: - /// - `requireReturnForObjectLiteral`: `boolean` (default: `false`) - requires braces and an explicit return for object literals. + /// `as-needed` enforces no braces where they can be omitted (default). /// - /// Note: This option only applies when the first option is `"as-needed"`. - /// - /// Example configuration: - /// ```json - /// { - /// "arrow-body-style": ["error", "as-needed", { "requireReturnForObjectLiteral": true }] - /// } - /// ``` - /// - /// ### Examples - /// - /// #### `"never"` (default) - /// - /// Examples of **incorrect** code for this rule with the `never` option: + /// Examples of **incorrect** code for this rule with the `as-needed` option: /// ```js - /// /* arrow-body-style: ["error", "never"] */ + /// /* arrow-body-style: ["error", "as-needed"] */ /// /// /* ✘ Bad: */ /// const foo = () => { @@ -100,16 +40,25 @@ declare_oxc_lint!( /// }; /// ``` /// - /// Examples of **correct** code for this rule with the `never` option: + /// Examples of **correct** code for this rule with the `as-needed` option: /// ```js - /// /* arrow-body-style: ["error", "never"] */ + /// /* arrow-body-style: ["error", "as-needed"] */ /// /// /* ✔ Good: */ - /// const foo = () => 0; - /// const bar = () => ({ foo: 0 }); - /// ``` + /// const foo1 = () => 0; /// - /// #### `"always"` + /// const foo2 = (retv, name) => { + /// retv[name] = true; + /// return retv; + /// }; + /// + /// const foo3 = () => { + /// bar(); + /// }; + /// ``` + #[default] + AsNeeded, + /// `always` enforces braces around the function body. /// /// Examples of **incorrect** code for this rule with the `always` option: /// ```js @@ -128,12 +77,12 @@ declare_oxc_lint!( /// return 0; /// }; /// ``` + Always, + /// `never` enforces no braces around the function body (constrains arrow functions to the role of returning an expression) /// - /// #### `"as-needed"` - /// - /// Examples of **incorrect** code for this rule with the `as-needed` option: + /// Examples of **incorrect** code for this rule with the `never` option: /// ```js - /// /* arrow-body-style: ["error", "as-needed"] */ + /// /* arrow-body-style: ["error", "never"] */ /// /// /* ✘ Bad: */ /// const foo = () => { @@ -141,24 +90,27 @@ declare_oxc_lint!( /// }; /// ``` /// - /// Examples of **correct** code for this rule with the `as-needed` option: + /// Examples of **correct** code for this rule with the `never` option: /// ```js - /// /* arrow-body-style: ["error", "as-needed"] */ + /// /* arrow-body-style: ["error", "never"] */ /// /// /* ✔ Good: */ - /// const foo1 = () => 0; - /// - /// const foo2 = (retv, name) => { - /// retv[name] = true; - /// return retv; - /// }; - /// - /// const foo3 = () => { - /// bar(); - /// }; + /// const foo = () => 0; + /// const bar = () => ({ foo: 0 }); /// ``` + Never, +} + +#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(default)] +pub struct ArrowBodyStyle(Mode, ArrowBodyStyleConfig); + +#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase", default)] +struct ArrowBodyStyleConfig { + /// If `true`, requires braces and an explicit return for object literals. /// - /// #### `"as-needed"` with `requireReturnForObjectLiteral` + /// Note: This option only applies when the first option is `"as-needed"`. /// /// Examples of **incorrect** code for this rule with the `{ "requireReturnForObjectLiteral": true }` option: /// ```js @@ -177,23 +129,42 @@ declare_oxc_lint!( /// const foo = () => {}; /// const bar = () => { return { bar: 0 }; }; /// ``` + require_return_for_object_literal: bool, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule can enforce or disallow the use of braces around arrow function body. + /// Arrow functions can use either: + /// - a block body `() => { ... }` + /// - or a concise body `() => expression` with an implicit return. + /// + /// ### Why is this bad? + /// + /// Inconsistent use of block vs. concise bodies makes code harder to read. + /// Concise bodies are limited to a single expression, whose value is implicitly returned. + /// + /// ### Options + /// + /// Example configuration: + /// ```json + /// { + /// "arrow-body-style": ["error", "as-needed", { "requireReturnForObjectLiteral": true }] + /// } + /// ``` ArrowBodyStyle, eslint, style, fix, + config = ArrowBodyStyle, ); impl Rule for ArrowBodyStyle { fn from_configuration(value: Value) -> Self { - let mode = value.get(0).and_then(Value::as_str).map(Mode::from).unwrap_or_default(); - - let require_return_for_object_literal = value - .get(1) - .and_then(|v| v.get("requireReturnForObjectLiteral")) - .and_then(Value::as_bool) - .unwrap_or(false); - - Self { mode, require_return_for_object_literal } + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { @@ -218,12 +189,13 @@ impl ArrowBodyStyle { arrow_func_expr: &ArrowFunctionExpression<'a>, ctx: &LintContext<'a>, ) { + let ArrowBodyStyle(mode, config) = &self; let body = &arrow_func_expr.body; let inner_expr = arrow_func_expr.get_expression().map(Expression::get_inner_expression); - let should_report = self.mode == Mode::Always - || (self.mode == Mode::AsNeeded - && self.require_return_for_object_literal + let should_report = mode == &Mode::Always + || (mode == &Mode::AsNeeded + && config.require_return_for_object_literal && matches!(inner_expr, Some(Expression::ObjectExpression(_)))); if !should_report { @@ -247,8 +219,9 @@ impl ArrowBodyStyle { ctx: &LintContext<'a>, ) { let body = &arrow_func_expr.body; + let ArrowBodyStyle(mode, config) = &self; - match self.mode { + match mode { Mode::Never => { // Mode::Never: report any block body if body.statements.is_empty() { @@ -289,7 +262,7 @@ impl ArrowBodyStyle { Mode::AsNeeded if body.statements.len() == 1 => { if let Statement::ReturnStatement(return_statement) = &body.statements[0] { // Skip if requireReturnForObjectLiteral and returning an object - if self.require_return_for_object_literal + if config.require_return_for_object_literal && matches!( return_statement.argument, Some(Expression::ObjectExpression(_)) diff --git a/crates/oxc_linter/tests/rule_configuration_documentation_test.rs b/crates/oxc_linter/tests/rule_configuration_documentation_test.rs index 43e53c5d4c2d4..345edcfcd716b 100644 --- a/crates/oxc_linter/tests/rule_configuration_documentation_test.rs +++ b/crates/oxc_linter/tests/rule_configuration_documentation_test.rs @@ -30,7 +30,6 @@ fn test_rules_with_custom_configuration_have_schema() { // list - newly-created rules should always be documented before being merged! let exceptions: &[&str] = &[ // eslint - "eslint/arrow-body-style", "eslint/func-names", "eslint/no-empty-function", "eslint/no-restricted-imports", From 536781c65263a4617b86ea6f8f05676beb3a5208 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:56:49 -0700 Subject: [PATCH 17/31] Revert no_inner_declarations.rs back to its state on main. I'd rather just leave it as-is for now so that we can focus on the other changes in this PR. I'll open another PR with the code cleanup on this rule. --- .../src/rules/eslint/no_inner_declarations.rs | 57 +++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs index bd25c7465b963..cb69e1abe1153 100644 --- a/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs +++ b/crates/oxc_linter/src/rules/eslint/no_inner_declarations.rs @@ -5,11 +5,7 @@ use oxc_span::Span; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::{ - AstNode, - context::LintContext, - rule::{DefaultRuleConfig, Rule}, -}; +use crate::{AstNode, context::LintContext, rule::Rule}; fn no_inner_declarations_diagnostic(decl_type: &str, body: &str, span: Span) -> OxcDiagnostic { OxcDiagnostic::warn("Variable or `function` declarations are not allowed in nested blocks") @@ -19,11 +15,9 @@ fn no_inner_declarations_diagnostic(decl_type: &str, body: &str, span: Span) -> #[derive(Debug, Default, Clone, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase", default)] -pub struct NoInnerDeclarations(NoInnerDeclarationsMode, NoInnerDeclarationsConfigObject); - -#[derive(Debug, Default, Clone, Deserialize, JsonSchema)] -#[serde(rename_all = "camelCase", default)] -struct NoInnerDeclarationsConfigObject { +pub struct NoInnerDeclarations { + /// Determines what type of declarations to check. + config: NoInnerDeclarationsConfig, /// Controls whether function declarations in nested blocks are allowed in strict mode (ES6+ behavior). #[schemars(with = "BlockScopedFunctions")] block_scoped_functions: Option, @@ -31,7 +25,7 @@ struct NoInnerDeclarationsConfigObject { #[derive(Debug, Default, Clone, Copy, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] -enum NoInnerDeclarationsMode { +enum NoInnerDeclarationsConfig { /// Disallows function declarations in nested blocks. #[default] Functions, @@ -84,31 +78,48 @@ declare_oxc_lint!( impl Rule for NoInnerDeclarations { fn from_configuration(value: serde_json::Value) -> Self { - serde_json::from_value::>(value) - .unwrap_or_default() - .into_inner() + let config = value.get(0).and_then(serde_json::Value::as_str).map_or_else( + NoInnerDeclarationsConfig::default, + |value| match value { + "functions" => NoInnerDeclarationsConfig::Functions, + _ => NoInnerDeclarationsConfig::Both, + }, + ); + + let block_scoped_functions = if value.is_array() && !value.is_null() { + value + .get(1) + .and_then(|v| v.get("blockScopedFunctions")) + .and_then(serde_json::Value::as_str) + .map(|value| match value { + "disallow" => BlockScopedFunctions::Disallow, + _ => BlockScopedFunctions::Allow, + }) + .or(Some(BlockScopedFunctions::Allow)) + } else { + None + }; + + Self { config, block_scoped_functions } } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { AstKind::VariableDeclaration(decl) => { - let NoInnerDeclarations(mode, _config) = &self; - - if mode == &NoInnerDeclarationsMode::Functions || !decl.kind.is_var() { + if self.config == NoInnerDeclarationsConfig::Functions || !decl.kind.is_var() { return; } check_rule(node, ctx); } AstKind::Function(func) => { - let NoInnerDeclarations(mode, config) = &self; - if !func.is_function_declaration() { return; } - if mode == &NoInnerDeclarationsMode::Functions - && config.block_scoped_functions == Some(BlockScopedFunctions::Allow) + if self.config == NoInnerDeclarationsConfig::Functions + && let Some(block_scoped_functions) = self.block_scoped_functions + && block_scoped_functions == BlockScopedFunctions::Allow { let is_module = ctx.source_type().is_module(); let scope_id = node.scope_id(); @@ -189,8 +200,8 @@ fn test() { ("if (test) { var foo; }", None), ("if (test) { let x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 6 }, ("if (test) { const x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 6 }, - ("if (test) { using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module" }, - ("if (test) { await using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module" }, + ("if (test) { using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module", }, + ("if (test) { await using x = 1; }", Some(serde_json::json!(["both"]))), // { "ecmaVersion": 2026, "sourceType": "module", }, ("function doSomething() { while (test) { var foo; } }", None), ("var foo;", Some(serde_json::json!(["both"]))), ("var foo = 42;", Some(serde_json::json!(["both"]))), From 962bfe2adf76697a26121ba485e35c9189b3acfb Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sat, 6 Dec 2025 00:02:13 -0700 Subject: [PATCH 18/31] Appease the linter. --- .../oxc_linter/src/rules/eslint/func_style.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/func_style.rs b/crates/oxc_linter/src/rules/eslint/func_style.rs index 609d53b8a3ced..29d8b92b458e8 100644 --- a/crates/oxc_linter/src/rules/eslint/func_style.rs +++ b/crates/oxc_linter/src/rules/eslint/func_style.rs @@ -13,8 +13,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; -fn func_style_diagnostic(span: Span, style: &Style) -> OxcDiagnostic { - let style_str = if style == &Style::Declaration { "declaration" } else { "expression" }; +fn func_style_diagnostic(span: Span, style: Style) -> OxcDiagnostic { + let style_str = if style == Style::Declaration { "declaration" } else { "expression" }; OxcDiagnostic::warn(format!("Expected a function {style_str}.")) .with_help("Enforce the consistent use of either `function` declarations or expressions assigned to variables") .with_label(span) @@ -244,17 +244,14 @@ impl Rule for FuncStyle { _ => true, }; if should_diagnostic { - ctx.diagnostic(func_style_diagnostic(func.span, style)); + ctx.diagnostic(func_style_diagnostic(func.span, *style)); } } if config.overrides.named_exports == Some(NamedExports::Expression) && matches!(parent.kind(), AstKind::ExportNamedDeclaration(_)) { - ctx.diagnostic(func_style_diagnostic( - func.span, - &Style::Expression, - )); + ctx.diagnostic(func_style_diagnostic(func.span, Style::Expression)); } } FunctionType::FunctionExpression => { @@ -269,7 +266,7 @@ impl Rule for FuncStyle { && (config.overrides.named_exports.is_none() || !is_ancestor_export) { - ctx.diagnostic(func_style_diagnostic(decl.span, style)); + ctx.diagnostic(func_style_diagnostic(decl.span, *style)); } if config.overrides.named_exports == Some(NamedExports::Declaration) @@ -277,7 +274,7 @@ impl Rule for FuncStyle { { ctx.diagnostic(func_style_diagnostic( decl.span, - &Style::Declaration, + Style::Declaration, )); } } @@ -318,13 +315,13 @@ impl Rule for FuncStyle { if is_decl_style && (config.overrides.named_exports.is_none() || !is_ancestor_export) { - ctx.diagnostic(func_style_diagnostic(decl.span, &Style::Declaration)); + ctx.diagnostic(func_style_diagnostic(decl.span, Style::Declaration)); } if config.overrides.named_exports == Some(NamedExports::Declaration) && is_ancestor_export { - ctx.diagnostic(func_style_diagnostic(decl.span, &Style::Declaration)); + ctx.diagnostic(func_style_diagnostic(decl.span, Style::Declaration)); } } } From fdc479501231ac23223e0b17adf1954c1644769e Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 12:05:57 -0700 Subject: [PATCH 19/31] Remove all but one usage of clone in DefaultRuleConfig deserialization. --- crates/oxc_linter/src/rule.rs | 80 +++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index b12514906aeae..8603c177f129b 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -130,57 +130,75 @@ where let value = serde_json::Value::deserialize(deserializer)?; - // Even if it only has a single type parameter T, we still expect an array - // for ESLint-style rule configurations. + // Expect an array for ESLint-style rule configs. + // + // The shape should generally be like one of the following: + // `[{ "bar": "baz" }]`, this is the most common + // `["foo"]`, some rules use a single enum/string value + // `["foo", { "bar": "baz" }]`, some rules use a tuple of values let serde_json::Value::Array(arr) = value else { return Err(D::Error::custom("Expected array for rule configuration")); }; - // Empty array -> use T::default() (fast-path; no clones) + // Empty array => use defaults. if arr.is_empty() { return Ok(DefaultRuleConfig(T::default())); } - // Match on the array contents: - // - [arg] where arg is an object => try parsing `arg` as T; if that - // fails, attempt to parse [arg, {}] into T (helps tuple-of-objects) - // otherwise fall back to T::default(). - // - otherwise => try parsing the whole array as T (tuple form). If - // that fails, parse the first element into T or fall back to default. - match arr.as_slice() { - [first] if first.is_object() => { - // Try the simple object-as-T path first. - if let Ok(config) = serde_json::from_value::(first.clone()) { - return Ok(DefaultRuleConfig(config)); - } - - // Attempt the tuple-of-objects case by appending an empty object - // (so `[obj]` -> `[obj, {}]`). If that deserializes to T, accept it. + // Case 1: single element array where the element is an object. + // For rules that use tuple-of-objects configs (e.g. `(Obj, Obj2)`), + // prefer the tuple path and fill in the second element with an + // empty object (letting serde apply defaults). Otherwise, parse the + // single object as T (object config). + if arr.len() == 1 && arr[0].is_object() { + // Consume the single value. + let first = arr.into_iter().next().unwrap(); + + // TODO: This can almost certainly be replaced by a more efficient check for the shape of T? + // + // Detect tuple-of-two-objects by checking whether T can be + // deserialized from two empty objects. If so, attempt the + // `[first, {}]` path and return that result if it succeeds. If + // the tuple attempt fails, or if T doesn't look like a tuple, + // we fall back to parsing the single object as T (or default). + let two_empty = serde_json::Value::Array(vec![ + serde_json::Value::Object(serde_json::Map::new()), + serde_json::Value::Object(serde_json::Map::new()), + ]); + + if serde_json::from_value::(two_empty).is_ok() { let arr_two = serde_json::Value::Array(vec![ - first.clone(), + first, serde_json::Value::Object(serde_json::Map::new()), ]); if let Ok(config) = serde_json::from_value::(arr_two) { return Ok(DefaultRuleConfig(config)); } - // Nothing worked; use T::default(). - Ok(DefaultRuleConfig(T::default())) + // Tuple detection succeeded but parsing failed for `[first, {}]`. + // Use default to be safe. + return Ok(DefaultRuleConfig(T::default())); } - _ => { - let first = arr.first().cloned(); - if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr)) { - return Ok(DefaultRuleConfig(t)); - } + // Not a tuple-of-two-objects; try parsing the single object as T. + if let Ok(config) = serde_json::from_value::(first) { + return Ok(DefaultRuleConfig(config)); + } - // Parsing the whole array failed; parse first element if present. - let config = - first.and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + return Ok(DefaultRuleConfig(T::default())); + } - Ok(DefaultRuleConfig(config)) - } + // Case 2: non-single object shapes (arrays of multiple elements, enums, + // tuples, etc.) — try parsing the whole array into T. If that fails, + // try parsing the first element into T, otherwise fall back to default. + let first = arr.first().cloned(); + + if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr)) { + return Ok(DefaultRuleConfig(t)); } + + let config = first.and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); + Ok(DefaultRuleConfig(config)) } } From 332f8ea3d64a10c7e79c4ef5b50cc31ac8a80e32 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 12:39:37 -0700 Subject: [PATCH 20/31] refactor(linter): Remove support for double-object tuples. This simplifies the config parsing logic *a ton*. --- crates/oxc_linter/src/rule.rs | 76 +++-------------------------------- 1 file changed, 6 insertions(+), 70 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 8603c177f129b..77508c747e2cf 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -145,47 +145,14 @@ where return Ok(DefaultRuleConfig(T::default())); } - // Case 1: single element array where the element is an object. - // For rules that use tuple-of-objects configs (e.g. `(Obj, Obj2)`), - // prefer the tuple path and fill in the second element with an - // empty object (letting serde apply defaults). Otherwise, parse the - // single object as T (object config). + // Case 1: If the first element is an object, assume this is a single-object + // configuration (the most common case). Attempt to parse that object + // into T. If that fails, fall back to default. if arr.len() == 1 && arr[0].is_object() { - // Consume the single value. - let first = arr.into_iter().next().unwrap(); - - // TODO: This can almost certainly be replaced by a more efficient check for the shape of T? - // - // Detect tuple-of-two-objects by checking whether T can be - // deserialized from two empty objects. If so, attempt the - // `[first, {}]` path and return that result if it succeeds. If - // the tuple attempt fails, or if T doesn't look like a tuple, - // we fall back to parsing the single object as T (or default). - let two_empty = serde_json::Value::Array(vec![ - serde_json::Value::Object(serde_json::Map::new()), - serde_json::Value::Object(serde_json::Map::new()), - ]); - - if serde_json::from_value::(two_empty).is_ok() { - let arr_two = serde_json::Value::Array(vec![ - first, - serde_json::Value::Object(serde_json::Map::new()), - ]); - if let Ok(config) = serde_json::from_value::(arr_two) { - return Ok(DefaultRuleConfig(config)); - } - - // Tuple detection succeeded but parsing failed for `[first, {}]`. - // Use default to be safe. - return Ok(DefaultRuleConfig(T::default())); - } + let obj = arr.into_iter().next().unwrap(); + let t = serde_json::from_value::(obj).unwrap_or_else(|_| T::default()); - // Not a tuple-of-two-objects; try parsing the single object as T. - if let Ok(config) = serde_json::from_value::(first) { - return Ok(DefaultRuleConfig(config)); - } - - return Ok(DefaultRuleConfig(T::default())); + return Ok(DefaultRuleConfig(t)); } // Case 2: non-single object shapes (arrays of multiple elements, enums, @@ -706,37 +673,6 @@ mod test { ); } - #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] - #[serde(default)] - struct Obj2 { - bar: bool, - } - - #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] - #[serde(default)] - struct ExampleTupleConfig(Obj, Obj2); - - #[test] - fn test_deserialize_default_rule_with_two_object_tuple() { - // Test a rule config that is a tuple of two objects. - let json = r#"[{ "foo": "fooval" }, { "bar": true }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - - assert_eq!( - de.into_inner(), - ExampleTupleConfig(Obj { foo: "fooval".to_string() }, Obj2 { bar: true }) - ); - - // Ensure that we can pass just one value and it'll provide the default for the second. - let json = r#"[{ "foo": "onlyfooval" }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - - assert_eq!( - de.into_inner(), - ExampleTupleConfig(Obj { foo: "onlyfooval".to_string() }, Obj2 { bar: false }) - ); - } - #[derive(serde::Deserialize, Debug, PartialEq, Eq)] #[serde(default)] struct ExampleObjConfig { From 948e1735d60515ebbad1d9d8c3fd583363ea71c7 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 12:44:47 -0700 Subject: [PATCH 21/31] Add another test case. --- crates/oxc_linter/src/rule.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 77508c747e2cf..525677fa9cda4 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -635,6 +635,12 @@ mod test { let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); assert_eq!(de.into_inner(), EnumOptions::OptionA); + + // Works with non-default value as well. + let json = r#"["optionB"]"#; + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + + assert_eq!(de.into_inner(), EnumOptions::OptionB); } #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] From 3fea7572636caf4bf3ed4d04eaa033d8a2c5f055 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 12:46:55 -0700 Subject: [PATCH 22/31] Fix tuple test to use an actual tuple struct. --- crates/oxc_linter/src/rule.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 525677fa9cda4..f78da1c092b1a 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -645,24 +645,18 @@ mod test { #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] #[serde(default)] - struct TupleWithEnumAndObjectConfig { - option: EnumOptions, - extra: Obj, - } + struct TupleWithEnumAndObjectConfig(EnumOptions, Obj); #[test] fn test_deserialize_default_rule_config_with_enum_and_object() { - // A basic enum config option. + // A basic enum config option with an object. let json = r#"["optionA", { "foo": "bar" }]"#; let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); assert_eq!( de.into_inner(), - TupleWithEnumAndObjectConfig { - option: EnumOptions::OptionA, - extra: Obj { foo: "bar".to_string() } - } + TupleWithEnumAndObjectConfig(EnumOptions::OptionA, Obj { foo: "bar".to_string() }) ); // Ensure that we can pass just one value and it'll provide the default for the second. @@ -672,10 +666,10 @@ mod test { assert_eq!( de.into_inner(), - TupleWithEnumAndObjectConfig { - option: EnumOptions::OptionB, - extra: Obj { foo: "defaultval".to_string() } - } + TupleWithEnumAndObjectConfig( + EnumOptions::OptionB, + Obj { foo: "defaultval".to_string() } + ) ); } From 8375a6d94d248a7a4e601f2ae3de1b944bf34844 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 13:04:41 -0700 Subject: [PATCH 23/31] Remove test for handling of extra elements, no need to support that. --- crates/oxc_linter/src/rule.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index f78da1c092b1a..e3a37ad854527 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -593,10 +593,6 @@ mod test { let de: DefaultRuleConfig = serde_json::from_str(r#"[{ "foo": "xyz" }]"#).unwrap(); assert_eq!(de.into_inner(), Obj { foo: "xyz".to_string() }); - // Extra elements in the array should be ignored when parsing the object - let de: DefaultRuleConfig = serde_json::from_str(r#"[{ "foo": "yyy" }, 42]"#).unwrap(); - assert_eq!(de.into_inner(), Obj { foo: "yyy".to_string() }); - // Empty array -> default let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); assert_eq!(de.into_inner(), Obj { foo: "defaultval".to_string() }); From 43c982addfd6293ee8c3c19d67023c432883118b Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 13:14:04 -0700 Subject: [PATCH 24/31] refactor: Avoid converting into an array and then back for no reason. Maybe we should consider adding a panic if the input array has more than one object passed to it, maybe only in debug mode? --- crates/oxc_linter/src/rule.rs | 40 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index e3a37ad854527..dc2a4c55eccfa 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -145,27 +145,31 @@ where return Ok(DefaultRuleConfig(T::default())); } - // Case 1: If the first element is an object, assume this is a single-object - // configuration (the most common case). Attempt to parse that object - // into T. If that fails, fall back to default. - if arr.len() == 1 && arr[0].is_object() { - let obj = arr.into_iter().next().unwrap(); - let t = serde_json::from_value::(obj).unwrap_or_else(|_| T::default()); - - return Ok(DefaultRuleConfig(t)); - } - - // Case 2: non-single object shapes (arrays of multiple elements, enums, - // tuples, etc.) — try parsing the whole array into T. If that fails, - // try parsing the first element into T, otherwise fall back to default. - let first = arr.first().cloned(); - - if let Ok(t) = serde_json::from_value::(serde_json::Value::Array(arr)) { + // Fast path: Single-element array. + if arr.len() == 1 { + let elem = arr.into_iter().next().unwrap(); + + // If it's an object, parse it directly (most common case). + // Otherwise, try parsing the element as T first (for primitives/enums). + // If that fails, try parsing as a single-element tuple. + let t = if elem.is_object() { + serde_json::from_value::(elem).unwrap_or_else(|_| T::default()) + } else { + // Try parsing element directly (for primitives, enums) + serde_json::from_value::(elem.clone()) + .or_else(|_| { + // If that fails, try as single-element array (for tuples with defaults) + serde_json::from_value::(serde_json::Value::Array(vec![elem])) + }) + .unwrap_or_else(|_| T::default()) + }; return Ok(DefaultRuleConfig(t)); } - let config = first.and_then(|v| serde_json::from_value(v).ok()).unwrap_or_else(T::default); - Ok(DefaultRuleConfig(config)) + // Multi-element arrays (tuples like [42, { "foo": "abc" }] or ["optionA", { "foo": "bar" }]). + let t = serde_json::from_value::(serde_json::Value::Array(arr)) + .unwrap_or_else(|_| T::default()); + Ok(DefaultRuleConfig(t)) } } From 200d05703fae8cfc9fd90f9e993d58a15d0f52a5 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 13:17:25 -0700 Subject: [PATCH 25/31] Flatten the code a bit. --- crates/oxc_linter/src/rule.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index dc2a4c55eccfa..92c5c3aac3ec1 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -150,25 +150,23 @@ where let elem = arr.into_iter().next().unwrap(); // If it's an object, parse it directly (most common case). - // Otherwise, try parsing the element as T first (for primitives/enums). - // If that fails, try parsing as a single-element tuple. - let t = if elem.is_object() { - serde_json::from_value::(elem).unwrap_or_else(|_| T::default()) - } else { - // Try parsing element directly (for primitives, enums) - serde_json::from_value::(elem.clone()) - .or_else(|_| { - // If that fails, try as single-element array (for tuples with defaults) - serde_json::from_value::(serde_json::Value::Array(vec![elem])) - }) - .unwrap_or_else(|_| T::default()) - }; + if elem.is_object() { + let t = serde_json::from_value::(elem).unwrap_or_else(|_| T::default()); + return Ok(DefaultRuleConfig(t)); + } + + // For non-objects, try parsing the element directly (primitives, enums). + // If that fails, try as a single-element array (for partial tuples with defaults). + let t = serde_json::from_value::(elem.clone()) + .or_else(|_| serde_json::from_value::(serde_json::Value::Array(vec![elem]))) + .unwrap_or_else(|_| T::default()); return Ok(DefaultRuleConfig(t)); } // Multi-element arrays (tuples like [42, { "foo": "abc" }] or ["optionA", { "foo": "bar" }]). let t = serde_json::from_value::(serde_json::Value::Array(arr)) .unwrap_or_else(|_| T::default()); + Ok(DefaultRuleConfig(t)) } } From 19b5d40a8149766b17bbbd660cabad1957a4497f Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 13:23:07 -0700 Subject: [PATCH 26/31] Add a few comments. --- crates/oxc_linter/src/rule.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 92c5c3aac3ec1..5876bb2ddaa1a 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -145,7 +145,9 @@ where return Ok(DefaultRuleConfig(T::default())); } - // Fast path: Single-element array. + // Single-element array. + // - `["foo"]` + // - `[{ "foo": "bar" }]` if arr.len() == 1 { let elem = arr.into_iter().next().unwrap(); @@ -163,7 +165,9 @@ where return Ok(DefaultRuleConfig(t)); } - // Multi-element arrays (tuples like [42, { "foo": "abc" }] or ["optionA", { "foo": "bar" }]). + // Multi-element arrays + // - `[42, { "foo": "abc" }]` + // - `["optionA", { "foo": "bar" }]` let t = serde_json::from_value::(serde_json::Value::Array(arr)) .unwrap_or_else(|_| T::default()); From 5d40c28d75bfdb8a035fb54450a5ea03b97eb3c8 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 13:58:39 -0700 Subject: [PATCH 27/31] Create an assert function to simplify testing the DefaultRuleConfig. --- crates/oxc_linter/src/rule.rs | 86 +++++++++++++++-------------------- 1 file changed, 37 insertions(+), 49 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 5876bb2ddaa1a..b5f994f8bf255 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -540,16 +540,12 @@ mod test { #[test] fn test_deserialize_default_rule_config_single() { // single element present - let de: DefaultRuleConfig = serde_json::from_str("[123]").unwrap(); - assert_eq!(de.into_inner(), 123u32); - let de: DefaultRuleConfig = serde_json::from_str("[true]").unwrap(); - assert!(de.into_inner()); - let de: DefaultRuleConfig = serde_json::from_str("[false]").unwrap(); - assert!(!de.into_inner()); + assert_default_rule_config("[123]", 123u32); + assert_default_rule_config("[true]", true); + assert_default_rule_config("[false]", false); // empty array should use defaults - let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); - assert_eq!(de.into_inner(), String::default()); + assert_default_rule_config("[]", String::default()); } #[derive(serde::Deserialize, Debug, PartialEq, Eq)] @@ -577,31 +573,28 @@ mod test { #[test] fn test_deserialize_default_rule_config_tuple() { // both elements present - let de: DefaultRuleConfig = - serde_json::from_str(r#"[42, { "foo": "abc" }]"#).unwrap(); - assert_eq!(de.into_inner(), Pair(42u32, Obj { foo: "abc".to_string() })); + assert_default_rule_config( + r#"[42, { "foo": "abc" }]"#, + Pair(42u32, Obj { foo: "abc".to_string() }), + ); // only first element present -> parsing the entire array into `Pair` // will fail, so we parse the first element. Since Pair has #[serde(default)], // serde will use the default value for the missing second field. - let de: DefaultRuleConfig = serde_json::from_str("[10]").unwrap(); - assert_eq!(de.into_inner(), Pair(10u32, Obj { foo: "defaultval".to_string() })); + assert_default_rule_config("[10]", Pair(10u32, Obj { foo: "defaultval".to_string() })); // empty array -> both default - let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); - assert_eq!(de.into_inner(), Pair(123u32, Obj { foo: "defaultval".to_string() })); + assert_default_rule_config("[]", Pair(123u32, Obj { foo: "defaultval".to_string() })); } #[test] fn test_deserialize_default_rule_config_object_in_array() { // Single-element array containing an object should parse into the object // configuration (fallback behavior, not the "entire-array as T" path). - let de: DefaultRuleConfig = serde_json::from_str(r#"[{ "foo": "xyz" }]"#).unwrap(); - assert_eq!(de.into_inner(), Obj { foo: "xyz".to_string() }); + assert_default_rule_config(r#"[{ "foo": "xyz" }]"#, Obj { foo: "xyz".to_string() }); // Empty array -> default - let de: DefaultRuleConfig = serde_json::from_str("[]").unwrap(); - assert_eq!(de.into_inner(), Obj { foo: "defaultval".to_string() }); + assert_default_rule_config("[]", Obj { foo: "defaultval".to_string() }); } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] @@ -633,16 +626,10 @@ mod test { #[test] fn test_deserialize_default_rule_config_with_enum_config() { // A basic enum config option. - let json = r#"["optionA"]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - - assert_eq!(de.into_inner(), EnumOptions::OptionA); + assert_default_rule_config(r#"["optionA"]"#, EnumOptions::OptionA); // Works with non-default value as well. - let json = r#"["optionB"]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - - assert_eq!(de.into_inner(), EnumOptions::OptionB); + assert_default_rule_config(r#"["optionB"]"#, EnumOptions::OptionB); } #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] @@ -652,26 +639,18 @@ mod test { #[test] fn test_deserialize_default_rule_config_with_enum_and_object() { // A basic enum config option with an object. - let json = r#"["optionA", { "foo": "bar" }]"#; - let de: DefaultRuleConfig = - serde_json::from_str(json).unwrap(); - - assert_eq!( - de.into_inner(), - TupleWithEnumAndObjectConfig(EnumOptions::OptionA, Obj { foo: "bar".to_string() }) + assert_default_rule_config( + r#"["optionA", { "foo": "bar" }]"#, + TupleWithEnumAndObjectConfig(EnumOptions::OptionA, Obj { foo: "bar".to_string() }), ); // Ensure that we can pass just one value and it'll provide the default for the second. - let json = r#"["optionB"]"#; - let de: DefaultRuleConfig = - serde_json::from_str(json).unwrap(); - - assert_eq!( - de.into_inner(), + assert_default_rule_config( + r#"["optionB"]"#, TupleWithEnumAndObjectConfig( EnumOptions::OptionB, - Obj { foo: "defaultval".to_string() } - ) + Obj { foo: "defaultval".to_string() }, + ), ); } @@ -691,16 +670,25 @@ mod test { #[test] fn test_deserialize_default_rule_with_object_with_multiple_fields() { // Test a rule config that is a simple object with multiple fields. - let json = r#"[{ "baz": "fooval", "qux": true }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - - assert_eq!(de.into_inner(), ExampleObjConfig { baz: "fooval".to_string(), qux: true }); + assert_default_rule_config( + r#"[{ "baz": "fooval", "qux": true }]"#, + ExampleObjConfig { baz: "fooval".to_string(), qux: true }, + ); // Ensure that missing fields get their default values. - let json = r#"[{ "qux": true }]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + assert_default_rule_config( + r#"[{ "qux": true }]"#, + ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }, + ); + } - assert_eq!(de.into_inner(), ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }); + // Ensure that the provided JSON deserializes into the expected value with DefaultRuleConfig. + fn assert_default_rule_config(json: &str, expected: T) + where + T: serde::de::DeserializeOwned + Default + PartialEq + std::fmt::Debug, + { + let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); + assert_eq!(de.into_inner(), expected); } fn assert_rule_runs_on_node_types( From a6a0eb17ea9fa89fc05aa27a175cfa3be910127c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 14:05:19 -0700 Subject: [PATCH 28/31] Update one more test to use the custom assert fn --- crates/oxc_linter/src/rule.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index b5f994f8bf255..75c41d70da474 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -600,19 +600,17 @@ mod test { #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] #[serde(default)] struct ComplexConfig { - foo: FxHashMap, + foo: FxHashMap, } #[test] fn test_deserialize_default_rule_config_with_complex_shape() { // A complex object shape for the rule config, like // `[ { "foo": { "obj": "value" } } ]`. - let json = r#"[ { "foo": { "obj": "value" } } ]"#; - let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - let cfg = de.into_inner(); - - let val = cfg.foo.get("obj").expect("obj key present"); - assert_eq!(val, &serde_json::Value::String("value".to_string())); + assert_default_rule_config( + r#"[ { "foo": { "obj": "value" } } ]"#, + ComplexConfig { foo: [("obj".to_string(), "value".to_string())].into_iter().collect() }, + ); } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] From 5f2ee80380b2e12e167f69b224fc4252f3f5794d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 7 Dec 2025 14:13:37 -0700 Subject: [PATCH 29/31] Resolve lint violations. --- crates/oxc_linter/src/rule.rs | 38 ++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 75c41d70da474..ffdb5b6c85817 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -540,12 +540,12 @@ mod test { #[test] fn test_deserialize_default_rule_config_single() { // single element present - assert_default_rule_config("[123]", 123u32); - assert_default_rule_config("[true]", true); - assert_default_rule_config("[false]", false); + assert_default_rule_config("[123]", &123u32); + assert_default_rule_config("[true]", &true); + assert_default_rule_config("[false]", &false); // empty array should use defaults - assert_default_rule_config("[]", String::default()); + assert_default_rule_config("[]", &String::default()); } #[derive(serde::Deserialize, Debug, PartialEq, Eq)] @@ -575,26 +575,26 @@ mod test { // both elements present assert_default_rule_config( r#"[42, { "foo": "abc" }]"#, - Pair(42u32, Obj { foo: "abc".to_string() }), + &Pair(42u32, Obj { foo: "abc".to_string() }), ); // only first element present -> parsing the entire array into `Pair` // will fail, so we parse the first element. Since Pair has #[serde(default)], // serde will use the default value for the missing second field. - assert_default_rule_config("[10]", Pair(10u32, Obj { foo: "defaultval".to_string() })); + assert_default_rule_config("[10]", &Pair(10u32, Obj { foo: "defaultval".to_string() })); // empty array -> both default - assert_default_rule_config("[]", Pair(123u32, Obj { foo: "defaultval".to_string() })); + assert_default_rule_config("[]", &Pair(123u32, Obj { foo: "defaultval".to_string() })); } #[test] fn test_deserialize_default_rule_config_object_in_array() { // Single-element array containing an object should parse into the object // configuration (fallback behavior, not the "entire-array as T" path). - assert_default_rule_config(r#"[{ "foo": "xyz" }]"#, Obj { foo: "xyz".to_string() }); + assert_default_rule_config(r#"[{ "foo": "xyz" }]"#, &Obj { foo: "xyz".to_string() }); // Empty array -> default - assert_default_rule_config("[]", Obj { foo: "defaultval".to_string() }); + assert_default_rule_config("[]", &Obj { foo: "defaultval".to_string() }); } #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] @@ -609,7 +609,9 @@ mod test { // `[ { "foo": { "obj": "value" } } ]`. assert_default_rule_config( r#"[ { "foo": { "obj": "value" } } ]"#, - ComplexConfig { foo: [("obj".to_string(), "value".to_string())].into_iter().collect() }, + &ComplexConfig { + foo: std::iter::once(("obj".to_string(), "value".to_string())).collect(), + }, ); } @@ -624,10 +626,10 @@ mod test { #[test] fn test_deserialize_default_rule_config_with_enum_config() { // A basic enum config option. - assert_default_rule_config(r#"["optionA"]"#, EnumOptions::OptionA); + assert_default_rule_config(r#"["optionA"]"#, &EnumOptions::OptionA); // Works with non-default value as well. - assert_default_rule_config(r#"["optionB"]"#, EnumOptions::OptionB); + assert_default_rule_config(r#"["optionB"]"#, &EnumOptions::OptionB); } #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] @@ -639,13 +641,13 @@ mod test { // A basic enum config option with an object. assert_default_rule_config( r#"["optionA", { "foo": "bar" }]"#, - TupleWithEnumAndObjectConfig(EnumOptions::OptionA, Obj { foo: "bar".to_string() }), + &TupleWithEnumAndObjectConfig(EnumOptions::OptionA, Obj { foo: "bar".to_string() }), ); // Ensure that we can pass just one value and it'll provide the default for the second. assert_default_rule_config( r#"["optionB"]"#, - TupleWithEnumAndObjectConfig( + &TupleWithEnumAndObjectConfig( EnumOptions::OptionB, Obj { foo: "defaultval".to_string() }, ), @@ -670,23 +672,23 @@ mod test { // Test a rule config that is a simple object with multiple fields. assert_default_rule_config( r#"[{ "baz": "fooval", "qux": true }]"#, - ExampleObjConfig { baz: "fooval".to_string(), qux: true }, + &ExampleObjConfig { baz: "fooval".to_string(), qux: true }, ); // Ensure that missing fields get their default values. assert_default_rule_config( r#"[{ "qux": true }]"#, - ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }, + &ExampleObjConfig { baz: "defaultbaz".to_string(), qux: true }, ); } // Ensure that the provided JSON deserializes into the expected value with DefaultRuleConfig. - fn assert_default_rule_config(json: &str, expected: T) + fn assert_default_rule_config(json: &str, expected: &T) where T: serde::de::DeserializeOwned + Default + PartialEq + std::fmt::Debug, { let de: DefaultRuleConfig = serde_json::from_str(json).unwrap(); - assert_eq!(de.into_inner(), expected); + assert_eq!(de.into_inner(), *expected); } fn assert_rule_runs_on_node_types( From 232b8bae6da27d0f1e78783a6213a5f18ddf244d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 9 Dec 2025 23:59:34 -0500 Subject: [PATCH 30/31] docs(linter): Updated `eslint/yoda` rule with auto-gen config options, I have. (#16562) Uses DefaultRuleConfig now, this rule does. Updated to use a proper tuple config, it has been. Continue to pass, the tests do. Part of #14743 and dependent on #16555, this PR is. Generated docs: ```md ## Configuration ### The 1st option type: `"never" | "always"` #### `"never"` The default `"never"` option can have exception options in an object literal, via `exceptRange` and `onlyEquality`. #### `"always"` The `"always"` option requires that literal values must always come first in comparisons. ### The 2nd option This option is an object with the following properties: #### exceptRange type: `boolean` default: `false` If the `"exceptRange"` property is `true`, the rule _allows_ yoda conditions in range comparisons which are wrapped directly in parentheses, including the parentheses of an `if` or `while` condition. A _range_ comparison tests whether a variable is inside or outside the range between two literal values. #### onlyEquality type: `boolean` default: `false` If the `"onlyEquality"` property is `true`, the rule reports yoda conditions _only_ for the equality operators `==` and `===`. The `onlyEquality` option allows a superset of the exceptions which `exceptRange` allows, thus both options are not useful together. ``` --- crates/oxc_linter/src/rules/eslint/yoda.rs | 97 ++++++++++--------- .../rule_configuration_documentation_test.rs | 1 - 2 files changed, 52 insertions(+), 46 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/yoda.rs b/crates/oxc_linter/src/rules/eslint/yoda.rs index 70470cf7cf117..9003173e4dbf3 100644 --- a/crates/oxc_linter/src/rules/eslint/yoda.rs +++ b/crates/oxc_linter/src/rules/eslint/yoda.rs @@ -9,8 +9,15 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_ecmascript::{ToBigInt, WithoutGlobalReferenceInformation}; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; - -use crate::{AstNode, context::LintContext, rule::Rule, utils::is_same_expression}; +use schemars::JsonSchema; +use serde::Deserialize; + +use crate::{ + AstNode, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, + utils::is_same_expression, +}; fn yoda_diagnostic(span: Span, never: bool, operator: &str) -> OxcDiagnostic { let expected_side = if never { "right" } else { "left" }; @@ -19,16 +26,39 @@ fn yoda_diagnostic(span: Span, never: bool, operator: &str) -> OxcDiagnostic { .with_label(span) } -#[derive(Debug, Clone)] -pub struct Yoda { - never: bool, +#[derive(Debug, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "camelCase", default)] +pub struct Yoda(AllowYoda, YodaOptions); + +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "camelCase", default)] +pub struct YodaOptions { + /// If the `"exceptRange"` property is `true`, the rule *allows* yoda conditions + /// in range comparisons which are wrapped directly in parentheses, including the + /// parentheses of an `if` or `while` condition. + /// A *range* comparison tests whether a variable is inside or outside the range + /// between two literal values. except_range: bool, + /// If the `"onlyEquality"` property is `true`, the rule reports yoda + /// conditions *only* for the equality operators `==` and `===`. The `onlyEquality` + /// option allows a superset of the exceptions which `exceptRange` allows, thus + /// both options are not useful together. only_equality: bool, } +#[derive(Debug, Default, Clone, JsonSchema, Deserialize, PartialEq)] +#[serde(rename_all = "kebab-case")] +enum AllowYoda { + /// The default `"never"` option can have exception options in an object literal, via `exceptRange` and `onlyEquality`. + #[default] + Never, + /// The `"always"` option requires that literal values must always come first in comparisons. + Always, +} + impl Default for Yoda { fn default() -> Self { - Self { never: true, except_range: false, only_equality: false } + Self(AllowYoda::Never, YodaOptions { except_range: false, only_equality: false }) } } @@ -46,25 +76,20 @@ declare_oxc_lint!( // // ... /// } /// ``` + /// /// This is called a Yoda condition because it reads as, "if red equals the color", similar to the way the Star Wars character Yoda speaks. Compare to the other way of arranging the operands: + /// /// ```js /// if (color === "red") { /// // ... /// } /// ``` + /// /// This typically reads, "if the color equals red", which is arguably a more natural way to describe the comparison. /// Proponents of Yoda conditions highlight that it is impossible to mistakenly use `=` instead of `==` because you cannot assign to a literal value. Doing so will cause a syntax error and you will be informed of the mistake early on. This practice was therefore very common in early programming where tools were not yet available. /// Opponents of Yoda conditions point out that tooling has made us better programmers because tools will catch the mistaken use of `=` instead of `==` (ESLint will catch this for you). Therefore, they argue, the utility of the pattern doesn't outweigh the readability hit the code takes while using Yoda conditions. /// - /// ### Options - /// - /// This rule can take a string option: - /// * If it is the default `"never"`, then comparisons must never be Yoda conditions. - /// * If it is `"always"`, then the literal value must always come first. - /// The default `"never"` option can have exception options in an object literal: - /// * If the `"exceptRange"` property is `true`, the rule *allows* yoda conditions in range comparisons which are wrapped directly in parentheses, including the parentheses of an `if` or `while` condition. The default value is `false`. A *range* comparison tests whether a variable is inside or outside the range between two literal values. - /// * If the `"onlyEquality"` property is `true`, the rule reports yoda conditions *only* for the equality operators `==` and `===`. The default value is `false`. - /// The `onlyEquality` option allows a superset of the exceptions which `exceptRange` allows, thus both options are not useful together. + /// ### Examples /// /// #### never /// @@ -119,6 +144,7 @@ declare_oxc_lint!( /// #### exceptRange /// /// Examples of **correct** code for the `"never", { "exceptRange": true }` options: + /// /// ```js /// function isReddish(color) { /// return (color.hue < 60 || 300 < color.hue); @@ -189,34 +215,13 @@ declare_oxc_lint!( Yoda, eslint, style, - fix + fix, + config = Yoda, ); impl Rule for Yoda { fn from_configuration(value: serde_json::Value) -> Self { - let mut config = Self::default(); - - let Some(arr) = value.as_array() else { - return config; - }; - - let option1 = arr.first().and_then(serde_json::Value::as_str); - let option2 = arr.get(1).and_then(serde_json::Value::as_object); - - if option1 == Some("always") { - config.never = false; - } - - if let Some(option2) = option2 { - if option2.get("exceptRange").and_then(serde_json::Value::as_bool) == Some(true) { - config.except_range = true; - } - if option2.get("onlyEquality").and_then(serde_json::Value::as_bool) == Some(true) { - config.only_equality = true; - } - } - - config + serde_json::from_value::>(value).unwrap_or_default().into_inner() } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { @@ -224,11 +229,13 @@ impl Rule for Yoda { return; }; + let Yoda(allow_yoda, options) = self; + let parent_node = ctx.nodes().parent_node(node.id()); if let AstKind::LogicalExpression(logical_expr) = parent_node.kind() { let parent_logical_expr = ctx.nodes().parent_node(parent_node.id()); - if self.except_range + if options.except_range && is_parenthesized(parent_logical_expr) && is_range(logical_expr, ctx) { @@ -240,18 +247,18 @@ impl Rule for Yoda { return; } - if self.only_equality && !is_equality(expr) { + if options.only_equality && !is_equality(expr) { return; } // never - if self.never && is_yoda(expr) { - do_diagnostic_with_fix(expr, ctx, self.never); + if allow_yoda == &AllowYoda::Never && is_yoda(expr) { + do_diagnostic_with_fix(expr, ctx, true); } // always - if !self.never && is_not_yoda(expr) { - do_diagnostic_with_fix(expr, ctx, self.never); + if allow_yoda == &AllowYoda::Always && is_not_yoda(expr) { + do_diagnostic_with_fix(expr, ctx, false); } } } diff --git a/crates/oxc_linter/tests/rule_configuration_documentation_test.rs b/crates/oxc_linter/tests/rule_configuration_documentation_test.rs index 345edcfcd716b..c0654b8074069 100644 --- a/crates/oxc_linter/tests/rule_configuration_documentation_test.rs +++ b/crates/oxc_linter/tests/rule_configuration_documentation_test.rs @@ -34,7 +34,6 @@ fn test_rules_with_custom_configuration_have_schema() { "eslint/no-empty-function", "eslint/no-restricted-imports", "eslint/no-warning-comments", - "eslint/yoda", // jest "jest/valid-title", // react From 48cda351752c08d8c3686bade99146b217ec491b Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Wed, 10 Dec 2025 00:07:42 -0500 Subject: [PATCH 31/31] fix(linter): Fix default behavior of the `eslint/eqeqeq` rule, and also the config option docs for it (#16560) This is part of #16023. See [the tests for the original rule](https://github.com/eslint/eslint/blob/b017f094d4e53728f8d335b9cf8b16dc074afda3/tests/lib/rules/eqeqeq.js). We have a test in [the eqeqeq.rs implementation](https://github.com/oxc-project/oxc/blob/e24aabdfa65044a7223e4ea7b294ad3bf5dfb1ec/crates/oxc_linter/src/rules/eslint/eqeqeq.rs) like so: ```rs // Issue: ("href != null", Some(json!([{"null": "ignore"}]))), ``` The problem is that this test has an incorrect shape for the config object, see here: ```jsonc // Should always be in one of these three formats, all three work in the original rule as well: "eslint/eqeqeq": ["error", "always", { "null": "never" }], "eslint/eqeqeq": ["error", "always"], "eslint/eqeqeq": ["error"], // But right now the tests have a case where the string arg is skipped, while the ESLint rule does not allow this: "eslint/eqeqeq": ["error", { "null": "ignore" }], ``` The problem is that the code _did_ previously handle this config array as invalid. However, because the implementation of `from` on NullType would fall back to `ignore` if it received bad data, it looked like it worked: ```rs impl NullType { pub fn from(raw: &str) -> Self { match raw { "always" => Self::Always, "never" => Self::Never, _ => Self::Ignore, } } } ``` Because `always` is marked as the default value (and is also the default value in the original ESLint rule), and so should be the default case. The test was just hitting the fallback value, so it looked like it worked, but really the fallback value was incorrect previously and did not match the docs _or_ the ESLint behavior. This fixes that issue by correcting the fallback value, and also fixes the auto-generated config shape/docs, so it correctly represents itself as taking a tuple. Generated docs: ```md ## Configuration ### The 1st option type: `"always" | "smart"` #### `"always"` Always require triple-equal comparisons, `===`/`!==`. This is the default. #### `"smart"` Allow certain safe comparisons to use `==`/`!=` (`typeof`, literals, nullish). ### The 2nd option This option is an object with the following properties: #### null type: `"always" | "never" | "ignore"` ##### `"always"` Always require triple-equals when comparing with null, `=== null`/`!== null`. This is the default. ##### `"never"` Never require triple-equals when comparing with null, always use `== null`/`!= null` ##### `"ignore"` Ignore null comparisons, allow either `== null`/`!= null` and `=== null`/`!== null` ``` --- crates/oxc_linter/src/rules/eslint/eqeqeq.rs | 151 ++++++++----------- 1 file changed, 60 insertions(+), 91 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/eqeqeq.rs b/crates/oxc_linter/src/rules/eslint/eqeqeq.rs index ab4e92e7115e2..b77f221ecfa39 100644 --- a/crates/oxc_linter/src/rules/eslint/eqeqeq.rs +++ b/crates/oxc_linter/src/rules/eslint/eqeqeq.rs @@ -7,9 +7,13 @@ use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::{BinaryOperator, UnaryOperator}; use schemars::JsonSchema; +use serde::Deserialize; -use crate::fixer::{RuleFix, RuleFixer}; use crate::{AstNode, context::LintContext, rule::Rule}; +use crate::{ + fixer::{RuleFix, RuleFixer}, + rule::DefaultRuleConfig, +}; fn eqeqeq_diagnostic(actual: &str, expected: &str, span: Span) -> OxcDiagnostic { OxcDiagnostic::warn(format!("Expected {expected} and instead saw {actual}")) @@ -17,11 +21,40 @@ fn eqeqeq_diagnostic(actual: &str, expected: &str, span: Span) -> OxcDiagnostic .with_label(span) } -#[derive(Debug, Default, Clone, JsonSchema)] +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "camelCase", default)] +pub struct Eqeqeq(CompareType, EqeqeqOptions); + +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] #[serde(rename_all = "camelCase", default)] -pub struct Eqeqeq { - compare_type: CompareType, - null_type: NullType, +pub struct EqeqeqOptions { + /// Configuration for whether to allow/disallow comparisons against `null`, + /// e.g. `foo == null` or `foo != null` + null: NullType, +} + +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum CompareType { + /// Always require triple-equal comparisons, `===`/`!==`. + /// This is the default. + #[default] + Always, + /// Allow certain safe comparisons to use `==`/`!=` (`typeof`, literals, nullish). + Smart, +} + +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum NullType { + /// Always require triple-equals when comparing with null, `=== null`/`!== null`. + /// This is the default. + #[default] + Always, + /// Never require triple-equals when comparing with null, always use `== null`/`!= null`. + Never, + /// Ignore null comparisons, allow either `== null`/`!= null` or `=== null`/`!== null`. + Ignore, } declare_oxc_lint!( @@ -35,24 +68,6 @@ declare_oxc_lint!( /// /// ### Options /// - /// First option: - /// - Type: `string` - /// - Default: `"always"` - /// - /// Possible values: - /// * `"always"` - always require `===`/`!==` - /// * `"smart"` - allow safe comparisons (`typeof`, literals, nullish) - /// - /// Second option (only used with `"always"`): - /// - Type: `object` - /// - Properties: - /// - `null`: `string` (default: `"always"`) - `"ignore"` allows `== null` and `!= null`. - /// - /// Possible values for `null`: - /// * `"always"` - always require `=== null`/`!== null` - /// * `"never"` - always require `== null`/`!= null` - /// * `"ignore"` - allow both `== null`/`!= null` and `=== null`/`!== null` - /// /// Example JSON configuration: /// ```json /// { @@ -66,7 +81,7 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule: /// ```js - /// /* eslint eqeqeq: "error" */ + /// /* eqeqeq: "error" */ /// /// if (x == 42) {} /// if ("" == text) {} @@ -75,7 +90,7 @@ declare_oxc_lint!( /// /// Examples of **correct** code for this rule: /// ```js - /// /* eslint eqeqeq: "error" */ + /// /* eqeqeq: "error" */ /// /// if (x === 42) {} /// if ("" === text) {} @@ -86,7 +101,7 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule with the `"smart"` option: /// ```js - /// /* eslint eqeqeq: ["error", "smart"] */ + /// /* eqeqeq: ["error", "smart"] */ /// /// if (x == 42) {} /// if ("" == text) {} @@ -94,7 +109,7 @@ declare_oxc_lint!( /// /// Examples of **correct** code for this rule with the `"smart"` option: /// ```js - /// /* eslint eqeqeq: ["error", "smart"] */ + /// /* eqeqeq: ["error", "smart"] */ /// /// if (typeof foo == "undefined") {} /// if (foo == null) {} @@ -105,14 +120,14 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule with the `{ "null": "ignore" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "ignore" }] */ + /// /* eqeqeq: ["error", "always", { "null": "ignore" }] */ /// if (x == 42) {} /// if ("" == text) {} /// ``` /// /// Examples of **correct** code for this rule with the `{ "null": "ignore" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "ignore" }] */ + /// /* eqeqeq: ["error", "always", { "null": "ignore" }] */ /// if (foo == null) {} /// if (foo != null) {} /// ``` @@ -121,7 +136,7 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule with the `{ "null": "always" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "always" }] */ + /// /* eqeqeq: ["error", "always", { "null": "always" }] */ /// /// if (foo == null) {} /// if (foo != null) {} @@ -129,7 +144,7 @@ declare_oxc_lint!( /// /// Examples of **correct** code for this rule with the `{ "null": "always" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "always" }] */ + /// /* eqeqeq: ["error", "always", { "null": "always" }] */ /// /// if (foo === null) {} /// if (foo !== null) {} @@ -139,7 +154,7 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule with the `{ "null": "never" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "never" }] */ + /// /* eqeqeq: ["error", "always", { "null": "never" }] */ /// /// if (x == 42) {} /// if ("" == text) {} @@ -149,14 +164,13 @@ declare_oxc_lint!( /// /// Examples of **correct** code for this rule with the `{ "null": "never" }` option: /// ```js - /// /* eslint eqeqeq: ["error", "always", { "null": "never" }] */ + /// /* eqeqeq: ["error", "always", { "null": "never" }] */ /// /// if (x === 42) {} /// if ("" === text) {} /// if (foo == null) {} /// if (foo != null) {} /// ``` - /// Eqeqeq, eslint, pedantic, @@ -164,45 +178,9 @@ declare_oxc_lint!( config = Eqeqeq, ); -#[derive(Debug, Default, Clone, JsonSchema)] -#[serde(rename_all = "lowercase")] -enum CompareType { - #[default] - Always, - Smart, -} - -impl CompareType { - pub fn from(raw: &str) -> Self { - match raw { - "smart" => Self::Smart, - _ => Self::Always, - } - } -} - -#[derive(Debug, Default, Clone, JsonSchema)] -#[serde(rename_all = "lowercase")] -enum NullType { - #[default] - Always, - Never, - Ignore, -} - -impl NullType { - pub fn from(raw: &str) -> Self { - match raw { - "always" => Self::Always, - "never" => Self::Never, - _ => Self::Ignore, - } - } -} - impl Eqeqeq { fn report_inverse_null_comparison(&self, binary_expr: &BinaryExpression, ctx: &LintContext) { - if !matches!(self.null_type, NullType::Never) { + if !matches!(self.1.null, NullType::Never) { return; } let operator = binary_expr.operator.as_str(); @@ -216,26 +194,17 @@ impl Eqeqeq { impl Rule for Eqeqeq { fn from_configuration(value: serde_json::Value) -> Self { - let first_arg = value.get(0).and_then(serde_json::Value::as_str); - - let null_type = value - .get(usize::from(first_arg.is_some())) - .and_then(|v| v.get("null")) - .and_then(serde_json::Value::as_str) - .map(NullType::from) - .unwrap_or_default(); - - let compare_type = first_arg.map(CompareType::from).unwrap_or_default(); - - Self { compare_type, null_type } + serde_json::from_value::>(value).unwrap_or_default().into_inner() } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::BinaryExpression(binary_expr) = node.kind() else { return; }; + let Eqeqeq(compare_type, options) = self.clone(); + let is_null_comparison = is_null_check(binary_expr); - let must_enforce_null = matches!(self.null_type, NullType::Always); + let must_enforce_null = matches!(options.null, NullType::Always); if !matches!(binary_expr.operator, BinaryOperator::Equality | BinaryOperator::Inequality) { if is_null_comparison { @@ -251,7 +220,7 @@ impl Rule for Eqeqeq { // - Comparing two literal values // - Evaluating the value of typeof // - Comparing against null - if matches!(self.compare_type, CompareType::Smart) + if matches!(compare_type, CompareType::Smart) && (is_typeof_check || is_same_type_literals || is_null_comparison) { return; @@ -361,18 +330,18 @@ fn test() { ("foo == null", Some(json!(["smart"]))), ("foo === null", None), // Always use === or !== with `null` - ("null === null", Some(json!(["always", {"null": "always"}]))), + ("null === null", Some(json!(["always", { "null": "always" }]))), // Never use === or !== with `null` - ("null == null", Some(json!(["always", {"null": "never"}]))), + ("null == null", Some(json!(["always", { "null": "never" }]))), // Do not apply this rule to `null`. - ("null == null", Some(json!(["smart", {"null": "ignore"}]))), + ("null == null", Some(json!(["smart", { "null": "ignore" }]))), // Issue: - ("href != null", Some(json!([{"null": "ignore"}]))), + ("href != null", Some(json!(["always", { "null": "ignore" }]))), ]; let fail = vec![ // ESLint will perform like below case - ("null >= 1", Some(json!(["always", {"null": "never"}]))), + ("null >= 1", Some(json!(["always", { "null": "never" }]))), ("typeof foo == 'undefined'", None), ("'hello' != 'world'", None), ("0 == 0", None), @@ -382,7 +351,7 @@ fn test() { ("foo == true", None), ("bananas != 1", None), ("value == undefined", None), - ("null == null", Some(json!(["always", {"null": "always"}]))), + ("null == null", Some(json!(["always", { "null": "always" }]))), ]; let fix = vec![