From 88417aba95b1c793195ccb78992342fc13a8ba33 Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Wed, 5 Jun 2024 03:59:53 +0000 Subject: [PATCH 1/2] Added support for "opaque" return types These are return types that are left alone by the UCL interpretor and cannot be operated on or get fields of. --- ucl/objs.go | 20 +++++++++ ucl/userbuiltin.go | 29 ++++++++++++ ucl/userbuiltin_test.go | 99 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) diff --git a/ucl/objs.go b/ucl/objs.go index 804ff71..b82bb42 100644 --- a/ucl/objs.go +++ b/ucl/objs.go @@ -110,6 +110,8 @@ func (b boolObject) Truthy() bool { func toGoValue(obj object) (interface{}, bool) { switch v := obj.(type) { + case OpaqueObject: + return v.v, true case nil: return nil, true case strObject: @@ -149,6 +151,8 @@ func toGoValue(obj object) (interface{}, bool) { func fromGoValue(v any) (object, error) { switch t := v.(type) { + case OpaqueObject: + return t, nil case nil: return nil, nil case string: @@ -476,6 +480,22 @@ func (s structProxyObject) Each(fn func(k string, v object) error) error { return nil } +type OpaqueObject struct { + v any +} + +func Opaque(v any) OpaqueObject { + return OpaqueObject{v: v} +} + +func (p OpaqueObject) String() string { + return fmt.Sprintf("opaque{%T}", p.v) +} + +func (p OpaqueObject) Truthy() bool { + return p.v != nil +} + type errBreak struct { isCont bool ret object diff --git a/ucl/userbuiltin.go b/ucl/userbuiltin.go index 625bea8..b7480dc 100644 --- a/ucl/userbuiltin.go +++ b/ucl/userbuiltin.go @@ -121,6 +121,23 @@ func (ca CallArgs) bindArg(v interface{}, arg object) error { } switch t := arg.(type) { + case OpaqueObject: + if v == nil { + return errors.New("opaque object not bindable to nil") + } + + vr := reflect.ValueOf(v) + tr := reflect.ValueOf(t.v) + if vr.Kind() != reflect.Pointer { + return errors.New("expected pointer for an opaque object bind") + } + + if !tr.Type().AssignableTo(vr.Elem().Type()) { + return errors.New("opaque object not assignable to passed in value") + } + + vr.Elem().Set(tr) + return nil case proxyObject: return bindProxyObject(v, reflect.ValueOf(t.p)) case listableProxyObject: @@ -142,6 +159,18 @@ func canBindArg(v interface{}, arg object) bool { } switch t := arg.(type) { + case OpaqueObject: + vr := reflect.ValueOf(v) + tr := reflect.ValueOf(t.v) + if vr.Kind() != reflect.Pointer { + return false + } + + if !tr.Type().AssignableTo(vr.Elem().Type()) { + return false + } + + return true case proxyObject: return canBindProxyObject(v, reflect.ValueOf(t.p)) case listableProxyObject: diff --git a/ucl/userbuiltin_test.go b/ucl/userbuiltin_test.go index a4bd231..eea982b 100644 --- a/ucl/userbuiltin_test.go +++ b/ucl/userbuiltin_test.go @@ -183,6 +183,105 @@ func TestInst_SetBuiltin(t *testing.T) { }) } }) + + t.Run("opaques returned as is", func(t *testing.T) { + type opaqueThingType struct { + x string + y string + z string + } + opaqueThing := &opaqueThingType{x: "do", y: "not", z: "touch"} + + tests := []struct { + descr string + expr string + wantErr bool + }{ + {descr: "return as is", expr: `getOpaque`, wantErr: false}, + {descr: "carry around ok", expr: `set x (getOpaque) ; $x`, wantErr: false}, + {descr: "iterate over", expr: `foreach (countTo3) { |x| echo $x }`, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.descr, func(t *testing.T) { + + outW := bytes.NewBuffer(nil) + inst := ucl.New(ucl.WithOut(outW)) + + inst.SetBuiltin("getOpaque", func(ctx context.Context, args ucl.CallArgs) (any, error) { + return ucl.Opaque(opaqueThing), nil + }) + + res, err := inst.Eval(context.Background(), tt.expr) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Same(t, opaqueThing, res) + } + }) + } + }) + + t.Run("operate on opaques", func(t *testing.T) { + type opaqueThingType struct { + x string + y string + z string + } + opaqueThing := &opaqueThingType{x: "do", y: "not", z: "touch"} + + tests := []struct { + descr string + expr string + want opaqueThingType + }{ + {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"}}, + } + + for _, tt := range tests { + t.Run(tt.descr, func(t *testing.T) { + + outW := bytes.NewBuffer(nil) + inst := ucl.New(ucl.WithOut(outW)) + + inst.SetBuiltin("getOpaque", func(ctx context.Context, args ucl.CallArgs) (any, error) { + return ucl.Opaque(opaqueThing), nil + }) + inst.SetBuiltin("setProp", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var o *opaqueThingType + + if err := args.Bind(&o); err != nil { + return nil, err + } + + if args.HasSwitch("x") { + var s string + _ = args.BindSwitch("x", &s) + o.x = s + } + if args.HasSwitch("y") { + var s string + _ = args.BindSwitch("y", &s) + o.y = s + } + if args.HasSwitch("z") { + var s string + _ = args.BindSwitch("z", &s) + o.z = s + } + + return nil, nil + }) + + _, err := inst.Eval(context.Background(), tt.expr) + assert.NoError(t, err) + assert.Equal(t, tt.want, *opaqueThing) + }) + } + }) } func TestCallArgs_Bind(t *testing.T) { From 94cb920c51c017f9a0b759d1c07be9543e1d2392 Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Wed, 5 Jun 2024 11:30:45 +0000 Subject: [PATCH 2/2] 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) + } }) } })