diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 3220af9cc2d02..ffdb5b6c85817 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -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: +/// ```ignore +/// 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,16 +130,48 @@ where let value = serde_json::Value::deserialize(deserializer)?; - 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); - Ok(DefaultRuleConfig(config)) - } else { - Err(D::Error::custom("Expected array for rule configuration")) + // 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 defaults. + if arr.is_empty() { + return Ok(DefaultRuleConfig(T::default())); } + + // Single-element array. + // - `["foo"]` + // - `[{ "foo": "bar" }]` + if arr.len() == 1 { + let elem = arr.into_iter().next().unwrap(); + + // If it's an object, parse it directly (most common case). + 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 + // - `[42, { "foo": "abc" }]` + // - `["optionA", { "foo": "bar" }]` + let t = serde_json::from_value::(serde_json::Value::Array(arr)) + .unwrap_or_else(|_| T::default()); + + Ok(DefaultRuleConfig(t)) } } @@ -391,7 +436,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 +537,160 @@ 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); + + // empty array should use defaults + assert_default_rule_config("[]", &String::default()); + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq)] + #[serde(default)] + struct Obj { + foo: String, + } + + 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 + 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. + 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() })); + } + + #[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() }); + + // Empty array -> default + assert_default_rule_config("[]", &Obj { foo: "defaultval".to_string() }); + } + + #[derive(serde::Deserialize, Debug, PartialEq, Eq, Default)] + #[serde(default)] + struct ComplexConfig { + foo: FxHashMap, + } + + #[test] + fn test_deserialize_default_rule_config_with_complex_shape() { + // A complex object shape for the rule config, like + // `[ { "foo": { "obj": "value" } } ]`. + assert_default_rule_config( + r#"[ { "foo": { "obj": "value" } } ]"#, + &ComplexConfig { + foo: std::iter::once(("obj".to_string(), "value".to_string())).collect(), + }, + ); + } + + #[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. + assert_default_rule_config(r#"["optionA"]"#, &EnumOptions::OptionA); + + // Works with non-default value as well. + assert_default_rule_config(r#"["optionB"]"#, &EnumOptions::OptionB); + } + + #[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)] + #[serde(default)] + struct TupleWithEnumAndObjectConfig(EnumOptions, Obj); + + #[test] + fn test_deserialize_default_rule_config_with_enum_and_object() { + // A basic enum config option with an object. + 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. + assert_default_rule_config( + r#"["optionB"]"#, + &TupleWithEnumAndObjectConfig( + EnumOptions::OptionB, + Obj { foo: "defaultval".to_string() }, + ), + ); + } + + #[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. + assert_default_rule_config( + r#"[{ "baz": "fooval", "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 }, + ); + } + + // 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( rule: &R, node_types: &[oxc_ast::AstType], 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/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![ diff --git a/crates/oxc_linter/src/rules/eslint/func_style.rs b/crates/oxc_linter/src/rules/eslint/func_style.rs index 4594abf1a70d3..29d8b92b458e8 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,50 @@ 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 +293,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 +307,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 +577,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" } }]), ), ), ( diff --git a/crates/oxc_linter/src/rules/eslint/sort_keys.rs b/crates/oxc_linter/src/rules/eslint/sort_keys.rs index 22097dd7865c6..84d48aae07922 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(); } 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 43e53c5d4c2d4..c0654b8074069 100644 --- a/crates/oxc_linter/tests/rule_configuration_documentation_test.rs +++ b/crates/oxc_linter/tests/rule_configuration_documentation_test.rs @@ -30,12 +30,10 @@ 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", "eslint/no-warning-comments", - "eslint/yoda", // jest "jest/valid-title", // react