Skip to content

Commit 58b5601

Browse files
ptomato12wrigja
authored andcommitted
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
1 parent a907acf commit 58b5601

File tree

3 files changed

+36
-90
lines changed

3 files changed

+36
-90
lines changed

lib/ecmascript.ts

+36-76
Original file line numberDiff line numberDiff line change
@@ -590,27 +590,42 @@ export function ParseTemporalDurationString(isoString: string) {
590590
const weeks = ToInteger(match[4]) * sign;
591591
const days = ToInteger(match[5]) * sign;
592592
const hours = ToInteger(match[6]) * sign;
593-
let fHours: number | string = match[7];
594-
let minutes = ToInteger(match[8]) * sign;
595-
let fMinutes: number | string = match[9];
596-
let seconds = ToInteger(match[10]) * sign;
597-
const fSeconds = match[11] + '000000000';
598-
let milliseconds = ToInteger(fSeconds.slice(0, 3)) * sign;
599-
let microseconds = ToInteger(fSeconds.slice(3, 6)) * sign;
600-
let nanoseconds = ToInteger(fSeconds.slice(6, 9)) * sign;
601-
602-
fHours = fHours ? (sign * ToInteger(fHours)) / 10 ** fHours.length : 0;
603-
fMinutes = fMinutes ? (sign * ToInteger(fMinutes)) / 10 ** fMinutes.length : 0;
604-
605-
({ minutes, seconds, milliseconds, microseconds, nanoseconds } = DurationHandleFractions(
606-
fHours,
607-
minutes,
608-
fMinutes,
609-
seconds,
610-
milliseconds,
611-
microseconds,
612-
nanoseconds
613-
));
593+
const fHours = match[7];
594+
const minutesStr = match[8];
595+
const fMinutes = match[9];
596+
const secondsStr = match[10];
597+
const fSeconds = match[11];
598+
let minutes = 0;
599+
let seconds = 0;
600+
// fractional hours, minutes, or seconds, expressed in whole nanoseconds:
601+
let excessNanoseconds = 0;
602+
603+
if (fHours !== undefined) {
604+
if (minutesStr ?? fMinutes ?? secondsStr ?? fSeconds ?? false) {
605+
throw new RangeError('only the smallest unit can be fractional');
606+
}
607+
excessNanoseconds = ToInteger((fHours + '000000000').slice(0, 9)) * 3600 * sign;
608+
} else {
609+
minutes = ToInteger(minutesStr) * sign;
610+
if (fMinutes !== undefined) {
611+
if (secondsStr ?? fSeconds ?? false) {
612+
throw new RangeError('only the smallest unit can be fractional');
613+
}
614+
excessNanoseconds = ToInteger((fMinutes + '000000000').slice(0, 9)) * 60 * sign;
615+
} else {
616+
seconds = ToInteger(secondsStr) * sign;
617+
if (fSeconds !== undefined) {
618+
excessNanoseconds = ToInteger((fSeconds + '000000000').slice(0, 9)) * sign;
619+
}
620+
}
621+
}
622+
623+
const nanoseconds = excessNanoseconds % 1000;
624+
const microseconds = MathTrunc(excessNanoseconds / 1000) % 1000;
625+
const milliseconds = MathTrunc(excessNanoseconds / 1e6) % 1000;
626+
seconds += MathTrunc(excessNanoseconds / 1e9) % 60;
627+
minutes += MathTrunc(excessNanoseconds / 6e10);
628+
614629
RejectDuration(years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
615630
return { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds };
616631
}
@@ -715,61 +730,6 @@ export function RegulateISOYearMonth(
715730
return { year, month };
716731
}
717732

718-
function DurationHandleFractions(
719-
fHoursParam: number,
720-
minutesParam: number,
721-
fMinutesParam: number,
722-
secondsParam: number,
723-
millisecondsParam: number,
724-
microsecondsParam: number,
725-
nanosecondsParam: number
726-
) {
727-
let fHours = fHoursParam;
728-
let minutes = minutesParam;
729-
let fMinutes = fMinutesParam;
730-
let seconds = secondsParam;
731-
let milliseconds = millisecondsParam;
732-
let microseconds = microsecondsParam;
733-
let nanoseconds = nanosecondsParam;
734-
735-
if (fHours !== 0) {
736-
[minutes, fMinutes, seconds, milliseconds, microseconds, nanoseconds].forEach((val) => {
737-
if (val !== 0) throw new RangeError('only the smallest unit can be fractional');
738-
});
739-
const mins = fHours * 60;
740-
minutes = MathTrunc(mins);
741-
fMinutes = mins % 1;
742-
}
743-
744-
if (fMinutes !== 0) {
745-
[seconds, milliseconds, microseconds, nanoseconds].forEach((val) => {
746-
if (val !== 0) throw new RangeError('only the smallest unit can be fractional');
747-
});
748-
const secs = fMinutes * 60;
749-
seconds = MathTrunc(secs);
750-
const fSeconds = secs % 1;
751-
752-
if (fSeconds !== 0) {
753-
const mils = fSeconds * 1000;
754-
milliseconds = MathTrunc(mils);
755-
const fMilliseconds = mils % 1;
756-
757-
if (fMilliseconds !== 0) {
758-
const mics = fMilliseconds * 1000;
759-
microseconds = MathTrunc(mics);
760-
const fMicroseconds = mics % 1;
761-
762-
if (fMicroseconds !== 0) {
763-
const nans = fMicroseconds * 1000;
764-
nanoseconds = MathTrunc(nans);
765-
}
766-
}
767-
}
768-
}
769-
770-
return { minutes, seconds, milliseconds, microseconds, nanoseconds };
771-
}
772-
773733
function ToTemporalDurationRecord(item: Temporal.DurationLike | string) {
774734
if (!IsObject(item)) {
775735
return ParseTemporalDurationString(ToString(item));

test/expected-failures-todo-migrated-code.txt

-2
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,6 @@ built-ins/Temporal/Calendar/prototype/yearOfWeek/year-zero.js
148148
built-ins/Temporal/Duration/compare/order-of-operations.js
149149
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
150150
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-wrong-type.js
151-
built-ins/Temporal/Duration/from/argument-string-fractional-precision.js
152-
built-ins/Temporal/Duration/from/argument-string-fractional-with-zero-subparts.js
153151
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-safe-integer.js
154152
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-value-2.js
155153
built-ins/Temporal/Duration/prototype/add/order-of-operations.js

test/expected-failures.txt

-12
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,3 @@
33
# see expected-failures-es5.txt and expected-failures-opt.txt respectively.
44

55
# https://github.com/tc39/test262/pull/3548
6-
built-ins/Temporal/Duration/compare/argument-string-negative-fractional-units.js
7-
built-ins/Temporal/Duration/from/argument-string-negative-fractional-units.js
8-
built-ins/Temporal/Duration/prototype/add/argument-string-negative-fractional-units.js
9-
built-ins/Temporal/Duration/prototype/subtract/argument-string-negative-fractional-units.js
10-
built-ins/Temporal/Instant/prototype/add/argument-string-negative-fractional-units.js
11-
built-ins/Temporal/Instant/prototype/subtract/argument-string-negative-fractional-units.js
12-
built-ins/Temporal/PlainDateTime/prototype/add/argument-string-negative-fractional-units.js
13-
built-ins/Temporal/PlainDateTime/prototype/subtract/argument-string-negative-fractional-units.js
14-
built-ins/Temporal/PlainTime/prototype/add/argument-string-negative-fractional-units.js
15-
built-ins/Temporal/PlainTime/prototype/subtract/argument-string-negative-fractional-units.js
16-
built-ins/Temporal/ZonedDateTime/prototype/add/argument-string-negative-fractional-units.js
17-
built-ins/Temporal/ZonedDateTime/prototype/subtract/argument-string-negative-fractional-units.js

0 commit comments

Comments
 (0)