diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 24487181fda8c..d10f1788f4543 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -129,6 +129,7 @@ mod eslint { pub mod no_undef; pub mod no_undefined; pub mod no_unexpected_multiline; + pub mod no_unneeded_ternary; pub mod no_unreachable; pub mod no_unsafe_finally; pub mod no_unsafe_negation; @@ -553,6 +554,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_params, eslint::new_cap, eslint::no_useless_call, + eslint::no_unneeded_ternary, eslint::no_extra_label, eslint::no_multi_assign, eslint::no_nested_ternary, diff --git a/crates/oxc_linter/src/rules/eslint/no_unneeded_ternary.rs b/crates/oxc_linter/src/rules/eslint/no_unneeded_ternary.rs new file mode 100644 index 0000000000000..72afe988a1219 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_unneeded_ternary.rs @@ -0,0 +1,269 @@ +use crate::{context::LintContext, rule::Rule, AstNode}; +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +fn no_unneeded_ternary_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Unnecessary use of boolean literals in conditional expression") + .with_help("Remove this ternary operator") + .with_label(span) +} + +fn no_unneeded_ternary_conditional_expression_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Unnecessary use of conditional expression for default assignment") + .with_help("Remove this ternary operator and use the variable directly") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoUnneededTernary { + default_assignment: bool, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow ternary operators when simpler alternatives exist + /// + /// ### Why is this bad? + /// + /// It’s a common mistake in JavaScript to use a conditional expression to select between two + /// Boolean values instead of using ! to convert the test to a Boolean. + /// + /// Another common mistake is using a single variable as both the conditional test and the + /// consequent. In such cases, the logical OR can be used to provide the same functionality. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// const isYes = answer === 1 ? true : false; + /// const isNo = answer === 1 ? false : true; + /// + /// foo(bar ? bar : 1); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// const isYes = answer === 1; + /// const isNo = answer !== 1; + /// + /// foo(bar || 1); + /// ``` + NoUnneededTernary, + eslint, + suspicious, + pending +); + +impl Rule for NoUnneededTernary { + fn from_configuration(value: serde_json::Value) -> Self { + let default_assignment = value + .get(0) + .and_then(|v| v.get("defaultAssignment")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(true); + + Self { default_assignment } + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::ConditionalExpression(expr) = node.kind() else { + return; + }; + if matches!(expr.consequent, Expression::BooleanLiteral(_)) + && matches!(expr.alternate, Expression::BooleanLiteral(_)) + { + ctx.diagnostic(no_unneeded_ternary_diagnostic(expr.span)); + } else if let (Some(test), Some(cons)) = ( + (&expr.test.get_inner_expression().get_identifier_reference()), + (&expr.consequent.get_inner_expression().get_identifier_reference()), + ) { + if !self.default_assignment && test.name == cons.name { + ctx.diagnostic(no_unneeded_ternary_conditional_expression_diagnostic(expr.span)); + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("config.newIsCap = config.newIsCap !== false", None), + ("var a = x === 2 ? 'Yes' : 'No';", None), + ("var a = x === 2 ? true : 'No';", None), + ("var a = x === 2 ? 'Yes' : false;", None), + ("var a = x === 2 ? 'true' : 'false';", None), + ("var a = foo ? foo : bar;", None), + ("var value = 'a';var canSet = true;var result = value || (canSet ? 'unset' : 'can not set')", None), + ("var a = foo ? bar : foo;", None), + ("foo ? bar : foo;", None), + ("var a = f(x ? x : 1)", None), + ("f(x ? x : 1);", None), + ("foo ? foo : bar;", None), + ("var a = foo ? 'Yes' : foo;", None), + ("var a = foo ? 'Yes' : foo;", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ("var a = foo ? bar : foo;", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ("foo ? bar : foo;", Some(serde_json::json!([{ "defaultAssignment": false }]))) + ]; + + let fail = vec![ + ("var a = x === 2 ? true : false;", None), + ("var a = (x === 2) ? true : false;", None), + ("var a = x >= 2 ? true : false;", None), + ("var a = x ? true : false;", None), + ("var a = x === 1 ? false : true;", None), + ("var a = x != 1 ? false : true;", None), + ("var a = foo() ? false : true;", None), + ("var a = !foo() ? false : true;", None), + ("var a = foo + bar ? false : true;", None), + ("var a = x instanceof foo ? false : true;", None), + ("var a = foo ? false : false;", None), + ("var a = foo() ? false : false;", None), + ("var a = x instanceof foo ? true : false;", None), + ("var a = !foo ? true : false;", None), + ( + " + var value = 'a' + var canSet = true + var result = value ? value : canSet ? 'unset' : 'can not set' + ", + Some(serde_json::json!([{ "defaultAssignment": false }])), + ), + ( + "foo ? foo : (bar ? baz : qux)", + Some(serde_json::json!([{ "defaultAssignment": false }])), + ), + ( + "function* fn() { foo ? foo : yield bar }", + Some(serde_json::json!([{ "defaultAssignment": false }])), + ), // { "ecmaVersion": 6 }, + ("var a = foo ? foo : 'No';", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ( + "var a = ((foo)) ? (((((foo))))) : ((((((((((((((bar))))))))))))));", + Some(serde_json::json!([{ "defaultAssignment": false }])), + ), + ("var a = b ? b : c => c;", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ("var a = b ? b : c = 0;", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ("var a = b ? b : (c => c);", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ("var a = b ? b : (c = 0);", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ("var a = b ? b : (c) => (c);", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ( + "var a = b ? b : c, d; // this is ((b ? b : c), (d))", + Some(serde_json::json!([{ "defaultAssignment": false }])), + ), // { "ecmaVersion": 2015 }, + ("var a = b ? b : (c, d);", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2015 }, + ("f(x ? x : 1);", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ("x ? x : 1;", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ("var a = foo ? foo : bar;", Some(serde_json::json!([{ "defaultAssignment": false }]))), + ("var a = foo ? foo : a ?? b;", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "ecmaVersion": 2020 }, + ("foo as any ? false : true", None), // { "parser": require(parser("typescript-parsers/unneeded-ternary-1")), "ecmaVersion": 6 }, + ("foo ? foo : bar as any", Some(serde_json::json!([{ "defaultAssignment": false }]))), // { "parser": require(parser("typescript-parsers/unneeded-ternary-2")), "ecmaVersion": 6 } + ]; + + // I keep the fix tets commented until they are implemented + // let fix = vec![ + // ("var a = x === 2 ? true : false;", "var a = x === 2;", None), + // ("var a = x >= 2 ? true : false;", "var a = x >= 2;", None), + // ("var a = x ? true : false;", "var a = !!x;", None), + // ("var a = x === 1 ? false : true;", "var a = x !== 1;", None), + // ("var a = x != 1 ? false : true;", "var a = x == 1;", None), + // ("var a = foo() ? false : true;", "var a = !foo();", None), + // ("var a = !foo() ? false : true;", "var a = !!foo();", None), + // ("var a = foo + bar ? false : true;", "var a = !(foo + bar);", None), + // ("var a = x instanceof foo ? false : true;", "var a = !(x instanceof foo);", None), + // ("var a = foo ? false : false;", "var a = false;", None), + // ("var a = x instanceof foo ? true : false;", "var a = x instanceof foo;", None), + // ("var a = !foo ? true : false;", "var a = !foo;", None), + // ( + // " + // var value = 'a' + // var canSet = true + // var result = value ? value : canSet ? 'unset' : 'can not set' + // ", + // " + // var value = 'a' + // var canSet = true + // var result = value || (canSet ? 'unset' : 'can not set') + // ", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "foo ? foo : (bar ? baz : qux)", + // "foo || (bar ? baz : qux)", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "function* fn() { foo ? foo : yield bar }", + // "function* fn() { foo || (yield bar) }", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = foo ? foo : 'No';", + // "var a = foo || 'No';", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = ((foo)) ? (((((foo))))) : ((((((((((((((bar))))))))))))));", + // "var a = ((foo)) || ((((((((((((((bar))))))))))))));", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : c => c;", + // "var a = b || (c => c);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : c = 0;", + // "var a = b || (c = 0);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : (c => c);", + // "var a = b || (c => c);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : (c = 0);", + // "var a = b || (c = 0);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : (c) => (c);", + // "var a = b || ((c) => (c));", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : c, d; // this is ((b ? b : c), (d))", + // "var a = b || c, d; // this is ((b ? b : c), (d))", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = b ? b : (c, d);", + // "var a = b || (c, d);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ("f(x ? x : 1);", "f(x || 1);", Some(serde_json::json!([{ "defaultAssignment": false }]))), + // ("x ? x : 1;", "x || 1;", Some(serde_json::json!([{ "defaultAssignment": false }]))), + // ( + // "var a = foo ? foo : bar;", + // "var a = foo || bar;", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ( + // "var a = foo ? foo : a ?? b;", + // "var a = foo || (a ?? b);", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ("foo as any ? false : true", "!(foo as any)", None), + // ( + // "foo ? foo : bar as any", + // "foo || (bar as any)", + // Some(serde_json::json!([{ "defaultAssignment": false }])), + // ), + // ]; + Tester::new(NoUnneededTernary::NAME, NoUnneededTernary::PLUGIN, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_unneeded_ternary.snap b/crates/oxc_linter/src/snapshots/eslint_no_unneeded_ternary.snap new file mode 100644 index 0000000000000..45a6d3d856abb --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_unneeded_ternary.snap @@ -0,0 +1,228 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x === 2 ? true : false; + · ────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = (x === 2) ? true : false; + · ──────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x >= 2 ? true : false; + · ───────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x ? true : false; + · ──────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x === 1 ? false : true; + · ────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x != 1 ? false : true; + · ───────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo() ? false : true; + · ──────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = !foo() ? false : true; + · ───────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo + bar ? false : true; + · ──────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x instanceof foo ? false : true; + · ─────────────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo ? false : false; + · ─────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo() ? false : false; + · ───────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = x instanceof foo ? true : false; + · ─────────────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = !foo ? true : false; + · ─────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:4:33] + 3 │ var canSet = true + 4 │ var result = value ? value : canSet ? 'unset' : 'can not set' + · ──────────────────────────────────────────────── + 5 │ + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:1] + 1 │ foo ? foo : (bar ? baz : qux) + · ───────────────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:18] + 1 │ function* fn() { foo ? foo : yield bar } + · ───────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo ? foo : 'No'; + · ──────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = ((foo)) ? (((((foo))))) : ((((((((((((((bar)))))))))))))); + · ───────────────────────────────────────────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : c => c; + · ────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : c = 0; + · ───────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : (c => c); + · ──────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : (c = 0); + · ─────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : (c) => (c); + · ────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : c, d; // this is ((b ? b : c), (d)) + · ───────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = b ? b : (c, d); + · ────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:3] + 1 │ f(x ? x : 1); + · ───────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:1] + 1 │ x ? x : 1; + · ───────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo ? foo : bar; + · ─────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:9] + 1 │ var a = foo ? foo : a ?? b; + · ────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of boolean literals in conditional expression + ╭─[no_unneeded_ternary.tsx:1:1] + 1 │ foo as any ? false : true + · ───────────────────────── + ╰──── + help: Remove this ternary operator + + ⚠ eslint(no-unneeded-ternary): Unnecessary use of conditional expression for default assignment + ╭─[no_unneeded_ternary.tsx:1:1] + 1 │ foo ? foo : bar as any + · ────────────────────── + ╰──── + help: Remove this ternary operator and use the variable directly