diff --git a/crates/oxc_linter/src/rules/eslint/eqeqeq.rs b/crates/oxc_linter/src/rules/eslint/eqeqeq.rs index 3102ea4f74ce0..19ece8b49be57 100644 --- a/crates/oxc_linter/src/rules/eslint/eqeqeq.rs +++ b/crates/oxc_linter/src/rules/eslint/eqeqeq.rs @@ -8,6 +8,7 @@ use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::{BinaryOperator, UnaryOperator}; use schemars::JsonSchema; +use crate::fixer::{RuleFix, RuleFixer}; use crate::{AstNode, context::LintContext, rule::Rule}; fn eqeqeq_diagnostic(actual: &str, expected: &str, span: Span) -> OxcDiagnostic { @@ -26,65 +27,136 @@ pub struct Eqeqeq { declare_oxc_lint!( /// ### What it does /// - /// Requires the use of the `===` and `!==` operators. + /// Requires the use of the `===` and `!==` operators, disallowing the use of `==` and `!=`. /// /// ### Why is this bad? /// - /// Using non-strict equality operators leads to hard to track bugs due to type coercion. + /// Using non-strict equality operators leads to unexpected behavior due to type coercion, which can cause hard-to-find bugs. + /// + /// ### 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 + /// { + /// "eqeqeq": ["error", "always", { "null": "ignore" }] + /// } + /// ``` /// /// ### Examples /// + /// #### `"always"` (default) + /// /// Examples of **incorrect** code for this rule: /// ```js - /// const a = []; - /// const b = true; - /// a == b + /// /* eslint eqeqeq: "error" */ + /// + /// if (x == 42) {} + /// if ("" == text) {} + /// if (obj.getStuff() != undefined) {} /// ``` - /// The above will evaluate to `true`, but that is almost surely not what you want. /// /// Examples of **correct** code for this rule: /// ```js - /// const a = []; - /// const b = true; - /// a === b + /// /* eslint eqeqeq: "error" */ + /// + /// if (x === 42) {} + /// if ("" === text) {} + /// if (obj.getStuff() !== undefined) {} /// ``` - /// The above will evaluate to `false` (an array is not boolean true). /// - /// ### Options + /// #### `"smart"` /// - /// #### null + /// Examples of **incorrect** code for this rule with the `"smart"` option: + /// ```js + /// /* eslint eqeqeq: ["error", "smart"] */ /// - /// ```json - /// "eslint/eqeqeq": ["error", "always", {"null": "ignore"}] + /// if (x == 42) {} + /// if ("" == text) {} /// ``` /// - /// Allow nullish comparison (`foo == null`). The alternative (`foo === null || foo === undefined`) is verbose and has no other benefit. + /// Examples of **correct** code for this rule with the `"smart"` option: + /// ```js + /// /* eslint eqeqeq: ["error", "smart"] */ + /// + /// if (typeof foo == "undefined") {} + /// if (foo == null) {} + /// if (foo != null) {} + /// ``` /// - /// #### smart + /// #### `{"null": "ignore"}` (with `"always"` first option) /// - /// ```json - /// "eslint/eqeqeq": ["error", "smart"] + /// Examples of **incorrect** code for this rule with the `{ "null": "ignore" }` option: + /// ```js + /// /* eslint 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" }] */ + /// if (foo == null) {} + /// if (foo != null) {} /// ``` /// - /// Allow `==` when comparing: + /// #### `{"null": "always"}` (default - with `"always"` first option) + /// + /// Examples of **incorrect** code for this rule with the `{ "null": "always" }` option: + /// ```js + /// /* eslint eqeqeq: ["error", "always", { "null": "always" }] */ + /// + /// if (foo == null) {} + /// if (foo != null) {} + /// ``` + /// + /// Examples of **correct** code for this rule with the `{ "null": "always" }` option: + /// ```js + /// /* eslint eqeqeq: ["error", "always", { "null": "always" }] */ + /// + /// if (foo === null) {} + /// if (foo !== null) {} + /// ``` /// - /// * the result from `typeof` - /// * literal values - /// * nullish + /// #### `{"null": "never"}` (with `"always"` first option) /// - /// Examples of **incorrect** code for this option: + /// Examples of **incorrect** code for this rule with the `{ "null": "never" }` option: /// ```js - /// a == b - /// [] == true + /// /* eslint eqeqeq: ["error", "always", { "null": "never" }] */ + /// + /// if (x == 42) {} + /// if ("" == text) {} + /// if (foo === null) {} + /// if (foo !== null) {} /// ``` /// - /// Examples of **correct** code for this option: + /// Examples of **correct** code for this rule with the `{ "null": "never" }` option: /// ```js - /// typeof foo == 'undefined' - /// 'foo' == 'bar' - /// 42 == 42 - /// foo == null + /// /* eslint eqeqeq: ["error", "always", { "null": "never" }] */ + /// + /// if (x === 42) {} + /// if ("" === text) {} + /// if (foo == null) {} + /// if (foo != null) {} /// ``` + /// Eqeqeq, eslint, pedantic, @@ -92,6 +164,56 @@ 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) { + return; + } + let operator = binary_expr.operator.as_str(); + let truncated_operator = &operator[..operator.len() - 1]; + // There are some uncontrolled cases to auto fix. + // In ESLint, `null >= null` will be auto fixed to `null > null` which is also wrong. + // So I just report it. + ctx.diagnostic(eqeqeq_diagnostic(operator, truncated_operator, binary_expr.span)); + } +} + 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); @@ -112,40 +234,30 @@ impl Rule for Eqeqeq { let AstKind::BinaryExpression(binary_expr) = node.kind() else { return; }; - let is_null = is_null_check(binary_expr); - let enforce_rule_for_null = matches!(self.null_type, NullType::Always); - let enforce_inverse_rule_for_null = matches!(self.null_type, NullType::Never); + let is_null_comparison = is_null_check(binary_expr); + let must_enforce_null = matches!(self.null_type, NullType::Always); if !matches!(binary_expr.operator, BinaryOperator::Equality | BinaryOperator::Inequality) { - if enforce_inverse_rule_for_null && is_null { - let operator = binary_expr.operator.as_str(); - // There are some uncontrolled cases to auto fix. - // In ESlint, `null >= null` will be auto fixed to `null > null` which is also wrong. - // So I just report it. - ctx.diagnostic(eqeqeq_diagnostic( - operator, - &operator[0..operator.len() - 1], - binary_expr.span, - )); + if is_null_comparison { + self.report_inverse_null_comparison(binary_expr, ctx); } - return; } - let is_type_of_binary_bool = is_type_of_binary(binary_expr); - let are_literals_and_same_type_bool = + let is_typeof_check = is_type_of_binary(binary_expr); + let is_same_type_literals = are_literals_and_same_type(&binary_expr.left, &binary_expr.right); // The "smart" option enforces the use of `===` and `!==` except for these cases: // - Comparing two literal values // - Evaluating the value of typeof // - Comparing against null if matches!(self.compare_type, CompareType::Smart) - && (is_type_of_binary_bool || are_literals_and_same_type_bool || is_null) + && (is_typeof_check || is_same_type_literals || is_null_comparison) { return; } - if !enforce_rule_for_null && is_null { + if !must_enforce_null && is_null_comparison { return; } @@ -153,21 +265,9 @@ impl Rule for Eqeqeq { let (preferred_operator, preferred_operator_with_padding) = to_strict_eq_operator_str(binary_expr.operator); - #[expect(clippy::cast_possible_truncation)] - let operator_span = { - let left_end = binary_expr.left.span().end; - let right_start = binary_expr.right.span().start; - let offset = Span::new(left_end, right_start) - .source_text(ctx.source_text()) - .find(operator) - .unwrap_or(0) as u32; - - let operator_start = left_end + offset; - let operator_end = operator_start + operator.len() as u32; - Span::new(operator_start, operator_end) - }; + let operator_span = get_operator_span(binary_expr, operator, ctx); - let fix_kind = if is_type_of_binary_bool || are_literals_and_same_type_bool { + let fix_kind = if is_typeof_check || is_same_type_literals { FixKind::SafeFix } else { FixKind::DangerousFix @@ -176,70 +276,54 @@ impl Rule for Eqeqeq { ctx.diagnostic_with_fix_of_kind( eqeqeq_diagnostic(operator, preferred_operator, operator_span), fix_kind, - |fixer| { - let start = binary_expr.left.span().end; - let end = binary_expr.right.span().start; - let span = Span::new(start, end); - - fixer.replace(span, preferred_operator_with_padding) - }, + |fixer| apply_rule_fix(&fixer, binary_expr, preferred_operator_with_padding), ); } } -#[derive(Debug, Default, Clone, JsonSchema)] -#[serde(rename_all = "lowercase")] -enum CompareType { - #[default] - Always, - Smart, -} +#[expect(clippy::cast_possible_truncation)] +fn get_operator_span(binary_expr: &BinaryExpression, operator: &str, ctx: &LintContext) -> Span { + let left_end = binary_expr.left.span().end; + let right_start = binary_expr.right.span().start; + let between_text = Span::new(left_end, right_start).source_text(ctx.source_text()); + let offset = between_text.find(operator).unwrap_or(0) as u32; -impl CompareType { - pub fn from(raw: &str) -> Self { - match raw { - "smart" => Self::Smart, - _ => Self::Always, - } - } -} + let operator_start = left_end + offset; + let operator_end = operator_start + operator.len() as u32; -#[derive(Debug, Default, Clone, JsonSchema)] -#[serde(rename_all = "lowercase")] -enum NullType { - #[default] - Always, - Never, - Ignore, + Span::new(operator_start, operator_end) } -impl NullType { - pub fn from(raw: &str) -> Self { - match raw { - "always" => Self::Always, - "never" => Self::Never, - _ => Self::Ignore, - } - } +fn apply_rule_fix<'a>( + fixer: &RuleFixer<'_, 'a>, + binary_expr: &BinaryExpression, + preferred_operator_with_padding: &'a str, +) -> RuleFix<'a> { + let span = Span::new(binary_expr.left.span().end, binary_expr.right.span().start); + + fixer.replace(span, preferred_operator_with_padding) } fn to_strict_eq_operator_str(operator: BinaryOperator) -> (&'static str, &'static str) { match operator { BinaryOperator::Equality => ("===", " === "), BinaryOperator::Inequality => ("!==", " !== "), - _ => unreachable!(), + _ => unreachable!( + "Only Equality and Inequality operators are supported in to_strict_eq_operator_str" + ), } } -/// Checks if either operand of a binary expression is a typeof operation +fn is_type_of(expr: &Expression) -> bool { + matches!( + expr, + Expression::UnaryExpression(unary_expr) if matches!(unary_expr.operator, UnaryOperator::Typeof) + ) +} + +/// Checks if either operand of a binary expression is a `typeof` operation. fn is_type_of_binary(binary_expr: &BinaryExpression) -> bool { - match (&binary_expr.left, &binary_expr.right) { - (Expression::UnaryExpression(unary_expr), _) - | (_, Expression::UnaryExpression(unary_expr)) => { - matches!(unary_expr.operator, UnaryOperator::Typeof) - } - _ => false, - } + is_type_of(&binary_expr.left) || is_type_of(&binary_expr.right) } /// Checks if operands are literals of the same type