Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Ical.Net.Tests/Calendars/Recurrence/RecurrenceTestCases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ RRULE:FREQ=HOURLY;BYHOUR=0;BYYEARDAY=360,-2;UNTIL=20260101T000000
DTSTART:20251220T000000
INSTANCES:20251226T000000,20251230T000000

# BYYEARDAY with 1st day of the year and UNTIL
RRULE:FREQ=YEARLY;BYYEARDAY=1,10;UNTIL=20260105
DTSTART:20250110
INSTANCES:20250110,20260101

# BYHOUR, BYMINUTE, BYSECOND limit behaviour
# note that the max number of increments is 1000, so we can only observer a limited time span
RRULE:FREQ=SECONDLY;BYHOUR=1,2;BYMINUTE=3,4,59;BYSECOND=5,6;UNTIL=20250216T020500
Expand Down Expand Up @@ -156,3 +161,23 @@ RRULE:FREQ=YEARLY;BYWEEKNO=-1;BYDAY=SU;COUNT=3
DTSTART:99971228
EXCEPTION:Ical.Net.Evaluation.EvaluationOutOfRangeException
EXCEPTION-STEP:Enumeration

# Yearly recurrence starting on Feb 28 of a leap year, 5 occurrences
DTSTART:20240228
RRULE:FREQ=YEARLY;COUNT=5
INSTANCES:20240228,20250228,20260228,20270228,20280228

# Yearly recurrence starting on Feb 29 of a leap year, 4 occurrences
DTSTART:20240229
RRULE:FREQ=YEARLY;COUNT=4
INSTANCES:20240229,20280229,20320229,20360229

# First and last day of the year, 3 occurrences with first/last day of year expansion, issue #889
DTSTART:20250101
RRULE:FREQ=YEARLY;BYYEARDAY=-1,1;BYMONTHDAY=-1,1;COUNT=3
INSTANCES:20250101,20251231,20260101

# Second and second last day of June, all occurrences in June 2025, issue #889
DTSTART:20250601
RRULE:FREQ=YEARLY;BYMONTH=6;BYMONTHDAY=2,-2;UNTIL=20250630
INSTANCES:20250602,20250629
4 changes: 2 additions & 2 deletions Ical.Net.Tests/RecurrenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ int eventIndex

private void EventOccurrenceTest(
Calendar cal,
CalDateTime fromDate,
CalDateTime toDate,
CalDateTime? fromDate,
CalDateTime? toDate,
Period[] expectedPeriods,
string[]? timeZones
) => EventOccurrenceTest(cal, fromDate, toDate, expectedPeriods, timeZones, 0);
Expand Down
9 changes: 3 additions & 6 deletions Ical.Net/Evaluation/Evaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,9 @@ protected void IncrementDate(ref CalDateTime dt, RecurrencePattern pattern, int
dt = old.AddDays(-old.Day + 1).AddMonths(interval);
break;
case FrequencyType.Yearly:
// When a rule uses BYWEEKNO, recurrence enumeration
// is based on week numbers relative to the year.
// So we preserve January 1st when adding years; otherwise, preserve month/day.
dt = (pattern.ByWeekNo.Count != 0)
? old.AddDays(-old.DayOfYear + 1).AddYears(interval)
: old.AddYears(interval);
// RecurrencePatternEvaluator relies on the assumption that after incrementing, the new refDate
// is usually at the first day of an interval.
dt = old.AddDays(-old.DayOfYear + 1).AddYears(interval);
break;
default:
// Frequency should always be valid at this stage.
Expand Down
73 changes: 36 additions & 37 deletions Ical.Net/Evaluation/RecurrencePatternEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,8 @@ private IEnumerable<CalDateTime> EnumerateDates(CalDateTime originalDate, CalDat
if (searchEndDate < lowerLimit)
break;

// Use the original DTSTART month as the anchor for YEARLY rules when BYMONTH is not specified.
// This ensures BYMONTHDAY expansion/limiting is performed relative to the DTSTART month
// (the expected behavior for yearly rules without an explicit BYMONTH), instead of the
// current interval reference month.
var anchorMonth = pattern is { Frequency: FrequencyType.Yearly, ByMonth.Count: 0 }
? originalDate.Month
: intervalRefTime.Month;

var candidates = GetCandidates(intervalRefTime, pattern, expandBehavior, anchorMonth);
var candidates =
GetCandidates((lowerLimit > intervalRefTime) ? lowerLimit : intervalRefTime, pattern, expandBehavior);

foreach (var t in candidates.Where(t => t >= originalDate))
{
Expand Down Expand Up @@ -226,7 +219,8 @@ private IEnumerable<CalDateTime> EnumerateDates(CalDateTime originalDate, CalDat
/// might fall earlier in the year than the intervalRefTime's month/day. In
/// that case we compute the earliest possible date/time that could be
/// generated for the interval (earliest month/day/hour/minute/second).
/// - If BYWEEKNO is present, the interval may contain days from the previous
/// - If neither BYMONTH nor BYWEEKNO is present, we use the original date's month
/// - If only BYWEEKNO is present, the interval may contain days from the previous
/// or next year (ISO week boundaries). In that case we adjust the interval
/// start to the first day of the configured week so we don't miss candidates
/// that belong to the week containing Jan 1st.
Expand All @@ -235,6 +229,20 @@ private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, Re
{
switch (pattern)
{
case { Frequency: FrequencyType.Yearly, ByMonth.Count: 0, ByWeekNo.Count: 0 }:
{
// Return intervalRefTime but use the month from the original DTSTART.
// Else, the earliest candidate for the interval might be too early
// Do this by shifting the intervalRefTime by the difference in months.
// This preserves the day/time from intervalRefTime and relies on AddMonths
// to perform month-end semantics (e.g. Jan 31 -> Feb 28/29) instead of
// manually clamping the day.
var monthDelta = originalDate.Month - intervalRefTime.Month;
var adjusted = intervalRefTime.AddMonths(monthDelta);

return new CalDateTime(adjusted.Year, adjusted.Month, adjusted.Day, adjusted.Hour, adjusted.Minute, adjusted.Second, intervalRefTime.TzId);
}

case { Frequency: FrequencyType.Yearly, ByMonth.Count: > 0, ByWeekNo.Count: 0 }:
{
// When evaluating a YEARLY rule that restricts months (BYMONTH) but not
Expand Down Expand Up @@ -263,8 +271,12 @@ private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, Re
int day;
if (pattern.ByMonthDay.Count > 0)
{
var md = pattern.ByMonthDay.Min();
day = md > 0 ? Math.Min(md, daysInMonth) : Math.Max(1, daysInMonth + md + 1);
// Map BYMONTHDAY entries (positive and negative) to absolute days
// in the target month, then pick the smallest. This handles cases
// where BYMONTHDAY mixes positive and negative values (e.g. 2,-2).
var mappedDays = pattern.ByMonthDay
Copy link
Collaborator

Choose a reason for hiding this comment

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

Getting back to https://github.com/ical-org/ical.net/pull/884/files/84eb777d104f900184e58cb7cbc11c6126e17816#r2575960238

I was digging into the code (and git history) to remember, why I introduced this method. The code historically relies on the assumption that after Evaluator.IncrementDate() the new refDate is usually set to the first day of the given interval. For cases where this assumption doesn't hold, this method is intended to return the interval's actual start date (lower limit). So far this was only required for yearly recurrences when using BYWEEKNO, because for yearly recurrences the date was always set to Jan 1 but Week 1 could start in the previous year. In all other cases the code worked without that adjustment, but that's not quite obvious too. At least the time of the day is never set to 0:00, which is compensated by the fact, that we add 1d in GetSearchEndDate() but this is not documented at all and by no means obvious.

For now I'd recommend to set the date to Jan 1st in Evaluator.IncrementDate() in case of yearly recurrences. In this method I recommend to only deal with the special case of yearly/byweekno or alternatively return Jan 1st for the yearly case if byweekno is not used (but not only in BYMONTH is set).

At a later time we might investigate how to improve GetIntervalLowerLimit and GetSearchEndDate. The 1d adjustment in GetSearchEndDate is only intended to compensate for potential DST changes, not for refDate not being set to the beginning of the interval. I feel we should improve this so it is always set to the beginning of the interval, including the time part, and ideally also for the yearly/byweekno case. If we manage to do so, we can get rid of GetIntervalLowerLimit altogether.

Copy link
Collaborator Author

@axunonb axunonb Dec 12, 2025

Choose a reason for hiding this comment

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

only deal with the special case of yearly

Fixed with next commit

.Select(md => md > 0 ? Math.Min(md, daysInMonth) : Math.Max(1, daysInMonth + md + 1));
day = mappedDays.Min();
}
else
{
Expand All @@ -279,13 +291,15 @@ private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, Re

return new CalDateTime(year, month, day, hour, minute, second, intervalRefTime.TzId);
}

case { Frequency: FrequencyType.Yearly, ByWeekNo.Count: not 0 }:
{
// YEARLY with BYWEEKNO: weeks may span year boundaries. Move the
// interval lower limit to the first day of the week so expansion over
// the week (including days before Jan 1st) is handled correctly.
return GetFirstDayOfWeekDate(intervalRefTime, pattern.FirstDayOfWeek);
}

default:
{
return intervalRefTime;
Expand All @@ -312,17 +326,15 @@ private struct ExpandContext
/// <param name="pattern"></param>
/// <param name="expandBehaviors"></param>
/// <returns>A list of possible dates.</returns>
private IEnumerable<CalDateTime> GetCandidates(CalDateTime date, RecurrencePattern pattern, bool?[] expandBehaviors, int anchorMonth)
private IEnumerable<CalDateTime> GetCandidates(CalDateTime date, RecurrencePattern pattern, bool?[] expandBehaviors)
{
var expandContext = new ExpandContext() { DatesFullyExpanded = false };

IEnumerable<CalDateTime> dates = [date];
dates = GetMonthVariants(dates, pattern, expandBehaviors[0]);
dates = GetWeekNoVariants(dates, pattern, expandBehaviors[1], ref expandContext);
dates = GetYearDayVariants(dates, pattern, expandBehaviors[2], ref expandContext);
// Use the provided anchorMonth (typically the original DTSTART month) so BYMONTHDAY expansion
// is performed relative to the intended month when BYMONTH is not specified.
dates = GetMonthDayVariants(dates, pattern, expandBehaviors[3], ref expandContext, anchorMonth: anchorMonth);
dates = GetMonthDayVariants(dates, pattern, expandBehaviors[3], ref expandContext);
dates = GetDayVariants(dates, pattern, expandBehaviors[4], ref expandContext);
dates = GetHourVariants(dates, pattern, expandBehaviors[5]);
dates = GetMinuteVariants(dates, pattern, expandBehaviors[6]);
Expand Down Expand Up @@ -542,7 +554,7 @@ from yearDay in pattern.ByYearDay
/// If no BYMONTHDAY rules are specified, the date list is returned unmodified.
/// </summary>
/// <returns>The modified list of dates after applying the BYMONTHDAY rules.</returns>
private static IEnumerable<CalDateTime> GetMonthDayVariants(IEnumerable<CalDateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext, int anchorMonth)
private static IEnumerable<CalDateTime> GetMonthDayVariants(IEnumerable<CalDateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext)
{
if (expand == null || pattern.ByMonthDay.Count == 0)
return dates;
Expand All @@ -554,10 +566,10 @@ private static IEnumerable<CalDateTime> GetMonthDayVariants(IEnumerable<CalDateT
}

// limit behavior
return GetMonthDayVariantsLimited(dates, pattern, anchorMonth);
return GetMonthDayVariantsLimited(dates, pattern);
}

private static IEnumerable<CalDateTime> GetMonthDayVariantsLimited(IEnumerable<CalDateTime> dates, RecurrencePattern pattern, int anchorMonth)
private static IEnumerable<CalDateTime> GetMonthDayVariantsLimited(IEnumerable<CalDateTime> dates, RecurrencePattern pattern)
{
// Helper that checks whether the given candidate matches any BYMONTHDAY entry
// taking negative values into account (relative to the month's length).
Expand All @@ -575,25 +587,12 @@ static bool MatchesAnyMonthDay(CalDateTime candidate, IEnumerable<int> monthDays

foreach (var date in dates)
{
if (pattern.ByMonth.Count > 0)
{
// If BYMONTH is specified, the date must be in one of those months
// and match a BYMONTHDAY value.
if (!pattern.ByMonth.Contains(date.Month))
continue;

if (MatchesAnyMonthDay(date, pattern.ByMonthDay))
yield return date;
}
else
{
// When BYMONTH is not specified, only consider dates in the anchor month.
if (date.Month != anchorMonth)
continue;
// If BYMONTH is specified and this date's month is not included, skip it.
if (pattern.ByMonth.Count > 0 && !pattern.ByMonth.Contains(date.Month))
continue;

if (MatchesAnyMonthDay(date, pattern.ByMonthDay))
yield return date;
}
if (MatchesAnyMonthDay(date, pattern.ByMonthDay))
yield return date;
}
}

Expand Down
Loading