LibJS: Re-implement String.localeCompare using the StringCompare AO

This follows the ECMA402 spec and means String.prototype.localeCompare
will automatically become actually locale aware once StringCompare is
actually implemented based on UTS #10.

2 files changed, 21 insertions, 13 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
index d739cedf33..47fcf30f57 100644
--- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
@@ -16,6 +16,9 @@
 #include <LibJS/Runtime/Error.h>
 #include <LibJS/Runtime/GlobalObject.h>
 #include <LibJS/Runtime/Intl/AbstractOperations.h>
+#include <LibJS/Runtime/Intl/Collator.h>
+#include <LibJS/Runtime/Intl/CollatorCompareFunction.h>
+#include <LibJS/Runtime/Intl/CollatorConstructor.h>
 #include <LibJS/Runtime/PrimitiveString.h>
 #include <LibJS/Runtime/RegExpObject.h>
 #include <LibJS/Runtime/StringIterator.h>
@@ -1002,19 +1005,24 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::sup)
 }
 
 // 22.1.3.11 String.prototype.localeCompare ( that [ , reserved1 [ , reserved2 ] ] ), https://tc39.es/ecma262/#sec-string.prototype.localecompare
-// NOTE: This is the minimum localeCompare implementation for engines without ECMA-402.
+// 19.1.1 String.prototype.localeCompare ( that [ , locales [ , options ] ] ), https://tc39.es/ecma402/#sup-String.prototype.localeCompare
 JS_DEFINE_NATIVE_FUNCTION(StringPrototype::locale_compare)
 {
-    auto string = TRY(ak_string_from(vm, global_object));
-    auto that_string = TRY(vm.argument(0).to_string(global_object));
+    // FIXME: This can throw (spec issue)
+    // 1. Let O be RequireObjectCoercible(this value).
+    auto object = TRY(require_object_coercible(global_object, vm.this_value(global_object)));
+
+    // 2. Let S be ? ToString(O).
+    auto string = TRY(object.to_string(global_object));
+
+    // 3. Let thatValue be ? ToString(that).
+    auto that_value = TRY(vm.argument(0).to_string(global_object));
 
-    // FIXME: Actually compare the string not just according to their bits.
-    if (string == that_string)
-        return Value(0);
-    if (string < that_string)
-        return Value(-1);
+    // 4. Let collator be ? Construct(%Collator%, « locales, options »).
+    auto* collator = TRY(construct(global_object, *global_object.intl_collator_constructor(), vm.argument(1), vm.argument(2)));
 
-    return Value(1);
+    // 5. Return CompareStrings(collator, S, thatValue).
+    return Intl::compare_strings(static_cast<Intl::Collator&>(*collator), Utf8View(string), Utf8View(that_value));
 }
 
 }
diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.localeCompare.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.localeCompare.js
index 6762638b78..5d6bebc2d8 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.localeCompare.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.localeCompare.js
@@ -9,7 +9,7 @@ test("basic functionality", () => {
         const aTob = a.localeCompare(b);
         const bToa = b.localeCompare(a);
 
-        expect(aTob).toBe(1);
+        expect(aTob > 0).toBeTrue();
         expect(aTob).toBe(-bToa);
     }
 
@@ -24,7 +24,7 @@ test("basic functionality", () => {
 
     expect("null".localeCompare(null)).toBe(0);
     expect("null".localeCompare(undefined)).not.toBe(0);
-    expect("null".localeCompare()).toBe(-1);
+    expect("null".localeCompare() < 0).toBeTrue();
 
     expect(() => {
         String.prototype.localeCompare.call(undefined, undefined);
@@ -34,6 +34,6 @@ test("basic functionality", () => {
 test("UTF-16", () => {
     var s = "😀😀";
     expect(s.localeCompare("😀😀")).toBe(0);
-    expect(s.localeCompare("\ud83d")).toBe(1);
-    expect(s.localeCompare("😀😀s")).toBe(-1);
+    expect(s.localeCompare("\ud83d") > 0);
+    expect(s.localeCompare("😀😀s") < 0);
 });