feat(rome_js_analyze): noGlobalIsNan (#4612)

CHANGELOG.md

@@ -81,6 +81,11 @@ multiple files:
   The _Rome_ formatter takes care of removing extra semicolons.
   Thus, there is no need for this rule.
 
+#### New rules
+
+- Add [`noGlobalThis`](https://docs.rome.tools/lint/rules/noglobalthis/)
+
+  This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion.
 
 #### Other changes
 

crates/rome_diagnostics_categories/src/categories.rs

@@ -99,6 +99,7 @@ define_categories! {
     "lint/nursery/noStaticOnlyClass": "https://docs.rome.tools/lint/rules/noStaticOnlyClass",
     "lint/nursery/noDuplicateJsonKeys": "https://docs.rome.tools/lint/rules/noDuplicateJsonKeys",
     "lint/nursery/useNamingConvention": "https://docs.rome.tools/lint/rules/useNamingConvention",
+"lint/nursery/noGlobalIsNan": "https://docs.rome.tools/lint/rules/noGlobalIsNan",
     // Insert new nursery rule here
 
 

crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs

@@ -0,0 +1,132 @@
+use crate::{semantic_services::Semantic, JsRuleAction};
+use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_diagnostics::Applicability;
+use rome_js_factory::make;
+use rome_js_syntax::{AnyJsExpression, T};
+use rome_rowan::{AstNode, BatchMutationExt};
+
+declare_rule! {
+    /// Use `Number.isNaN` instead of global `isNaN`.
+    ///
+    /// `Number.isNaN()` and `isNaN()` [have not the same behavior](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#description).
+    /// When the argument to `isNaN()` is not a number, the value is first coerced to a number.
+    /// `Number.isNaN()` does not perform this coercion.
+    /// Therefore, it is a more reliable way to test whether a value is `NaN`.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// isNaN({}); // true
+    /// ```
+    ///
+    /// ## Valid
+    ///
+    /// ```js
+    /// Number.isNaN({}); // false
+    /// ```
+    ///
+    pub(crate) NoGlobalIsNan {
+        version: "next",
+        name: "noGlobalIsNan",
+        recommended: true,
+    }
+}
+
+impl Rule for NoGlobalIsNan {
+    type Query = Semantic<AnyJsExpression>;
+    type State = ();
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let node = ctx.query();
+        let model = ctx.model();
+        match node {
+            AnyJsExpression::JsIdentifierExpression(expression) => {
+                let name = expression.name().ok()?;
+                if name.has_name("isNaN") && model.binding(&name).is_none() {
+                    return Some(());
+                }
+            }
+            AnyJsExpression::JsStaticMemberExpression(expression) => {
+                let object_name = expression
+                    .object()
+                    .ok()?
+                    .omit_parentheses()
+                    .as_js_identifier_expression()?
+                    .name()
+                    .ok()?;
+                let member = expression.member().ok()?;
+                if object_name.is_global_this()
+                    && model.binding(&object_name).is_none()
+                    && member.as_js_name()?.value_token().ok()?.text_trimmed() == "isNaN"
+                {
+                    return Some(());
+                }
+            }
+            AnyJsExpression::JsComputedMemberExpression(expression) => {
+                let object_name = expression
+                    .object()
+                    .ok()?
+                    .omit_parentheses()
+                    .as_js_identifier_expression()?
+                    .name()
+                    .ok()?;
+                let member = expression.member().ok()?.omit_parentheses();
+                let member = member
+                    .as_any_js_literal_expression()?
+                    .as_js_string_literal_expression()?;
+                if object_name.is_global_this()
+                    && model.binding(&object_name).is_none()
+                    && member.value_token().ok()?.text_trimmed() == "isNaN"
+                {
+                    return Some(());
+                }
+            }
+            _ => (),
+        }
+        None
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
+        let node = ctx.query();
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                node.range(),
+                markup! {
+                    <Emphasis>"isNaN"</Emphasis>" is unsafe. It attempts a type coercion. Use "<Emphasis>"Number.isNaN"</Emphasis>" instead."
+                },
+            )
+            .note(markup! {
+                "See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#description">"the MDN documentation"</Hyperlink>" for more details."
+            }),
+        )
+    }
+
+    fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
+        let node = ctx.query();
+        let mut mutation = ctx.root().begin();
+        let number_constructor =
+            make::js_identifier_expression(make::js_reference_identifier(make::ident("Number")));
+        let is_nan_name = make::js_name(make::ident("isNaN"));
+        let expression = make::js_static_member_expression(
+            number_constructor.into(),
+            make::token(T![.]),
+            is_nan_name.into(),
+        );
+        mutation.replace_node(node.clone(), expression.into());
+        Some(JsRuleAction {
+            category: ActionCategory::QuickFix,
+            applicability: Applicability::MaybeIncorrect,
+            message: markup! {
+                "Use "<Emphasis>"Number.isNaN"</Emphasis>" instead."
+            }
+            .to_owned(),
+            mutation,
+        })
+    }
+}

crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js

@@ -0,0 +1,17 @@
+isNaN({});
+
+(isNaN)({});
+
+globalThis.isNaN({});
+
+(globalThis).isNaN({});
+
+globalThis["isNaN"]({});
+
+(globalThis)[("isNaN")]({});
+
+function localIsNaN(isNaN) {
+    globalThis.isNaN({});
+}
+
+localIsNaN(isNaN);

crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap

@@ -0,0 +1,177 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: invalid.js
+---
+# Input
+```js
+isNaN({});
+
+(isNaN)({});
+
+globalThis.isNaN({});
+
+(globalThis).isNaN({});
+
+globalThis["isNaN"]({});
+
+(globalThis)[("isNaN")]({});
+
+function localIsNaN(isNaN) {
+    globalThis.isNaN({});
+}
+
+localIsNaN(isNaN);
+
+```
+
+# Diagnostics
+```
+invalid.js:1:1 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+  > 1 │ isNaN({});
+      │ ^^^^^
+    2 │ 
+    3 │ (isNaN)({});
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+     1    │ - isNaN({});
+        1 │ + Number.isNaN({});
+     2  2 │   
+     3  3 │   (isNaN)({});
+  
+
+```
+
+```
+invalid.js:3:2 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+    1 │ isNaN({});
+    2 │ 
+  > 3 │ (isNaN)({});
+      │  ^^^^^
+    4 │ 
+    5 │ globalThis.isNaN({});
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+     1  1 │   isNaN({});
+     2  2 │   
+     3    │ - (isNaN)({});
+        3 │ + (Number.isNaN)({});
+     4  4 │   
+     5  5 │   globalThis.isNaN({});
+  
+
+```
+
+```
+invalid.js:5:1 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+    3 │ (isNaN)({});
+    4 │ 
+  > 5 │ globalThis.isNaN({});
+      │ ^^^^^^^^^^^^^^^^
+    6 │ 
+    7 │ (globalThis).isNaN({});
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+     3  3 │   (isNaN)({});
+     4  4 │   
+     5    │ - globalThis.isNaN({});
+        5 │ + Number.isNaN({});
+     6  6 │   
+     7  7 │   (globalThis).isNaN({});
+  
+
+```
+
+```
+invalid.js:7:1 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+    5 │ globalThis.isNaN({});
+    6 │ 
+  > 7 │ (globalThis).isNaN({});
+      │ ^^^^^^^^^^^^^^^^^^
+    8 │ 
+    9 │ globalThis["isNaN"]({});
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+     5  5 │   globalThis.isNaN({});
+     6  6 │   
+     7    │ - (globalThis).isNaN({});
+        7 │ + Number.isNaN({});
+     8  8 │   
+     9  9 │   globalThis["isNaN"]({});
+  
+
+```
+
+```
+invalid.js:14:5 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+    13 │ function localIsNaN(isNaN) {
+  > 14 │     globalThis.isNaN({});
+       │     ^^^^^^^^^^^^^^^^
+    15 │ }
+    16 │ 
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+    12 12 │   
+    13 13 │   function localIsNaN(isNaN) {
+    14    │ - ····globalThis.isNaN({});
+       14 │ + ····Number.isNaN({});
+    15 15 │   }
+    16 16 │   
+  
+
+```
+
+```
+invalid.js:17:12 lint/nursery/noGlobalIsNan  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
+  
+    15 │ }
+    16 │ 
+  > 17 │ localIsNaN(isNaN);
+       │            ^^^^^
+    18 │ 
+  
+  i See the MDN documentation for more details.
+  
+  i Suggested fix: Use Number.isNaN instead.
+  
+    15 15 │   }
+    16 16 │   
+    17    │ - localIsNaN(isNaN);
+       17 │ + localIsNaN(Number.isNaN);
+    18 18 │   
+  
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js

@@ -0,0 +1,14 @@
+Number.isNaN(Number.NaN);
+
+globalThis.Number.isNaN(Number.NaN);
+
+function localIsNaN(isNaN) {
+    isNaN({});
+}
+
+function localVar() {
+    var isNaN;
+    isNaN()
+}
+
+localIsNaN(Number.isNaN);

crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js.snap

@@ -0,0 +1,24 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: valid.js
+---
+# Input
+```js
+Number.isNaN(Number.NaN);
+
+globalThis.Number.isNaN(Number.NaN);
+
+function localIsNaN(isNaN) {
+    isNaN({});
+}
+
+function localVar() {
+    var isNaN;
+    isNaN()
+}
+
+localIsNaN(Number.isNaN);
+
+```
+
+