{{ message }}
refactor: rewrite matcher-walker to infer next run from cron fields#533
Merged
Conversation
Fixes #518. The old field-by-field walker resolved a non-existent local time (the DST spring-forward gap) forward, then carried the drifted hour as it advanced day/month/year. That overshot ~10 months and returned a date that did not even match (e.g. "30 2 * * *" in America/New_York returned 2027-01-01). Rewrite getNextMatch as infer + verify: walk the calendar day by day, and on each day that matches month and day-of-month try the matching times in ascending order, materializing each to a real instant in the timezone and confirming it with the real matcher. The matcher enforces the weekday constraint and rejects DST-gap times by construction, so a gap day is skipped to the next day where the time exists. No date is returned unverified. Exposes localTimeToTimestamp from localized-time for the walker. Adds regression tests for the gap-daily case (#518), weekday overshoot (#510), Feb 29 (multi-year jump) and skipped day-31 months.
- Sort the time fields once in the constructor instead of per qualifying day. - Raise the calendar search bound to 100 years so valid-but-sparse expressions (e.g. Feb 29 on a specific weekday, ~28y apart) resolve instead of throwing; the loop is cheap, so the impossible-expression case stays fast. - Add tests: impossible expression throws, and the fall-back repeated hour returns the first occurrence.
…ons field Replace the three inline lower-bound guards with a single named predicate (isLaterInDay) plus the cheap whole-hour skip, and remove the now-unused public `expressions` field (the sorted private fields cover every use). No behavior change.
…mestamp The rewritten walker calls localTimeToTimestamp directly and never mutates a LocalizedTime, so set() had no remaining caller. Remove it and add direct unit tests for localTimeToTimestamp covering the three DST cases (normal, the non-existent spring-forward gap resolved forward, and the ambiguous fall-back resolved to the first occurrence). Restores localized-time.ts to 100% line and function coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #518.
Problem
The field-by-field walker resolved a non-existent local time (DST spring-forward gap) forward and carried the drifted hour as it advanced day/month/year.
30 2 * * *inAmerica/New_Yorkreturned2027-01-01(the 02:xx hour does not exist on the transition day), and the final fallthrough returned the date without re-checking that it matched.Approach
Infer the next run from the fields instead of scanning time tick by tick:
match()).The matcher enforces the weekday constraint and rejects DST-gap times by construction (a wall-clock that does not exist materializes to a different instant whose hour will not match), so a gap day is skipped to the next day where the time exists. No date is ever returned unverified. The search is bounded (~10 years of days) and throws for impossible expressions like
0 0 31 2 *.Verified
30 2 * * *gap day -> next day 02:30 (getNextMatch overshoots ~1 year for a daily schedule whose time falls in the spring-forward gap #518).0 18 * * 0..6-> next occurrence, no 2032/2034 overshoot (Wrong timestamp returned for getNextRun() #510).