Skip to content

prefer-number-isnan: misleading "wrap with Number(...)" caveat + suggestion-only fires on already-numeric args (0/51 live sites [Content truncated due to length] #43486

Description

@github-actions

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

  • Rule distinguishes provably-numeric args from unknown args as above.
  • Provably-numeric case: safe autofix applied under --fix; message omits the misleading Number(...) caveat.
  • Non-numeric case: unchanged (suggest-only + caveat), preserving the string-semantics-flip guard.
  • Tests: add invalid cases for isNaN(parseInt(x, 10)), isNaN(parseFloat(x)), isNaN(d.getTime()) asserting the autofix output; keep an unknown-arg case asserting suggest-only.
  • No regression on the 51 live sites (all should still flag; provably-numeric ones become --fix-able).

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 ·

  • expires on Jul 11, 2026, 10:38 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions