From b113e8ac1054733ae441b13be31c3636c2cf2c09 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Sat, 28 Dec 2024 01:26:18 +0900 Subject: [PATCH 1/3] feat(linter): implement `no-nested-ternary` rule --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/no_nested_ternary.rs | 69 +++++++++++++++++++ .../snapshots/eslint_no_nested_ternary.snap | 15 ++++ 3 files changed, 86 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 0cfa9eb7a90da..053fec48b501e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -93,6 +93,7 @@ mod eslint { pub mod no_loss_of_precision; pub mod no_magic_numbers; pub mod no_multi_str; + pub mod no_nested_ternary; pub mod no_new; pub mod no_new_func; pub mod no_new_native_nonconstructor; @@ -535,6 +536,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_classes_per_file, eslint::max_lines, eslint::max_params, + eslint::no_nested_ternary, eslint::no_labels, eslint::no_restricted_imports, eslint::no_object_constructor, diff --git a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs new file mode 100644 index 0000000000000..c70f480466f0f --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs @@ -0,0 +1,69 @@ +use oxc_ast::ast::Expression; +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_nested_ternary_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Do not nest ternary expressions.").with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoNestedTernary; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows nested ternary expressions to improve code readability and maintainability. + /// + /// ### Why is this bad? + /// + /// Nested ternary expressions make code harder to read and understand. They can lead to complex, difficult-to-debug logic. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// const result = condition1 ? (condition2 ? "a" : "b") : "c"; + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// let result; + /// if (condition1) { + /// result = condition2 ? "a" : "b"; + /// } else { + /// result = "c"; + /// } + /// ``` + NoNestedTernary, + style, +); + +impl Rule for NoNestedTernary { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::ConditionalExpression(node) = node.kind() { + if matches!(node.alternate, Expression::ConditionalExpression(_)) + || matches!(node.consequent, Expression::ConditionalExpression(_)) + { + ctx.diagnostic(no_nested_ternary_diagnostic(node.span)); + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec!["foo ? doBar() : doBaz();", "var foo = bar === baz ? qux : quxx;"]; + + let fail = vec![ + "foo ? bar : baz === qux ? quxx : foobar;", + "foo ? baz === qux ? quxx : foobar : bar;", + ]; + + Tester::new(NoNestedTernary::NAME, NoNestedTernary::CATEGORY, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap new file mode 100644 index 0000000000000..b20a7618c2bba --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap @@ -0,0 +1,15 @@ +--- +source: crates/oxc_linter/src/tester.rs +snapshot_kind: text +--- + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? bar : baz === qux ? quxx : foobar; + · ─────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? baz === qux ? quxx : foobar : bar; + · ─────────────────────────────────────── + ╰──── From 106875ab62fd927f6106159b9549f5327cbb7b91 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Sat, 28 Dec 2024 04:11:39 +0900 Subject: [PATCH 2/3] enhance rule --- .../src/rules/eslint/no_nested_ternary.rs | 42 ++++++++++++++-- .../snapshots/eslint_no_nested_ternary.snap | 48 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs index c70f480466f0f..331cd09b8a84c 100644 --- a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs +++ b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs @@ -45,9 +45,16 @@ declare_oxc_lint!( impl Rule for NoNestedTernary { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::ConditionalExpression(node) = node.kind() { - if matches!(node.alternate, Expression::ConditionalExpression(_)) - || matches!(node.consequent, Expression::ConditionalExpression(_)) - { + let check_nested_ternary = |expr: &Expression| { + if matches!(expr, Expression::ConditionalExpression(_)) { + true + } else { + let inner_expr = expr.get_inner_expression(); + matches!(inner_expr, Expression::ConditionalExpression(_)) + } + }; + + if check_nested_ternary(&node.consequent) || check_nested_ternary(&node.alternate) { ctx.diagnostic(no_nested_ternary_diagnostic(node.span)); } } @@ -58,11 +65,38 @@ impl Rule for NoNestedTernary { fn test() { use crate::tester::Tester; - let pass = vec!["foo ? doBar() : doBaz();", "var foo = bar === baz ? qux : quxx;"]; + let pass = vec![ + "foo ? doBar() : doBaz();", + "var foo = bar === baz ? qux : quxx;", + "var result = foo && bar ? baz : qux || quux;", + "var result = foo ? bar : baz === qux;", + "foo ? doSomething(a, b) : doSomethingElse(c, d);", + // Parenthesized Expressions + "var result = (foo ? bar : baz) || qux;", + "var result = (foo ? bar : baz) && qux;", + "var result = foo === bar ? (baz || qux) : quux;", + "var result = (foo ? bar : baz) ? qux : quux;", + // TypeScript + "var result = foo! ? bar : baz;", + "var result = foo ? bar! : baz;", + "var result = (foo as boolean) ? bar : baz;", + "var result = foo ? (bar as string) : baz;", + ]; let fail = vec![ "foo ? bar : baz === qux ? quxx : foobar;", "foo ? baz === qux ? quxx : foobar : bar;", + // Parenthesized Expressions + "var result = foo ? (bar ? baz : qux) : quux;", + "var result = foo ? (bar === baz ? qux : quux) : foobar;", + "doSomething(foo ? bar : baz ? qux : quux);", + // Comment + "var result = foo /* comment */ ? bar : baz ? qux : quux;", + // TypeScript + "var result = foo! ? bar : baz! ? qux : quux;", + "var result = foo ? bar! : (baz! ? qux : quux);", + "var result = (foo as boolean) ? bar : (baz as string) ? qux : quux;", + "var result = foo ? (bar as string) : (baz as number ? qux : quux);", ]; Tester::new(NoNestedTernary::NAME, NoNestedTernary::CATEGORY, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap index b20a7618c2bba..9bdafa63bd9d2 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap @@ -13,3 +13,51 @@ snapshot_kind: text 1 │ foo ? baz === qux ? quxx : foobar : bar; · ─────────────────────────────────────── ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar ? baz : qux) : quux; + · ────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar === baz ? qux : quux) : foobar; + · ───────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:13] + 1 │ doSomething(foo ? bar : baz ? qux : quux); + · ──────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo /* comment */ ? bar : baz ? qux : quux; + · ────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo! ? bar : baz! ? qux : quux; + · ────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? bar! : (baz! ? qux : quux); + · ──────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = (foo as boolean) ? bar : (baz as string) ? qux : quux; + · ───────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar as string) : (baz as number ? qux : quux); + · ──────────────────────────────────────────────────── + ╰──── From e43a7591b732bfd94c6f6074f63ad0fbdddddc5b Mon Sep 17 00:00:00 2001 From: Cameron Clark Date: Fri, 27 Dec 2024 23:22:48 +0000 Subject: [PATCH 3/3] refactor: minor cleanup --- .../src/rules/eslint/no_nested_ternary.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs index 331cd09b8a84c..2508793abe4cd 100644 --- a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs +++ b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs @@ -45,16 +45,13 @@ declare_oxc_lint!( impl Rule for NoNestedTernary { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::ConditionalExpression(node) = node.kind() { - let check_nested_ternary = |expr: &Expression| { - if matches!(expr, Expression::ConditionalExpression(_)) { - true - } else { - let inner_expr = expr.get_inner_expression(); - matches!(inner_expr, Expression::ConditionalExpression(_)) - } - }; - - if check_nested_ternary(&node.consequent) || check_nested_ternary(&node.alternate) { + if matches!( + node.consequent.get_inner_expression(), + Expression::ConditionalExpression(_) + ) || matches!( + node.alternate.get_inner_expression(), + Expression::ConditionalExpression(_) + ) { ctx.diagnostic(no_nested_ternary_diagnostic(node.span)); } }