Skip to content

Detect Date.parse(...) comparisons in require-invalid-date-check-before-compare - #52961

Merged
pelikhan merged 2 commits into
mainfrom
copilot/require-invalid-date-check-before-compare
Aug 15, 2026
Merged

Detect Date.parse(...) comparisons in require-invalid-date-check-before-compare#52961
pelikhan merged 2 commits into
mainfrom
copilot/require-invalid-date-check-before-compare

Conversation

Copilot AI commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

require-invalid-date-check-before-compare only tracked new Date(...) + .getTime() construction, but Date.parse(x) follows the exact same hazard: it returns NaN for unparseable input, and a subsequent relational comparison (<, >, <=, >=) silently evaluates to false instead of surfacing an error. Date.parse is actually the more common idiom for timestamp comparisons in actions/setup/js, so the rule was blind to its most likely regression site.

// previously undetected
const createdAtMs = Date.parse(run.created_at || "");
if (createdAtMs < cutoffMs) { hasMore = false; } // NaN < x is always false

// now correctly recognized as guarded
const createdAtMs = Date.parse(run.created_at || "");
if (!Number.isFinite(createdAtMs) || createdAtMs < cutoffMs) { ... }

Detection

  • Added isPotentiallyInvalidDateParseCall to recognize Date.parse(x) the same way isPotentiallyInvalidDateConstruction recognizes new Date(x).
  • Track variables initialized from Date.parse(x) alongside new Date(x) variables, tagged with a source: "construct" | "parse" so guard/message logic can differentiate them.
  • BinaryExpression handling now also treats an inline Date.parse(...) call as a flaggable comparison side, mirroring inline new Date(...).

Guard recognition

  • Added isIsNaNCallee/isIsFiniteCallee helpers to recognize both global (isNaN/isFinite) and static (Number.isNaN/Number.isFinite) forms.
  • Added extractDirectNaNCheckTarget / extractIsFiniteCheckTarget to recognize Number.isNaN(name) / !Number.isNaN(name) and Number.isFinite(name) applied directly to the parsed number (no .getTime() involved) as valid guards.
  • Reused the existing dominance/reachability logic (guardDominatesComparison, isExitingIfGuard, guardDirectlyGatesComparison) unchanged, with one addition: when a Number.isFinite(name) guard is wrapped in !, the enclosing negation is also registered so exiting-guard patterns like if (!Number.isFinite(a)) return false; are recognized.

Messaging

  • Added a new requireInvalidDateCheckParse message describing the Date.parse/Number.isFinite idiom, keeping the original requireInvalidDateCheck message (and its wording) for new Date(...) cases untouched.
  • When both sides of a comparison are unvalidated but come from different sources (one new Date, one Date.parse), each side is now reported individually with its own tailored message instead of forcing a single combined message.

… comparisons

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix date comparison rule to include Date.parse checks Detect Date.parse(...) comparisons in require-invalid-date-check-before-compare Aug 15, 2026
Copilot AI requested a review from pelikhan August 15, 2026 20:28
@pelikhan
pelikhan marked this pull request as ready for review August 15, 2026 20:38
Copilot AI balanced review requested due to automatic review settings August 15, 2026 20:38
@pelikhan
pelikhan merged commit c02e323 into main Aug 15, 2026
1 check passed
@pelikhan
pelikhan deleted the copilot/require-invalid-date-check-before-compare branch August 15, 2026 20:39

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

Extends the ESLint rule to detect unsafe relational comparisons involving Date.parse() results.

Changes:

  • Tracks variable and inline Date.parse() expressions.
  • Recognizes finite/NaN guards and adds tailored diagnostics.
  • Adds tests for guarded and unguarded comparisons.
Show a summary per file
File Description
eslint-factory/src/rules/require-invalid-date-check-before-compare.ts Adds Date.parse() detection, guard handling, and diagnostics.
eslint-factory/src/rules/require-invalid-date-check-before-compare.test.ts Tests new detection and guard patterns.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

eslint-factory/src/rules/require-invalid-date-check-before-compare.ts:290

  • Recording Number.isFinite(a) as a generic guard loses the predicate's polarity. The existing dominance logic will therefore accept unsafe forms such as if (Number.isFinite(a)) return; return a >= cutoff and return Number.isFinite(a) || a >= cutoff, where the comparison is reached specifically when a is invalid. Preserve whether a guard is valid-when-true and only accept the matching exit/short-circuit branch (for example, finite && comparison or an exit on !finite).
        const isFiniteTarget = extractIsFiniteCheckTarget(node);
        if (isFiniteTarget) {
          const variable = resolveVariable(sourceCode, isFiniteTarget);
          if (variable) {
            const ancestors = sourceCode.getAncestors(node);
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines +279 to +283
const directNaNTarget = extractDirectNaNCheckTarget(node);
if (directNaNTarget) {
const variable = resolveVariable(sourceCode, directNaNTarget);
if (variable) addGuardPath(variable, sourceCode.getAncestors(node), node);
return;
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.87.0

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

Labels

None yet

Projects

None yet

3 participants