From 2d56517de93b8ed0f3382ad20133d85d688c542a Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Tue, 28 Jan 2025 08:54:48 +1100 Subject: [PATCH 1/2] Fixed a panic when not is called with a nil value Also added the "error" builtin. --- _docs/mod/core.md | 9 ++++++ ucl/builtins.go | 14 ++++++++- ucl/inst.go | 1 + ucl/objs.go | 11 +++++++ ucl/testbuiltins_test.go | 62 ++++++++++++++++++++++++++-------------- 5 files changed, 74 insertions(+), 23 deletions(-) diff --git a/_docs/mod/core.md b/_docs/mod/core.md index 711ccd7..af6ad92 100644 --- a/_docs/mod/core.md +++ b/_docs/mod/core.md @@ -20,6 +20,15 @@ echo [ARGS...] Displays the string representation of ARGS to stdout followed by a new line. +### error + +``` +error MSG +``` + +Raises an error with MSG as the given value. This will start unrolling the stack until a `try` block is +encountered, or until it reaches the top level stack, at which it will be displayed as a "runtime error". + ### foreach ``` diff --git a/ucl/builtins.go b/ucl/builtins.go index 539d119..24eee57 100644 --- a/ucl/builtins.go +++ b/ucl/builtins.go @@ -297,7 +297,12 @@ func notBuiltin(ctx context.Context, args invocationArgs) (Object, error) { return nil, err } - return boolObject(!args.args[0].Truthy()), nil + v := args.args[0] + if v == nil { + return boolObject(true), nil + } + + return boolObject(!v.Truthy()), nil } var errObjectsNotEqual = errors.New("objects not equal") @@ -889,6 +894,13 @@ func foreachBuiltin(ctx context.Context, args macroArgs) (Object, error) { return last, nil } +func errorBuiltin(ctx context.Context, args invocationArgs) (Object, error) { + if len(args.args) < 1 { + return nil, errors.New("need at least one arguments") + } + return nil, errRuntime{val: args.args[0]} +} + func breakBuiltin(ctx context.Context, args invocationArgs) (Object, error) { if len(args.args) < 1 { return nil, errBreak{} diff --git a/ucl/inst.go b/ucl/inst.go index 6332256..550975d 100644 --- a/ucl/inst.go +++ b/ucl/inst.go @@ -92,6 +92,7 @@ func New(opts ...InstOption) *Inst { rootEC.addCmd("break", invokableFunc(breakBuiltin)) rootEC.addCmd("continue", invokableFunc(continueBuiltin)) rootEC.addCmd("return", invokableFunc(returnBuiltin)) + rootEC.addCmd("error", invokableFunc(errorBuiltin)) rootEC.addMacro("if", macroFunc(ifBuiltin)) rootEC.addMacro("foreach", macroFunc(foreachBuiltin)) diff --git a/ucl/objs.go b/ucl/objs.go index 8cdd011..f5cb0ef 100644 --- a/ucl/objs.go +++ b/ucl/objs.go @@ -622,3 +622,14 @@ func (e errReturn) Error() string { } var ErrHalt = errors.New("halt") + +type errRuntime struct { + val Object +} + +func (e errRuntime) Error() string { + if e.val == nil { + return "runtime error: (nil)" + } + return "runtime error: " + e.val.String() +} diff --git a/ucl/testbuiltins_test.go b/ucl/testbuiltins_test.go index 455680f..26e15e7 100644 --- a/ucl/testbuiltins_test.go +++ b/ucl/testbuiltins_test.go @@ -3,7 +3,6 @@ package ucl import ( "bytes" "context" - "errors" "fmt" "strings" "testing" @@ -43,13 +42,6 @@ func WithTestBuiltin() InstOption { return &a, nil })) - i.rootEC.addCmd("error", invokableFunc(func(ctx context.Context, args invocationArgs) (Object, error) { - if len(args.args) == 0 { - return nil, errors.New("an error occurred") - } - return nil, errors.New(args.args[0].String()) - })) - i.rootEC.addCmd("joinpipe", invokableFunc(func(ctx context.Context, args invocationArgs) (Object, error) { sb := strings.Builder{} @@ -230,7 +222,7 @@ func TestBuiltins_Try(t *testing.T) { try { error "bang" } - echo "after"`, wantErr: "bang"}, + echo "after"`, wantErr: "runtime error: bang"}, {desc: "try with catch - successful", expr: ` try { echo "good" @@ -251,7 +243,7 @@ func TestBuiltins_Try(t *testing.T) { } catch { |err| echo (cat "the error was = " $err) } - echo "after"`, want: "the error was = error:bang\nafter\n(nil)\n"}, + echo "after"`, want: "the error was = error:runtime error: bang\nafter\n(nil)\n"}, {desc: "try with two catch - successful", expr: ` try { echo "i'm good" @@ -279,18 +271,18 @@ func TestBuiltins_Try(t *testing.T) { echo "wow, we made it here" } echo "after"`, want: "wow, we made it here\nafter\n(nil)\n"}, - {desc: "return value - single try", expr: ` + {desc: "return value - single try 1", expr: ` set x (try { error "bang" }) - $x`, wantErr: "bang"}, - {desc: "return value - single try", expr: ` + $x`, wantErr: "runtime error: bang"}, + {desc: "return value - single try 2", expr: ` set x (try { error "bang" } catch { |err| $err }) - $x`, want: "error:bang\n"}, + $x`, want: "error:runtime error: bang\n"}, {desc: "return value - try and catch - successful", expr: ` set x (try { error "bang" } catch { "hello" }) $x`, want: "hello\n"}, {desc: "return value - try and catch - unsuccessful", expr: ` set x (try { error "bang" } catch { error "boom" }) - $x`, wantErr: "boom"}, + $x`, wantErr: "runtime error: boom"}, {desc: "try with finally - successful", expr: ` try { echo "all good" @@ -304,7 +296,7 @@ func TestBuiltins_Try(t *testing.T) { } finally { echo "always at end" } - echo "after"`, want: "always at end\n", wantErr: "bang"}, + echo "after"`, want: "always at end\n", wantErr: "runtime error: bang"}, {desc: "try with catch and finally - successful", expr: ` try { echo "all good" @@ -331,7 +323,7 @@ func TestBuiltins_Try(t *testing.T) { } finally { echo "always at end" } - echo "after"`, want: "always at end\n", wantErr: "boom"}, + echo "after"`, want: "always at end\n", wantErr: "runtime error: boom"}, {desc: "try with finally - finally result discarded", expr: ` set a (try { "return me" @@ -344,13 +336,13 @@ func TestBuiltins_Try(t *testing.T) { error "bang" } finally { error "kaboom" - }`, wantErr: "bang"}, + }`, wantErr: "runtime error: bang"}, {desc: "try with finally - error not discarded if try succeeds", expr: ` try { echo "all good" } finally { error "kaboom" - }`, want: "all good\n", wantErr: "kaboom"}, + }`, want: "all good\n", wantErr: "runtime error: kaboom"}, {desc: "try with finally with error - successful", expr: ` try { echo "all good" @@ -359,13 +351,13 @@ func TestBuiltins_Try(t *testing.T) { if (eq $err ()) { echo "that's nil" } } echo "after"`, want: "all good\nthe error was \nthat's nil\nafter\n(nil)\n"}, - {desc: "try with finally - unsuccessful", expr: ` + {desc: "try with finally - unsuccessful 2", expr: ` try { error "bang" } finally { |err| echo (cat "the error was " $err) } - echo "after"`, want: "the error was error:bang\n", wantErr: "bang"}, + echo "after"`, want: "the error was error:runtime error: bang\n", wantErr: "runtime error: bang"}, {desc: "try with too many finallies - unsuccessful", expr: ` try { error "bang" @@ -387,7 +379,7 @@ func TestBuiltins_Try(t *testing.T) { } finally { echo "outer" } - echo "after"`, want: "the error was error:bang\nouter caught\nouter\nafter\n(nil)\n"}, + echo "after"`, want: "the error was error:runtime error: bang\nouter caught\nouter\nafter\n(nil)\n"}, } for _, tt := range tests { @@ -1496,6 +1488,7 @@ func TestBuiltins_AndOrNot(t *testing.T) { {desc: "not 1", expr: `not $true`, want: false}, {desc: "not 2", expr: `not $false`, want: true}, {desc: "not 3", expr: `not $false $true`, want: true}, + {desc: "not 4", expr: `not ()`, want: true}, {desc: "short circuit and 1", expr: `and "hello" "world"`, want: "world"}, {desc: "short circuit and 2", expr: `and () "world"`, want: nil}, @@ -1564,6 +1557,31 @@ func TestBuiltins_Cat(t *testing.T) { } } +func TestBuiltins_Error(t *testing.T) { + tests := []struct { + desc string + expr string + wantErr string + }{ + {desc: "error 1", expr: `error "bang"`, wantErr: "runtime error: bang"}, + {desc: "error 2", expr: `error 123`, wantErr: "runtime error: 123"}, + {desc: "error 3", expr: `error ()`, wantErr: "runtime error: (nil)"}, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + ctx := context.Background() + outW := bytes.NewBuffer(nil) + + inst := New(WithOut(outW), WithTestBuiltin()) + + _, err := inst.Eval(ctx, tt.expr) + assert.Error(t, err) + assert.Equal(t, tt.wantErr, err.Error()) + }) + } +} + func evalAndDisplay(ctx context.Context, inst *Inst, expr string) error { res, err := inst.eval(ctx, expr) if err != nil { From c433e4bf530cbbab2a5c17db47c73fc10c5c0e99 Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Tue, 28 Jan 2025 09:25:52 +1100 Subject: [PATCH 2/2] Made undefined variables return an error Previously undefined variables would be set to nil, which introduced subtle errors. Changed this so that referencing an undefined variable will produce an error. This behaviour can changed as part of the instance configuration by declaring a missing-var handler. --- ucl/eval.go | 11 +++++++- ucl/inst.go | 7 +++++ ucl/inst_test.go | 58 ++++++++++++++++++++++++++++++++++++++++ ucl/testbuiltins_test.go | 8 ++++-- ucl/userbuiltin.go | 1 + 5 files changed, 82 insertions(+), 3 deletions(-) diff --git a/ucl/eval.go b/ucl/eval.go index 237864b..98e352e 100644 --- a/ucl/eval.go +++ b/ucl/eval.go @@ -186,7 +186,16 @@ func (e evaluator) evalArg(ctx context.Context, ec *evalCtx, n astCmdArg) (Objec if v, ok := ec.getVar(*n.Var); ok { return v, nil } - return nil, nil + + if e.inst.missingVarHandler != nil { + dv, err := e.inst.missingVarHandler(ctx, *n.Var) + if err != nil { + return nil, err + } + return fromGoValue(dv) + } + + return nil, errors.New("undefined variable: " + *n.Var) case n.MaybeSub != nil: sub := n.MaybeSub.Sub if sub == nil { diff --git a/ucl/inst.go b/ucl/inst.go index 550975d..5414753 100644 --- a/ucl/inst.go +++ b/ucl/inst.go @@ -11,6 +11,7 @@ import ( type Inst struct { out io.Writer missingBuiltinHandler MissingBuiltinHandler + missingVarHandler MissingVarHandler echoPrinter EchoPrinter rootEC *evalCtx @@ -30,6 +31,12 @@ func WithMissingBuiltinHandler(handler MissingBuiltinHandler) InstOption { } } +func WithMissingVarHandler(handler MissingVarHandler) InstOption { + return func(i *Inst) { + i.missingVarHandler = handler + } +} + func WithModule(module Module) InstOption { return func(i *Inst) { for name, builtin := range module.Builtins { diff --git a/ucl/inst_test.go b/ucl/inst_test.go index 1da27e5..1faa97c 100644 --- a/ucl/inst_test.go +++ b/ucl/inst_test.go @@ -119,6 +119,64 @@ func TestInst_Eval(t *testing.T) { } } +func TestInst_MissingVarHandler(t *testing.T) { + tests := []struct { + desc string + expr string + handler ucl.MissingVarHandler + want any + wantErr string + }{ + { + desc: "default - error", + expr: `$bla`, + wantErr: "undefined variable: bla", + }, + { + desc: "handler - set to nil", + handler: func(ctx context.Context, name string) (any, error) { + return nil, nil + }, + expr: `$bla`, + want: nil, + }, + { + desc: "handler - set to a string 1", + handler: func(ctx context.Context, name string) (any, error) { + return "I am " + name, nil + }, + expr: `$bla`, + want: "I am bla", + }, + { + desc: "handler - set to a string 2", + handler: func(ctx context.Context, name string) (any, error) { + return "I am " + name, nil + }, + expr: `$something`, + want: "I am something", + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + ctx := context.Background() + outW := bytes.NewBuffer(nil) + + inst := ucl.New(ucl.WithOut(outW), ucl.WithTestBuiltin(), ucl.WithMissingVarHandler(tt.handler)) + res, err := inst.Eval(ctx, tt.expr) + + if tt.wantErr != "" { + assert.Error(t, err) + assert.Equal(t, err.Error(), tt.wantErr) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, res) + } + }) + } +} + var parseComments1 = ` proc lookup { |file| foreach { |toks| diff --git a/ucl/testbuiltins_test.go b/ucl/testbuiltins_test.go index 26e15e7..917a4ac 100644 --- a/ucl/testbuiltins_test.go +++ b/ucl/testbuiltins_test.go @@ -197,7 +197,9 @@ func TestBuiltins_If(t *testing.T) { ctx := context.Background() outW := bytes.NewBuffer(nil) - inst := New(WithOut(outW), WithTestBuiltin()) + inst := New(WithOut(outW), WithTestBuiltin(), WithMissingVarHandler(func(ctx context.Context, name string) (any, error) { + return nil, nil + })) err := evalAndDisplay(ctx, inst, tt.expr) assert.NoError(t, err) @@ -387,7 +389,9 @@ func TestBuiltins_Try(t *testing.T) { ctx := context.Background() outW := bytes.NewBuffer(nil) - inst := New(WithOut(outW), WithTestBuiltin()) + inst := New(WithOut(outW), WithTestBuiltin(), WithMissingVarHandler(func(ctx context.Context, name string) (any, error) { + return nil, nil + })) err := evalAndDisplay(ctx, inst, tt.expr) if tt.wantErr != "" { diff --git a/ucl/userbuiltin.go b/ucl/userbuiltin.go index ce26e40..5e2ee07 100644 --- a/ucl/userbuiltin.go +++ b/ucl/userbuiltin.go @@ -11,6 +11,7 @@ import ( type BuiltinHandler func(ctx context.Context, args CallArgs) (any, error) type MissingBuiltinHandler func(ctx context.Context, name string, args CallArgs) (any, error) +type MissingVarHandler func(ctx context.Context, name string) (any, error) type CallArgs struct { args invocationArgs