Fixes in ParseTemporalDurationString

There were two problems in ParseTemporalDurationString: 1) The previous code didn't make any difference between absent duration string components and components that were present but 0. This led to strings like 'PT1.5H0M' being accepted, which should have been rejected according to the spec. 2) It was possible to incur rounding error in the nanoseconds when parsing fractional components. Instead, express all fractional components as a whole number of nanoseconds, which is possible to do without rounding error, and calculate nanoseconds, microseconds, milliseconds, excess seconds, and excess minutes resulting from fractional components at the end. Inlines DurationHandleFractions into ParseTemporalDurationString; this operation was already inlined in the spec text anyway. UPSTREAM_COMMIT=0bf0b5b4b1b50a1cf2d8e2447f88f2cb6c360b36
js-temporal · Feb 22, 2023 · 58b5601 · 58b5601
1 parent a907acf
commit 58b5601
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 90 deletions.
diff --git a/lib/ecmascript.ts b/lib/ecmascript.ts
@@ -590,27 +590,42 @@ export function ParseTemporalDurationString(isoString: string) {
   const weeks = ToInteger(match[4]) * sign;
   const days = ToInteger(match[5]) * sign;
   const hours = ToInteger(match[6]) * sign;
-  let fHours: number | string = match[7];
-  let minutes = ToInteger(match[8]) * sign;
-  let fMinutes: number | string = match[9];
-  let seconds = ToInteger(match[10]) * sign;
-  const fSeconds = match[11] + '000000000';
-  let milliseconds = ToInteger(fSeconds.slice(0, 3)) * sign;
-  let microseconds = ToInteger(fSeconds.slice(3, 6)) * sign;
-  let nanoseconds = ToInteger(fSeconds.slice(6, 9)) * sign;
-
-  fHours = fHours ? (sign * ToInteger(fHours)) / 10 ** fHours.length : 0;
-  fMinutes = fMinutes ? (sign * ToInteger(fMinutes)) / 10 ** fMinutes.length : 0;
-
-  ({ minutes, seconds, milliseconds, microseconds, nanoseconds } = DurationHandleFractions(
-    fHours,
-    minutes,
-    fMinutes,
-    seconds,
-    milliseconds,
-    microseconds,
-    nanoseconds
-  ));
+  const fHours = match[7];
+  const minutesStr = match[8];
+  const fMinutes = match[9];
+  const secondsStr = match[10];
+  const fSeconds = match[11];
+  let minutes = 0;
+  let seconds = 0;
+  // fractional hours, minutes, or seconds, expressed in whole nanoseconds:
+  let excessNanoseconds = 0;
+
+  if (fHours !== undefined) {
+    if (minutesStr ?? fMinutes ?? secondsStr ?? fSeconds ?? false) {
+      throw new RangeError('only the smallest unit can be fractional');
+    }
+    excessNanoseconds = ToInteger((fHours + '000000000').slice(0, 9)) * 3600 * sign;
+  } else {
+    minutes = ToInteger(minutesStr) * sign;
+    if (fMinutes !== undefined) {
+      if (secondsStr ?? fSeconds ?? false) {
+        throw new RangeError('only the smallest unit can be fractional');
+      }
+      excessNanoseconds = ToInteger((fMinutes + '000000000').slice(0, 9)) * 60 * sign;
+    } else {
+      seconds = ToInteger(secondsStr) * sign;
+      if (fSeconds !== undefined) {
+        excessNanoseconds = ToInteger((fSeconds + '000000000').slice(0, 9)) * sign;
+      }
+    }
+  }
+
+  const nanoseconds = excessNanoseconds % 1000;
+  const microseconds = MathTrunc(excessNanoseconds / 1000) % 1000;
+  const milliseconds = MathTrunc(excessNanoseconds / 1e6) % 1000;
+  seconds += MathTrunc(excessNanoseconds / 1e9) % 60;
+  minutes += MathTrunc(excessNanoseconds / 6e10);
+
   RejectDuration(years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
   return { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds };
 }
@@ -715,61 +730,6 @@ export function RegulateISOYearMonth(
   return { year, month };
 }
 
-function DurationHandleFractions(
-  fHoursParam: number,
-  minutesParam: number,
-  fMinutesParam: number,
-  secondsParam: number,
-  millisecondsParam: number,
-  microsecondsParam: number,
-  nanosecondsParam: number
-) {
-  let fHours = fHoursParam;
-  let minutes = minutesParam;
-  let fMinutes = fMinutesParam;
-  let seconds = secondsParam;
-  let milliseconds = millisecondsParam;
-  let microseconds = microsecondsParam;
-  let nanoseconds = nanosecondsParam;
-
-  if (fHours !== 0) {
-    [minutes, fMinutes, seconds, milliseconds, microseconds, nanoseconds].forEach((val) => {
-      if (val !== 0) throw new RangeError('only the smallest unit can be fractional');
-    });
-    const mins = fHours * 60;
-    minutes = MathTrunc(mins);
-    fMinutes = mins % 1;
-  }
-
-  if (fMinutes !== 0) {
-    [seconds, milliseconds, microseconds, nanoseconds].forEach((val) => {
-      if (val !== 0) throw new RangeError('only the smallest unit can be fractional');
-    });
-    const secs = fMinutes * 60;
-    seconds = MathTrunc(secs);
-    const fSeconds = secs % 1;
-
-    if (fSeconds !== 0) {
-      const mils = fSeconds * 1000;
-      milliseconds = MathTrunc(mils);
-      const fMilliseconds = mils % 1;
-
-      if (fMilliseconds !== 0) {
-        const mics = fMilliseconds * 1000;
-        microseconds = MathTrunc(mics);
-        const fMicroseconds = mics % 1;
-
-        if (fMicroseconds !== 0) {
-          const nans = fMicroseconds * 1000;
-          nanoseconds = MathTrunc(nans);
-        }
-      }
-    }
-  }
-
-  return { minutes, seconds, milliseconds, microseconds, nanoseconds };
-}
-
 function ToTemporalDurationRecord(item: Temporal.DurationLike | string) {
   if (!IsObject(item)) {
     return ParseTemporalDurationString(ToString(item));

diff --git a/test/expected-failures-todo-migrated-code.txt b/test/expected-failures-todo-migrated-code.txt
@@ -148,8 +148,6 @@ built-ins/Temporal/Calendar/prototype/yearOfWeek/year-zero.js
 built-ins/Temporal/Duration/compare/order-of-operations.js
 built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
 built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-wrong-type.js
-built-ins/Temporal/Duration/from/argument-string-fractional-precision.js
-built-ins/Temporal/Duration/from/argument-string-fractional-with-zero-subparts.js
 built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-safe-integer.js
 built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-value-2.js
 built-ins/Temporal/Duration/prototype/add/order-of-operations.js

diff --git a/test/expected-failures.txt b/test/expected-failures.txt
@@ -3,15 +3,3 @@
 # see expected-failures-es5.txt and expected-failures-opt.txt respectively.
 
 # https://github.com/tc39/test262/pull/3548
-built-ins/Temporal/Duration/compare/argument-string-negative-fractional-units.js
-built-ins/Temporal/Duration/from/argument-string-negative-fractional-units.js
-built-ins/Temporal/Duration/prototype/add/argument-string-negative-fractional-units.js
-built-ins/Temporal/Duration/prototype/subtract/argument-string-negative-fractional-units.js
-built-ins/Temporal/Instant/prototype/add/argument-string-negative-fractional-units.js
-built-ins/Temporal/Instant/prototype/subtract/argument-string-negative-fractional-units.js
-built-ins/Temporal/PlainDateTime/prototype/add/argument-string-negative-fractional-units.js
-built-ins/Temporal/PlainDateTime/prototype/subtract/argument-string-negative-fractional-units.js
-built-ins/Temporal/PlainTime/prototype/add/argument-string-negative-fractional-units.js
-built-ins/Temporal/PlainTime/prototype/subtract/argument-string-negative-fractional-units.js
-built-ins/Temporal/ZonedDateTime/prototype/add/argument-string-negative-fractional-units.js
-built-ins/Temporal/ZonedDateTime/prototype/subtract/argument-string-negative-fractional-units.js