Browse Source

Properly convert reflect map keys into strings. Fixes #590.

Dmitry Panov 1 year ago
parent
commit
c665f0b58f
3 changed files with 76 additions and 4 deletions
  1. 32 3
      object_gomap_reflect.go
  2. 41 0
      object_gomap_reflect_test.go
  3. 3 1
      runtime.go

+ 32 - 3
object_gomap_reflect.go

@@ -1,6 +1,7 @@
 package goja
 
 import (
+	"fmt"
 	"reflect"
 
 	"github.com/dop251/goja/unistring"
@@ -114,7 +115,7 @@ func (o *objectGoMapReflect) _put(key reflect.Value, val Value, throw bool) bool
 			}
 			o.fieldsValue.SetMapIndex(key, v)
 		} else {
-			o.val.runtime.typeErrorResult(throw, "Cannot set property %s, object is not extensible", key.String())
+			o.val.runtime.typeErrorResult(throw, "Cannot set property %v, object is not extensible", key)
 			return false
 		}
 		return true
@@ -241,7 +242,7 @@ func (i *gomapReflectPropIter) next() (propIterItem, iterNextFunc) {
 		v := i.o.fieldsValue.MapIndex(key)
 		i.idx++
 		if v.IsValid() {
-			return propIterItem{name: newStringValue(key.String()), enumerable: _ENUM_TRUE}, i.next
+			return propIterItem{name: i.o.keyToString(key), enumerable: _ENUM_TRUE}, i.next
 		}
 	}
 
@@ -258,8 +259,36 @@ func (o *objectGoMapReflect) iterateStringKeys() iterNextFunc {
 func (o *objectGoMapReflect) stringKeys(_ bool, accum []Value) []Value {
 	// all own keys are enumerable
 	for _, key := range o.fieldsValue.MapKeys() {
-		accum = append(accum, newStringValue(key.String()))
+		accum = append(accum, o.keyToString(key))
 	}
 
 	return accum
 }
+
+func (*objectGoMapReflect) keyToString(key reflect.Value) String {
+	kind := key.Kind()
+
+	if kind == reflect.String {
+		return newStringValue(key.String())
+	}
+
+	str := fmt.Sprintf("%v", key)
+
+	switch kind {
+	case reflect.Int,
+		reflect.Int8,
+		reflect.Int16,
+		reflect.Int32,
+		reflect.Int64,
+		reflect.Uint,
+		reflect.Uint8,
+		reflect.Uint16,
+		reflect.Uint32,
+		reflect.Uint64,
+		reflect.Float32,
+		reflect.Float64:
+		return asciiString(str)
+	default:
+		return newStringValue(str)
+	}
+}

+ 41 - 0
object_gomap_reflect_test.go

@@ -1,6 +1,8 @@
 package goja
 
 import (
+	"sort"
+	"strings"
 	"testing"
 )
 
@@ -307,3 +309,42 @@ func TestGoMapReflectElt(t *testing.T) {
 
 	r.testScript(SCRIPT, valueTrue, t)
 }
+
+func TestGoMapReflectKeyToString(t *testing.T) {
+	vm := New()
+
+	test := func(v any, t *testing.T) {
+		o1 := vm.ToValue(v).ToObject(vm)
+		keys := o1.Keys()
+		sort.Strings(keys)
+		if len(keys) != 2 || keys[0] != "1" || keys[1] != "2" {
+			t.Fatal(keys)
+		}
+
+		keys1 := o1.self.stringKeys(true, nil)
+		sort.Slice(keys1, func(a, b int) bool {
+			return strings.Compare(keys1[a].String(), keys1[b].String()) < 0
+		})
+		if len(keys1) != 2 || keys1[0] != asciiString("1") || keys1[1] != asciiString("2") {
+			t.Fatal(keys1)
+		}
+	}
+
+	t.Run("int", func(t *testing.T) {
+		m1 := map[int]any{
+			1: 2,
+			2: 3,
+		}
+		test(m1, t)
+	})
+
+	t.Run("CustomString", func(t *testing.T) {
+		type CustomString string
+		m2 := map[CustomString]any{
+			"1": 2,
+			"2": 3,
+		}
+		test(m2, t)
+	})
+
+}

+ 3 - 1
runtime.go

@@ -1720,7 +1720,9 @@ Note that Value.Export() for a `Date` value returns time.Time in local timezone.
 
 # Maps
 
-Maps with string or integer key type are converted into host objects that largely behave like a JavaScript Object.
+Maps with string, integer, or float key types are converted into host objects that largely behave like a JavaScript Object.
+One noticeable difference is that the key order is not stable, as with maps in Go.
+Keys are converted to strings following the fmt.Sprintf("%v") convention.
 
 # Maps with methods