Rule
eslint-factory/src/rules/prefer-number-isnan.ts (first refinement review of this rule)
Summary
The rule's diagnostic and its suggestion caveat describe the isNaN() coercion footgun — but that footgun applies to none of the live call sites in actions/setup/js, because every one passes an argument that is already a number. For those sites the isNaN(x) → Number.isNaN(x) transform is a guaranteed semantic-preserving equivalence, yet the rule (a) offers only a manual suggest (never an autofix) and (b) attaches a caveat — "review whether the argument should be wrapped with Number(...)" — that is actively misleading there.
Grounding (live corpus)
Global isNaN(...) is used at 51 call sites across actions/setup/js/**/*.cjs. Sampling the full set, every argument is provably numeric — the result of parseInt/parseFloat/Number(...), or a Date#getTime(), or a value already narrowed to number:
| Site |
Argument |
templatable.cjs:62 |
isNaN(n) where const n = parseInt(String(value), 10) |
check_skip_if_no_match.cjs:23 |
isNaN(minMatches); minMatches = parseInt(minMatchesStr, 10) |
pr_helpers.cjs:52 |
isNaN(prNumber); prNumber = parseInt(String(...), 10) |
update_project.cjs:579 |
isNaN(numValue); numValue = ... parseFloat(String(fieldValue)) |
error_recovery.cjs:138,147 |
isNaN(seconds) / isNaN(resetTimestampMs); both parseInt(...) |
generate_usage_activity_summary.cjs:149 |
isNaN(code); code = parseInt(status, 10) |
expired_entity_cleanup_helpers.cjs:35, check_stop_time.cjs:25, create_project_status_update.cjs:240 |
isNaN(<date>.getTime()) |
The files that already migrated to Number.isNaN confirm the idiom: mcp_cli_bridge.cjs:776/798/805 and project_timezone.cjs:73 all read Number.isNaN(parsed) where parsed came from a numeric parse. Zero sites pass a raw string/unknown directly to isNaN — i.e. the case the message warns about ("coerces non-number inputs", "hide invalid raw values") never occurs in this corpus.
Why this matters
- For
isNaN(parseInt(x, 10)), isNaN and Number.isNaN are exactly equivalent (the arg is already a number). The caveat "review whether the argument should be wrapped with Number(...)" is wrong here — wrapping would be redundant, and there is nothing to review. 51 warnings each carrying that caveat is alarm-fatigue on a false premise.
- Because the rule only
suggests (no fix), these ~51 mechanical, equivalence-preserving replacements cannot be applied via eslint --fix, even though they are provably safe.
Proposed refinement
Detect a provably-numeric argument and branch behavior:
- Provably-numeric = the single argument is a
CallExpression to parseInt/parseFloat/Number/Number.parseInt/Number.parseFloat, or a CallExpression whose callee property is getTime/getTimezoneOffset/valueOf, or a numeric Literal.
- When provably-numeric: emit a message without the "wrap with
Number(...)" caveat and offer a real autofix (fix, not just suggest) — the transform is a guaranteed equivalence.
- When NOT provably-numeric (raw/unknown arg): keep today's cautious
suggest-only + caveat behavior (correct — replacement could flip semantics for strings).
Acceptance criteria
Notes
- Non-blocking: the current suggestion is disclosed and not auto-applied, so this is a quality/UX refinement, not a correctness bug. The value is removing a systematically-wrong caveat and unlocking safe autofix on the dominant idiom.
Generated by 🤖 ESLint Refiner · 313.5 AIC · ⌖ 13 AIC · ⊞ 4.7K · ◷
Rule
eslint-factory/src/rules/prefer-number-isnan.ts(first refinement review of this rule)Summary
The rule's diagnostic and its suggestion caveat describe the
isNaN()coercion footgun — but that footgun applies to none of the live call sites inactions/setup/js, because every one passes an argument that is already a number. For those sites theisNaN(x)→Number.isNaN(x)transform is a guaranteed semantic-preserving equivalence, yet the rule (a) offers only a manualsuggest(never an autofix) and (b) attaches a caveat — "review whether the argument should be wrapped withNumber(...)" — that is actively misleading there.Grounding (live corpus)
Global
isNaN(...)is used at 51 call sites acrossactions/setup/js/**/*.cjs. Sampling the full set, every argument is provably numeric — the result ofparseInt/parseFloat/Number(...), or aDate#getTime(), or a value already narrowed tonumber:templatable.cjs:62isNaN(n)whereconst n = parseInt(String(value), 10)check_skip_if_no_match.cjs:23isNaN(minMatches);minMatches = parseInt(minMatchesStr, 10)pr_helpers.cjs:52isNaN(prNumber);prNumber = parseInt(String(...), 10)update_project.cjs:579isNaN(numValue);numValue = ... parseFloat(String(fieldValue))error_recovery.cjs:138,147isNaN(seconds)/isNaN(resetTimestampMs); bothparseInt(...)generate_usage_activity_summary.cjs:149isNaN(code);code = parseInt(status, 10)expired_entity_cleanup_helpers.cjs:35,check_stop_time.cjs:25,create_project_status_update.cjs:240isNaN(<date>.getTime())The files that already migrated to
Number.isNaNconfirm the idiom:mcp_cli_bridge.cjs:776/798/805andproject_timezone.cjs:73all readNumber.isNaN(parsed)whereparsedcame from a numeric parse. Zero sites pass a raw string/unknown directly toisNaN— i.e. the case the message warns about ("coerces non-number inputs", "hide invalid raw values") never occurs in this corpus.Why this matters
isNaN(parseInt(x, 10)),isNaNandNumber.isNaNare exactly equivalent (the arg is already anumber). The caveat "review whether the argument should be wrapped withNumber(...)" is wrong here — wrapping would be redundant, and there is nothing to review. 51 warnings each carrying that caveat is alarm-fatigue on a false premise.suggests (nofix), these ~51 mechanical, equivalence-preserving replacements cannot be applied viaeslint --fix, even though they are provably safe.Proposed refinement
Detect a provably-numeric argument and branch behavior:
CallExpressiontoparseInt/parseFloat/Number/Number.parseInt/Number.parseFloat, or aCallExpressionwhose callee property isgetTime/getTimezoneOffset/valueOf, or a numericLiteral.Number(...)" caveat and offer a real autofix (fix, not justsuggest) — the transform is a guaranteed equivalence.suggest-only + caveat behavior (correct — replacement could flip semantics for strings).Acceptance criteria
--fix; message omits the misleading Number(...) caveat.isNaN(parseInt(x, 10)),isNaN(parseFloat(x)),isNaN(d.getTime())asserting the autofix output; keep an unknown-arg case asserting suggest-only.--fix-able).Notes