Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 74 additions & 13 deletions eslint-factory/src/rules/prefer-number-isnan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,79 @@ describe("prefer-number-isnan", () => {
});
});

it("invalid: global isNaN() is flagged with a replacement suggestion", () => {
it("invalid: provably numeric arguments are autofixed", () => {
cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
valid: [],
invalid: [
{
code: `isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
code: `isNaN(parseInt(value, 10));`,
output: `Number.isNaN(parseInt(value, 10));`,
errors: [{ messageId: "preferNumberIsNaN" }],
},
{
code: `isNaN(parseFloat(value));`,
output: `Number.isNaN(parseFloat(value));`,
errors: [{ messageId: "preferNumberIsNaN" }],
},
{
code: `isNaN(Number(value));`,
output: `Number.isNaN(Number(value));`,
errors: [{ messageId: "preferNumberIsNaN" }],
},
{
code: `isNaN(Number.parseInt(value, 10));`,
output: `Number.isNaN(Number.parseInt(value, 10));`,
errors: [{ messageId: "preferNumberIsNaN" }],
},
{
code: `isNaN(42);`,
output: `Number.isNaN(42);`,
errors: [{ messageId: "preferNumberIsNaN" }],
},
],
});
});

it("valid: method calls on arbitrary receivers are not treated as provably numeric", () => {
cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
valid: [],
invalid: [
// getTime/valueOf can be defined on any object; must remain suggestion-only
{
code: `isNaN(d.getTime());`,
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(d.getTime());` }] }],
},
{
code: `isNaN(x.valueOf());`,
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(x.valueOf());` }] }],
},
],
});
});

it("invalid: shadowed parseInt/parseFloat/Number fall back to suggestion-only", () => {
esmRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
valid: [],
invalid: [
{
code: `function parseInt() { return "x"; } isNaN(parseInt(v));`,
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `function parseInt() { return "x"; } Number.isNaN(parseInt(v));` }] }],
},
{
// Raw string argument (e.g. env var) — suggestion preserves argument so callers must review whether to wrap with Number(...)
code: `const Number = {}; isNaN(0);`,
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `const Number = {}; Number.isNaN(0);` }] }],
},
],
});
});

it("invalid: unknown/raw arguments remain suggestion-only with coercion caveat", () => {
cjsRuleTester.run("prefer-number-isnan", preferNumberIsNanRule, {
valid: [],
invalid: [
{
code: `isNaN(process.env.PORT);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(process.env.PORT);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(process.env.PORT);` }] }],
},
],
});
Expand All @@ -78,27 +139,27 @@ describe("prefer-number-isnan", () => {
invalid: [
{
code: `globalThis.isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `globalThis["isNaN"](value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `window.isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `window["isNaN"](value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `global.isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `global["isNaN"](value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
],
});
Expand All @@ -110,11 +171,11 @@ describe("prefer-number-isnan", () => {
invalid: [
{
code: `isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
{
code: `window.isNaN(value);`,
errors: [{ messageId: "preferNumberIsNaN", suggestions: [{ messageId: "replaceWithNumberIsNaN", output: `Number.isNaN(value);` }] }],
errors: [{ messageId: "preferNumberIsNaNWithCoercionCaveat", suggestions: [{ messageId: "replaceWithNumberIsNaNWithNumberWrapReview", output: `Number.isNaN(value);` }] }],
},
],
});
Expand Down
63 changes: 59 additions & 4 deletions eslint-factory/src/rules/prefer-number-isnan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ export const preferNumberIsNanRule = createRule({
name: "prefer-number-isnan",
meta: {
type: "suggestion",
fixable: "code",
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(...).",
preferNumberIsNaN: "Prefer Number.isNaN(...) over global isNaN(...).",
preferNumberIsNaNWithCoercionCaveat: "Prefer Number.isNaN(...) over global isNaN(...). Global isNaN() coerces non-number inputs and can hide invalid raw values.",
replaceWithNumberIsNaN: "Replace callee with Number.isNaN.",
replaceWithNumberIsNaNWithNumberWrapReview: "Replace callee with Number.isNaN — review whether the argument should be wrapped with Number(...).",
},
},
defaultOptions: [],
Expand Down Expand Up @@ -58,13 +61,65 @@ export const preferNumberIsNanRule = createRule({
return isDirectAccess || isComputedAccess;
}

function isNumberParseMethod(node: TSESTree.MemberExpression): boolean {
if (node.object.type !== "Identifier" || node.object.name !== "Number") {
return false;
}

const property = node.property;
const isDirectAccess = !node.computed && property.type === "Identifier" && (property.name === "parseInt" || property.name === "parseFloat");
const isComputedAccess = node.computed && property.type === "Literal" && (property.value === "parseInt" || property.value === "parseFloat");

return isDirectAccess || isComputedAccess;
}

function isProvablyNumericArgument(node: TSESTree.CallExpressionArgument): boolean {
if (node.type === "Literal") {
return typeof node.value === "number";
}

if (node.type !== "CallExpression") {
return false;
}

// Unqualified global functions are only provably numeric when unshadowed.
if (node.callee.type === "Identifier") {
const name = node.callee.name;

return (name === "parseInt" || name === "parseFloat" || name === "Number") && !hasLocalBinding(node, name);
}

// Number.parseInt / Number.parseFloat are only provably numeric when Number is unshadowed.
if (node.callee.type === "MemberExpression" && isNumberParseMethod(node.callee) && !hasLocalBinding(node, "Number")) {
return true;
}

// Arbitrary method calls (getTime, valueOf, etc.) are not provably numeric —
// the receiver type is unknown and the method can be overridden to return anything.
return false;
}

function report(node: TSESTree.CallExpression): void {
const numberUnshadowed = !hasLocalBinding(node, "Number");
const hasSingleProvablyNumericArgument = node.arguments.length === 1 && isProvablyNumericArgument(node.arguments[0]);

if (hasSingleProvablyNumericArgument && numberUnshadowed) {
context.report({
node: node.callee,
messageId: "preferNumberIsNaN",
fix(fixer: TSESLint.RuleFixer) {
return fixer.replaceText(node.callee, "Number.isNaN");
},
});
return;
}

context.report({
node: node.callee,
messageId: "preferNumberIsNaN",
messageId: "preferNumberIsNaNWithCoercionCaveat",
suggest: [
{
messageId: "replaceWithNumberIsNaN",
messageId: "replaceWithNumberIsNaNWithNumberWrapReview",
fix(fixer: TSESLint.RuleFixer) {
return fixer.replaceText(node.callee, "Number.isNaN");
},
Expand Down
Loading