From 5543e8c435f8cb76cae7aac6cbe0405141196a37 Mon Sep 17 00:00:00 2001 From: Hao Cheng Date: Sun, 19 Nov 2023 04:06:48 +0100 Subject: [PATCH] feat(linter): eslint-plugin-unicorn/no-nested-ternary (#1417) This PR implements the [eslint-plugin-unicorn/no-nested-ternary](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-nested-ternary.md) rule. Tests taken from https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/test/no-nested-ternary.mjs. Related issue: #684 --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/unicorn/no_nested_ternary.rs | 186 ++++++++++++++++++ .../src/snapshots/no_nested_ternary.snap | 70 +++++++ 3 files changed, 258 insertions(+) create mode 100644 crates/oxc_linter/src/rules/unicorn/no_nested_ternary.rs create mode 100644 crates/oxc_linter/src/snapshots/no_nested_ternary.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index f34708c1c935..37fba3467964 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -159,6 +159,7 @@ mod unicorn { pub mod no_invalid_remove_event_listener; pub mod no_lonely_if; pub mod no_negated_condition; + pub mod no_nested_ternary; pub mod no_new_array; pub mod no_new_buffer; pub mod no_object_as_default_parameter; @@ -317,6 +318,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::no_invalid_remove_event_listener, unicorn::no_lonely_if, unicorn::no_negated_condition, + unicorn::no_nested_ternary, unicorn::no_new_array, unicorn::no_new_buffer, unicorn::no_object_as_default_parameter, diff --git a/crates/oxc_linter/src/rules/unicorn/no_nested_ternary.rs b/crates/oxc_linter/src/rules/unicorn/no_nested_ternary.rs new file mode 100644 index 000000000000..72f585a7d439 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_nested_ternary.rs @@ -0,0 +1,186 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode, Fix}; + +#[derive(Debug, Error, Diagnostic)] +enum NoNestedTernaryDiagnostic { + #[error("eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses.")] + #[diagnostic(severity(warning), help("Add parentheses around the nested ternary expression."))] + UnparenthesizedNestedTernary(#[label] Span), + #[error( + "eslint-plugin-unicorn(no-nested-ternary): Unexpected deeply nested ternary expression." + )] + #[diagnostic( + severity(warning), + help("Avoid nesting ternary expressions for more than one level.") + )] + DeeplyNestedTernary(#[label] Span), +} + +#[derive(Debug, Default, Clone)] +pub struct NoNestedTernary; + +declare_oxc_lint!( + /// ### What it does + /// This rule disallows deeply nested ternary expressions. + /// Nested ternary expressions that are only one level deep and wrapped in parentheses are allowed. + /// + /// ### Why is this bad? + /// Nesting ternary expressions can make code more difficult to understand. + /// + /// ### Example + /// ```javascript + /// // Fail + /// const foo = i > 5 ? i < 100 ? true : false : true; + /// const foo = i > 5 ? true : (i < 100 ? true : (i < 1000 ? true : false)); + /// + /// // Pass + /// const foo = i > 5 ? (i < 100 ? true : false) : true; + /// const foo = i > 5 ? (i < 100 ? true : false) : (i < 100 ? true : false); + /// ``` + NoNestedTernary, + restriction +); + +impl Rule for NoNestedTernary { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::ConditionalExpression(cond_expr) = node.kind() else { return }; + + if matches!(&cond_expr.test.get_inner_expression(), Expression::ConditionalExpression(_)) + || matches!( + &cond_expr.consequent.get_inner_expression(), + Expression::ConditionalExpression(_) + ) + || matches!( + &cond_expr.alternate.get_inner_expression(), + Expression::ConditionalExpression(_) + ) + { + return; + } + + let mut nested_level = 0; + let mut current_node = node; + while let Some(parent_node) = ctx.nodes().parent_node(current_node.id()) { + match parent_node.kind() { + AstKind::ConditionalExpression(_) => { + nested_level += 1; + } + AstKind::ParenthesizedExpression(_) => {} + _ => break, + } + if nested_level == 2 { + break; + } + current_node = parent_node; + } + + match nested_level { + 0 => {} + 1 => { + let Some(parent_node) = ctx.nodes().parent_node(node.id()) else { unreachable!() }; + if let AstKind::ParenthesizedExpression(_) = parent_node.kind() { + return; + } + ctx.diagnostic_with_fix( + NoNestedTernaryDiagnostic::UnparenthesizedNestedTernary(cond_expr.span), + || { + Fix::new( + format!("({})", cond_expr.span.source_text(ctx.source_text())), + cond_expr.span, + ) + }, + ); + } + 2 => { + ctx.diagnostic(NoNestedTernaryDiagnostic::DeeplyNestedTernary(cond_expr.span)); + } + _ => unreachable!(), + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r"const foo = i > 5 ? true : false;", + r"const foo = i > 5 ? true : (i < 100 ? true : false);", + r"const foo = i > 5 ? (i < 100 ? true : false) : true;", + r"const foo = i > 5 ? (i < 100 ? true : false) : (i < 100 ? true : false);", + r"const foo = i > 5 ? true : (i < 100 ? FOO(i > 50 ? false : true) : false);", + r"foo ? doBar() : doBaz();", + r"var foo = bar === baz ? qux : quxx;", + r" + const pluginName = isAbsolute ? + pluginPath.slice(pluginPath.lastIndexOf('/') + 1) : + ( + isNamespaced ? + pluginPath.split('@')[1].split('/')[1] : + pluginPath + ); + ", + ]; + + let fail = vec![ + r"const foo = i > 5 ? true : (i < 100 ? true : (i < 1000 ? true : false));", + r"const foo = i > 5 ? true : (i < 100 ? (i > 50 ? false : true) : false);", + r"const foo = i > 5 ? i < 100 ? true : false : true;", + r"const foo = i > 5 ? i < 100 ? true : false : i < 100 ? true : false;", + r"const foo = i > 5 ? true : i < 100 ? true : false;", + r"foo ? bar : baz === qux ? quxx : foobar;", + r"foo ? baz === qux ? quxx : foobar : bar;", + r" + const foo = a ? + b : + ( + c ? + d : + ( + e ? + f : + (g ? h : i) + ) + ) + ", + ]; + + let fix = vec![ + ( + "const foo = i > 5 ? i < 100 ? true : false : true;", + "const foo = i > 5 ? (i < 100 ? true : false) : true;", + None, + ), + ( + "const foo = i > 5 ? i < 100 ? true : false : i < 100 ? true : false;", + "const foo = i > 5 ? (i < 100 ? true : false) : (i < 100 ? true : false);", + None, + ), + ( + "const foo = i > 5 ? true : i < 100 ? true : false;", + "const foo = i > 5 ? true : (i < 100 ? true : false);", + None, + ), + ( + "foo ? bar : baz === qux ? quxx : foobar;", + "foo ? bar : (baz === qux ? quxx : foobar);", + None, + ), + ( + "foo ? baz === qux ? quxx : foobar : bar;", + "foo ? (baz === qux ? quxx : foobar) : bar;", + None, + ), + ]; + + Tester::new_without_config(NoNestedTernary::NAME, pass, fail) + .expect_fix(fix) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_nested_ternary.snap b/crates/oxc_linter/src/snapshots/no_nested_ternary.snap new file mode 100644 index 000000000000..8cb2037630a8 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_nested_ternary.snap @@ -0,0 +1,70 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_nested_ternary +--- + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected deeply nested ternary expression. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? true : (i < 100 ? true : (i < 1000 ? true : false)); + · ─────────────────────── + ╰──── + help: Avoid nesting ternary expressions for more than one level. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected deeply nested ternary expression. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? true : (i < 100 ? (i > 50 ? false : true) : false); + · ───────────────────── + ╰──── + help: Avoid nesting ternary expressions for more than one level. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? i < 100 ? true : false : true; + · ────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? i < 100 ? true : false : i < 100 ? true : false; + · ────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? i < 100 ? true : false : i < 100 ? true : false; + · ────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ const foo = i > 5 ? true : i < 100 ? true : false; + · ────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? bar : baz === qux ? quxx : foobar; + · ─────────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected nested ternary expression without parentheses. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? baz === qux ? quxx : foobar : bar; + · ─────────────────────────── + ╰──── + help: Add parentheses around the nested ternary expression. + + ⚠ eslint-plugin-unicorn(no-nested-ternary): Unexpected deeply nested ternary expression. + ╭─[no_nested_ternary.tsx:9:1] + 9 │ f : + 10 │ (g ? h : i) + · ───────── + 11 │ ) + ╰──── + help: Avoid nesting ternary expressions for more than one level. + +