LibJS: Disallow Temporal.Duration input values to be non-integers

This is a normative change in the Temporal spec.

See: https://github.com/tc39/proposal-temporal/commit/8c85450

6 files changed, 60 insertions, 48 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
index bc01bb89b0..14df4ebd6d 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
@@ -1477,7 +1477,7 @@ ThrowCompletionOr<double> to_positive_integer(GlobalObject& global_object, Value
     return integer;
 }
 
-// 13.48 PrepareTemporalFields ( fields, fieldNames, requiredFields ), https://tc39.es/proposal-temporal/#sec-temporal-preparetemporalfields
+// 13.49 PrepareTemporalFields ( fields, fieldNames, requiredFields ), https://tc39.es/proposal-temporal/#sec-temporal-preparetemporalfields
 ThrowCompletionOr<Object*> prepare_temporal_fields(GlobalObject& global_object, Object const& fields, Vector<String> const& field_names, Vector<StringView> const& required_fields)
 {
     auto& vm = global_object.vm();
@@ -1528,7 +1528,7 @@ ThrowCompletionOr<Object*> prepare_temporal_fields(GlobalObject& global_object,
     return result;
 }
 
-// 13.49 PreparePartialTemporalFields ( fields, fieldNames ), https://tc39.es/proposal-temporal/#sec-temporal-preparepartialtemporalfields
+// 13.50 PreparePartialTemporalFields ( fields, fieldNames ), https://tc39.es/proposal-temporal/#sec-temporal-preparepartialtemporalfields
 ThrowCompletionOr<Object*> prepare_partial_temporal_fields(GlobalObject& global_object, Object const& fields, Vector<String> const& field_names)
 {
     auto& vm = global_object.vm();
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.h b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.h
index ed909cb8ca..e553e4f3d8 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.h
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.h
@@ -142,7 +142,7 @@ ThrowCompletionOr<double> to_positive_integer(GlobalObject&, Value argument);
 ThrowCompletionOr<Object*> prepare_temporal_fields(GlobalObject&, Object const& fields, Vector<String> const& field_names, Vector<StringView> const& required_fields);
 ThrowCompletionOr<Object*> prepare_partial_temporal_fields(GlobalObject&, Object const& fields, Vector<String> const& field_names);
 
-// 13.46 ToIntegerThrowOnInfinity ( argument ), https://tc39.es/proposal-temporal/#sec-temporal-tointegerthrowoninfinity
+// 13.47 ToIntegerThrowOnInfinity ( argument ), https://tc39.es/proposal-temporal/#sec-temporal-tointegerthrowoninfinity
 template<typename... Args>
 ThrowCompletionOr<double> to_integer_throw_on_infinity(GlobalObject& global_object, Value argument, ErrorType error_type, Args... args)
 {
@@ -161,4 +161,25 @@ ThrowCompletionOr<double> to_integer_throw_on_infinity(GlobalObject& global_obje
     return integer;
 }
 
+// 13.48 ToIntegerWithoutRounding ( argument ), https://tc39.es/proposal-temporal/#sec-temporal-tointegerwithoutrounding
+template<typename... Args>
+ThrowCompletionOr<double> to_integer_without_rounding(GlobalObject& global_object, Value argument, ErrorType error_type, Args... args)
+{
+    auto& vm = global_object.vm();
+
+    // 1. Let number be ? ToNumber(argument).
+    auto number = TRY(argument.to_number(global_object));
+
+    // 2. If number is NaN, +0𝔽, or −0𝔽 return 0.
+    if (number.is_nan() || number.is_positive_zero() || number.is_negative_zero())
+        return 0;
+
+    // 3. If ! IsIntegralNumber(number) is false, throw a RangeError exception.
+    if (!number.is_integral_number())
+        return vm.template throw_completion<RangeError>(global_object, error_type, args...);
+
+    // 4. Return ℝ(number).
+    return number.as_double();
+}
+
 }
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/Duration.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/Duration.cpp
index 25d0ef2748..55e542d86c 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/Duration.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/Duration.cpp
@@ -102,16 +102,10 @@ ThrowCompletionOr<TemporalDuration> to_temporal_duration_record(GlobalObject& gl
             // i. Set any to true.
             any = true;
 
-            // ii. Let val be ? ToNumber(val).
-            value = TRY(value.to_number(global_object));
+            // ii. Let val be 𝔽(? ToIntegerWithoutRounding(val)).
+            value = Value(TRY(to_integer_without_rounding(global_object, value, ErrorType::TemporalInvalidDurationPropertyValueNonIntegral, property.as_string(), value.to_string_without_side_effects())));
 
-            // iii. If ! IsIntegralNumber(val) is false, then
-            if (!value.is_integral_number()) {
-                // 1. Throw a RangeError exception.
-                return vm.throw_completion<RangeError>(global_object, ErrorType::TemporalInvalidDurationPropertyValueNonIntegral, property.as_string(), value.to_string_without_side_effects());
-            }
-
-            // iv. Set result's internal slot whose name is the Internal Slot value of the current row to val.
+            // iii. Set result's internal slot whose name is the Internal Slot value of the current row to val.
             result.*internal_slot = value.as_double();
         }
     }
@@ -198,16 +192,10 @@ ThrowCompletionOr<PartialDuration> to_partial_duration(GlobalObject& global_obje
             // i. Set any to true.
             any = true;
 
-            // ii. Set value to ? ToNumber(value).
-            value = TRY(value.to_number(global_object));
-
-            // iii. If ! IsIntegralNumber(value) is false, then
-            if (!value.is_integral_number()) {
-                // 1. Throw a RangeError exception.
-                return vm.throw_completion<RangeError>(global_object, ErrorType::TemporalInvalidDurationPropertyValueNonIntegral, property.as_string(), value.to_string_without_side_effects());
-            }
+            // ii. Set value to 𝔽(? ToIntegerWithoutRounding(value)).
+            value = Value(TRY(to_integer_without_rounding(global_object, value, ErrorType::TemporalInvalidDurationPropertyValueNonIntegral, property.as_string(), value.to_string_without_side_effects())));
 
-            // iv. Set result's internal slot whose name is the Internal Slot value of the current row to value.
+            // iii. Set result's internal slot whose name is the Internal Slot value of the current row to value.
             result.*internal_slot = value.as_double();
         }
     }
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/DurationConstructor.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/DurationConstructor.cpp
index d65e5d630d..42ddc27bcc 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/DurationConstructor.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/DurationConstructor.cpp
@@ -51,35 +51,35 @@ ThrowCompletionOr<Object*> DurationConstructor::construct(FunctionObject& new_ta
     auto& vm = this->vm();
     auto& global_object = this->global_object();
 
-    // 2. Let y be ? ToIntegerThrowOnInfinity(years).
-    auto y = TRY(to_integer_throw_on_infinity(global_object, vm.argument(0), ErrorType::TemporalInvalidDuration));
+    // 2. Let y be ? ToIntegerWithoutRounding(years).
+    auto y = TRY(to_integer_without_rounding(global_object, vm.argument(0), ErrorType::TemporalInvalidDuration));
 
-    // 3. Let mo be ? ToIntegerThrowOnInfinity(months).
-    auto mo = TRY(to_integer_throw_on_infinity(global_object, vm.argument(1), ErrorType::TemporalInvalidDuration));
+    // 3. Let mo be ? ToIntegerWithoutRounding(months).
+    auto mo = TRY(to_integer_without_rounding(global_object, vm.argument(1), ErrorType::TemporalInvalidDuration));
 
-    // 4. Let w be ? ToIntegerThrowOnInfinity(weeks).
-    auto w = TRY(to_integer_throw_on_infinity(global_object, vm.argument(2), ErrorType::TemporalInvalidDuration));
+    // 4. Let w be ? ToIntegerWithoutRounding(weeks).
+    auto w = TRY(to_integer_without_rounding(global_object, vm.argument(2), ErrorType::TemporalInvalidDuration));
 
-    // 5. Let d be ? ToIntegerThrowOnInfinity(days).
-    auto d = TRY(to_integer_throw_on_infinity(global_object, vm.argument(3), ErrorType::TemporalInvalidDuration));
+    // 5. Let d be ? ToIntegerWithoutRounding(days).
+    auto d = TRY(to_integer_without_rounding(global_object, vm.argument(3), ErrorType::TemporalInvalidDuration));
 
-    // 6. Let h be ? ToIntegerThrowOnInfinity(hours).
-    auto h = TRY(to_integer_throw_on_infinity(global_object, vm.argument(4), ErrorType::TemporalInvalidDuration));
+    // 6. Let h be ? ToIntegerWithoutRounding(hours).
+    auto h = TRY(to_integer_without_rounding(global_object, vm.argument(4), ErrorType::TemporalInvalidDuration));
 
-    // 7. Let m be ? ToIntegerThrowOnInfinity(minutes).
-    auto m = TRY(to_integer_throw_on_infinity(global_object, vm.argument(5), ErrorType::TemporalInvalidDuration));
+    // 7. Let m be ? ToIntegerWithoutRounding(minutes).
+    auto m = TRY(to_integer_without_rounding(global_object, vm.argument(5), ErrorType::TemporalInvalidDuration));
 
-    // 8. Let s be ? ToIntegerThrowOnInfinity(seconds).
-    auto s = TRY(to_integer_throw_on_infinity(global_object, vm.argument(6), ErrorType::TemporalInvalidDuration));
+    // 8. Let s be ? ToIntegerWithoutRounding(seconds).
+    auto s = TRY(to_integer_without_rounding(global_object, vm.argument(6), ErrorType::TemporalInvalidDuration));
 
-    // 9. Let ms be ? ToIntegerThrowOnInfinity(milliseconds).
-    auto ms = TRY(to_integer_throw_on_infinity(global_object, vm.argument(7), ErrorType::TemporalInvalidDuration));
+    // 9. Let ms be ? ToIntegerWithoutRounding(milliseconds).
+    auto ms = TRY(to_integer_without_rounding(global_object, vm.argument(7), ErrorType::TemporalInvalidDuration));
 
-    // 10. Let mis be ? ToIntegerThrowOnInfinity(microseconds).
-    auto mis = TRY(to_integer_throw_on_infinity(global_object, vm.argument(8), ErrorType::TemporalInvalidDuration));
+    // 10. Let mis be ? ToIntegerWithoutRounding(microseconds).
+    auto mis = TRY(to_integer_without_rounding(global_object, vm.argument(8), ErrorType::TemporalInvalidDuration));
 
-    // 11. Let ns be ? ToIntegerThrowOnInfinity(nanoseconds).
-    auto ns = TRY(to_integer_throw_on_infinity(global_object, vm.argument(9), ErrorType::TemporalInvalidDuration));
+    // 11. Let ns be ? ToIntegerWithoutRounding(nanoseconds).
+    auto ns = TRY(to_integer_without_rounding(global_object, vm.argument(9), ErrorType::TemporalInvalidDuration));
 
     // 12. Return ? CreateTemporalDuration(y, mo, w, d, h, m, s, ms, mis, ns, NewTarget).
     return TRY(create_temporal_duration(global_object, y, mo, w, d, h, m, s, ms, mis, ns, &new_target));
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.from.js b/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.from.js
index 91441327a3..da847e985e 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.from.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.from.js
@@ -39,6 +39,13 @@ describe("correct behavior", () => {
         expectDurationOneToTen(duration);
     });
 
+    test("NaN value becomes zero", () => {
+        // NOTE: NaN does *not* throw a RangeError anymore - which is questionable, IMO - as of:
+        // https://github.com/tc39/proposal-temporal/commit/8c854507a52efbc6e9eb2642f0f928df38e5c021
+        const duration = Temporal.Duration.from({ years: "foo" });
+        expect(duration.years).toBe(0);
+    });
+
     // Un-skip once ParseTemporalDurationString is implemented
     test.skip("Duration string argument", () => {
         const duration = Temporal.Duration.from("TODO");
@@ -60,11 +67,5 @@ describe("errors", () => {
             RangeError,
             "Invalid value for duration property 'years': must be an integer, got 1.2" // ...29999999999999 - let's not include that in the test :^)
         );
-        expect(() => {
-            Temporal.Duration.from({ years: "foo" });
-        }).toThrowWithMessage(
-            RangeError,
-            "Invalid value for duration property 'years': must be an integer, got NaN"
-        );
     });
 });
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.with.js b/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.with.js
index dae778ad22..4f4a7c2819 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.with.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.with.js
@@ -79,7 +79,9 @@ describe("errors", () => {
 
     test("invalid duration value", () => {
         for (const property of DURATION_PROPERTIES) {
-            for (const value of [1.23, NaN, Infinity]) {
+            // NOTE: NaN does *not* throw a RangeError anymore - which is questionable, IMO - as of:
+            // https://github.com/tc39/proposal-temporal/commit/8c854507a52efbc6e9eb2642f0f928df38e5c021
+            for (const value of [1.23, Infinity]) {
                 expect(() => {
                     new Temporal.Duration().with({ [property]: value });
                 }).toThrowWithMessage(