Make Array.prototype.sort stable

[DanielRosenwasser]

This change removes the Sorting class and its corresponding test, and just uses Arrays.sort to get a stable sort.

Fixes #1151.

[gbrail]

Ah hah -- I figured out how this used to fail. There is a test called "ComparatorTest" that tests with a comparator that returns a random number -- other JavaScript engines could run this test without error, but Java throws an exception SOMETIMES. We got lucky on this CI run but it still happens. Here's the stack trace:

ava.lang.IllegalArgumentException: Comparison method violates its general contract!
at java.base/java.util.TimSort.mergeHi(TimSort.java:903)
at java.base/java.util.TimSort.mergeAt(TimSort.java:520)
at java.base/java.util.TimSort.mergeCollapse(TimSort.java:448)
at java.base/java.util.TimSort.sort(TimSort.java:245)
at java.base/java.util.Arrays.sort(Arrays.java:1441)
at org.mozilla.javascript.NativeArray.js_sort(NativeArray.java:1353)
at org.mozilla.javascript.NativeArray.execIdCall(NativeArray.java:400)
at org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:84)
at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:35)
at org.mozilla.javascript.gen.custom_comparators_js_13._c_script_0(custom-comparators.js:38)
at org.mozilla.javascript.gen.custom_comparators_js_13.call(custom-comparators.js)
at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:383)
at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3943)
at org.mozilla.javascript.gen.custom_comparators_js_13.call(custom-comparators.js)
at org.mozilla.javascript.gen.custom_comparators_js_13.exec(custom-comparators.js)
at org.mozilla.javascript.Context.evaluateReader(Context.java:1197)
at org.mozilla.javascript.tests.ComparatorTest.customComparator(ComparatorTest.java:82)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:60)
at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:52)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
at com.sun.proxy.$Proxy5.processTestClass(Unknown Source)
at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

I suppose that we COULD just catch this exception and leave the array unsorted at this point, since once the comparator is behaving in a terrible way there's not much else we could do -- I would support that if it made sense.

[p-bakker]

Agreed, if that makes us more spec-compliant and doesn't break anything we already have, I'd say go for it!

[DanielRosenwasser]

Okay, the implementation now performs a try/catch with an early return if it detects an IllegalArgumentException. Does that match what you had in mind?

[p-bakker]

Could you validate whether #1315 is resolved by this PR as well?

@tuchida suggested that issue might be due to lack of a stable Array.sort()

[gbrail]

Looks good -- you just need to run "./gradlew spotlessApply", and unfortunately that has to be run on Java 11 because that tool doesn't work on multiple Java versions for us. Thanks!

[gbrail]

I created a new PR, #1526, which contains these commits plus a fix to spotless so that it builds.