Skip to content

Commit 34c3ec3

Browse files
committed
feat(linter/prefer-logical-operator-over-ternary): implement fixer (#18545)
1 parent 1117c44 commit 34c3ec3

1 file changed

Lines changed: 109 additions & 8 deletions

File tree

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

Lines changed: 109 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ declare_oxc_lint!(
4040
PreferLogicalOperatorOverTernary,
4141
unicorn,
4242
style,
43-
pending
43+
suggestion,
4444
);
4545

4646
impl Rule for PreferLogicalOperatorOverTernary {
@@ -49,24 +49,103 @@ impl Rule for PreferLogicalOperatorOverTernary {
4949
return;
5050
};
5151

52+
// `foo ? foo : bar` -> `foo || bar`
5253
if is_same_node(&conditional_expression.test, &conditional_expression.consequent, ctx) {
53-
ctx.diagnostic(prefer_logical_operator_over_ternary_diagnostic(
54-
conditional_expression.span,
55-
));
54+
let test_expr = preferred_test_expr(
55+
&conditional_expression.test,
56+
&conditional_expression.consequent,
57+
);
58+
let test_text =
59+
wrap_nullish_coalesce_operand(test_expr, ctx.source_range(test_expr.span()));
60+
let alternate_text = wrap_nullish_coalesce_operand(
61+
&conditional_expression.alternate,
62+
ctx.source_range(conditional_expression.alternate.span()),
63+
);
64+
let span = conditional_expression.span;
65+
66+
ctx.diagnostic_with_suggestion(
67+
prefer_logical_operator_over_ternary_diagnostic(span),
68+
|fixer| fixer.replace(span, format!("{test_text} || {alternate_text}")),
69+
);
70+
return;
5671
}
5772

58-
// `!bar ? foo : bar;`
73+
// `!bar ? foo : bar` -> `bar || foo`
5974
if let Expression::UnaryExpression(unary_expression) = &conditional_expression.test
6075
&& unary_expression.operator == UnaryOperator::LogicalNot
6176
&& is_same_node(&unary_expression.argument, &conditional_expression.alternate, ctx)
6277
{
63-
ctx.diagnostic(prefer_logical_operator_over_ternary_diagnostic(
64-
conditional_expression.span,
65-
));
78+
let alternate_expr = preferred_alternate_expr(
79+
&conditional_expression.test,
80+
&conditional_expression.alternate,
81+
ctx,
82+
);
83+
let alternate_text = wrap_nullish_coalesce_operand(
84+
alternate_expr,
85+
ctx.source_range(alternate_expr.span()),
86+
);
87+
let consequent_text = wrap_nullish_coalesce_operand(
88+
&conditional_expression.consequent,
89+
ctx.source_range(conditional_expression.consequent.span()),
90+
);
91+
let span = conditional_expression.span;
92+
93+
ctx.diagnostic_with_suggestion(
94+
prefer_logical_operator_over_ternary_diagnostic(span),
95+
|fixer| fixer.replace(span, format!("{alternate_text} || {consequent_text}")),
96+
);
6697
}
6798
}
6899
}
69100

101+
fn preferred_test_expr<'a>(
102+
test: &'a Expression<'a>,
103+
consequent: &'a Expression<'a>,
104+
) -> &'a Expression<'a> {
105+
if matches!(test, Expression::ParenthesizedExpression(_)) {
106+
return consequent.get_inner_expression();
107+
}
108+
109+
test.get_inner_expression()
110+
}
111+
112+
fn preferred_alternate_expr<'a>(
113+
test: &'a Expression<'a>,
114+
alternate: &'a Expression<'a>,
115+
ctx: &LintContext<'a>,
116+
) -> &'a Expression<'a> {
117+
let Expression::UnaryExpression(outer_unary) = test else {
118+
return alternate;
119+
};
120+
if outer_unary.operator != UnaryOperator::LogicalNot {
121+
return alternate;
122+
}
123+
if !is_same_node(&outer_unary.argument, alternate, ctx) {
124+
return alternate;
125+
}
126+
let Expression::UnaryExpression(inner_unary) = &outer_unary.argument else {
127+
return alternate;
128+
};
129+
if inner_unary.operator != UnaryOperator::LogicalNot {
130+
return alternate;
131+
}
132+
133+
&inner_unary.argument
134+
}
135+
136+
fn wrap_nullish_coalesce_operand(expr: &Expression, text: &str) -> String {
137+
if matches!(expr, Expression::ParenthesizedExpression(_)) {
138+
return text.to_string();
139+
}
140+
141+
match expr.without_parentheses() {
142+
Expression::LogicalExpression(logical_expr) if logical_expr.operator.is_coalesce() => {
143+
format!("({text})")
144+
}
145+
_ => text.to_string(),
146+
}
147+
}
148+
70149
fn is_same_node(left: &Expression, right: &Expression, ctx: &LintContext) -> bool {
71150
if is_same_expression(left, right, ctx) {
72151
return true;
@@ -138,11 +217,33 @@ fn test() {
138217
a && b ? a && b : 1",
139218
];
140219

220+
let fix = vec![
221+
("foo ? foo : bar;", "foo || bar;"),
222+
("foo.bar ? foo.bar : foo.baz", "foo.bar || foo.baz"),
223+
("foo?.bar ? foo.bar : baz", "foo?.bar || baz"),
224+
("foo?.bar ? foo?.bar : baz", "foo?.bar || baz"),
225+
("!bar ? foo : bar;", "bar || foo;"),
226+
("!!bar ? foo : !bar;", "bar || foo;"),
227+
("foo() ? foo() : bar", "foo() || bar"),
228+
("foo ? foo : a && b", "foo || a && b"),
229+
("foo ? foo : a || b", "foo || a || b"),
230+
("foo ? foo : a ?? b", "foo || (a ?? b)"),
231+
("a && b ? a && b : bar", "a && b || bar"),
232+
("a || b ? a || b : bar", "a || b || bar"),
233+
("a ?? b ? a ?? b : bar", "(a ?? b) || bar"),
234+
("foo ? foo : await a", "foo || await a"),
235+
("await a ? await a : foo", "await a || foo"),
236+
("await a ? (await (a)) : (foo)", "await a || (foo)"),
237+
("(await a) ? await (a) : (foo)", "await (a) || (foo)"),
238+
("(await a) ? (await (a)) : (foo)", "await (a) || (foo)"),
239+
];
240+
141241
Tester::new(
142242
PreferLogicalOperatorOverTernary::NAME,
143243
PreferLogicalOperatorOverTernary::PLUGIN,
144244
pass,
145245
fail,
146246
)
247+
.expect_fix(fix)
147248
.test_and_snapshot();
148249
}

0 commit comments

Comments
 (0)