Skip to content

feat(eslint-factory): add require-nan-check-after-env-numeric-parse rule - #49962

Merged
pelikhan merged 6 commits into
mainfrom
copilot/eslint-miner-add-require-nan-check-rule
Aug 3, 2026
Merged

feat(eslint-factory): add require-nan-check-after-env-numeric-parse rule#49962
pelikhan merged 6 commits into
mainfrom
copilot/eslint-miner-add-require-nan-check-rule

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

parseInt/parseFloat/Number() silently return NaN for malformed process.env input. Without an explicit Number.isNaN()/isNaN() guard, that NaN propagates into rate-limit thresholds, timeouts, loop bounds, and API payloads with no error surfacing. Scanning actions/setup/js found 12 unguarded occurrences across 9 files.

New rule: require-nan-check-after-env-numeric-parse

  • Detection: flags parseInt, parseFloat, Number.parseInt, Number.parseFloat, and Number() calls whose first argument traces back to process.env, when the assigned variable is never passed to isNaN()/Number.isNaN()
  • Env-access patterns covered: direct (process.env.FOO), logical fallbacks (|| / ??), optional chaining (?.trim()), ternary
  • Cleared by: any isNaN(x) or Number.isNaN(x) call with the variable as sole argument anywhere in the file scope
  • Severity: warn in eslint.config.cjs
// flagged
const maxRuns = parseInt(process.env.MAX_RUNS, 10);

// safe
const maxRuns = parseInt(process.env.MAX_RUNS, 10);
if (Number.isNaN(maxRuns)) throw new Error("MAX_RUNS must be a valid integer");

Files

  • src/rules/require-nan-check-after-env-numeric-parse.ts — rule implementation
  • src/rules/require-nan-check-after-env-numeric-parse.test.ts — 22 unit tests
  • src/index.ts — rule registration
  • eslint.config.cjs — enabled as warn
  • README.md — rule documented

Copilot AI and others added 2 commits August 3, 2026 12:17
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add require-nan-check-after-env-numeric-parse rule to ESLint feat(eslint-factory): add require-nan-check-after-env-numeric-parse rule Aug 3, 2026
Copilot AI requested a review from pelikhan August 3, 2026 12:28
@pelikhan

pelikhan commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

@copilot merge main and recompile

Copilot AI and others added 2 commits August 3, 2026 12:40
…add-require-nan-check-rule

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@copilot merge main and recompile

Done — merged main and recompiled all 269 workflows (269 succeeded).

@pelikhan
pelikhan marked this pull request as ready for review August 3, 2026 13:02
Copilot AI review requested due to automatic review settings August 3, 2026 13:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an ESLint rule warning when numeric process.env parsing lacks NaN validation.

Changes:

  • Implements and tests env numeric-parse detection.
  • Registers, enables, and documents the rule.
  • Unintentionally removes an agentic-workflow guidance entry.
Show a summary per file
File Description
eslint-factory/src/rules/require-nan-check-after-env-numeric-parse.ts Implements detection and validation tracking.
eslint-factory/src/rules/require-nan-check-after-env-numeric-parse.test.ts Adds rule tests.
eslint-factory/src/index.ts Registers the rule.
eslint-factory/eslint.config.cjs Enables the rule as a warning.
eslint-factory/README.md Documents the rule.
.github/skills/agentic-workflows/SKILL.md Removes an unrelated guidance entry.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Suppressed comments (2)

eslint-factory/src/rules/require-nan-check-after-env-numeric-parse.ts:116

  • The rule keys declarations and validations only by identifier text, so distinct bindings are conflated and same-named declarations overwrite each other. For example, Number.isNaN(count) in one function suppresses an unvalidated count parsed in another function. Track the ESLint variable/binding identity and resolve each validation argument to that binding instead.
          unvalidated.set(node.id.name, node);

eslint-factory/src/rules/require-nan-check-after-env-numeric-parse.ts:102

  • A locally defined isNaN (or local Number object below) is accepted as validation solely by name. A no-op helper such as const isNaN = () => false therefore suppresses the warning even though the parsed value was never checked by the intended built-in. Require these validator callees to resolve to unshadowed globals.
      // Global isNaN(x)
      if (callee.type === "Identifier" && callee.name === "isNaN") {
        return true;
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Balanced

@@ -71,7 +71,6 @@ Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/test-coverage.md`
- `.github/aw/test-expression.md`
- `.github/aw/token-optimization-caching-budgets.md`
Comment on lines +69 to +70
// Global parseInt(envExpr, ...) or parseFloat(envExpr)
if (callee.type === "Identifier" && (callee.name === "parseInt" || callee.name === "parseFloat")) {
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔍 PR Triage

Category: feature · Risk: low · Total score: 50/100
Batch: batch-quality-1 (grouped with #49971, #49972 — low-risk quality/tooling PRs)

Impact Urgency Quality
24/50 8/30 18/20

New eslint-factory rule (require-nan-check-after-env-numeric-parse) with 22 unit tests and README documentation.

Recommended action: batch_review — low-risk, well-tested; suitable for grouped review with similar tooling PRs.

Generated by 🔧 PR Triage Agent · auto · 55.7 AIC · ⌖ 4.24 AIC · ⊞ 8K ·

@pelikhan
pelikhan merged commit 6d26b8f into main Aug 3, 2026
@pelikhan
pelikhan deleted the copilot/eslint-miner-add-require-nan-check-rule branch August 3, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[eslint-miner] eslint-factory: add require-nan-check-after-env-numeric-parse rule

3 participants