Skip to content

Commit

Permalink
feat(lint/useIsNan): retain global this and window
Browse files Browse the repository at this point in the history
  • Loading branch information
@victor-teles
Victor Teles authored and victor-teles committed Sep 5, 2023
1 parent 6ced677 commit 2064586
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 80 deletions.
164 changes: 105 additions & 59 deletions crates/rome_js_analyze/src/semantic_analyzers/correctness/use_is_nan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,91 +153,137 @@ impl Rule for UseIsNan {
let mut mutation = ctx.root().begin();

match query {
UseIsNanQuery::JsBinaryExpression(bin_expr) => {
if bin_expr.is_comparison_operator()
&& (has_nan(bin_expr.left().ok()?, model)
|| has_nan(bin_expr.right().ok()?, model))
{
let literal = get_literal(bin_expr, model)?;
let with_inequality = contains_inequality(bin_expr).unwrap_or(false);
let is_nan_expression = create_is_nan_expression(&with_inequality);
UseIsNanQuery::JsBinaryExpression(binary_expression) => {
let has_nan = binary_expression.is_comparison_operator()
&& (has_nan(binary_expression.left().ok()?, model)
|| has_nan(binary_expression.right().ok()?, model));

if !has_nan {
return None;
}

let arg = AnyJsCallArgument::AnyJsExpression(
literal
.with_leading_trivia_pieces([])?
.with_trailing_trivia_pieces([])?,
);
let args = make::js_call_arguments(
make::token(T!['(']),
make::js_call_argument_list([arg], []),
make::token(T![')']),
);
let (literal, nan) = get_literal(binary_expression, model)?;
let with_inequality = contains_inequality(binary_expression).unwrap_or(false);
let is_nan_expression: AnyJsExpression = create_is_nan_expression(nan)
.and_then(|result| create_unary_expression(with_inequality, result))?;

let call = make::js_call_expression(is_nan_expression, args).build();
let arg = AnyJsCallArgument::AnyJsExpression(literal);
let args = make::js_call_arguments(
make::token(T!['(']),
make::js_call_argument_list([arg], []),
make::token(T![')']),
);

mutation.replace_node(
AnyJsExpression::JsBinaryExpression(bin_expr.clone()),
call.into(),
);
let call = make::js_call_expression(is_nan_expression, args).build();

return Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! {
"Use "<Emphasis>"Number.isNaN()"</Emphasis>" instead."
}
.to_owned(),
mutation,
});
}
mutation.replace_node(
AnyJsExpression::JsBinaryExpression(binary_expression.clone()),
call.into(),
);

None
return Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! {
"Use "<Emphasis>"Number.isNaN()"</Emphasis>" instead."
}
.to_owned(),
mutation,
});
}
UseIsNanQuery::JsCaseClause(_) => None,
UseIsNanQuery::JsSwitchStatement(_) => None,
}
}
}

fn create_is_nan_expression(with_inequality: &bool) -> AnyJsExpression {
let is_nan_expression = make::js_static_member_expression(
make::js_identifier_expression(make::js_reference_identifier(make::ident("Number"))).into(),
make::token(T![.]),
make::js_name(make::ident("isNaN")).into(),
);

if *with_inequality {
let unary = make::js_unary_expression(make::token(T![!]), is_nan_expression.into());
return unary.into();
fn create_unary_expression(
with_inequality: bool,
nan_expression: AnyJsExpression,
) -> Option<AnyJsExpression> {
if with_inequality {
let unary = make::js_unary_expression(make::token(T![!]), nan_expression.into());
return Some(unary.into());
}

is_nan_expression.into()
Some(nan_expression)
}

fn contains_inequality(bin_expr: &JsBinaryExpression) -> Option<bool> {
let binary_operator = bin_expr.operator().ok()?;
fn create_is_nan_expression(nan: AnyJsExpression) -> Option<AnyJsExpression> {
match nan {
AnyJsExpression::JsIdentifierExpression(_)
| AnyJsExpression::JsComputedMemberExpression(_) => {
let is_nan_expression = make::js_static_member_expression(
make::js_identifier_expression(make::js_reference_identifier(make::ident(
"Number",
)))
.into(),
make::token(T![.]),
make::js_name(make::ident("isNaN")).into(),
);

return Some(is_nan_expression.into());
}
AnyJsExpression::JsStaticMemberExpression(member_expression) => {
let is_nan_expression =
member_expression.with_member(make::js_name(make::ident("isNaN")).into());
let member_object = is_nan_expression.object().ok()?.omit_parentheses();
let (reference, _) = global_identifier(&member_object)?;
let number_identifier_exists = is_nan_expression
.object()
.ok()?
.as_js_static_member_expression()
.is_some_and(|y| y.member().is_ok_and(|z| z.text() == "Number"));

if !reference.is_global_this() && !reference.has_name("window")
|| number_identifier_exists
{
return Some(is_nan_expression.into());
}

let member_expression = make::js_static_member_expression(
is_nan_expression.object().ok()?,
make::token(T![.]),
make::js_name(make::ident("Number")).into(),
);

return Some(
is_nan_expression
.with_object(member_expression.into())
.into(),
);
}
_ => None,
}
}

fn contains_inequality(bin_expr: &JsBinaryExpression) -> Option<bool> {
Some(matches!(
binary_operator,
bin_expr.operator().ok()?,
JsBinaryOperator::Inequality | JsBinaryOperator::StrictInequality
))
}

fn get_literal(bin_expr: &JsBinaryExpression, model: &SemanticModel) -> Option<AnyJsExpression> {
let left_expression = bin_expr.left().ok();
let right_expression = bin_expr.right().ok();
fn get_literal(
bin_expr: &JsBinaryExpression,
model: &SemanticModel,
) -> Option<(AnyJsExpression, AnyJsExpression)> {
let left_expression = bin_expr.left().ok()?;
let right_expression = bin_expr.right().ok()?;
let is_nan_on_left = has_nan(left_expression.clone(), model);

if let (Some(left), Some(right)) = (left_expression, right_expression) {
let is_nan_on_left = has_nan(left.clone(), model);
let left = left_expression
.with_leading_trivia_pieces([])?
.with_trailing_trivia_pieces([])?;
let right = right_expression
.with_leading_trivia_pieces([])?
.with_trailing_trivia_pieces([])?;

return if is_nan_on_left {
Some(right)
} else {
Some(left)
};
if !is_nan_on_left {
return Some((left, right));
}

None
return Some((right, left));
}

/// Checks whether an expression has `NaN`, `Number.NaN`, or `Number['NaN']`.
Expand Down
8 changes: 4 additions & 4 deletions crates/rome_js_analyze/tests/specs/correctness/useIsNan/invalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ invalid.js:33:1 lint/correctness/useIsNan FIXABLE ━━━━━━━━━
31 31 │ Number.NaN >= "abc";
32 32 │ "abc" >= Number.NaN;
33 │ - x·===·Number?.NaN;
33 │ + Number.isNaN(x);
33 │ + Number?.isNaN(x);
34 34 │ x === Number['NaN'];
35 35 │

Expand Down Expand Up @@ -897,7 +897,7 @@ invalid.js:36:1 lint/correctness/useIsNan FIXABLE ━━━━━━━━━
34 34 │ x === Number['NaN'];
35 35 │
36 │ - 123·==·globalThis.NaN;
36 │ + Number.isNaN(123);
36 │ + globalThis.Number.isNaN(123);
37 37 │ 123 == window.NaN;
38 38 │ 123 == globalThis.Number.NaN;

Expand All @@ -920,7 +920,7 @@ invalid.js:37:1 lint/correctness/useIsNan FIXABLE ━━━━━━━━━
35 35 │
36 36 │ 123 == globalThis.NaN;
37 │ - 123·==·window.NaN;
37 │ + Number.isNaN(123);
37 │ + window.Number.isNaN(123);
38 38 │ 123 == globalThis.Number.NaN;
39 39 │

Expand All @@ -944,7 +944,7 @@ invalid.js:38:1 lint/correctness/useIsNan FIXABLE ━━━━━━━━━
36 36 │ 123 == globalThis.NaN;
37 37 │ 123 == window.NaN;
38 │ - 123·==·globalThis.Number.NaN;
38 │ + Number.isNaN(123);
38 │ + globalThis.Number.isNaN(123);
39 39 │
40 40 │ // switch-case

Expand Down
Loading

0 comments on commit 2064586

Please sign in to comment.