From 94cb920c51c017f9a0b759d1c07be9543e1d2392 Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Wed, 5 Jun 2024 11:30:45 +0000 Subject: [PATCH] Fixed a number of bugs - Fixed the non-opaque slice and struct types returned from builtins as pointers to be returned from eval as pointers - Fixed syntax problems with empty blocks that contain new-lines --- ucl/ast.go | 4 ++-- ucl/objs.go | 30 +++++++++++++++-------- ucl/testbuiltins_test.go | 52 ++++++++++++++++++++++++++++++++++++++++ ucl/userbuiltin_test.go | 40 +++++++++++++++++++++++++++---- 4 files changed, 109 insertions(+), 17 deletions(-) diff --git a/ucl/ast.go b/ucl/ast.go index fd4252d..d2444e1 100644 --- a/ucl/ast.go +++ b/ucl/ast.go @@ -40,8 +40,8 @@ type astListOrHash struct { } type astBlock struct { - Names []string `parser:"LC NL? (PIPE @Ident+ PIPE NL?)?"` - Statements []*astStatements `parser:"@@ NL? RC"` + Names []string `parser:"LC NL* (PIPE @Ident+ PIPE NL*)?"` + Statements []*astStatements `parser:"@@? NL* RC"` } type astMaybeSub struct { diff --git a/ucl/objs.go b/ucl/objs.go index b82bb42..a405f96 100644 --- a/ucl/objs.go +++ b/ucl/objs.go @@ -141,9 +141,9 @@ func toGoValue(obj object) (interface{}, bool) { case proxyObject: return v.p, true case listableProxyObject: - return v.v.Interface(), true + return v.orig.Interface(), true case structProxyObject: - return v.v.Interface(), true + return v.orig.Interface(), true } return nil, false @@ -171,10 +171,17 @@ func fromGoReflectValue(resVal reflect.Value) (object, error) { switch resVal.Kind() { case reflect.Slice: - return listableProxyObject{resVal}, nil + return listableProxyObject{v: resVal, orig: resVal}, nil case reflect.Struct: - return newStructProxyObject(resVal), nil + return newStructProxyObject(resVal, resVal), nil case reflect.Pointer: + switch resVal.Elem().Kind() { + case reflect.Slice: + return listableProxyObject{v: resVal.Elem(), orig: resVal}, nil + case reflect.Struct: + return newStructProxyObject(resVal.Elem(), resVal), nil + } + return fromGoReflectValue(resVal.Elem()) } @@ -399,7 +406,8 @@ func (p proxyObject) Truthy() bool { } type listableProxyObject struct { - v reflect.Value + v reflect.Value + orig reflect.Value } func (p listableProxyObject) String() string { @@ -423,14 +431,16 @@ func (p listableProxyObject) Index(i int) object { } type structProxyObject struct { - v reflect.Value - vf []reflect.StructField + v reflect.Value + orig reflect.Value + vf []reflect.StructField } -func newStructProxyObject(v reflect.Value) structProxyObject { +func newStructProxyObject(v reflect.Value, orig reflect.Value) structProxyObject { return structProxyObject{ - v: v, - vf: slices.Filter(reflect.VisibleFields(v.Type()), func(t reflect.StructField) bool { return t.IsExported() }), + v: v, + orig: orig, + vf: slices.Filter(reflect.VisibleFields(v.Type()), func(t reflect.StructField) bool { return t.IsExported() }), } } diff --git a/ucl/testbuiltins_test.go b/ucl/testbuiltins_test.go index c8fc3e7..615c139 100644 --- a/ucl/testbuiltins_test.go +++ b/ucl/testbuiltins_test.go @@ -389,6 +389,36 @@ func TestBuiltins_Return(t *testing.T) { expr string want string }{ + // syntax tests + {desc: "empty proc 1", expr: ` + proc greet {} + greet + `, want: "(nil)\n"}, + {desc: "empty proc 2", expr: ` + proc greet { + } + + greet + `, want: "(nil)\n"}, + {desc: "empty proc 3", expr: ` + proc greet { + + + } + + greet + `, want: "(nil)\n"}, + {desc: "empty proc 4", expr: ` + proc greet { + # bla + + # di + # bla! + } + + greet + `, want: "(nil)\n"}, + {desc: "nil return", expr: ` proc greet { echo "Hello" @@ -398,6 +428,27 @@ func TestBuiltins_Return(t *testing.T) { greet `, want: "Hello\n(nil)\n"}, + + {desc: "simple arg 1", expr: ` + proc greet { |x| + return (cat "Hello, " $x) + } + + greet "person" + `, want: "Hello, person\n"}, + {desc: "simple arg 2", expr: ` + proc greet { + # This will greet someone + # here are the args: + |x| + + # And here is the code + return (cat "Hello, " $x) + } + + greet "person" + `, want: "Hello, person\n"}, + {desc: "simple return", expr: ` proc greet { return "Hello, world" @@ -406,6 +457,7 @@ func TestBuiltins_Return(t *testing.T) { greet `, want: "Hello, world\n"}, + {desc: "only return current frame", expr: ` proc greetWhat { echo "Greet the" diff --git a/ucl/userbuiltin_test.go b/ucl/userbuiltin_test.go index eea982b..681182d 100644 --- a/ucl/userbuiltin_test.go +++ b/ucl/userbuiltin_test.go @@ -3,6 +3,7 @@ package ucl_test import ( "bytes" "context" + "errors" "fmt" "strings" "testing" @@ -113,6 +114,27 @@ func TestInst_SetBuiltin(t *testing.T) { assert.Equal(t, pair{"Hello", "World"}, res) }) + t.Run("builtin return proxy object ptr", func(t *testing.T) { + type pair struct { + x, y string + } + + inst := ucl.New() + inst.SetBuiltin("add2", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var x, y string + + if err := args.Bind(&x, &y); err != nil { + return nil, err + } + + return &pair{x, y}, nil + }) + + res, err := inst.Eval(context.Background(), `add2 "Hello" "World"`) + assert.NoError(t, err) + assert.Equal(t, &pair{"Hello", "World"}, res) + }) + t.Run("builtin operating on and returning proxy object", func(t *testing.T) { type pair struct { x, y string @@ -232,13 +254,15 @@ func TestInst_SetBuiltin(t *testing.T) { opaqueThing := &opaqueThingType{x: "do", y: "not", z: "touch"} tests := []struct { - descr string - expr string - want opaqueThingType + descr string + expr string + want opaqueThingType + wantErr bool }{ {descr: "return as is", expr: `getOpaque`, want: *opaqueThing}, {descr: "update pointer 1", expr: `set x (getOpaque) ; setProp $x -x "do" -y "touch" -z "this"`, want: opaqueThingType{x: "do", y: "touch", z: "this"}}, {descr: "update pointer 2", expr: `set x (getOpaque) ; setProp $x -x "yes" ; setProp $x -y "this" -z "too"`, want: opaqueThingType{x: "yes", y: "this", z: "too"}}, + {descr: "bad args", expr: `set x (getOpaque) ; setProp $t -x "yes" ; setProp $bla -y "this" -z "too"`, want: *opaqueThing, wantErr: true}, } for _, tt := range tests { @@ -255,6 +279,8 @@ func TestInst_SetBuiltin(t *testing.T) { if err := args.Bind(&o); err != nil { return nil, err + } else if o == nil { + return nil, errors.New("is nil") } if args.HasSwitch("x") { @@ -277,8 +303,12 @@ func TestInst_SetBuiltin(t *testing.T) { }) _, err := inst.Eval(context.Background(), tt.expr) - assert.NoError(t, err) - assert.Equal(t, tt.want, *opaqueThing) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, *opaqueThing) + } }) } })