From c433e4bf530cbbab2a5c17db47c73fc10c5c0e99 Mon Sep 17 00:00:00 2001
From: Leon Mika <lmika@lmika.org>
Date: Tue, 28 Jan 2025 09:25:52 +1100
Subject: [PATCH] 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