Fix month parsing overflowing into the next month - #50
Open
dualfroz wants to merge 1 commit into
Open
Conversation
Context.Time applied the parsed month while the day was still the one of the reference date. When that day does not exist in the target month, time.Date normalizes it forward, so "september 5th" parsed on the 31st of October resolved to the 5th of October. The parsed day was only applied afterwards, to the already wrong month. Clamp the reference day to the last day of the target month before switching the month. The parsed day still overwrites it afterwards, so the ordering of the branches in Context.Time is unchanged. The existing fixtures pin the reference date to 2016-01-06 and the 6th exists in every month, so the overflow never triggered. Add a regression test that parses against 2016-10-31. Fixes olebedev#12
dualfroz
force-pushed
the
fix/month-day-overflow
branch
from
September 5, 2026 22:44
9fc1f12 to
dcaedc8
Compare
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
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.
Problem
Fixes #12
Problem
Parsing a bare month, or a month with a day, against a reference date whose day
does not exist in the target month resolves to the wrong month. With the 31st of
October as the reference date:
september 5thfebruary 111st of juneThis is why the issue reports "September" turning into October and why a later
comment reports February turning into March at the end of January: the bug only
shows up when the reference date is a day that the parsed month does not have,
which is also why it was not reproducible with an arbitrary fixed date.
The last comment in the thread asks whether this has already been fixed. It has
not - the results above come from a run against current master (faea3c4).
Root cause
rules/context.go, thec.Monthbranch ofContext.Time(lines 31-34 beforethis change):
The day passed to
time.Dateist.Day(), still the day of the referencedate, because the parsed day is only applied further down in the same function
(the
c.Daybranch).time.Datenormalizes out-of-range days, so October 31stplus month September becomes October 1st, and the
c.Daybranch then appliesthe parsed day to the already wrong month.
Solution
Clamp the reference day to the last day the target month actually has before
switching the month. The parsed day, when the text carries one, still overwrites
it afterwards, so the ordering of the existing branches is untouched:
september 5th), the clamped value is a throwawayplaceholder that only has to stay inside the target month;
september), the result is the last day of themonth instead of a silent jump into the next one.
A small
daysInhelper does the lookup with the usualtime.Date(y, m+1, 0)trick. No rule files were touched.
How this was tested
Gate, on the fixed tree:
52 test cases pass, no pre-existing failures,
gofmt -l .is clean.New regression test
TestExactMonthDateFromLongMonthinrules/en/exact_month_date_test.go. The existing fixtures cannot catch thisbug:
ApplyFixturespins the reference date to 2016-01-06, and the 6th existsin every month, so the overflow never triggers. The new test parses against
2016-10-31 instead and checks the resulting dates directly.
With the fix reverted and the test in place:
With the fix applied:
Go 1.22.2.