From 5029dfde5a5bf9f8528fb8932821ca82fb3d1a23 Mon Sep 17 00:00:00 2001 From: Cameron Date: Thu, 26 Oct 2023 04:03:12 +0100 Subject: [PATCH] feat(linter) eslint-plugin-unicorn - prefer-logical-operator-over-ternary (#1064) --- crates/oxc_linter/src/rules.rs | 34 +-- .../prefer_logical_operator_over_ternary.rs | 208 ++++++++++++++++++ .../prefer_logical_operator_over_ternary.snap | 110 +++++++++ 3 files changed, 336 insertions(+), 16 deletions(-) create mode 100644 crates/oxc_linter/src/rules/unicorn/prefer_logical_operator_over_ternary.rs create mode 100644 crates/oxc_linter/src/snapshots/prefer_logical_operator_over_ternary.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 47e2349c9..a6950e74e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/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 } diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_logical_operator_over_ternary.rs b/crates/oxc_linter/src/rules/unicorn/prefer_logical_operator_over_ternary.rs new file mode 100644 index 000000000..29ef638f1 --- /dev/null +++ b/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(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_logical_operator_over_ternary.snap b/crates/oxc_linter/src/snapshots/prefer_logical_operator_over_ternary.snap new file mode 100644 index 000000000..ade2edf6f --- /dev/null +++ b/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 + +