feat(linter) eslint-plugin-unicorn - prefer-logical-operator-over-ternary (#1064)

crates/oxc_linter/src/rules.rs

@@ -147,6 +147,7 @@ mod unicorn {
     pub mod no_thenable;
     pub mod no_unnecessary_await;
     pub mod prefer_array_flat_map;
+    pub mod prefer_logical_operator_over_ternary;
     pub mod text_encoding_identifier_case;
     pub mod throw_new_error;
 }
@@ -184,9 +185,9 @@ oxc_macros::declare_all_lint_rules! {
     eslint::no_dupe_else_if,
     eslint::no_dupe_keys,
     eslint::no_duplicate_case,
+    eslint::no_empty,
     eslint::no_empty_character_class,
     eslint::no_empty_pattern,
-    eslint::no_empty,
     eslint::no_eval,
     eslint::no_ex_assign,
     eslint::no_extra_boolean_cast,
@@ -225,31 +226,31 @@ oxc_macros::declare_all_lint_rules! {
     typescript::no_empty_interface,
     typescript::no_explicit_any,
     typescript::no_extra_non_null_assertion,
+    typescript::no_misused_new,
+    typescript::no_namespace,
     typescript::no_non_null_asserted_optional_chain,
+    typescript::no_this_alias,
     typescript::no_unnecessary_type_constraint,
     typescript::no_unsafe_declaration_merging,
-    typescript::no_misused_new,
-    typescript::no_this_alias,
-    typescript::no_namespace,
     typescript::no_var_requires,
     typescript::prefer_as_const,
-    jest::no_disabled_tests,
-    jest::no_test_prefixes,
-    jest::no_focused_tests,
-    jest::valid_describe_callback,
-    jest::valid_expect,
-    jest::no_commented_out_tests,
     jest::expect_expect,
     jest::no_alias_methods,
+    jest::no_commented_out_tests,
     jest::no_conditional_expect,
     jest::no_confusing_set_timeout,
+    jest::no_disabled_tests,
     jest::no_done_callback,
+    jest::no_export,
+    jest::no_focused_tests,
+    jest::no_identical_title,
     jest::no_interpolation_in_snapshots,
     jest::no_jasmine_globals,
     jest::no_mocks_import,
-    jest::no_export,
     jest::no_standalone_expect,
-    jest::no_identical_title,
+    jest::no_test_prefixes,
+    jest::valid_describe_callback,
+    jest::valid_expect,
     jest::valid_title,
     unicorn::catch_error_name,
     unicorn::error_message,
@@ -257,23 +258,24 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::no_console_spaces,
     unicorn::no_empty_file,
     unicorn::no_instanceof_array,
-    unicorn::no_unnecessary_await,
     unicorn::no_thenable,
+    unicorn::no_unnecessary_await,
+    unicorn::prefer_array_flat_map,
+    unicorn::prefer_logical_operator_over_ternary,
     unicorn::text_encoding_identifier_case,
     unicorn::throw_new_error,
-    unicorn::prefer_array_flat_map,
     react::jsx_key,
     react::jsx_no_comment_text_nodes,
     react::jsx_no_duplicate_props,
-    react::no_unescaped_entities,
     react::jsx_no_useless_fragment,
     react::no_children_prop,
     react::no_dangerously_set_inner_html,
     react::no_find_dom_node,
     react::no_render_return_value,
     react::no_string_refs,
+    react::no_unescaped_entities,
+    import::default,
     import::named,
     import::no_cycle,
     import::no_self_import,
-    import::default
 }

crates/oxc_linter/src/rules/unicorn/prefer_logical_operator_over_ternary.rs

@@ -0,0 +1,208 @@
+use oxc_ast::{
+    ast::{ChainElement, Expression, MemberExpression},
+    AstKind,
+};
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::Error,
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_span::{GetSpan, Span};
+use oxc_syntax::operator::UnaryOperator;
+
+use crate::{context::LintContext, rule::Rule, AstNode};
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.")]
+#[diagnostic(severity(warning), help("Switch to \"||\" or \"??\" operator"))]
+struct PreferLogicalOperatorOverTernaryDiagnostic(#[label] pub Span);
+
+#[derive(Debug, Default, Clone)]
+pub struct PreferLogicalOperatorOverTernary;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// This rule finds ternary expressions that can be simplified to a logical operator.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using a logical operator is shorter and simpler than a ternary expression.
+    ///
+    /// ### Example
+    /// ```javascript
+    ///
+    /// // Bad
+    /// const foo = bar ? bar : baz;
+    /// console.log(foo ? foo : bar);
+    ///
+    /// // Good
+    /// const foo = bar || baz;
+    /// console.log(foo ?? bar);
+    ///
+    /// ```
+    PreferLogicalOperatorOverTernary,
+    correctness
+);
+
+impl Rule for PreferLogicalOperatorOverTernary {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::ConditionalExpression(conditional_expression) = node.kind() else { return };
+
+        if is_same_node(&conditional_expression.test, &conditional_expression.consequent, ctx) {
+            ctx.diagnostic(PreferLogicalOperatorOverTernaryDiagnostic(conditional_expression.span));
+        }
+
+        // `!bar ? foo : bar;`
+        if let Expression::UnaryExpression(unary_expression) = &conditional_expression.test {
+            if unary_expression.operator == UnaryOperator::LogicalNot
+                && is_same_node(&unary_expression.argument, &conditional_expression.alternate, ctx)
+            {
+                ctx.diagnostic(PreferLogicalOperatorOverTernaryDiagnostic(
+                    conditional_expression.span,
+                ));
+            }
+        }
+    }
+}
+
+fn is_same_node(left: &Expression, right: &Expression, ctx: &LintContext) -> bool {
+    if is_same_reference(left, right, ctx) {
+        return true;
+    }
+
+    match (left, right) {
+        (
+            Expression::AwaitExpression(left_await_expr),
+            Expression::AwaitExpression(right_await_expr),
+        ) => return is_same_node(&left_await_expr.argument, &right_await_expr.argument, ctx),
+        (
+            Expression::LogicalExpression(left_await_expr),
+            Expression::LogicalExpression(right_await_expr),
+        ) => {
+            return is_same_node(&left_await_expr.left, &right_await_expr.left, ctx)
+                && is_same_node(&left_await_expr.right, &right_await_expr.right, ctx)
+        }
+        (
+            Expression::UnaryExpression(left_await_expr),
+            Expression::UnaryExpression(right_await_expr),
+        ) => return is_same_node(&left_await_expr.argument, &right_await_expr.argument, ctx),
+        (Expression::UpdateExpression(_), Expression::UpdateExpression(_)) => return false,
+        _ => {}
+    }
+
+    left.span().source_text(ctx.source_text()) == right.span().source_text(ctx.source_text())
+}
+
+fn is_same_reference(left: &Expression, right: &Expression, ctx: &LintContext) -> bool {
+    match (left, right) {
+        (
+            Expression::ChainExpression(left_chain_expr),
+            Expression::MemberExpression(right_member_expr),
+        ) => {
+            if let ChainElement::MemberExpression(v) = &left_chain_expr.expression {
+                return is_same_member_expression(v, right_member_expr, ctx);
+            }
+        }
+        (
+            Expression::MemberExpression(left_chain_expr),
+            Expression::ChainExpression(right_member_expr),
+        ) => {
+            if let ChainElement::MemberExpression(v) = &right_member_expr.expression {
+                return is_same_member_expression(left_chain_expr, v, ctx);
+            }
+        }
+
+        // super // this
+        (Expression::Super(_), Expression::Super(_))
+        | (Expression::ThisExpression(_), Expression::ThisExpression(_))
+        | (Expression::NullLiteral(_), Expression::NullLiteral(_)) => return true,
+
+        (Expression::Identifier(left_ident), Expression::Identifier(right_ident)) => {
+            return left_ident.name == right_ident.name
+        }
+
+        (Expression::StringLiteral(left_str), Expression::StringLiteral(right_str)) => {
+            return left_str.value == right_str.value
+        }
+        (Expression::NumberLiteral(left_num), Expression::NumberLiteral(right_num)) => {
+            return left_num.raw == right_num.raw
+        }
+        (Expression::RegExpLiteral(left_regexp), Expression::RegExpLiteral(right_regexp)) => {
+            return left_regexp.regex.pattern == right_regexp.regex.pattern
+                && left_regexp.regex.flags == right_regexp.regex.flags
+        }
+        (Expression::BooleanLiteral(left_bool), Expression::BooleanLiteral(right_bool)) => {
+            return left_bool.value == right_bool.value
+        }
+
+        (
+            Expression::ChainExpression(left_chain_expr),
+            Expression::ChainExpression(right_chain_expr),
+        ) => {
+            if let ChainElement::MemberExpression(left_member_expr) = &left_chain_expr.expression {
+                if let ChainElement::MemberExpression(right_member_expr) =
+                    &right_chain_expr.expression
+                {
+                    return is_same_member_expression(left_member_expr, right_member_expr, ctx);
+                }
+            }
+        }
+        (
+            Expression::MemberExpression(left_member_expr),
+            Expression::MemberExpression(right_member_expr),
+        ) => return is_same_member_expression(left_member_expr, right_member_expr, ctx),
+        _ => {}
+    }
+
+    false
+}
+
+fn is_same_member_expression(
+    left: &MemberExpression,
+    right: &MemberExpression,
+    ctx: &LintContext,
+) -> bool {
+    let left_static_property_name = left.static_property_name();
+    let right_static_property_name = right.static_property_name();
+
+    if left_static_property_name != right_static_property_name {
+        return false;
+    };
+
+    return is_same_reference(left.object(), right.object(), ctx);
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "foo ? foo1 : bar;",
+        "foo.bar ? foo.bar1 : foo.baz",
+        "foo.bar ? foo1.bar : foo.baz",
+        "++foo ? ++foo : bar;",
+        "!!bar ? foo : bar;",
+    ];
+
+    let fail = vec![
+        "foo ? foo : bar;",
+        "foo.bar ? foo.bar : foo.baz",
+        "foo?.bar ? foo.bar : baz",
+        "foo?.bar ? foo?.bar : baz",
+        "!bar ? foo : bar;",
+        "!!bar ? foo : !bar;",
+        "foo() ? foo() : bar",
+        "foo ? foo : a && b",
+        "foo ? foo : a || b",
+        "foo ? foo : a ?? b",
+        "a && b ? a && b : bar",
+        "a || b ? a || b : bar",
+        "a ?? b ? a ?? b : bar",
+        "foo ? foo : await a",
+        "await a ? await a : foo",
+    ];
+
+    Tester::new_without_config(PreferLogicalOperatorOverTernary::NAME, pass, fail)
+        .test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/prefer_logical_operator_over_ternary.snap

@@ -0,0 +1,110 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: prefer_logical_operator_over_ternary
+---
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo ? foo : bar;
+   · ───────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo.bar ? foo.bar : foo.baz
+   · ───────────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo?.bar ? foo.bar : baz
+   · ────────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo?.bar ? foo?.bar : baz
+   · ─────────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ !bar ? foo : bar;
+   · ────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ !!bar ? foo : !bar;
+   · ──────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo() ? foo() : bar
+   · ───────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo ? foo : a && b
+   · ──────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo ? foo : a || b
+   · ──────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo ? foo : a ?? b
+   · ──────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ a && b ? a && b : bar
+   · ─────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ a || b ? a || b : bar
+   · ─────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ a ?? b ? a ?? b : bar
+   · ─────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ foo ? foo : await a
+   · ───────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+  ⚠ eslint-plugin-unicorn(prefer-logical-operator-over-ternary): Prefer using a logical operator over a ternary.
+   ╭─[prefer_logical_operator_over_ternary.tsx:1:1]
+ 1 │ await a ? await a : foo
+   · ───────────────────────
+   ╰────
+  help: Switch to "||" or "??" operator
+
+