Rule
eslint-factory/src/rules/prefer-number-isnan.ts
Observation
hasLocalBinding decides a name is locally bound only when it has a non-import definition:
if (variable?.defs.some(d => d.type !== "ImportBinding")) {
return true;
}
So a name bound solely via an ESM import is treated as not locally bound → the rule proceeds to flag it as the global. Two consequences:
import { isNaN } from "lodash"; (lodash ships an isNaN) — the call isNaN(x) is flagged as the global. lodash's isNaN does not coerce, so the diagnostic text ("Global isNaN() coerces non-number inputs") is factually wrong for that binding, and the Number.isNaN suggestion changes which function is called.
import { window } from "./browser-shim"; then window.isNaN(x) — flagged as if window were the real global object, even though it is a user shim.
Why it may still be intentional — but needs to be pinned down
The target corpus is CommonJS (require), where bindings are Variable defs (not ImportBinding), so const isNaN = Number.isNaN and const window = {...} are correctly treated as shadows and skipped — this is covered by tests. The ImportBinding branch therefore only affects ESM consumers, of which there are zero in actions/setup/js today, so this is a latent concern, not a live false positive.
But the exclusion reads as inverted (an import binding is a local binding that shadows the global), it has no test, and its rationale is not documented. If the intent was to keep flagging globalThis/window/global even when re-declared, that logic is surprising for isNaN itself.
Proposed refinement (pick one)
- Document + test the intended semantics: add a comment explaining why import bindings are excluded, and add valid/invalid ESM test cases pinning the behavior for
import { isNaN } and import { window }.
- or Fix: treat any def (including
ImportBinding) as a shadow — i.e. drop the d.type !== "ImportBinding" filter — so imported isNaN/window/global are not flagged as the global. This aligns with the existing intent that "locally shadowed bindings are intentionally excluded."
Acceptance criteria
Priority
Medium — hardening/soundness of the rule's shadow detection; not grounded in a live corpus FP (no ESM consumers today).
Generated by 🤖 ESLint Refiner · 313.5 AIC · ⌖ 13 AIC · ⊞ 4.7K · ◷
Rule
eslint-factory/src/rules/prefer-number-isnan.tsObservation
hasLocalBindingdecides a name is locally bound only when it has a non-import definition:So a name bound solely via an ESM import is treated as not locally bound → the rule proceeds to flag it as the global. Two consequences:
import { isNaN } from "lodash";(lodash ships anisNaN) — the callisNaN(x)is flagged as the global. lodash'sisNaNdoes not coerce, so the diagnostic text ("Global isNaN() coerces non-number inputs") is factually wrong for that binding, and theNumber.isNaNsuggestion changes which function is called.import { window } from "./browser-shim";thenwindow.isNaN(x)— flagged as ifwindowwere the real global object, even though it is a user shim.Why it may still be intentional — but needs to be pinned down
The target corpus is CommonJS (
require), where bindings areVariabledefs (notImportBinding), soconst isNaN = Number.isNaNandconst window = {...}are correctly treated as shadows and skipped — this is covered by tests. TheImportBindingbranch therefore only affects ESM consumers, of which there are zero inactions/setup/jstoday, so this is a latent concern, not a live false positive.But the exclusion reads as inverted (an import binding is a local binding that shadows the global), it has no test, and its rationale is not documented. If the intent was to keep flagging
globalThis/window/globaleven when re-declared, that logic is surprising forisNaNitself.Proposed refinement (pick one)
import { isNaN }andimport { window }.ImportBinding) as a shadow — i.e. drop thed.type !== "ImportBinding"filter — so importedisNaN/window/globalare not flagged as the global. This aligns with the existing intent that "locally shadowed bindings are intentionally excluded."Acceptance criteria
isNaN/window/globalis deliberate, documented, and covered by ESM RuleTester cases.isNaNsites still flag).Priority
Medium — hardening/soundness of the rule's shadow detection; not grounded in a live corpus FP (no ESM consumers today).