[eslint-miner] feat(eslint): add require-parseInt-radix rule — ESLint Miner 2026-06-27#41961
Conversation
Adds a new custom ESLint rule that requires all parseInt() and Number.parseInt() calls to include an explicit radix argument. ## Evidence Five violations in parse_codex_log.cjs (lines 32, 145, 146, 798, 806) where parseInt() is called without a radix on regex-captured digit strings. All are decimal-only captures (\d+) but omitting the radix leaves the behaviour implicit and fragile — any string prefixed with 0x/0X would silently parse as hexadecimal. ## Rule behaviour Flags: parseInt(str) → require explicit radix Number.parseInt(str) → require explicit radix Allows: parseInt(str, 10) Number.parseInt(str, 10) parseInt(str, 16) (any explicit radix) Enabled as 'warn' in eslint.config.cjs alongside the existing require-json-parse-try-catch rule. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. The PR adds a new ESLint rule (require-parseInt-radix.ts) and updates index.ts and eslint.config.cjs, but no *_test.go, *.test.cjs, or *.test.js files were changed. Test Quality Sentinel skipped. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Adds a new custom ESLint rule to the actions/setup/js lint pipeline to proactively require an explicit radix for parseInt() and Number.parseInt() calls, improving consistency and preventing implicit base-detection surprises.
Changes:
- Introduces
gh-aw-custom/require-parseInt-radixrule implementation to flagparseIntcalls missing a radix argument. - Registers the new rule in the
eslint-factoryplugin export. - Enables the rule as a
warnin theeslint-factoryESLint config.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/eslint-factory/src/rules/require-parseInt-radix.ts | New ESLint rule that reports parseInt/Number.parseInt calls with no radix argument. |
| actions/setup/js/eslint-factory/src/index.ts | Registers the new rule in the plugin’s rules map. |
| actions/setup/js/eslint-factory/eslint.config.cjs | Enables the rule as gh-aw-custom/require-parseInt-radix: warn. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Low
| messages: { | ||
| requireRadix: | ||
| "parseInt() must be called with an explicit radix (e.g., parseInt(str, 10)) to avoid implicit octal/hex parsing in actions/setup/js.", | ||
| }, |
There was a problem hiding this comment.
Updated the message to 'avoid implicit base detection in actions/setup/js' (febbcaf).
| docs: { | ||
| description: | ||
| "Require parseInt() calls in actions/setup/js scripts to include an explicit radix argument to avoid implicit octal or hexadecimal parsing", | ||
| }, |
There was a problem hiding this comment.
Updated the description to 'avoid implicit base detection (e.g., 0x prefix silently parsed as hexadecimal)' (febbcaf).
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — approving with two minor suggestions on test coverage and an edge-case gap.
📋 Key Themes & Highlights
Key Themes
- Missing unit tests:
@typescript-eslint/utilsshipsRuleTester; a co-located.test.tsfile would make future changes safer and serve as living documentation. parseInt(str, undefined)false negative: The currentnode.arguments.length < 2guard does not flag calls that passundefinedexplicitly as the radix — which is spec-equivalent to omitting it.
Positive Highlights
- ✅ Evidence-driven: PR identifies exactly 5 real violations in
parse_codex_log.cjsand confirms zero false positives acrossactions/setup/js. - ✅ Implementation is consistent with the existing
require-json-parse-try-catchrule pattern. - ✅ Both
parseIntandNumber.parseIntvariants are handled. - ✅ Rule is added at
warnseverity — appropriate for a newly introduced linting gate. - ✅ Clean, focused PR: three small, purposeful files.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 42 AIC · ⌖ 9.81 AIC · ⊞ 6.6K
| }, | ||
| }, | ||
| defaultOptions: [], | ||
| create(context) { |
There was a problem hiding this comment.
[/tdd] No unit tests are included — @typescript-eslint/utils ships RuleTester / @typescript-eslint/rule-tester for exactly this use case.
💡 Suggested test skeleton
// require-parseInt-radix.test.ts
import { RuleTester } from "`@typescript-eslint/rule-tester`";
import { requireParseIntRadixRule } from "./require-parseInt-radix";
const ruleTester = new RuleTester();
ruleTester.run("require-parseInt-radix", requireParseIntRadixRule, {
valid: [
{ code: "parseInt(str, 10);" },
{ code: "parseInt(str, 16);" },
{ code: "Number.parseInt(str, 10);" },
{ code: "parseInt(str, 8);" },
],
invalid: [
{
code: "parseInt(str);",
errors: [{ messageId: "requireRadix" }],
},
{
code: "Number.parseInt(str);",
errors: [{ messageId: "requireRadix" }],
},
{
code: "parseInt(str, undefined);",
errors: [{ messageId: "requireRadix" }],
},
],
});The existing require-json-parse-try-catch rule also lacks tests, so this follows current convention — but co-locating a test file with the rule is the most reliable way to prevent regressions and document intent.
@copilot please address this.
| if ( | ||
| node.callee.type === "Identifier" && | ||
| node.callee.name === "parseInt" && | ||
| node.arguments.length < 2 |
There was a problem hiding this comment.
[/tdd] node.arguments.length < 2 does not catch parseInt(str, undefined) — passing undefined as radix is spec-equivalent to omitting it (the engine falls back to a heuristic base).
💡 Tighten the guard to cover the undefined case
function isMissingRadix(node: TSESTree.CallExpression): boolean {
if (node.arguments.length < 2) return true;
const radix = node.arguments[1];
return radix.type === "Identifier" && radix.name === "undefined";
}Then use isMissingRadix(node) in both branches. The same pattern should apply to the Number.parseInt branch below.
A unit test for parseInt(str, undefined) in the invalid list would lock in this edge-case behaviour.
@copilot please address this.
There was a problem hiding this comment.
Non-blocking observations
The rule logic is correct for the targeted call patterns and the integration wiring (index.ts, eslint.config.cjs) is clean. Two non-blocking issues worth addressing before the rule grows:
Findings summary
1. meta.type: "problem" should be "suggestion" (line 8) — The preventive nature of this rule and the fact that all 5 violations are currently correct (digit-only regex captures) align with "suggestion", not "problem". ESLint's own radix built-in uses "suggestion" for the same check.
2. No scope analysis (line 25, 38) — node.callee.name === "parseInt" matches by name only. A locally-shadowed parseInt or Number will false-positive. Low risk in the current codebase, but fragile if test doubles are ever introduced.
Existing comments on lines 12 and 17 (wording accuracy) are also valid and should be addressed.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
patchdiff.githubusercontent.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "patchdiff.githubusercontent.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · 68.9 AIC · ⌖ 6.88 AIC · ⊞ 5.2K
| export const requireParseIntRadixRule = createRule({ | ||
| name: "require-parseInt-radix", | ||
| meta: { | ||
| type: "problem", |
There was a problem hiding this comment.
type: "problem" overstates the severity — ESLint's own built-in radix rule uses "suggestion" for the identical check.
💡 Details
ESLint rule types:
"problem"— code that will or may cause a bug right now"suggestion"— code that could be improved but does not currently cause a bug
All five flagged call-sites parse digit-only strings from \d+ regex captures and are currently correct. This rule is preventive, not diagnostic. Classifying it as "problem" misrepresents its risk level and may cause tooling (VS Code ESLint extension grouping, eslint --print-docs-json consumers) to surface it with unwarranted urgency.
Suggested change:
type: "suggestion",For reference, ESLint's own [radix built-in]((eslint.org/redacted) — which enforces the exact same property — uses type: "suggestion".
| CallExpression(node) { | ||
| // Global parseInt(x) — missing radix | ||
| if ( | ||
| node.callee.type === "Identifier" && |
There was a problem hiding this comment.
No scope analysis: any local variable named parseInt triggers a false positive, including test doubles or polyfills that shadow the global.
💡 Details
The check node.callee.name === "parseInt" matches the name of the identifier but does not verify it resolves to the global built-in. The same gap applies at line 38 (node.callee.object.name === "Number" for Number.parseInt).
A false positive example:
// A local test helper that shadows parseInt
function parseInt(s) { return +s; }
parseInt(x); // rule fires — false positiveESLint provides scope analysis to distinguish global from local bindings:
const scope = sourceCode.getScope(node);
const variable = scope.references
.find(ref => ref.identifier === node.callee)
?.resolved;
// If variable is null, it resolved to the global
if (variable !== null) return; // local shadow — skipIn this specific codebase the risk is low (shadowing parseInt would also violate no-shadow-restricted-names), but the rule is more fragile than it looks and will cause friction if the project ever adds test files that mock or override these globals.
|
@copilot please run the
|
…ction not octal Update docs description and message to accurately describe the risk as implicit base detection (e.g. 0x prefix → hex), removing the misleading reference to octal which modern JS does not do via parseInt. Also commit lock.yml recompile changes. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…parseint-radix-771629c4e240f5b5 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Done: fixed both wording issues (febbcaf), merged main (ecee02d), and local |
pelikhan
deleted the
eslint-miner/require-parseint-radix-771629c4e240f5b5
branch
Summary
Adds a new custom ESLint rule
require-parseInt-radixto thegh-aw-customESLint plugin used inactions/setup/jsscripts.What changed
New rule:
require-parseInt-radixFile:
actions/setup/js/eslint-factory/src/rules/require-parseInt-radix.ts(new, 42 lines)ESLintUtils.RuleCreatorfrom@typescript-eslint/utils."problem".parseInt()andNumber.parseInt()calls missing a radix argument (called with fewer than 2 arguments).parseInt(x)(Identifier callee) andNumber.parseInt(x)(MemberExpression callee).parseInt(str, 10)to avoid implicit base detection.Plugin registration
File:
actions/setup/js/eslint-factory/src/index.tsrequireParseIntRadixRuleand registers it as"require-parseInt-radix"in the plugin rules map.Config activation
File:
actions/setup/js/eslint-factory/eslint.config.cjs"gh-aw-custom/require-parseInt-radix": "warn"to the shared ESLint factory config.Motivation
Without an explicit radix,
parseInt()infers the numeric base from the string prefix (e.g.,0x→ hexadecimal). Enforcing an explicit radix prevents subtle bugs and makes numeric parsing intent unambiguous.Scope
Limited to
actions/setup/js/eslint-factory. No runtime logic, workflow YAML, or generated files changed.Generated by ESLint Miner · Run 28305716756