Skip to content

Commit

Permalink
fix: array writes (#454)
Browse files Browse the repository at this point in the history
Fix array writes not being persisted by passing in writeable
reflect.Value when available instead of .Interface() which looses that
property.

Also:
* Use value.IsValid() instead of comparison with zero entry.
* Use propertyLength instead of literal.

Fixes #386
  • Loading branch information
stevenh committed Nov 26, 2022
1 parent b6f2991 commit 08e7a8d
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 22 deletions.
53 changes: 41 additions & 12 deletions issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,18 @@ func Test_issue13(t *testing.T) {
"number": 42,
"array": []string{"def", "ghi"},
})
if err != nil {
t.Error(err)
t.FailNow()
}
require.NoError(t, err)

fn, err := vm.Object(`
(function(value){
return ""+[value.string, value.number, value.array]
})
`)
if err != nil {
t.Error(err)
t.FailNow()
}
require.NoError(t, err)

result, err := fn.Value().Call(fn.Value(), value)
if err != nil {
t.Error(err)
t.FailNow()
}
require.NoError(t, err)

is(result.string(), "Xyzzy,42,def,ghi")

anything := struct {
Expand Down Expand Up @@ -899,3 +891,40 @@ func Test_issue390(t *testing.T) {
require.NoError(t, err)
require.Equal(t, int64(1), rows)
}

type testSetType struct {
String string
Array [1]string
Slice []string
}

func Test_issue386(t *testing.T) {
var msg testSetType
msg.String = "string"
msg.Slice = []string{"slice"}
msg.Array[0] = "array"

vm := New()
err := vm.Set("msg", &msg)
require.NoError(t, err)

tests := map[string]string{
"string": `
msg.TypeMessage = 'something';
msg.TypeMessage;`,
"array": `
msg.Array[0] = 'something';
msg.Array[0]`,
"slice": `
msg.Slice[0] = 'something';
msg.Slice[0]`,
}

for name, code := range tests {
t.Run(name, func(t *testing.T) {
val, err := vm.Run(code)
require.NoError(t, err)
require.Equal(t, "something", val.String())
})
}
}
9 changes: 9 additions & 0 deletions runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,11 @@ func (self *_runtime) convertCallParameter(v Value, t reflect.Type) (reflect.Val
}

func (self *_runtime) toValue(value interface{}) Value {
rv, ok := value.(reflect.Value)
if ok {
value = rv.Interface()
}

switch value := value.(type) {
case Value:
return value
Expand Down Expand Up @@ -666,6 +671,10 @@ func (self *_runtime) toValue(value interface{}) Value {
default:
{
value := reflect.ValueOf(value)
if ok && value.Kind() == rv.Kind() {
// Use passed in rv which may be writable.
value = rv
}

switch value.Kind() {
case reflect.Ptr:
Expand Down
2 changes: 1 addition & 1 deletion type_go_array.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type _goArrayObject struct {
}

func _newGoArrayObject(value reflect.Value) *_goArrayObject {
writable := value.Kind() == reflect.Ptr // The Array is addressable (like a Slice)
writable := value.Kind() == reflect.Ptr || value.CanSet() // The Array is addressable (like a Slice)
mode := _propertyMode(0010)
if writable {
mode = 0110
Expand Down
2 changes: 1 addition & 1 deletion type_go_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func goMapGetOwnProperty(self *_object, name string) *_property {
}

// Other methods
if method := self.value.(*_goMapObject).value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.(*_goMapObject).value.MethodByName(name); method.IsValid() {
return &_property{
value: self.runtime.toValue(method.Interface()),
mode: 0110,
Expand Down
7 changes: 3 additions & 4 deletions type_go_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ func goSliceGetOwnProperty(self *_object, name string) *_property {
}

// .0, .1, .2, ...
index := stringToArrayIndex(name)
if index >= 0 {
if index := stringToArrayIndex(name); index >= 0 {
value := Value{}
reflectValue, exists := self.value.(*_goSliceObject).getValue(index)
if exists {
Expand All @@ -68,7 +67,7 @@ func goSliceGetOwnProperty(self *_object, name string) *_property {
}

// Other methods
if method := self.value.(*_goSliceObject).value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.(*_goSliceObject).value.MethodByName(name); method.IsValid() {
return &_property{
value: self.runtime.toValue(method.Interface()),
mode: 0110,
Expand Down Expand Up @@ -106,7 +105,7 @@ func goSliceDefineOwnProperty(self *_object, name string, descriptor _property,

func goSliceDelete(self *_object, name string, throw bool) bool {
// length
if name == "length" {
if name == propertyLength {
return self.runtime.typeErrorResult(throw)
}

Expand Down
8 changes: 4 additions & 4 deletions type_go_struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func (self _goStructObject) getValue(name string) reflect.Value {
}

if validGoStructName(name) {
// Do not reveal hidden or unexported fields
if field := reflect.Indirect(self.value).FieldByName(name); (field != reflect.Value{}) {
// Do not reveal hidden or unexported fields.
if field := reflect.Indirect(self.value).FieldByName(name); field.IsValid() {
return field
}

if method := self.value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.MethodByName(name); method.IsValid() {
return method
}
}
Expand Down Expand Up @@ -81,7 +81,7 @@ func goStructGetOwnProperty(self *_object, name string) *_property {
object := self.value.(*_goStructObject)
value := object.getValue(name)
if value.IsValid() {
return &_property{self.runtime.toValue(value.Interface()), 0110}
return &_property{self.runtime.toValue(value), 0110}
}

return objectGetOwnProperty(self, name)
Expand Down

0 comments on commit 08e7a8d

Please sign in to comment.