Skip to content

Commit

Permalink
Properly convert reflect map keys into strings. Fixes #590.
Browse files Browse the repository at this point in the history
  • Loading branch information
dop251 committed Jul 31, 2024
1 parent b1681fb commit c665f0b
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 4 deletions.
35 changes: 32 additions & 3 deletions object_gomap_reflect.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package goja

import (
"fmt"
"reflect"

"github.com/dop251/goja/unistring"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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 changes: 41 additions & 0 deletions object_gomap_reflect_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package goja

import (
"sort"
"strings"
"testing"
)

Expand Down Expand Up @@ -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)
})

}
4 changes: 3 additions & 1 deletion runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c665f0b

Please sign in to comment.