Skip to content

Commit 699ec64

Browse files
huangtiandi1999autofix-ci[bot]
huangtiandi1999
and
autofix-ci[bot]
authored
feat(linter): add autofix to eslint/no-unneeded-ternary (oxc-project#11184)
### Auto-fix Example ```js let a = x === 2 ? true : false; // let a = x === 2; ``` ```js foo() ? false : true; // !foo(); ``` ```typescript foo ? foo : bar as any // foo || (bar as any) ``` ```typescript let a = {} satisfies User ? true : false // let a = !!({} satisfies User) ``` --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 35761ae commit 699ec64

File tree

1 file changed

+202
-106
lines changed

1 file changed

+202
-106
lines changed

‎crates/oxc_linter/src/rules/eslint/no_unneeded_ternary.rs‎

Lines changed: 202 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use crate::{AstNode, context::LintContext, rule::Rule};
2-
use oxc_ast::{AstKind, ast::Expression};
2+
use oxc_ast::{
3+
AstKind,
4+
ast::{BinaryOperator, Expression},
5+
};
36
use oxc_diagnostics::OxcDiagnostic;
47
use oxc_macros::declare_oxc_lint;
5-
use oxc_span::Span;
8+
use oxc_span::{GetSpan, Span};
69

710
fn no_unneeded_ternary_diagnostic(span: Span) -> OxcDiagnostic {
811
OxcDiagnostic::warn("Unnecessary use of boolean literals in conditional expression")
@@ -54,7 +57,7 @@ declare_oxc_lint!(
5457
NoUnneededTernary,
5558
eslint,
5659
suspicious,
57-
pending
60+
fix_dangerous
5861
);
5962

6063
impl Rule for NoUnneededTernary {
@@ -75,18 +78,111 @@ impl Rule for NoUnneededTernary {
7578
if matches!(expr.consequent, Expression::BooleanLiteral(_))
7679
&& matches!(expr.alternate, Expression::BooleanLiteral(_))
7780
{
78-
ctx.diagnostic(no_unneeded_ternary_diagnostic(expr.span));
81+
ctx.diagnostic_with_dangerous_fix(no_unneeded_ternary_diagnostic(expr.span), |fixer| {
82+
let (Expression::BooleanLiteral(left), Expression::BooleanLiteral(right)) =
83+
(&expr.consequent, &expr.alternate)
84+
else {
85+
return fixer.noop();
86+
};
87+
if left.value == right.value {
88+
return fixer.replace_with(expr, &expr.consequent);
89+
}
90+
let replacement;
91+
let test_expr_source = ctx.source_range(expr.test.span());
92+
match &expr.test {
93+
Expression::BinaryExpression(binary) => {
94+
if left.value {
95+
replacement = test_expr_source.to_string();
96+
} else {
97+
replacement = match binary.operator {
98+
BinaryOperator::Equality | BinaryOperator::StrictEquality => {
99+
let op = if matches!(binary.operator, BinaryOperator::Equality)
100+
{
101+
"!="
102+
} else {
103+
"!=="
104+
};
105+
format!(
106+
"{} {op} {}",
107+
ctx.source_range(binary.left.span()),
108+
ctx.source_range(binary.right.span())
109+
)
110+
}
111+
BinaryOperator::Inequality | BinaryOperator::StrictInequality => {
112+
let op =
113+
if matches!(binary.operator, BinaryOperator::Inequality) {
114+
"=="
115+
} else {
116+
"==="
117+
};
118+
format!(
119+
"{} {op} {}",
120+
ctx.source_range(binary.left.span()),
121+
ctx.source_range(binary.right.span())
122+
)
123+
}
124+
_ => {
125+
format!("!({test_expr_source})")
126+
}
127+
};
128+
}
129+
}
130+
Expression::UnaryExpression(unary) if left.value && unary.operator.is_not() => {
131+
// !x ? true : false => !x
132+
replacement = test_expr_source.to_string();
133+
}
134+
_ => {
135+
let prefix = if left.value { "!!" } else { "!" };
136+
replacement = if without_parenthesize(&expr.test) {
137+
format!("{prefix}{test_expr_source}")
138+
} else {
139+
format!("{prefix}({test_expr_source})")
140+
};
141+
}
142+
}
143+
fixer.replace(expr.span, replacement)
144+
});
79145
} else if let (Some(test), Some(cons)) = (
80146
(&expr.test.get_inner_expression().get_identifier_reference()),
81147
(&expr.consequent.get_inner_expression().get_identifier_reference()),
82148
) {
83149
if !self.default_assignment && test.name == cons.name {
84-
ctx.diagnostic(no_unneeded_ternary_conditional_expression_diagnostic(expr.span));
150+
ctx.diagnostic_with_dangerous_fix(
151+
no_unneeded_ternary_conditional_expression_diagnostic(expr.span),
152+
|fixer| {
153+
// x ? x : 1 => x || 1
154+
// x ? x : y ? 1 : 2 => x || (y ? 1 : 2)
155+
let prefix = ctx.source_range(expr.test.span());
156+
let alternate_str = ctx.source_range(expr.alternate.span());
157+
let suffix = if expr.alternate.is_primary_expression() {
158+
alternate_str.to_string()
159+
} else {
160+
format!("({alternate_str})")
161+
};
162+
let replacement = format!("{prefix} || {suffix}");
163+
fixer.replace(expr.span, replacement)
164+
},
165+
);
85166
}
86167
}
87168
}
88169
}
89170

171+
fn without_parenthesize(node: &Expression) -> bool {
172+
matches!(
173+
node,
174+
Expression::Identifier(_)
175+
| Expression::UnaryExpression(_)
176+
| Expression::StaticMemberExpression(_)
177+
| Expression::AwaitExpression(_)
178+
| Expression::UpdateExpression(_)
179+
| Expression::CallExpression(_)
180+
| Expression::ChainExpression(_)
181+
| Expression::ImportExpression(_)
182+
| Expression::NewExpression(_)
183+
)
184+
}
185+
90186
#[test]
91187
fn test() {
92188
use crate::tester::Tester;
@@ -168,105 +264,105 @@ fn test() {
168264
];
169265

170266
// I keep the fix tets commented until they are implemented
171-
// let fix = vec![
172-
// ("var a = x === 2 ? true : false;", "var a = x === 2;", None),
173-
// ("var a = x >= 2 ? true : false;", "var a = x >= 2;", None),
174-
// ("var a = x ? true : false;", "var a = !!x;", None),
175-
// ("var a = x === 1 ? false : true;", "var a = x !== 1;", None),
176-
// ("var a = x != 1 ? false : true;", "var a = x == 1;", None),
177-
// ("var a = foo() ? false : true;", "var a = !foo();", None),
178-
// ("var a = !foo() ? false : true;", "var a = !!foo();", None),
179-
// ("var a = foo + bar ? false : true;", "var a = !(foo + bar);", None),
180-
// ("var a = x instanceof foo ? false : true;", "var a = !(x instanceof foo);", None),
181-
// ("var a = foo ? false : false;", "var a = false;", None),
182-
// ("var a = x instanceof foo ? true : false;", "var a = x instanceof foo;", None),
183-
// ("var a = !foo ? true : false;", "var a = !foo;", None),
184-
// (
185-
// "
186-
// var value = 'a'
187-
// var canSet = true
188-
// var result = value ? value : canSet ? 'unset' : 'can not set'
189-
// ",
190-
// "
191-
// var value = 'a'
192-
// var canSet = true
193-
// var result = value || (canSet ? 'unset' : 'can not set')
194-
// ",
195-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
196-
// ),
197-
// (
198-
// "foo ? foo : (bar ? baz : qux)",
199-
// "foo || (bar ? baz : qux)",
200-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
201-
// ),
202-
// (
203-
// "function* fn() { foo ? foo : yield bar }",
204-
// "function* fn() { foo || (yield bar) }",
205-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
206-
// ),
207-
// (
208-
// "var a = foo ? foo : 'No';",
209-
// "var a = foo || 'No';",
210-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
211-
// ),
212-
// (
213-
// "var a = ((foo)) ? (((((foo))))) : ((((((((((((((bar))))))))))))));",
214-
// "var a = ((foo)) || ((((((((((((((bar))))))))))))));",
215-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
216-
// ),
217-
// (
218-
// "var a = b ? b : c => c;",
219-
// "var a = b || (c => c);",
220-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
221-
// ),
222-
// (
223-
// "var a = b ? b : c = 0;",
224-
// "var a = b || (c = 0);",
225-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
226-
// ),
227-
// (
228-
// "var a = b ? b : (c => c);",
229-
// "var a = b || (c => c);",
230-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
231-
// ),
232-
// (
233-
// "var a = b ? b : (c = 0);",
234-
// "var a = b || (c = 0);",
235-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
236-
// ),
237-
// (
238-
// "var a = b ? b : (c) => (c);",
239-
// "var a = b || ((c) => (c));",
240-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
241-
// ),
242-
// (
243-
// "var a = b ? b : c, d; // this is ((b ? b : c), (d))",
244-
// "var a = b || c, d; // this is ((b ? b : c), (d))",
245-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
246-
// ),
247-
// (
248-
// "var a = b ? b : (c, d);",
249-
// "var a = b || (c, d);",
250-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
251-
// ),
252-
// ("f(x ? x : 1);", "f(x || 1);", Some(serde_json::json!([{ "defaultAssignment": false }]))),
253-
// ("x ? x : 1;", "x || 1;", Some(serde_json::json!([{ "defaultAssignment": false }]))),
254-
// (
255-
// "var a = foo ? foo : bar;",
256-
// "var a = foo || bar;",
257-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
258-
// ),
259-
// (
260-
// "var a = foo ? foo : a ?? b;",
261-
// "var a = foo || (a ?? b);",
262-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
263-
// ),
264-
// ("foo as any ? false : true", "!(foo as any)", None),
265-
// (
266-
// "foo ? foo : bar as any",
267-
// "foo || (bar as any)",
268-
// Some(serde_json::json!([{ "defaultAssignment": false }])),
269-
// ),
270-
// ];
271-
Tester::new(NoUnneededTernary::NAME, NoUnneededTernary::PLUGIN, pass, fail).test_and_snapshot();
267+
let fix = vec![
268+
("var a = x === 2 ? true : false;", "var a = x === 2;", None),
269+
("var a = x >= 2 ? true : false;", "var a = x >= 2;", None),
270+
("var a = x ? true : false;", "var a = !!x;", None),
271+
("var a = x === 1 ? false : true;", "var a = x !== 1;", None),
272+
("var a = x != 1 ? false : true;", "var a = x == 1;", None),
273+
("var a = foo() ? false : true;", "var a = !foo();", None),
274+
("var a = !foo() ? false : true;", "var a = !!foo();", None),
275+
("var a = foo + bar ? false : true;", "var a = !(foo + bar);", None),
276+
("var a = x instanceof foo ? false : true;", "var a = !(x instanceof foo);", None),
277+
("var a = foo ? false : false;", "var a = false;", None),
278+
("var a = x instanceof foo ? true : false;", "var a = x instanceof foo;", None),
279+
("var a = !foo ? true : false;", "var a = !foo;", None),
280+
(
281+
"var result = value ? value : canSet ? 'unset' : 'can not set'",
282+
"var result = value || (canSet ? 'unset' : 'can not set')",
283+
Some(serde_json::json!([{ "defaultAssignment": false }])),
284+
),
285+
(
286+
"foo ? foo : (bar ? baz : qux)",
287+
"foo || (bar ? baz : qux)",
288+
Some(serde_json::json!([{ "defaultAssignment": false }])),
289+
),
290+
(
291+
"function* fn() { foo ? foo : yield bar }",
292+
"function* fn() { foo || (yield bar) }",
293+
Some(serde_json::json!([{ "defaultAssignment": false }])),
294+
),
295+
(
296+
"var a = foo ? foo : 'No';",
297+
"var a = foo || 'No';",
298+
Some(serde_json::json!([{ "defaultAssignment": false }])),
299+
),
300+
(
301+
"var a = ((foo)) ? (((((foo))))) : ((((((((((((((bar))))))))))))));",
302+
"var a = ((foo)) || ((((((((((((((bar))))))))))))));",
303+
Some(serde_json::json!([{ "defaultAssignment": false }])),
304+
),
305+
(
306+
"var a = b ? b : c => c;",
307+
"var a = b || (c => c);",
308+
Some(serde_json::json!([{ "defaultAssignment": false }])),
309+
),
310+
(
311+
"var a = b ? b : c = 0;",
312+
"var a = b || (c = 0);",
313+
Some(serde_json::json!([{ "defaultAssignment": false }])),
314+
),
315+
(
316+
"var a = b ? b : (c => c);",
317+
"var a = b || (c => c);",
318+
Some(serde_json::json!([{ "defaultAssignment": false }])),
319+
),
320+
(
321+
"var a = b ? b : (c = 0);",
322+
"var a = b || (c = 0);",
323+
Some(serde_json::json!([{ "defaultAssignment": false }])),
324+
),
325+
(
326+
"var a = b ? b : (c) => (c);",
327+
"var a = b || ((c) => (c));",
328+
Some(serde_json::json!([{ "defaultAssignment": false }])),
329+
),
330+
(
331+
"var a = b ? b : c, d; // this is ((b ? b : c), (d))",
332+
"var a = b || c, d; // this is ((b ? b : c), (d))",
333+
Some(serde_json::json!([{ "defaultAssignment": false }])),
334+
),
335+
(
336+
"var a = b ? b : (c, d);",
337+
"var a = b || (c, d);",
338+
Some(serde_json::json!([{ "defaultAssignment": false }])),
339+
),
340+
("f(x ? x : 1);", "f(x || 1);", Some(serde_json::json!([{ "defaultAssignment": false }]))),
341+
("x ? x : 1;", "x || 1;", Some(serde_json::json!([{ "defaultAssignment": false }]))),
342+
(
343+
"var a = foo ? foo : bar;",
344+
"var a = foo || bar;",
345+
Some(serde_json::json!([{ "defaultAssignment": false }])),
346+
),
347+
(
348+
"var a = foo ? foo : a ?? b;",
349+
"var a = foo || (a ?? b);",
350+
Some(serde_json::json!([{ "defaultAssignment": false }])),
351+
),
352+
("foo as any ? false : true", "!(foo as any)", None),
353+
(
354+
"foo ? foo : bar as any",
355+
"foo || (bar as any)",
356+
Some(serde_json::json!([{ "defaultAssignment": false }])),
357+
),
358+
(
359+
"a ? a : b ?? 2",
360+
"a || (b ?? 2)",
361+
Some(serde_json::json!([{ "defaultAssignment": false }])),
362+
),
363+
("let a = {} satisfies User ? true : false", "let a = !!({} satisfies User)", None),
364+
];
365+
Tester::new(NoUnneededTernary::NAME, NoUnneededTernary::PLUGIN, pass, fail)
366+
.expect_fix(fix)
367+
.test_and_snapshot();
272368
}

0 commit comments

Comments
 (0)