eslint-factory: add gh-aw-custom/prefer-number-isnan rule for unsafe global isNaN usage

eslint-factory/eslint.config.cjs

@@ -16,6 +16,7 @@ module.exports = [
       "gh-aw-custom/no-unsafe-catch-error-property": "warn",
       "gh-aw-custom/no-unsafe-promise-catch-error-property": "warn",
       "gh-aw-custom/prefer-get-error-message": "warn",
+      "gh-aw-custom/prefer-number-isnan": "warn",
       "gh-aw-custom/require-async-entrypoint-catch": "warn",
       "gh-aw-custom/require-await-core-summary-write": "warn",
       "gh-aw-custom/require-json-parse-try-catch": "warn",

eslint-factory/src/index.ts

@@ -2,6 +2,7 @@ import { noCoreSetOutputNonStringRule } from "./rules/no-core-setoutput-non-stri
 import { noUnsafeCatchErrorPropertyRule } from "./rules/no-unsafe-catch-error-property";
 import { noUnsafePromiseCatchErrorPropertyRule } from "./rules/no-unsafe-promise-catch-error-property";
 import { preferGetErrorMessageRule } from "./rules/prefer-get-error-message";
+import { preferNumberIsNanRule } from "./rules/prefer-number-isnan";
 import { requireAsyncEntrypointCatchRule } from "./rules/require-async-entrypoint-catch";
 import { requireAwaitCoreSummaryWriteRule } from "./rules/require-await-core-summary-write";
 import { requireJsonParseTryCatchRule } from "./rules/require-json-parse-try-catch";
@@ -17,6 +18,7 @@ const plugin = {
     "no-unsafe-catch-error-property": noUnsafeCatchErrorPropertyRule,
     "no-unsafe-promise-catch-error-property": noUnsafePromiseCatchErrorPropertyRule,
     "prefer-get-error-message": preferGetErrorMessageRule,
+    "prefer-number-isnan": preferNumberIsNanRule,
     "require-async-entrypoint-catch": requireAsyncEntrypointCatchRule,
     "require-await-core-summary-write": requireAwaitCoreSummaryWriteRule,
     "require-json-parse-try-catch": requireJsonParseTryCatchRule,

eslint-factory/src/rules/prefer-number-isnan.test.ts

@@ -0,0 +1,87 @@
+import { RuleTester } from "eslint";
+import { describe, expect, it } from "vitest";
+import { preferNumberIsNanRule } from "./prefer-number-isnan";
+
+const cjsRuleTester = new RuleTester({
+  languageOptions: {
+    ecmaVersion: 2022,
+    sourceType: "commonjs",
+  },
+});
+
+const esmRuleTester = new RuleTester({
+  languageOptions: {
+    ecmaVersion: 2022,
+    sourceType: "module",
+  },
+});
+
+describe("prefer-number-isnan", () => {
+  it("uses the correct docs URL", () => {
+    expect(preferNumberIsNanRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#prefer-number-isnan");
+  });
+
+  it("valid: Number.isNaN and non-global forms are accepted", () => {
+    cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
+      valid: [`Number.isNaN(value);`, `Number["isNaN"](value);`, `foo.isNaN(value);`],
+      invalid: [],
+    });
+  });
+
+  it("valid: locally shadowed bindings are intentionally excluded", () => {
+    esmRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
+      valid: [
+        `function isNaN(value) { return false; } isNaN(value);`,
+        `const isNaN = Number.isNaN; isNaN(value);`,
+        `const globalThis = { isNaN(value) { return value; } }; globalThis.isNaN(value);`,
+        `const window = { isNaN(value) { return value; } }; window["isNaN"](value);`,
+        `const global = { isNaN(value) { return value; } }; global.isNaN(value);`,
+      ],
+      invalid: [],
+    });
+  });
+
+  it("invalid: global isNaN() is flagged with a replacement suggestion", () => {
+    cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `isNaN(value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+      ],
+    });
+  });
+
+  it("invalid: global object isNaN() access is flagged for direct and computed forms", () => {
+    cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `globalThis.isNaN(value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+        {
+          code: `globalThis["isNaN"](value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+        {
+          code: `window.isNaN(value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+        {
+          code: `window["isNaN"](value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+        {
+          code: `global.isNaN(value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+        {
+          code: `global["isNaN"](value);`,
+          errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
+        },
+      ],
+    });
+  });
+});

eslint-factory/src/rules/prefer-number-isnan.ts

@@ -0,0 +1,88 @@
+import { ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils";
+
+const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`);
+const GLOBAL_IS_NAN_OBJECTS = new Set(["globalThis", "window", "global"]);
+
+export const preferNumberIsNanRule = createRule({
+  name: "prefer-number-isnan",
+  meta: {
+    type: "suggestion",
+    hasSuggestions: true,
+    docs: {
+      description: "Prefer Number.isNaN() over global isNaN() to avoid coercion footguns when validating unknown inputs.",
+    },
+    schema: [],
+    messages: {
+      preferNumberIsNaN: "Prefer Number.isNaN(...) over global isNaN(...). Global isNaN() coerces non-number inputs and can hide invalid raw values.",
+      replaceWithNumberIsNaN: "Replace callee with Number.isNaN — review whether the argument should be wrapped with Number(...).",
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    const sourceCode = context.sourceCode;
+    type SourceCodeScope = ReturnType<typeof sourceCode.getScope>;
+
+    /**
+     * Checks whether a given identifier name is locally bound in the current scope chain.
+     * @param node AST node to start the scope search from.
+     * @param name Identifier name to search for.
+     * @returns true if the name has a local binding, false otherwise.
+     */
+    function hasLocalBinding(node: TSESTree.Node, name: string): boolean {
+      let scope: SourceCodeScope | null = sourceCode.getScope(node);
+
+      while (scope) {
+        const variable = scope.set.get(name);
+
+        if (variable?.defs.length) {
+          return true;
+        }
+
+        scope = scope.upper;
+      }
+
+      return false;
+    }
+
+    /**
+     * Checks whether a MemberExpression property is isNaN, either direct or computed.
+     * @param node MemberExpression node to inspect.
+     * @returns true if the property is isNaN.
+     */
+    function isIsNaNProperty(node: TSESTree.MemberExpression): boolean {
+      const property = node.property;
+      const isDirectAccess = property.type === "Identifier" && property.name === "isNaN";
+      const isComputedAccess = property.type === "Literal" && property.value === "isNaN";
+
+      return isDirectAccess || isComputedAccess;
+    }
+
+    function report(node: TSESTree.CallExpression): void {
+      context.report({
+        node: node.callee,
+        messageId: "preferNumberIsNaN",
+        suggest: [
+          {
+            messageId: "replaceWithNumberIsNaN",
+            fix(fixer: TSESLint.RuleFixer) {
+              return fixer.replaceText(node.callee, "Number.isNaN");
+            },
+          },
+        ],
+      });
+    }
+
+    return {
+      CallExpression(node) {
+        if (node.callee.type === "Identifier" && node.callee.name === "isNaN" && !hasLocalBinding(node, "isNaN")) {
+          report(node);
+          return;
+        }
+
+        if (node.callee.type === "MemberExpression" && node.callee.object.type === "Identifier" && GLOBAL_IS_NAN_OBJECTS.has(node.callee.object.name) && !hasLocalBinding(node, node.callee.object.name) && isIsNaNProperty(node.callee)) {
+          report(node);
+        }
+      },
+    };
+  },
+});