Skip to content

Commit 5b2fc39

Browse files
DonIsaacBoshen
DonIsaac
and
Boshen
authored
feat(linter): add use-isnan fixer for (in)equality operations (#3284)
Co-authored-by: Boshen <boshenc@gmail.com>
1 parent 508dae6 commit 5b2fc39

1 file changed

Lines changed: 72 additions & 23 deletions

File tree

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

Lines changed: 72 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
use oxc_ast::{ast::Expression, AstKind};
1+
use oxc_ast::{
2+
ast::{BinaryExpression, Expression},
3+
AstKind,
4+
};
25
use oxc_diagnostics::OxcDiagnostic;
36
use oxc_macros::declare_oxc_lint;
47
use oxc_span::{GetSpan, Span};
8+
use oxc_syntax::operator::BinaryOperator;
59

6-
use crate::{context::LintContext, rule::Rule, AstNode};
10+
use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
711

812
fn comparison_with_na_n(span0: Span) -> OxcDiagnostic {
913
OxcDiagnostic::warning("eslint(use-isnan): Requires calls to isNaN() when checking for NaN")
@@ -76,21 +80,30 @@ declare_oxc_lint!(
7680
impl Rule for UseIsnan {
7781
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
7882
match node.kind() {
79-
AstKind::BinaryExpression(expr)
80-
if expr.operator.is_compare() || expr.operator.is_equality() =>
81-
{
83+
AstKind::BinaryExpression(expr) if expr.operator.is_compare() => {
8284
if is_nan_identifier(&expr.left) {
8385
ctx.diagnostic(comparison_with_na_n(expr.left.span()));
8486
}
8587
if is_nan_identifier(&expr.right) {
8688
ctx.diagnostic(comparison_with_na_n(expr.right.span()));
8789
}
8890
}
91+
AstKind::BinaryExpression(expr) if expr.operator.is_equality() => {
92+
if is_nan_identifier(&expr.left) {
93+
ctx.diagnostic_with_fix(comparison_with_na_n(expr.left.span()), || {
94+
Fix::new(make_equality_fix(true, expr, ctx), expr.span)
95+
});
96+
}
97+
if is_nan_identifier(&expr.right) {
98+
ctx.diagnostic_with_fix(comparison_with_na_n(expr.right.span()), || {
99+
Fix::new(make_equality_fix(false, expr, ctx), expr.span)
100+
});
101+
}
102+
}
89103
AstKind::SwitchCase(case) if self.enforce_for_switch_case => {
90-
if let Some(test) = &case.test {
91-
if is_nan_identifier(test) {
92-
ctx.diagnostic(case_na_n(test.span()));
93-
}
104+
let Some(test) = &case.test else { return };
105+
if is_nan_identifier(test) {
106+
ctx.diagnostic(case_na_n(test.span()));
94107
}
95108
}
96109
AstKind::SwitchStatement(switch) if self.enforce_for_switch_case => {
@@ -99,14 +112,16 @@ impl Rule for UseIsnan {
99112
}
100113
}
101114
AstKind::CallExpression(call) if self.enforce_for_index_of => {
102-
// Match target array prototype methods whose only argument is NaN
103-
if let Some(method) = is_target_callee(&call.callee) {
104-
if call.arguments.len() == 1 {
105-
if let Some(expr) = call.arguments[0].as_expression() {
106-
if is_nan_identifier(expr) {
107-
ctx.diagnostic(index_of_na_n(method, expr.span()));
108-
}
109-
}
115+
// do this check first b/c it's cheaper than is_target_callee
116+
if call.arguments.len() != 1 {
117+
return;
118+
};
119+
// Match target array prototype methods whose only argument is
120+
// NaN
121+
let Some(method) = is_target_callee(&call.callee) else { return };
122+
if let Some(expr) = call.arguments[0].as_expression() {
123+
if is_nan_identifier(expr) {
124+
ctx.diagnostic(index_of_na_n(method, expr.span()));
110125
}
111126
}
112127
}
@@ -149,16 +164,35 @@ fn is_target_callee<'a>(callee: &'a Expression<'a>) -> Option<&'static str> {
149164
}
150165

151166
if let Expression::ChainExpression(chain) = callee {
152-
if let Some(expr) = chain.expression.as_member_expression() {
153-
return expr.static_property_name().and_then(|property| {
154-
TARGET_METHODS.iter().find(|method| **method == property).copied()
155-
});
156-
}
167+
let expr = chain.expression.as_member_expression()?;
168+
return expr.static_property_name().and_then(|property| {
169+
TARGET_METHODS.iter().find(|method| **method == property).copied()
170+
});
157171
}
158172

159173
None
160174
}
161175

176+
fn make_equality_fix<'a>(
177+
nan_on_left: bool,
178+
comparison: &BinaryExpression<'a>,
179+
ctx: &LintContext<'a>,
180+
) -> String {
181+
let non_nan = if nan_on_left {
182+
comparison.right.span().source_text(ctx.source_text())
183+
} else {
184+
comparison.left.span().source_text(ctx.source_text())
185+
};
186+
187+
let maybe_bang = match comparison.operator {
188+
BinaryOperator::Equality | BinaryOperator::StrictEquality => "",
189+
BinaryOperator::Inequality | BinaryOperator::StrictInequality => "!",
190+
_ => unreachable!(),
191+
};
192+
193+
format!("{maybe_bang}isNaN({non_nan})")
194+
}
195+
162196
#[test]
163197
fn test() {
164198
use crate::tester::Tester;
@@ -427,5 +461,20 @@ fn test() {
427461
("(foo?.indexOf)(Number.NaN)", Some(serde_json::json!([{ "enforceForIndexOf": true }]))),
428462
];
429463

430-
Tester::new(UseIsnan::NAME, pass, fail).test_and_snapshot();
464+
let fix = vec![
465+
("1 == NaN", "isNaN(1)", None),
466+
("1 === NaN", "isNaN(1)", None),
467+
("1 != NaN", "!isNaN(1)", None),
468+
("1 !== NaN", "!isNaN(1)", None),
469+
("NaN == 'foo'", "isNaN('foo')", None),
470+
("NaN === 'foo'", "isNaN('foo')", None),
471+
("NaN != 'foo'", "!isNaN('foo')", None),
472+
("NaN !== 'foo'", "!isNaN('foo')", None),
473+
("1 == Number.NaN", "isNaN(1)", None),
474+
("1 === Number.NaN", "isNaN(1)", None),
475+
("1 != Number.NaN", "!isNaN(1)", None),
476+
("1 !== Number.NaN", "!isNaN(1)", None),
477+
];
478+
479+
Tester::new(UseIsnan::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
431480
}

0 commit comments

Comments
 (0)