Browse Source

Deleting from a Go slice sets the element to a zero value rather than failing. Fixes #253, see #262, closes #265.

Dmitry Panov 4 years ago
parent
commit
0b7b48cdab
6 changed files with 74 additions and 31 deletions
  1. 5 7
      object_dynamic.go
  2. 8 8
      object_goslice.go
  3. 10 10
      object_goslice_reflect.go
  4. 16 1
      object_goslice_reflect_test.go
  5. 31 1
      object_goslice_test.go
  6. 4 4
      runtime.go

+ 5 - 7
object_dynamic.go

@@ -39,9 +39,8 @@ ones) is treated as an index and passed to the trap methods of the DynamicArray.
 the regular ECMAScript arrays which only support positive indexes up to 2^32-1.
 the regular ECMAScript arrays which only support positive indexes up to 2^32-1.
 
 
 DynamicArray cannot be sparse, i.e. hasOwnProperty(num) will return true for num >= 0 && num < Len(). Deleting
 DynamicArray cannot be sparse, i.e. hasOwnProperty(num) will return true for num >= 0 && num < Len(). Deleting
-such a property will fail. Note that this creates a slight peculiarity because the properties are reported as
-configurable (and therefore should be deletable). Reporting them as non-configurable is not an option though
-as it breaks an ECMAScript invariant where non-configurable properties cannot disappear.
+such a property is equivalent to setting it to undefined. Note that this creates a slight peculiarity because
+hasOwnProperty() will still return true, even after deletion.
 
 
 Note that Runtime.ToValue() does not have any special treatment for DynamicArray. The only way to create
 Note that Runtime.ToValue() does not have any special treatment for DynamicArray. The only way to create
 a dynamic array is by using the Runtime.NewDynamicArray() method. This is done deliberately to avoid
 a dynamic array is by using the Runtime.NewDynamicArray() method. This is done deliberately to avoid
@@ -657,11 +656,10 @@ func (a *dynamicArray) defineOwnPropertyIdx(name valueInt, desc PropertyDescript
 }
 }
 
 
 func (a *dynamicArray) _delete(idx int, throw bool) bool {
 func (a *dynamicArray) _delete(idx int, throw bool) bool {
-	if !a._has(idx) {
-		return true
+	if a._has(idx) {
+		a._setIdx(idx, _undefined, throw)
 	}
 	}
-	a.val.runtime.typeErrorResult(throw, "Cannot delete index property %d from a dynamic array", idx)
-	return false
+	return true
 }
 }
 
 
 func (a *dynamicArray) deleteStr(name unistring.String, throw bool) bool {
 func (a *dynamicArray) deleteStr(name unistring.String, throw bool) bool {

+ 8 - 8
object_goslice.go

@@ -252,12 +252,15 @@ func (o *objectGoSlice) toPrimitive() Value {
 	return o.toPrimitiveString()
 	return o.toPrimitiveString()
 }
 }
 
 
+func (o *objectGoSlice) _deleteIdx(idx int64) {
+	if idx < int64(len(*o.data)) {
+		(*o.data)[idx] = nil
+	}
+}
+
 func (o *objectGoSlice) deleteStr(name unistring.String, throw bool) bool {
 func (o *objectGoSlice) deleteStr(name unistring.String, throw bool) bool {
 	if idx := strToIdx64(name); idx >= 0 {
 	if idx := strToIdx64(name); idx >= 0 {
-		if idx < int64(len(*o.data)) {
-			o.val.runtime.typeErrorResult(throw, "Can't delete from Go slice")
-			return false
-		}
+		o._deleteIdx(idx)
 		return true
 		return true
 	}
 	}
 	return o.baseObject.deleteStr(name, throw)
 	return o.baseObject.deleteStr(name, throw)
@@ -266,10 +269,7 @@ func (o *objectGoSlice) deleteStr(name unistring.String, throw bool) bool {
 func (o *objectGoSlice) deleteIdx(i valueInt, throw bool) bool {
 func (o *objectGoSlice) deleteIdx(i valueInt, throw bool) bool {
 	idx := int64(i)
 	idx := int64(i)
 	if idx >= 0 {
 	if idx >= 0 {
-		if idx < int64(len(*o.data)) {
-			o.val.runtime.typeErrorResult(throw, "Can't delete from Go slice")
-			return false
-		}
+		o._deleteIdx(idx)
 	}
 	}
 	return true
 	return true
 }
 }

+ 10 - 10
object_goslice_reflect.go

@@ -265,12 +265,15 @@ func (o *objectGoSliceReflect) toPrimitive() Value {
 	return o.toPrimitiveString()
 	return o.toPrimitiveString()
 }
 }
 
 
+func (o *objectGoSliceReflect) _deleteIdx(idx int) {
+	if idx < o.value.Len() {
+		o.value.Index(idx).Set(reflect.Zero(o.value.Type().Elem()))
+	}
+}
+
 func (o *objectGoSliceReflect) deleteStr(name unistring.String, throw bool) bool {
 func (o *objectGoSliceReflect) deleteStr(name unistring.String, throw bool) bool {
-	if idx := strToIdx64(name); idx >= 0 {
-		if idx < int64(o.value.Len()) {
-			o.val.runtime.typeErrorResult(throw, "Can't delete from Go slice")
-			return false
-		}
+	if idx := strToGoIdx(name); idx >= 0 {
+		o._deleteIdx(idx)
 		return true
 		return true
 	}
 	}
 
 
@@ -278,12 +281,9 @@ func (o *objectGoSliceReflect) deleteStr(name unistring.String, throw bool) bool
 }
 }
 
 
 func (o *objectGoSliceReflect) deleteIdx(i valueInt, throw bool) bool {
 func (o *objectGoSliceReflect) deleteIdx(i valueInt, throw bool) bool {
-	idx := int64(i)
+	idx := toIntStrict(int64(i))
 	if idx >= 0 {
 	if idx >= 0 {
-		if idx < int64(o.value.Len()) {
-			o.val.runtime.typeErrorResult(throw, "Can't delete from Go slice")
-			return false
-		}
+		o._deleteIdx(idx)
 	}
 	}
 	return true
 	return true
 }
 }

+ 16 - 1
object_goslice_reflect_test.go

@@ -295,7 +295,7 @@ func TestGoSliceReflectDelete(t *testing.T) {
 	a := []*Object{{}, nil, {}}
 	a := []*Object{{}, nil, {}}
 	r.Set("a", a)
 	r.Set("a", a)
 	v, err := r.RunString(`
 	v, err := r.RunString(`
-	!delete a[0] && !delete a[1] && delete a[3];
+	delete a[0] && delete a[1] && delete a[3];
 	`)
 	`)
 	if err != nil {
 	if err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
@@ -304,3 +304,18 @@ func TestGoSliceReflectDelete(t *testing.T) {
 		t.Fatalf("not true: %v", v)
 		t.Fatalf("not true: %v", v)
 	}
 	}
 }
 }
+
+func TestGoSliceReflectPop(t *testing.T) {
+	r := New()
+	a := []string{"1", "", "3"}
+	r.Set("a", &a)
+	v, err := r.RunString(`
+	a.pop()
+	`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !v.SameAs(asciiString("3")) {
+		t.Fatal(v)
+	}
+}

+ 31 - 1
object_goslice_test.go

@@ -175,7 +175,7 @@ func TestGoSliceDelete(t *testing.T) {
 	a := []interface{}{1, nil, 3}
 	a := []interface{}{1, nil, 3}
 	r.Set("a", a)
 	r.Set("a", a)
 	v, err := r.RunString(`
 	v, err := r.RunString(`
-	!delete a[0] && !delete a[1] && delete a[3];
+	delete a[0] && delete a[1] && delete a[3];
 	`)
 	`)
 	if err != nil {
 	if err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
@@ -184,3 +184,33 @@ func TestGoSliceDelete(t *testing.T) {
 		t.Fatalf("not true: %v", v)
 		t.Fatalf("not true: %v", v)
 	}
 	}
 }
 }
+
+func TestGoSlicePop(t *testing.T) {
+	r := New()
+	a := []interface{}{1, nil, 3}
+	r.Set("a", &a)
+	v, err := r.RunString(`
+	a.pop()
+	`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !v.SameAs(intToValue(3)) {
+		t.Fatal(v)
+	}
+}
+
+func TestGoSliceShift(t *testing.T) {
+	r := New()
+	a := []interface{}{1, nil, 3}
+	r.Set("a", &a)
+	v, err := r.RunString(`
+	a.shift()
+	`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !v.SameAs(intToValue(1)) {
+		t.Fatal(v)
+	}
+}

+ 4 - 4
runtime.go

@@ -1316,7 +1316,7 @@ operator:
 
 
     // If return value is a non-nil *Object, it will be used instead of call.This
     // If return value is a non-nil *Object, it will be used instead of call.This
     // This way it is possible to return a Go struct or a map converted
     // This way it is possible to return a Go struct or a map converted
-    // into goja.Value using runtime.ToValue(), however in this case
+    // into goja.Value using ToValue(), however in this case
     // instanceof will not work as expected.
     // instanceof will not work as expected.
     return nil
     return nil
  }
  }
@@ -1420,9 +1420,9 @@ prototype and all the usual methods should work. There are, however, some caveat
 - If the slice is not addressable, the array cannot be extended or shrunk. Any attempt to do so (by setting an index
 - If the slice is not addressable, the array cannot be extended or shrunk. Any attempt to do so (by setting an index
 beyond the current length or by modifying the length) will result in a TypeError.
 beyond the current length or by modifying the length) will result in a TypeError.
 
 
-- Converted Arrays may not contain holes (because Go slices cannot). This means that hasOwnProperty(n) will always
-return `true` if n < length. Attempt to delete an item with an index < length will fail. Nil slice elements will be
-converted to `null`. Accessing an element beyond `length` will return `undefined`.
+- Converted Arrays may not contain holes (because Go slices cannot). This means that hasOwnProperty(n) always
+returns `true` if n < length. Deleting an item with an index < length will set it to a zero value (but the property will
+remain). Nil slice elements are be converted to `null`. Accessing an element beyond `length` returns `undefined`.
 
 
 Any other type is converted to a generic reflect based host object. Depending on the underlying type it behaves similar
 Any other type is converted to a generic reflect based host object. Depending on the underlying type it behaves similar
 to a Number, String, Boolean or Object.
 to a Number, String, Boolean or Object.