prefer-number-isnan: autofix for provably-numeric args, drop misleading caveat#43515
Conversation
- Add isProvablyNumeric() to detect parseInt/parseFloat/Number/ Number.parseInt/Number.parseFloat calls, getTime/getTimezoneOffset/ valueOf method calls, and numeric literals as provably-numeric args - Add fixable: "code" to rule meta so --fix can apply the replacement - Branch report(): provably-numeric → real fix (no misleading caveat); unknown arg → unchanged suggest-only + "review Number(...)" caveat - Add 9 test cases asserting autofix output for all provably-numeric patterns; existing suggestion-only tests unchanged Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey This PR ticks all the boxes: the implementation is tightly scoped to a single rule, test coverage spans every provably-numeric pattern the helper recognises (9 new
|
There was a problem hiding this comment.
Pull request overview
This PR updates the prefer-number-isnan ESLint rule to provide a true autofix (--fix) when the argument to isNaN(...) is deemed “provably numeric”, while keeping suggestion-only behavior (with the existing caveat message) for unknown inputs.
Changes:
- Added
fixable: "code"and a newisProvablyNumeric(...)helper to decide when an autofix is safe. - Split
report()behavior: autofix for “provably numeric” args; suggestions-only for everything else. - Added a new test block asserting autofix outputs for several “provably numeric” patterns.
Show a summary per file
| File | Description |
|---|---|
| eslint-factory/src/rules/prefer-number-isnan.ts | Adds provably-numeric detection and enables real autofixes for certain isNaN(...) call sites. |
| eslint-factory/src/rules/prefer-number-isnan.test.ts | Adds invalid cases asserting the new autofix outputs for provably-numeric argument patterns. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Low
| // parseInt(x), parseFloat(x), Number(x) | ||
| if (callee.type === "Identifier" && NUMERIC_CALL_NAMES.has(callee.name)) { | ||
| return true; | ||
| } | ||
| // Number.parseInt(x), Number.parseFloat(x) | ||
| if ( | ||
| callee.type === "MemberExpression" && | ||
| !callee.computed && | ||
| callee.object.type === "Identifier" && | ||
| callee.object.name === "Number" && | ||
| callee.property.type === "Identifier" && | ||
| (callee.property.name === "parseInt" || callee.property.name === "parseFloat") | ||
| ) { | ||
| return true; | ||
| } |
| // x.getTime(), x.getTimezoneOffset(), x.valueOf() | ||
| if (callee.type === "MemberExpression" && !callee.computed && callee.property.type === "Identifier" && NUMERIC_METHOD_NAMES.has(callee.property.name)) { | ||
| return true; | ||
| } |
| // Date method results | ||
| { | ||
| code: `isNaN(d.getTime());`, | ||
| output: `Number.isNaN(d.getTime());`, | ||
| errors: [{ messageId: "preferNumberIsNaN" }], | ||
| }, | ||
| { | ||
| code: `isNaN(d.getTimezoneOffset());`, | ||
| output: `Number.isNaN(d.getTimezoneOffset());`, | ||
| errors: [{ messageId: "preferNumberIsNaN" }], | ||
| }, | ||
| { | ||
| code: `isNaN(x.valueOf());`, | ||
| output: `Number.isNaN(x.valueOf());`, | ||
| errors: [{ messageId: "preferNumberIsNaN" }], | ||
| }, |
|
@copilot please run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed unresolved review feedback in
I also fetched |
🤖 PR Triage — Run §28741077056
Rationale: Promotes
|
|
@copilot please run the
|
The rule always emitted a
suggest-only fix with a "review whether to wrap withNumber(...)" caveat — but every one of the 51 liveisNaN()call sites inactions/setup/jspasses an already-numeric argument (parseInt(...),parseFloat(...),Date#getTime(), etc.), making that caveat wrong and the replacement un---fix-able.Rule changes
isProvablyNumeric(arg)— new helper that returnstruefor: calls toparseInt/parseFloat/Number/Number.parseInt/Number.parseFloat; method calls togetTime/getTimezoneOffset/valueOf; numeric literals.report()now branches onisProvablyNumeric:fix(applied by--fix), no caveat message.suggest-only + "review whether to wrap withNumber(...)" caveat.fixable: "code"to rule meta.Behavior comparison
Tests
Added a new test group with 9
invalidcases covering every provably-numeric pattern, each asserting theoutputafter autofix. Existing suggestion-only cases are unchanged.