FIx non-ISO months arithmetic

Port of tc39/proposal-temporal#1761

lib/calendar.ts

@@ -829,10 +829,29 @@ const nonIsoHelperBase: NonIsoHelperBase = {
   addMonthsCalendar(calendarDate, months, overflow, cache) {
     const { day } = calendarDate;
     for (let i = 0, absMonths = MathAbs(months); i < absMonths; i++) {
-      const days = months < 0 ? -this.daysInPreviousMonth(calendarDate, cache) : this.daysInMonth(calendarDate, cache);
+      const { month } = calendarDate;
+      const oldCalendarDate = calendarDate;
+      const days =
+        months < 0
+          ? -Math.max(day, this.daysInPreviousMonth(calendarDate, cache))
+          : this.daysInMonth(calendarDate, cache);
       const isoDate = this.calendarToIsoDate(calendarDate, 'constrain', cache);
-      const addedIso = this.addDaysIso(isoDate, days, cache);
+      let addedIso = this.addDaysIso(isoDate, days, cache);
       calendarDate = this.isoToCalendarDate(addedIso, cache);
+
+      // Normally, we can advance one month by adding the number of days in the
+      // current month. However, if we're at the end of the current month and
+      // the next month has fewer days, then we rolled over to the after-next
+      // month. Below we detect this condition and back up until we're back in
+      // the desired month.
+      if (months > 0) {
+        const monthsInOldYear = this.monthsInYear(oldCalendarDate, cache);
+        while (calendarDate.month - 1 !== month % monthsInOldYear) {
+          addedIso = this.addDaysIso(addedIso, -1, cache);
+          calendarDate = this.isoToCalendarDate(addedIso, cache);
+        }
+      }
+
       if (calendarDate.day !== day) {
         // try to retain the original day-of-month, if possible
         calendarDate = this.regulateDate({ ...calendarDate, day }, 'constrain', cache);
@@ -882,18 +901,17 @@ const nonIsoHelperBase: NonIsoHelperBase = {
         let current;
         let next = yearsAdded;
         do {
-          months++;
+          months += sign;
           current = next;
           next = this.addMonthsCalendar(current, sign, 'constrain', cache);
           if (next.day !== calendarOne.day) {
             // In case the day was constrained down, try to un-constrain it
             next = this.regulateDate({ ...next, day: calendarOne.day }, 'constrain', cache);
           }
         } while (this.compareCalendarDates(calendarTwo, next) * sign >= 0);
-        months--; // correct for loop above which overshoots by 1
+        months -= sign; // correct for loop above which overshoots by 1
         const remainingDays = this.calendarDaysUntil(current, calendarTwo, cache);
-        days = remainingDays % 7;
-        weeks = (remainingDays - days) / 7;
+        days = remainingDays;
         break;
       }
     }

test/datemath.mjs

@@ -58,13 +58,19 @@ describe('Date.since(normal, leap)', () => {
 });
 
 function build(name, sone, stwo) {
-  const [one, two] = [Temporal.PlainDate.from(sone), Temporal.PlainDate.from(stwo)].sort(Temporal.PlainDate.compare);
+  const calendars = ['iso8601', 'gregory'];
   describe(name, () => {
     const largestUnits = ['years', 'months', 'weeks', 'days'];
-    buildSub(one, two, largestUnits);
-    buildSub(one.with({ day: 25 }), two.with({ day: 5 }), largestUnits);
-    buildSub(one.with({ day: 30 }), two.with({ day: 29 }), largestUnits);
-    buildSub(one.with({ day: 30 }), two.with({ day: 5 }), largestUnits);
+    for (const calendar of calendars) {
+      const [one, two] = [
+        Temporal.PlainDate.from(sone).withCalendar(calendar),
+        Temporal.PlainDate.from(stwo).withCalendar(calendar)
+      ].sort(Temporal.PlainDate.compare);
+      buildSub(one, two, largestUnits);
+      buildSub(one.with({ day: 25 }), two.with({ day: 5 }), largestUnits);
+      buildSub(one.with({ day: 30 }), two.with({ day: 29 }), largestUnits);
+      buildSub(one.with({ day: 30 }), two.with({ day: 5 }), largestUnits);
+    }
   });
 }
 function buildSub(one, two, largestUnits) {

test/intl.mjs

@@ -690,7 +690,7 @@ describe('Intl', () => {
       roc: { year: 2001, month: 6, day: 1, monthCode: 'M06', eraYear: 2001, era: 'minguo' }
     };
     const addYearsMonthsDaysCases = Object.entries(addMonthsCases).reduce((obj, entry) => {
-      obj[entry[0]] = entry[1] === RangeError ? RangeError : { ...entry[1], day: 4 };
+      obj[entry[0]] = entry[1] === RangeError ? RangeError : { ...entry[1], day: 18 };
       return obj;
     }, {});
     const tests = {
@@ -701,7 +701,7 @@ describe('Intl', () => {
       // recognizes the leap month.
       months: { duration: { months: 6 }, results: addMonthsCases, startDate: { year: 2000, month: 12, day: 1 } },
       years: {
-        duration: { years: 3, months: 6, days: 3 },
+        duration: { years: 3, months: 6, days: 17 },
         results: addYearsMonthsDaysCases,
         startDate: { year: 1997, month: 12, day: 1 }
       }
@@ -729,6 +729,99 @@ describe('Intl', () => {
           equal(`start ${calculatedStart.toString()}`, `start ${start.toString()}`);
           const diff = start.until(end, { largestUnit: unit });
           equal(`diff ${unit} ${id}: ${diff}`, `diff ${unit} ${id}: ${duration}`);
+
+          if (unit === 'months') {
+            const startYesterday = start.subtract({ days: 1 });
+            let endYesterday = startYesterday.add(duration);
+            equal(
+              `add from end-of-month ${unit} ${id} day (initial): ${endYesterday.day}`,
+              `add from end-of-month ${unit} ${id} day (initial): ${Math.min(
+                startYesterday.day,
+                endYesterday.daysInMonth
+              )}`
+            );
+            // Now advance to the first day of the next month, which should be
+            // the same as the expected end date above.
+            let endYesterdayNextDay = endYesterday.add({ days: 1 });
+            while (endYesterdayNextDay.day !== 1) {
+              // It's possible that we may be more than one day off in some
+              // calendars, e.g. when original start date was March 1, so day
+              // before was Feb 28, so adding P6M to Feb 28 will be Oct 28, so
+              // need to advance three days.
+              endYesterdayNextDay = endYesterdayNextDay.add({ days: 1 });
+            }
+            equal(
+              `add from end-of-month ${unit} ${id} day: ${endYesterdayNextDay.day}`,
+              `add from end-of-month ${unit} ${id} day: ${values.day}`
+            );
+            equal(
+              `add from end-of-month ${unit} ${id} eraYear: ${endYesterdayNextDay.eraYear}`,
+              `add from end-of-month ${unit} ${id} eraYear: ${values.eraYear}`
+            );
+            equal(
+              `add from end-of-month ${unit} ${id} era: ${endYesterdayNextDay.era}`,
+              `add from end-of-month ${unit} ${id} era: ${values.era}`
+            );
+            equal(
+              `add from end-of-month ${unit} ${id} year: ${endYesterdayNextDay.year}`,
+              `add from end-of-month ${unit} ${id} year: ${values.year}`
+            );
+            equal(
+              `add from end-of-month ${unit} ${id} month: ${endYesterdayNextDay.month}`,
+              `add from end-of-month ${unit} ${id} month: ${values.month}`
+            );
+            equal(
+              `add from end-of-month ${unit} ${id} monthCode: ${endYesterdayNextDay.monthCode}`,
+              `add from end-of-month ${unit} ${id} monthCode: ${values.monthCode}`
+            );
+
+            // Now test the reverse operation: subtracting from the last day of
+            // the previous month.
+            const endReverse = endYesterdayNextDay.subtract({ days: 1 });
+            const startReverse = endReverse.subtract(duration);
+            equal(
+              `subtract from end-of-month ${unit} ${id} day (initial): ${startReverse.day}`,
+              `subtract from end-of-month ${unit} ${id} day (initial): ${Math.min(
+                endReverse.day,
+                startReverse.daysInMonth
+              )}`
+            );
+            // Now advance to the first day of the next month, which should be
+            // the same as the original start date above.
+            let startReverseNextDay = startReverse.add({ days: 1 });
+            while (startReverseNextDay.day !== 1) {
+              // It's possible that we may be more than one day off in some
+              // calendars, e.g. when original start date was March 1, so day
+              // before was Feb 28, so adding P6M to Feb 28 will be Oct 28, so
+              // need to advance three days.
+              startReverseNextDay = startReverseNextDay.add({ days: 1 });
+            }
+            equal(
+              `subtract from end-of-month ${unit} ${id} day: ${startReverseNextDay.day}`,
+              `subtract from end-of-month ${unit} ${id} day: ${start.day}`
+            );
+            equal(
+              `subtract from end-of-month ${unit} ${id} eraYear: ${startReverseNextDay.eraYear}`,
+              `subtract from end-of-month ${unit} ${id} eraYear: ${start.eraYear}`
+            );
+            equal(
+              `subtract from end-of-month ${unit} ${id} era: ${startReverseNextDay.era}`,
+              `subtract from end-of-month ${unit} ${id} era: ${start.era}`
+            );
+            equal(
+              `subtract from end-of-month ${unit} ${id} year: ${startReverseNextDay.year}`,
+              `subtract from end-of-month ${unit} ${id} year: ${start.year}`
+            );
+            equal(
+              `subtract from end-of-month ${unit} ${id} month: ${startReverseNextDay.month}`,
+              `subtract from end-of-month ${unit} ${id} month: ${start.month}`
+            );
+            equal(
+              `subtract from end-of-month ${unit} ${id} monthCode: ${startReverseNextDay.monthCode}`,
+              `subtract from end-of-month ${unit} ${id} monthCode: ${start.monthCode}`
+            );
+          }
+
           const ms = (globalThis.performance ? globalThis.performance.now() : Date.now()) - now;
           totalNow += ms;
           // eslint-disable-next-line no-console