From c665f0b58f6edef98c9b15c30ff9efeaae1eb887 Mon Sep 17 00:00:00 2001 From: Dmitry Panov Date: Wed, 31 Jul 2024 16:04:04 +0100 Subject: [PATCH] Properly convert reflect map keys into strings. Fixes #590. --- object_gomap_reflect.go | 35 +++++++++++++++++++++++++++--- object_gomap_reflect_test.go | 41 ++++++++++++++++++++++++++++++++++++ runtime.go | 4 +++- 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/object_gomap_reflect.go b/object_gomap_reflect.go index 531c1652..d4c1a061 100644 --- a/object_gomap_reflect.go +++ b/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) + } +} diff --git a/object_gomap_reflect_test.go b/object_gomap_reflect_test.go index d9d06f3b..8603c26e 100644 --- a/object_gomap_reflect_test.go +++ b/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) + }) + +} diff --git a/runtime.go b/runtime.go index 9f4e999f..ff7f7e6f 100644 --- a/runtime.go +++ b/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