Browse Source

Array.prototype.sort(): correctly handle float return values from a custom comparator function. Fixes #144.

Dmitry Panov 5 years ago
parent
commit
2de61b1174
2 changed files with 45 additions and 2 deletions
  1. 13 2
      builtin_array.go
  2. 32 0
      runtime_test.go

+ 13 - 2
builtin_array.go

@@ -2,6 +2,7 @@ package goja
 
 import (
 	"bytes"
+	"math"
 	"sort"
 	"strings"
 )
@@ -860,10 +861,20 @@ func (ctx *arraySortCtx) sortCompare(x, y Value) int {
 	}
 
 	if ctx.compare != nil {
-		return int(ctx.compare(FunctionCall{
+		f := ctx.compare(FunctionCall{
 			This:      _undefined,
 			Arguments: []Value{x, y},
-		}).ToInteger())
+		}).ToFloat()
+		if f > 0 {
+			return 1
+		}
+		if f < 0 {
+			return -1
+		}
+		if math.Signbit(f) {
+			return -1
+		}
+		return 0
 	}
 	return strings.Compare(x.String(), y.String())
 }

+ 32 - 0
runtime_test.go

@@ -924,6 +924,38 @@ func TestSortComparatorReturnValues(t *testing.T) {
 	testScript1(SCRIPT, _undefined, t)
 }
 
+func TestSortComparatorReturnValueFloats(t *testing.T) {
+	const SCRIPT = `
+	var a = [
+		5.97,
+		9.91,
+		4.13,
+		9.28,
+		3.29,
+	];
+	a.sort( function(a, b) { return a - b; } );
+	for (var i = 1; i < a.length; i++) {
+		if (a[i] < a[i-1]) {
+			throw new Error("Array is not sorted: " + a);
+		}
+	}
+	`
+	testScript1(SCRIPT, _undefined, t)
+}
+
+func TestSortComparatorReturnValueNegZero(t *testing.T) {
+	const SCRIPT = `
+	var a = [2, 1];
+	a.sort( function(a, b) { return a > b ? 0 : -0; } );
+	for (var i = 1; i < a.length; i++) {
+		if (a[i] < a[i-1]) {
+			throw new Error("Array is not sorted: " + a);
+		}
+	}
+	`
+	testScript1(SCRIPT, _undefined, t)
+}
+
 func TestNilApplyArg(t *testing.T) {
 	const SCRIPT = `
 	(function x(a, b) {