From cf2d1880911460361c2599d525592d1895ad0f82 Mon Sep 17 00:00:00 2001 From: axunonb Date: Sun, 30 Nov 2025 10:22:55 +0100 Subject: [PATCH 1/3] Refactored `RecurrencePatternEvaluator` and `Evaluator` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RecurrencePatternEvaluator`: 1. Extended `GetIntervalLowerLimit` calculation: * YEARLY + BYMONTH: compute a conservative earliest possible candidate inside the interval * YEARLY + BYWEEKNO: shift interval start to the week’s first day * default return intervalRefTime 2. Intoduced an `anchorMonth` as an explicit month anchor used when FREQ=YEARLY and BYMONTH is not specified so that BYMONTHDAY expansion/limiting is performed relative to the original DTSTART month rather than the current intervalRefTime month. `anchorMonth` only affects cases where BYMONTH is absent. `Evaluator`: Added a distinction for case `FrequencyType.Yearly`: Is BYWEEKNO present or not. `RecurrenceTests`: Extended `YearlyInterval1()` to test more occurrences and taking latest code changes into account. Fixes https://github.com/ical-org/ical.net/issues/791#issuecomment-3553325859 --- Ical.Net.Tests/RecurrenceTests.cs | 21 +-- Ical.Net/Evaluation/Evaluator.cs | 13 +- .../Evaluation/RecurrencePatternEvaluator.cs | 151 ++++++++++++++---- 3 files changed, 145 insertions(+), 40 deletions(-) diff --git a/Ical.Net.Tests/RecurrenceTests.cs b/Ical.Net.Tests/RecurrenceTests.cs index ac1e12750..c0eae8d59 100644 --- a/Ical.Net.Tests/RecurrenceTests.cs +++ b/Ical.Net.Tests/RecurrenceTests.cs @@ -407,7 +407,7 @@ public void ByMonth2() .TakeWhileBefore(new CalDateTime(2000, 12, 31)).ToList(); var evt2Occurrences = evt2.GetOccurrences(new CalDateTime(1997, 9, 1)) .TakeWhileBefore(new CalDateTime(2000, 12, 31)).ToList(); - Assert.That(evt1Occurrences.Count == evt2Occurrences.Count, Is.True, + Assert.That(evt1Occurrences.Count, Is.EqualTo(evt2Occurrences.Count), "ByMonth1 does not match ByMonth2 as it should"); for (var i = 0; i < evt1Occurrences.Count; i++) Assert.That(evt2Occurrences[i].Period, Is.EqualTo(evt1Occurrences[i].Period), @@ -1654,7 +1654,7 @@ public void HourlyUntil1() new Period(new CalDateTime(1997, 9, 2, 9, 0, 0, _tzid), Duration.FromHours(1)), new Period(new CalDateTime(1997, 9, 2, 12, 0, 0, _tzid), Duration.FromHours(1)), new Period(new CalDateTime(1997, 9, 2, 15, 0, 0, _tzid), Duration.FromHours(1)), - new Period(new CalDateTime(1997, 9, 2, 18, 0, 0, _tzid), Duration.FromHours(1)), + new Period(new CalDateTime(1997, 9, 2, 18, 0, 0, _tzid), Duration.FromHours(1)) }, timeZones: null ); @@ -2009,13 +2009,16 @@ public void YearlyInterval1() var iCal = Calendar.Load(IcsFiles.YearlyInterval1)!; EventOccurrenceTest( iCal, - new CalDateTime(2006, 1, 1, 7, 0, 0, _tzid), - new CalDateTime(2007, 1, 31, 7, 0, 0, _tzid), - new[] - { - new Period(new CalDateTime(2007, 1, 8, 7, 0, 0, _tzid), Duration.FromHours(24)), - new Period(new CalDateTime(2007, 1, 9, 7, 0, 0, _tzid), Duration.FromHours(24)) - }, + new CalDateTime(2005, 1, 11, 7, 0, 0, _tzid), + new CalDateTime(2010, 1, 31, 7, 0, 0, _tzid), + [ + new Period(new CalDateTime(2005, 4, 11, 7, 0, 0, _tzid), Duration.FromHours(24)), + new Period(new CalDateTime(2005, 4, 12, 7, 0, 0, _tzid), Duration.FromHours(24)), + new Period(new CalDateTime(2007, 4, 9, 7, 0, 0, _tzid), Duration.FromHours(24)), + new Period(new CalDateTime(2007, 4, 10, 7, 0, 0, _tzid), Duration.FromHours(24)), + new Period(new CalDateTime(2009, 4, 13, 7, 0, 0, _tzid), Duration.FromHours(24)), + new Period(new CalDateTime(2009, 4, 14, 7, 0, 0, _tzid), Duration.FromHours(24)) + ], null ); } diff --git a/Ical.Net/Evaluation/Evaluator.cs b/Ical.Net/Evaluation/Evaluator.cs index 9598b04e8..8f9f428a7 100644 --- a/Ical.Net/Evaluation/Evaluator.cs +++ b/Ical.Net/Evaluation/Evaluator.cs @@ -41,7 +41,18 @@ protected void IncrementDate(ref CalDateTime dt, RecurrencePattern pattern, int dt = old.AddDays(-old.Day + 1).AddMonths(interval); break; case FrequencyType.Yearly: - dt = old.AddDays(-old.DayOfYear + 1).AddYears(interval); + if (pattern.ByWeekNo.Count != 0) + { + // When a rule uses BYWEEKNO, recurrence enumeration + // is based on week numbers relative to the year. + // So we preserve January 1st when adding years + dt = old.AddDays(-old.DayOfYear + 1).AddYears(interval); + } + else + { + // preserve month/day when adding years + dt = old.AddYears(interval); + } break; default: // Frequency should always be valid at this stage. diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index 361893136..72e8179f9 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -162,10 +162,20 @@ private IEnumerable EnumerateDates(CalDateTime originalDate, CalDat var dateCount = 0; while (true) { - if (searchEndDate < GetIntervalLowerLimit(intervalRefTime, pattern)) + var lowerLimit = GetIntervalLowerLimit(intervalRefTime, pattern, originalDate); + + if (searchEndDate < lowerLimit) break; - var candidates = GetCandidates(intervalRefTime, pattern, expandBehavior); + // 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); foreach (var t in candidates.Where(t => t >= originalDate)) { @@ -210,16 +220,77 @@ private IEnumerable EnumerateDates(CalDateTime originalDate, CalDat /// Find the lowest possible date/time for a recurrence in the given interval. /// /// - /// Usually intervalRefTime is either at DTSTART or later at the start of the interval. - /// However, for YEARLY recurrences with BYWEEKNO=1 there could be recurrences before - /// Jan 1st, so we need to adjust the intervalRefTime to the start of the week. + /// For most frequencies the interval's lower limit is simply the provided + /// . YEARLY rules require special handling: + /// - If BYMONTH is present and BYWEEKNO is not, an occurrence for the interval + /// 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 + /// 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. /// - private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, RecurrencePattern pattern) + private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, RecurrencePattern pattern, CalDateTime originalDate) { - var intervalLowerLimit = intervalRefTime; - if ((pattern.Frequency == FrequencyType.Yearly) && (pattern.ByWeekNo.Count != 0)) - intervalLowerLimit = GetFirstDayOfWeekDate(intervalRefTime, pattern.FirstDayOfWeek); - return intervalLowerLimit; + switch (pattern) + { + case { Frequency: FrequencyType.Yearly, ByMonth.Count: > 0, ByWeekNo.Count: 0 }: + { + // When evaluating a YEARLY rule that restricts months (BYMONTH) but not + // week numbers, it's possible that the earliest candidate inside the + // interval is in an earlier month/day than `intervalRefTime` (for example + // BYMONTH=1 while intervalRefTime is anchored on a later month). To avoid + // terminating enumeration prematurely (with a coarse UNTIL cutoff), compute + // the earliest plausible date/time for this interval and use that as the + // lower limit. + // + // We pick: + // - year = intervalRefTime.Year + // - month = smallest BYMONTH or the original DTSTART month if BYMONTH absent + // - day = smallest BYMONTHDAY (clamped to daysInMonth) or original DTSTART day + // - time components = smallest BYHOUR/BYMINUTE/BYSECOND or original DTSTART time + // + // This is a conservative earliest-possible candidate; it must not exclude + // any valid occurrence for the interval. + var year = intervalRefTime.Year; + + // Determine the earliest month we could possibly generate for this interval. + var month = pattern.ByMonth.Count > 0 ? pattern.ByMonth.Min() : originalDate.Month; + + // Determine an appropriate day in the month. + var daysInMonth = Calendar.GetDaysInMonth(year, month); + 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); + } + else + { + // default to original date's day, constrained to the target month length + day = Math.Min(originalDate.Day, daysInMonth); + } + + // Determine earliest time components + var hour = pattern.ByHour.Count > 0 ? pattern.ByHour.Min() : originalDate.Hour; + var minute = pattern.ByMinute.Count > 0 ? pattern.ByMinute.Min() : originalDate.Minute; + var second = pattern.BySecond.Count > 0 ? pattern.BySecond.Min() : originalDate.Second; + + 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; + } + } } private struct ExpandContext @@ -241,15 +312,17 @@ private struct ExpandContext /// /// /// A list of possible dates. - private IEnumerable GetCandidates(CalDateTime date, RecurrencePattern pattern, bool?[] expandBehaviors) + private IEnumerable GetCandidates(CalDateTime date, RecurrencePattern pattern, bool?[] expandBehaviors, int anchorMonth) { var expandContext = new ExpandContext() { DatesFullyExpanded = false }; - IEnumerable dates = [ date ]; + IEnumerable dates = [date]; dates = GetMonthVariants(dates, pattern, expandBehaviors[0]); dates = GetWeekNoVariants(dates, pattern, expandBehaviors[1], ref expandContext); dates = GetYearDayVariants(dates, pattern, expandBehaviors[2], ref expandContext); - dates = GetMonthDayVariants(dates, pattern, expandBehaviors[3], 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 = GetDayVariants(dates, pattern, expandBehaviors[4], ref expandContext); dates = GetHourVariants(dates, pattern, expandBehaviors[5]); dates = GetMinuteVariants(dates, pattern, expandBehaviors[6]); @@ -316,10 +389,8 @@ private static IEnumerable GetMonthVariants(IEnumerable pattern.ByMonth.Contains(date.Month) || pattern.ByMonth.Contains(date.AddDays(6).Month)); } - else - { - return dates.Where(date => pattern.ByMonth.Contains(date.Month)); - } + + return dates.Where(date => pattern.ByMonth.Contains(date.Month)); } /// @@ -348,12 +419,12 @@ private static IEnumerable GetWeekNoVariants(IEnumerable GetWeekNoVariantsExpanded(IEnumerable dates, RecurrencePattern pattern) { - foreach ((var t, var weekNo) in dates.SelectMany(t => GetByWeekNoForYearNormalized(pattern, t.Year), (t, weekNo) => (t, weekNo))) + foreach (var (t, weekNo) in dates.SelectMany(t => GetByWeekNoForYearNormalized(pattern, t.Year), (t, weekNo) => (t, weekNo))) { var date = t; // Make sure we start from a reference date that is in a week that belongs to the current year. - // Its not important that the date lies in a certain week, but that the week belongs to the + // It's not important that the date lies in a certain week, but that the week belongs to the // current year and that the week day is preserved. if (date.Month == 1) date = date.AddDays(7); @@ -448,12 +519,11 @@ from yearDay in pattern.ByYearDay } /// - /// Applies BYMONTHDAY rules specified in this Recur instance to the specified date list. + /// Applies BYMONTHDAY rules specified in this Recurrence instance to the specified date list. /// If no BYMONTHDAY rules are specified, the date list is returned unmodified. /// - /// The list of dates to which the BYMONTHDAY rules will be applied. /// The modified list of dates after applying the BYMONTHDAY rules. - private static IEnumerable GetMonthDayVariants(IEnumerable dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext) + private static IEnumerable GetMonthDayVariants(IEnumerable dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext, int anchorMonth) { if (expand == null || pattern.ByMonthDay.Count == 0) return dates; @@ -465,21 +535,42 @@ private static IEnumerable GetMonthDayVariants(IEnumerable GetMonthDayVariantsLimited(IEnumerable dates, RecurrencePattern pattern) + private static IEnumerable GetMonthDayVariantsLimited(IEnumerable dates, RecurrencePattern pattern, int anchorMonth) { foreach (var date in dates) { - var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); - foreach (var monthDay in pattern.ByMonthDay) + // If BYMONTH is specified, limit according to it; otherwise limit to the anchor month. + if (pattern.ByMonth.Count > 0) + { + var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); + foreach (var monthDay in pattern.ByMonthDay) + { + var byMonthDay = (monthDay > 0) ? monthDay : (daysInMonth + monthDay + 1); + if (date.Day == byMonthDay && pattern.ByMonth.Contains(date.Month)) + { + yield return date; + break; + } + } + } + else { - var byMonthDay = (monthDay > 0) ? monthDay : (daysInMonth + monthDay + 1); - if (date.Day == byMonthDay) + // Only consider dates that are in the anchor month and match the BYMONTHDAY. + if (date.Month != anchorMonth) + continue; + + var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); + foreach (var monthDay in pattern.ByMonthDay) { - yield return date; - break; + var byMonthDay = (monthDay > 0) ? monthDay : (daysInMonth + monthDay + 1); + if (date.Day == byMonthDay) + { + yield return date; + break; + } } } } From 7062dcc2d9539ed81a03f49d2c225362a880dbe0 Mon Sep 17 00:00:00 2001 From: axunonb Date: Sun, 30 Nov 2025 10:49:27 +0100 Subject: [PATCH 2/3] chore: Remove code duplications in `RecurrencePatternEvaluator.GetMonthDayVariantsLimited` --- .../Evaluation/RecurrencePatternEvaluator.cs | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index 72e8179f9..737d19bd2 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -540,38 +540,40 @@ private static IEnumerable GetMonthDayVariants(IEnumerable GetMonthDayVariantsLimited(IEnumerable dates, RecurrencePattern pattern, int anchorMonth) { + // Helper that checks whether the given date matches any BYMONTHDAY entry + // taking negative values into account (relative to the month's length). + static bool MatchesAnyMonthDay(CalDateTime date, IEnumerable monthDays) + { + var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); + foreach (var monthDay in monthDays) + { + var byMonthDay = monthDay > 0 ? monthDay : (daysInMonth + monthDay + 1); + if (date.Day == byMonthDay) + return true; + } + return false; + } + foreach (var date in dates) { - // If BYMONTH is specified, limit according to it; otherwise limit to the anchor month. if (pattern.ByMonth.Count > 0) { - var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); - foreach (var monthDay in pattern.ByMonthDay) - { - var byMonthDay = (monthDay > 0) ? monthDay : (daysInMonth + monthDay + 1); - if (date.Day == byMonthDay && pattern.ByMonth.Contains(date.Month)) - { - yield return date; - break; - } - } + // 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 { - // Only consider dates that are in the anchor month and match the BYMONTHDAY. + // When BYMONTH is not specified, only consider dates in the anchor month. if (date.Month != anchorMonth) continue; - var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); - foreach (var monthDay in pattern.ByMonthDay) - { - var byMonthDay = (monthDay > 0) ? monthDay : (daysInMonth + monthDay + 1); - if (date.Day == byMonthDay) - { - yield return date; - break; - } - } + if (MatchesAnyMonthDay(date, pattern.ByMonthDay)) + yield return date; } } } From 84eb777d104f900184e58cb7cbc11c6126e17816 Mon Sep 17 00:00:00 2001 From: axunonb Date: Sun, 30 Nov 2025 13:36:39 +0100 Subject: [PATCH 3/3] Apply some GitHub Pilot review suggestions --- Ical.Net/Evaluation/Evaluator.cs | 18 +++------ .../Evaluation/RecurrencePatternEvaluator.cs | 39 ++++++++++++++----- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/Ical.Net/Evaluation/Evaluator.cs b/Ical.Net/Evaluation/Evaluator.cs index 8f9f428a7..8b35bb063 100644 --- a/Ical.Net/Evaluation/Evaluator.cs +++ b/Ical.Net/Evaluation/Evaluator.cs @@ -41,18 +41,12 @@ protected void IncrementDate(ref CalDateTime dt, RecurrencePattern pattern, int dt = old.AddDays(-old.Day + 1).AddMonths(interval); break; case FrequencyType.Yearly: - if (pattern.ByWeekNo.Count != 0) - { - // When a rule uses BYWEEKNO, recurrence enumeration - // is based on week numbers relative to the year. - // So we preserve January 1st when adding years - dt = old.AddDays(-old.DayOfYear + 1).AddYears(interval); - } - else - { - // preserve month/day when adding years - dt = old.AddYears(interval); - } + // 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); break; default: // Frequency should always be valid at this stage. diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index 737d19bd2..a7ca398c8 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -256,7 +256,7 @@ private static CalDateTime GetIntervalLowerLimit(CalDateTime intervalRefTime, Re var year = intervalRefTime.Year; // Determine the earliest month we could possibly generate for this interval. - var month = pattern.ByMonth.Count > 0 ? pattern.ByMonth.Min() : originalDate.Month; + var month = pattern.ByMonth.Min(); // Determine an appropriate day in the month. var daysInMonth = Calendar.GetDaysInMonth(year, month); @@ -465,11 +465,30 @@ private static List GetByWeekNoForYearNormalized(RecurrencePattern pattern, } /// - /// Applies BYYEARDAY rules specified in this Recur instance to the specified date list. - /// If no BYYEARDAY rules are specified, the date list is returned unmodified. + /// Applies BYYEARDAY rules from to a sequence of candidate dates. /// - /// The list of dates to which the BYYEARDAY rules will be applied. - /// The modified list of dates after applying the BYYEARDAY rules. + /// Sequence of candidate dates (typically interval reference dates or previously-expanded dates). + /// Recurrence pattern containing the BYYEARDAY values. + /// + /// Controls the operation mode: + /// - true : perform an expand operation — each input date is expanded into the concrete dates + /// represented by the BYYEARDAY values for that date's year. + /// - false : perform a limit/filter operation — only return input dates that match any of the + /// BYYEARDAY values for the input date's year. + /// - null : no operation, return unchanged. + /// + /// + /// Context that indicates whether earlier parts have already fully expanded the candidate set. + /// If is true then expansion must not be + /// performed again and the method should behave in limit mode. + /// When this method performs an expansion it will set + /// to true to prevent later parts from expanding again. + /// + /// + /// A sequence of dates after applying the BYYEARDAY rules. + /// Expanded dates produced for a given input are constrained to the same calendar year as the input date; + /// out-of-range BYYEARDAY values (e.g. +-366 in non-leap years) are ignored. + /// private static IEnumerable GetYearDayVariants(IEnumerable dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext) { if (expand is null || pattern.ByYearDay.Count == 0) @@ -519,7 +538,7 @@ from yearDay in pattern.ByYearDay } /// - /// Applies BYMONTHDAY rules specified in this Recurrence instance to the specified date list. + /// Applies BYMONTHDAY rules specified in this RecurrencePattern instance to the specified date list. /// If no BYMONTHDAY rules are specified, the date list is returned unmodified. /// /// The modified list of dates after applying the BYMONTHDAY rules. @@ -540,15 +559,15 @@ private static IEnumerable GetMonthDayVariants(IEnumerable GetMonthDayVariantsLimited(IEnumerable dates, RecurrencePattern pattern, int anchorMonth) { - // Helper that checks whether the given date matches any BYMONTHDAY entry + // Helper that checks whether the given candidate matches any BYMONTHDAY entry // taking negative values into account (relative to the month's length). - static bool MatchesAnyMonthDay(CalDateTime date, IEnumerable monthDays) + static bool MatchesAnyMonthDay(CalDateTime candidate, IEnumerable monthDays) { - var daysInMonth = Calendar.GetDaysInMonth(date.Year, date.Month); + var daysInMonth = Calendar.GetDaysInMonth(candidate.Year, candidate.Month); foreach (var monthDay in monthDays) { var byMonthDay = monthDay > 0 ? monthDay : (daysInMonth + monthDay + 1); - if (date.Day == byMonthDay) + if (candidate.Day == byMonthDay) return true; } return false;