Repository: d5/tengo
Pull request #250 (2020-02-16 21:46:10)

builtin delete for maps and arrays
builtin `delete` is ready. It works like Go's delete for maps but also works for array types. delete function returns only undefined value or raises runtime error. It mutates given map or array. doc and tests are added.

d5: Right now Tengo does not support negative indexing into arrays. See https://github.com/d5/tengo/blob/master/objects.go#L223 and https://github.com/d5/tengo/blob/master/objects.go#L873. So I think we should remove it from this PR for consistent behavior.
ozanh:  I will remove it soon you are right I missed it
geseq: A builtin for map deletes makes perfect sense, but now that I think about it, for arrays, the go syntax of creating slices in the form `a[i:j]` might be better than a delete function. I'm gonna look into this further tomorrow.
ozanh: Daniel, I thought that for a scripting language such a syntax may not be appealing; `a = a[:2] + a[3:]` if item is in the middle and garbage stays in the slice until the script ends (not a big problem though). Honestly, my future rule engine users, who won't be Gophers, can be confused, I guess.
ozanh: If tengo will have methods like `Array.pop(index)` or `Array.splice(...)` etc. in the future, delete builtin should not support arrays. There must be only one way to do it.
d5: Actually I really like the idea of having a separate builtin function of `splice` for array types. That way we can maintain the expected behavior of `delete` (that works for maps) and users can still choose to use `splice` for array manipulations. Thoughts? @geseq @ozanh 
ozanh: It sounds good to me.
geseq: Yeah splice sounds good.
ozanh: I will remove array support from delete function if you are OK. Later I am going to implement a splice builtin. Guide me please.
geseq: Perfect!

Patch:
diff --git a/builtins.go b/builtins.go
index 87f7fc3..a89e72f 100644
--- a/builtins.go
+++ b/builtins.go
@@ -13,6 +13,10 @@ var builtinFuncs = []*BuiltinFunction{
 		Name:  "append",
 		Value: builtinAppend,
 	},
+	{
+		Name:  "delete",
+		Value: builtinDelete,
+	},
 	{
 		Name:  "string",
 		Value: builtinString,
@@ -500,3 +504,31 @@ func builtinAppend(args ...Object) (Object, error) {
 		}
 	}
 }
+
+// builtinDelete deletes Map keys
+// usage: delete(map, "key")
+// key must be a string
+func builtinDelete(args ...Object) (Object, error) {
+	argsLen := len(args)
+	if argsLen != 2 {
+		return nil, ErrWrongNumArguments
+	}
+	switch arg := args[0].(type) {
+	case *Map:
+		if key, ok := args[1].(*String); ok {
+			delete(arg.Value, key.Value)
+			return UndefinedValue, nil
+		}
+		return nil, ErrInvalidArgumentType{
+			Name:     "second",
+			Expected: "string",
+			Found:    args[1].TypeName(),
+		}
+	default:
+		return nil, ErrInvalidArgumentType{
+			Name:     "first",
+			Expected: "map",
+			Found:    arg.TypeName(),
+		}
+	}
+}
diff --git a/docs/builtins.md b/docs/builtins.md
index 0e8e239..23db51e 100644
--- a/docs/builtins.md
+++ b/docs/builtins.md
@@ -45,6 +45,28 @@ v := [1]
 v = append(v, 2, 3) // v == [1, 2, 3]
 ```
 
+## delete
+
+Deletes the element with the specified key from the map type.
+First argument must be a map type and second argument must be a string type.
+(Like Go's `delete` builtin except keys are always string).
+`delete` returns `undefined` value if successful and it mutates given map.
+
+```golang
+v := {key: "value"}
+delete(v, "key") // v == {}
+```
+
+```golang
+v := {key: "value"}
+delete(v, "missing") // v == {"key": "value"}
+```
+
+```golang
+delete({}) // runtime error, second argument is missing
+delete({}, 1) // runtime error, second argument must be a string type
+```
+
 ## type_name
 
 Returns the type_name of an object.


Test patch:
diff --git a/builtins_test.go b/builtins_test.go
new file mode 100644
index 0000000..4f0d837
--- /dev/null
+++ b/builtins_test.go
@@ -0,0 +1,97 @@
+package tengo
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+)
+
+func Test_builtinDelete(t *testing.T) {
+	type args struct {
+		args []Object
+	}
+	tests := []struct {
+		name      string
+		args      args
+		want      Object
+		wantErr   bool
+		wantedErr error
+		target    interface{}
+	}{
+		//Map
+		{name: "invalid-arg", args: args{[]Object{&String{}, &String{}}}, wantErr: true,
+			wantedErr: ErrInvalidArgumentType{Name: "first", Expected: "map", Found: "string"}},
+		{name: "no-args", wantErr: true, wantedErr: ErrWrongNumArguments},
+		{name: "empty-args", args: args{[]Object{}}, wantErr: true, wantedErr: ErrWrongNumArguments},
+		{name: "3-args", args: args{[]Object{(*Map)(nil), (*String)(nil), (*String)(nil)}}, wantErr: true, wantedErr: ErrWrongNumArguments},
+		{name: "nil-map-empty-key", args: args{[]Object{&Map{}, &String{}}}, want: UndefinedValue},
+		{name: "nil-map-nonstr-key", args: args{[]Object{&Map{}, &Int{}}}, wantErr: true,
+			wantedErr: ErrInvalidArgumentType{Name: "second", Expected: "string", Found: "int"}},
+		{name: "nil-map-no-key", args: args{[]Object{&Map{}}}, wantErr: true,
+			wantedErr: ErrWrongNumArguments},
+		{name: "map-missing-key",
+			args: args{
+				[]Object{
+					&Map{Value: map[string]Object{
+						"key": &String{Value: "value"},
+					}},
+					&String{Value: "key1"},
+				}},
+			want:   UndefinedValue,
+			target: &Map{Value: map[string]Object{"key": &String{Value: "value"}}},
+		},
+		{name: "map-emptied",
+			args: args{
+				[]Object{
+					&Map{Value: map[string]Object{
+						"key": &String{Value: "value"},
+					}},
+					&String{Value: "key"},
+				}},
+			want:   UndefinedValue,
+			target: &Map{Value: map[string]Object{}},
+		},
+		{name: "map-multi-keys",
+			args: args{
+				[]Object{
+					&Map{Value: map[string]Object{
+						"key1": &String{Value: "value1"},
+						"key2": &Int{Value: 10},
+					}},
+					&String{Value: "key1"},
+				}},
+			want:   UndefinedValue,
+			target: &Map{Value: map[string]Object{"key2": &Int{Value: 10}}},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := builtinDelete(tt.args.args...)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("builtinDelete() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if tt.wantErr && !errors.Is(err, tt.wantedErr) {
+				if err.Error() != tt.wantedErr.Error() {
+					t.Errorf("builtinDelete() error = %v, wantedErr %v", err, tt.wantedErr)
+					return
+				}
+			}
+			if got != tt.want {
+				t.Errorf("builtinDelete() = %v, want %v", got, tt.want)
+				return
+			}
+			if !tt.wantErr && tt.target != nil {
+				switch v := tt.args.args[0].(type) {
+				case *Map, *Array:
+					if !reflect.DeepEqual(tt.target, tt.args.args[0]) {
+						t.Errorf("builtinDelete() objects are not equal got: %+v, want: %+v", tt.args.args[0], tt.target)
+					}
+				default:
+					t.Errorf("builtinDelete() unsuporrted arg[0] type %s", v.TypeName())
+					return
+				}
+			}
+		})
+	}
+}
diff --git a/vm_test.go b/vm_test.go
index c26bf0c..b59806e 100644
--- a/vm_test.go
+++ b/vm_test.go
@@ -726,6 +726,40 @@ func TestBuiltinFunction(t *testing.T) {
 	expectError(t, `format("%s", "1234567890")`,
 		nil, "exceeding string size limit")
 	tengo.MaxStringLen = 2147483647
+
+	// delete
+	expectError(t, `delete()`, nil, tengo.ErrWrongNumArguments.Error())
+	expectError(t, `delete(1)`, nil, tengo.ErrWrongNumArguments.Error())
+	expectError(t, `delete(1, 2, 3)`, nil, tengo.ErrWrongNumArguments.Error())
+	expectError(t, `delete({}, "", 3)`, nil, tengo.ErrWrongNumArguments.Error())
+	expectError(t, `delete(1, 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(1.0, 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete("str", 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(bytes("str"), 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(error("err"), 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(true, 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(char('c'), 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(undefined, 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(time(1257894000), 1)`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(immutable({}), "key")`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete(immutable([]), "")`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete([], "")`, nil, `invalid type for argument 'first'`)
+	expectError(t, `delete({}, 1)`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, 1.0)`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, undefined)`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, [])`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, {})`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, error("err"))`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, bytes("str"))`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, char(35))`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, time(1257894000))`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, immutable({}))`, nil, `invalid type for argument 'second'`)
+	expectError(t, `delete({}, immutable([]))`, nil, `invalid type for argument 'second'`)
+
+	expectRun(t, `out = delete({}, "")`, nil, tengo.UndefinedValue)
+	expectRun(t, `out = {key1: 1}; delete(out, "key1")`, nil, MAP{})
+	expectRun(t, `out = {key1: 1, key2: "2"}; delete(out, "key1")`, nil, MAP{"key2": "2"})
+	expectRun(t, `out = [1, "2", {a: "b", c: 10}]; delete(out[2], "c")`, nil, ARR{1, "2", MAP{"a": "b"}})
 }
 
 func TestBytesN(t *testing.T) {