[eslint-miner] eslint-miner: add require-nan-check-after-split-index-parse rule

eslint-factory/eslint.config.cjs

@@ -53,6 +53,7 @@ module.exports = [
       "gh-aw-custom/require-escaped-regexp-interpolation": "warn",
       "gh-aw-custom/require-fetch-timeout": "warn",
       "gh-aw-custom/require-nan-check-after-env-numeric-parse": "warn",
+      "gh-aw-custom/require-nan-check-after-split-index-parse": "warn",
       "gh-aw-custom/prefer-structured-clone": "warn",
       "gh-aw-custom/require-fetch-response-body-try-catch": "warn",
       "gh-aw-custom/require-error-code-in-thrown-error": "warn",

eslint-factory/src/index.ts

@@ -39,6 +39,7 @@ import { noDuplicateConstantValuesRule } from "./rules/no-duplicate-constant-val
 import { requireEscapedRegexpInterpolationRule } from "./rules/require-escaped-regexp-interpolation";
 import { requireFetchTimeoutRule } from "./rules/require-fetch-timeout";
 import { requireNanCheckAfterEnvNumericParseRule } from "./rules/require-nan-check-after-env-numeric-parse";
+import { requireNanCheckAfterSplitIndexParseRule } from "./rules/require-nan-check-after-split-index-parse";
 import { preferStructuredCloneRule } from "./rules/prefer-structured-clone";
 import { requireFetchResponseBodyTryCatchRule } from "./rules/require-fetch-response-body-try-catch";
 import { requireErrorCodeInThrownErrorRule } from "./rules/require-error-code-in-thrown-error";
@@ -91,6 +92,7 @@ const plugin = {
     "require-escaped-regexp-interpolation": requireEscapedRegexpInterpolationRule,
     "require-fetch-timeout": requireFetchTimeoutRule,
     "require-nan-check-after-env-numeric-parse": requireNanCheckAfterEnvNumericParseRule,
+    "require-nan-check-after-split-index-parse": requireNanCheckAfterSplitIndexParseRule,
     "prefer-structured-clone": preferStructuredCloneRule,
     "require-fetch-response-body-try-catch": requireFetchResponseBodyTryCatchRule,
     "require-error-code-in-thrown-error": requireErrorCodeInThrownErrorRule,

eslint-factory/src/rules/require-nan-check-after-split-index-parse.test.ts

@@ -0,0 +1,121 @@
+import { RuleTester } from "eslint";
+import { describe, expect, it } from "vitest";
+import { requireNanCheckAfterSplitIndexParseRule } from "./require-nan-check-after-split-index-parse";
+
+const cjsRuleTester = new RuleTester({
+  languageOptions: {
+    ecmaVersion: 2022,
+    sourceType: "commonjs",
+  },
+});
+
+describe("require-nan-check-after-split-index-parse", () => {
+  it("uses the correct docs URL", () => {
+    expect(requireNanCheckAfterSplitIndexParseRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#require-nan-check-after-split-index-parse");
+  });
+
+  it("valid: parseInt from split(...)[index] validated with Number.isNaN", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [`const discussionNumber = parseInt(endpoint.split(":")[1], 10); if (Number.isNaN(discussionNumber)) throw new Error("invalid");`],
+      invalid: [],
+    });
+  });
+
+  it("valid: parseInt from split(...)[index] validated with global isNaN", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [`const discussionNumber = parseInt(endpoint.split(":")[1], 10); if (isNaN(discussionNumber)) throw new Error("invalid");`],
+      invalid: [],
+    });
+  });
+
+  it("valid: Number.parseInt from split(...)[index] validated with Number.isNaN", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [`const discussionNumber = Number.parseInt(endpoint.split(":")[1], 10); if (Number.isNaN(discussionNumber)) throw new Error("invalid");`],
+      invalid: [],
+    });
+  });
+
+  it("valid: truthiness guard on parsed value", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [`const discussionNumber = parseInt(endpoint.split(":")[1], 10); if (!discussionNumber) throw new Error("invalid");`],
+      invalid: [],
+    });
+  });
+
+  it("valid: parseInt not from split(...)[index] is not flagged", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [`const count = parseInt(rawValue, 10); doSomething(count);`],
+      invalid: [],
+    });
+  });
+
+  it("invalid: parseInt from split(...)[index] without NaN check", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `const discussionNumber = parseInt(endpoint.split(":")[1], 10); getDiscussionNodeId(owner, repo, discussionNumber);`,
+          errors: [{ messageId: "requireNaNCheck" }],
+        },
+      ],
+    });
+  });
+
+  it("invalid: Number.parseInt from split(...)[index] without NaN check", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `const discussionNumber = Number.parseInt(endpoint.split(":")[1], 10); getDiscussionNodeId(owner, repo, discussionNumber);`,
+          errors: [{ messageId: "requireNaNCheck" }],
+        },
+      ],
+    });
+  });
+
+  it("invalid: parseFloat from split(...)[index] without NaN check", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `const version = parseFloat(tag.split("v")[1]); doSomething(version);`,
+          errors: [{ messageId: "requireNaNCheck" }],
+        },
+      ],
+    });
+  });
+
+  it("invalid: same-named variable validated in one scope does not suppress an unvalidated occurrence in another scope", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `
+function a() {
+  const discussionNumber = parseInt(endpoint.split(":")[1], 10);
+  if (!discussionNumber) throw new Error("invalid");
+  return discussionNumber;
+}
+function b() {
+  const discussionNumber = parseInt(endpoint.split(":")[1], 10);
+  return getDiscussionNodeId(owner, repo, discussionNumber);
+}
+          `.trim(),
+          errors: [{ messageId: "requireNaNCheck" }],
+        },
+      ],
+    });
+  });
+
+  it("invalid: multiple unvalidated split-index parse declarations are each reported", () => {
+    cjsRuleTester.run("require-nan-check-after-split-index-parse", requireNanCheckAfterSplitIndexParseRule, {
+      valid: [],
+      invalid: [
+        {
+          code: `const a = parseInt(x.split(":")[1], 10); const b = parseInt(y.split(":")[1], 10); use(a, b);`,
+          errors: [{ messageId: "requireNaNCheck" }, { messageId: "requireNaNCheck" }],
+        },
+      ],
+    });
+  });
+});

eslint-factory/src/rules/require-nan-check-after-split-index-parse.ts

@@ -0,0 +1,167 @@
+import { ESLintUtils, TSESTree } from "@typescript-eslint/utils";
+
+const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`);
+
+export const requireNanCheckAfterSplitIndexParseRule = createRule({
+  name: "require-nan-check-after-split-index-parse",
+  meta: {
+    type: "problem",
+    docs: {
+      description: "Require NaN validation after parsing a numeric value out of a string.split(...)[index] expression, since malformed delimited strings silently produce NaN that can propagate into API calls.",
+    },
+    schema: [],
+    messages: {
+      requireNaNCheck:
+        "Numeric value '{{name}}' parsed from a 'split(...)[index]' expression is never validated with Number.isNaN(), isNaN(), Number.isFinite(), isFinite(), or a truthiness check. A malformed delimited string will silently produce NaN, which can then be passed to downstream API calls.",
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    // Set of VariableDeclarator nodes for split(...)[index]-derived parses, keyed by node identity
+    // so that same-named variables in different scopes are not conflated.
+    const unvalidated = new Set<TSESTree.VariableDeclarator>();
+    // Set of VariableDeclarator nodes confirmed to be validated (via isNaN / Number.isNaN or a truthiness guard)
+    const validated = new Set<TSESTree.VariableDeclarator>();
+
+    /**
+     * Resolves an Identifier reference to the VariableDeclarator that declared it,
+     * using scope analysis so that same-named variables in different scopes are distinguished.
+     */
+    function resolveDeclarator(identifier: TSESTree.Identifier): TSESTree.VariableDeclarator | null {
+      const sourceCode = context.sourceCode;
+      let scope: ReturnType<typeof sourceCode.getScope> | null = sourceCode.getScope(identifier);
+      while (scope) {
+        const variable = scope.variables.find(v => v.name === identifier.name);
+        if (variable) {
+          const def = variable.defs.find(d => d.node.type === "VariableDeclarator");
+          if (def && def.node.type === "VariableDeclarator") {
+            return def.node as TSESTree.VariableDeclarator;
+          }
+          return null;
+        }
+        scope = scope.upper;
+      }
+      return null;
+    }
+
+    /**
+     * Returns true when the given node is a `<expr>.split(...)[<index>]` member access,
+     * e.g. `endpoint.split(":")[1]`.
+     */
+    function isSplitIndexAccess(node: TSESTree.Node): boolean {
+      if (node.type !== "MemberExpression" || !node.computed) return false;
+      const obj = node.object;
+      return obj.type === "CallExpression" && obj.callee.type === "MemberExpression" && !obj.callee.computed && obj.callee.property.type === "Identifier" && obj.callee.property.name === "split";
+    }
+
+    /**
+     * Returns true when the call expression is a numeric-parse function whose
+     * first argument is a `split(...)[index]` access.
+     */
+    function isNumericParseCallFromSplitIndex(node: TSESTree.CallExpression): boolean {
+      const { callee, arguments: args } = node;
+
+      if (args.length === 0 || args[0].type === "SpreadElement") return false;
+
+      const firstArg = args[0] as TSESTree.Expression;
+
+      // Global parseInt(splitExpr, ...) or parseFloat(splitExpr)
+      if (callee.type === "Identifier" && (callee.name === "parseInt" || callee.name === "parseFloat")) {
+        return isSplitIndexAccess(firstArg);
+      }
+
+      // Number.parseInt(splitExpr, ...) or Number.parseFloat(splitExpr)
+      if (
+        callee.type === "MemberExpression" &&
+        callee.object.type === "Identifier" &&
+        callee.object.name === "Number" &&
+        !callee.computed &&
+        callee.property.type === "Identifier" &&
+        (callee.property.name === "parseInt" || callee.property.name === "parseFloat")
+      ) {
+        return isSplitIndexAccess(firstArg);
+      }
+
+      return false;
+    }
+
+    /**
+     * Returns true when the call expression is a NaN-validating global:
+     * isNaN(...), Number.isNaN(...), isFinite(...) or Number.isFinite(...).
+     */
+    function isIsNaNCall(node: TSESTree.CallExpression): boolean {
+      const { callee } = node;
+
+      // Global isNaN(x) / isFinite(x)
+      if (callee.type === "Identifier" && (callee.name === "isNaN" || callee.name === "isFinite")) {
+        return true;
+      }
+
+      // Number.isNaN(x) / Number.isFinite(x)
+      if (
+        callee.type === "MemberExpression" &&
+        callee.object.type === "Identifier" &&
+        callee.object.name === "Number" &&
+        !callee.computed &&
+        callee.property.type === "Identifier" &&
+        (callee.property.name === "isNaN" || callee.property.name === "isFinite")
+      ) {
+        return true;
+      }
+
+      return false;
+    }
+
+    /**
+     * Marks a bare identifier (or `!identifier`) used as a condition test as validated,
+     * since NaN is falsy and such a truthiness guard rejects it.
+     */
+    function markTruthinessGuard(test: TSESTree.Node): void {
+      let expr = test;
+      while (expr.type === "UnaryExpression" && expr.operator === "!") {
+        expr = expr.argument;
+      }
+      if (expr.type === "Identifier") {
+        const declarator = resolveDeclarator(expr);
+        if (declarator) validated.add(declarator);
+      }
+    }
+
+    return {
+      VariableDeclarator(node) {
+        if (node.id.type === "Identifier" && node.init?.type === "CallExpression" && isNumericParseCallFromSplitIndex(node.init as TSESTree.CallExpression)) {
+          unvalidated.add(node);
+        }
+      },
+
+      CallExpression(node) {
+        // Track any isNaN(x) / Number.isNaN(x) call where x is an identifier
+        if (isIsNaNCall(node) && node.arguments.length === 1 && node.arguments[0].type === "Identifier") {
+          const declarator = resolveDeclarator(node.arguments[0] as TSESTree.Identifier);
+          if (declarator) validated.add(declarator);
+        }
+      },
+
+      IfStatement(node) {
+        markTruthinessGuard(node.test);
+      },
+
+      ConditionalExpression(node) {
+        markTruthinessGuard(node.test);
+      },
+
+      "Program:exit"() {
+        for (const declaratorNode of unvalidated) {
+          if (!validated.has(declaratorNode)) {
+            const name = declaratorNode.id.type === "Identifier" ? declaratorNode.id.name : "";
+            context.report({
+              node: declaratorNode,
+              messageId: "requireNaNCheck",
+              data: { name },
+            });
+          }
+        }
+      },
+    };
+  },
+});