From 045a51ec59c2abe0fd40d4714e5e7a1012fbb675 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:00:36 -0700 Subject: [PATCH 01/18] 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 073e17bb88e724ba34b52e1323cf2f216e244b25 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:17:58 -0700 Subject: [PATCH 02/18] 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 346f10e893b699b6ad687404e871120568f277cc Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:25:52 -0700 Subject: [PATCH 03/18] 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 834dd54f1bae91aaf978b3cffcef25ae6bbb4464 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:38:23 -0700 Subject: [PATCH 04/18] 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 4df8b19f4bdf4a800b01b40a2a2210b8070bca91 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:51:44 -0700 Subject: [PATCH 05/18] 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 63290eae42c59725e5ad2cc8ca7b37bdd73a3ac1 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 21:53:04 -0700 Subject: [PATCH 06/18] 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 6c2b76af09888fec748e972368b5b9f947724f6c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:08:41 -0700 Subject: [PATCH 07/18] 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 5a2384deda41d..d620fac1aac90 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(); } } @@ -277,7 +240,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 1e1aab3ad7f73ca668838942374e281fdd1027f5 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:08:50 -0700 Subject: [PATCH 08/18] 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 d620fac1aac90..da5c0b4f24007 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 91288f3d4f0b5c23a2e2249a6e6add33a6a51a43 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:19:48 -0700 Subject: [PATCH 09/18] 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 8d46d350492145eb856151d14384b8870e59af6c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 22:44:51 -0700 Subject: [PATCH 10/18] 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 5d1735746e225eb0fa2dd65503904cf2b7289fc8 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:00:09 -0700 Subject: [PATCH 11/18] 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 fb8a5d34a24e762c3db3e03dcfb22ffd42ac06b6 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:01:33 -0700 Subject: [PATCH 12/18] 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 6616190cc3840fb8ae52f854ac7ffd6810cd216c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:08:49 -0700 Subject: [PATCH 13/18] 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 ae7f9d4c56dda107e9575c44b192883a0c35710c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:10:15 -0700 Subject: [PATCH 14/18] 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 7b722571f7102965fa6c17e9c32fabf8f9545f36 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:42:21 -0700 Subject: [PATCH 15/18] 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 bfe07b9445aa81cf2b553d3e100caebb3b7ff8e3 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:53:44 -0700 Subject: [PATCH 16/18] 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 3abce5e4bc5d0f91fcf0c5ec14c83c4047efc4fb Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:56:49 -0700 Subject: [PATCH 17/18] 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 6f5894bccee073a8fbd53782af864cb318f848dd Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Dec 2025 23:57:49 -0700 Subject: [PATCH 18/18] docs(linter): Fix the rule config docs for `eslint/no-inner-declarations` rule. --- .../src/rules/eslint/no_inner_declarations.rs | 57 ++++++++----------- 1 file changed, 23 insertions(+), 34 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..bd25c7465b963 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,9 +19,11 @@ 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, @@ -25,7 +31,7 @@ pub struct NoInnerDeclarations { #[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,48 +84,31 @@ 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>) { match node.kind() { AstKind::VariableDeclaration(decl) => { - if self.config == NoInnerDeclarationsConfig::Functions || !decl.kind.is_var() { + let NoInnerDeclarations(mode, _config) = &self; + + if mode == &NoInnerDeclarationsMode::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 self.config == NoInnerDeclarationsConfig::Functions - && let Some(block_scoped_functions) = self.block_scoped_functions - && block_scoped_functions == BlockScopedFunctions::Allow + if mode == &NoInnerDeclarationsMode::Functions + && config.block_scoped_functions == Some(BlockScopedFunctions::Allow) { let is_module = ctx.source_type().is_module(); let scope_id = node.scope_id(); @@ -200,8 +189,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"]))),