Fixed bug which was not isolating procs

ucl/builtins.go

@@ -1161,7 +1161,7 @@ func procBuiltin(ctx context.Context, args macroArgs) (Object, error) {
 
 	obj := procObject{args.eval, args.ec, blockObj.block}
 	if procName != "" {
-		args.ec.addCmd(procName, obj)
+		args.ec.addUserCmd(procName, obj)
 	}
 	return obj, nil
 }

ucl/env.go

@@ -7,12 +7,11 @@ type evalCtx struct {
 	macros           map[string]macroable
 	vars             map[string]Object
 	pseudoVars       map[string]pseudoVar
+	userCommandFrame bool // Frame to use for user-defined commands
 }
 
 func (ec *evalCtx) forkAndIsolate() *evalCtx {
-	newEc := &evalCtx{parent: ec}
+	return &evalCtx{parent: ec, root: ec.root, userCommandFrame: true}
-	newEc.root = newEc
-	return newEc
 }
 
 func (ec *evalCtx) fork() *evalCtx {
@@ -27,6 +26,25 @@ func (ec *evalCtx) addCmd(name string, inv invokable) {
 	ec.root.commands[name] = inv
 }
 
+func (ec *evalCtx) addUserCmd(name string, inv invokable) {
+	frame := ec
+	for frame != nil {
+		if frame.userCommandFrame {
+			break
+		}
+		frame = frame.parent
+	}
+	if frame == nil {
+		panic("no user command frame found")
+	}
+
+	if frame.commands == nil {
+		frame.commands = make(map[string]invokable)
+	}
+
+	frame.commands[name] = inv
+}
+
 func (ec *evalCtx) addMacro(name string, inv macroable) {
 	if ec.root.macros == nil {
 		ec.root.macros = make(map[string]macroable)

ucl/inst.go

@@ -53,7 +53,9 @@ type Module struct {
 }
 
 func New(opts ...InstOption) *Inst {
-	rootEC := &evalCtx{}
+	rootEC := &evalCtx{
+		userCommandFrame: true,
+	}
 	rootEC.root = rootEC
 
 	rootEC.addCmd("echo", invokableFunc(echoBuiltin))
@@ -194,7 +196,7 @@ func (inst *Inst) eval(ctx context.Context, r io.Reader, opts evalOptions) (Obje
 
 	env := inst.rootEC
 	if opts.forkEnv {
-		env = env.fork()
+		env = env.forkAndIsolate()
 	}
 
 	return eval.evalScript(ctx, env, ast)

ucl/inst_test.go

@@ -145,7 +145,39 @@ func TestInst_Eval_WithSubEnv(t *testing.T) {
 		assert.Nil(t, res)
 	})
 
-	t.Run("when using hooks, reading vars are not permitted to leak across environment", func(t *testing.T) {
+	t.Run("environments should not leak when using hooks", func(t *testing.T) {
+		tests := []struct {
+			descr string
+			eval1 string
+			eval2 string
+			want1 any
+			want2 any
+		}{
+			{
+				descr: "reading vars",
+				eval1: `$a = "hello" ; hook { $a }`,
+				eval2: `$a = "world" ; hook { $a }`,
+				want1: "hello",
+				want2: "world",
+			},
+			{
+				descr: "modifying vars",
+				eval1: `$a = "hello" ; hook { $a = "new value" ; $a }`,
+				eval2: `$a = "world" ; hook { $a }`,
+				want1: "new value",
+				want2: "world",
+			},
+			{
+				descr: "defining procs",
+				eval1: `proc say_hello { "hello" } ; hook { say_hello }`,
+				eval2: `proc say_hello { "world" } ; hook { say_hello }`,
+				want1: "hello",
+				want2: "world",
+			},
+		}
+
+		for _, tt := range tests {
+			t.Run(tt.descr, func(t *testing.T) {
 				ctx := t.Context()
 
 				hooks := make([]ucl.Invokable, 0)
@@ -162,55 +194,23 @@ func TestInst_Eval_WithSubEnv(t *testing.T) {
 					return nil, nil
 				})
 
-		res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a }`), ucl.WithSubEnv())
+				res, err := inst.Eval(ctx, strings.NewReader(tt.eval1), ucl.WithSubEnv())
 				assert.NoError(t, err)
 				assert.Nil(t, res)
 
-		res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv())
+				res, err = inst.Eval(ctx, strings.NewReader(tt.eval2), ucl.WithSubEnv())
 				assert.NoError(t, err)
 				assert.Nil(t, res)
 
 				h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{})
 				assert.NoError(t, err)
-		assert.Equal(t, "hello", h1)
+				assert.Equal(t, tt.want1, h1)
 
 				h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{})
 				assert.NoError(t, err)
-		assert.Equal(t, "world", h2)
+				assert.Equal(t, tt.want2, h2)
 			})
-
-	t.Run("when using hooks, writing vars are not permitted to leak across environment", func(t *testing.T) {
-		ctx := t.Context()
-
-		hooks := make([]ucl.Invokable, 0)
-
-		inst := ucl.New()
-		inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) {
-			var hookProc ucl.Invokable
-
-			if err := args.Bind(&hookProc); err != nil {
-				return nil, err
 		}
-			hooks = append(hooks, hookProc)
-
-			return nil, nil
-		})
-
-		res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a = "new value" ; $a }`), ucl.WithSubEnv())
-		assert.NoError(t, err)
-		assert.Nil(t, res)
-
-		res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv())
-		assert.NoError(t, err)
-		assert.Nil(t, res)
-
-		h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{})
-		assert.NoError(t, err)
-		assert.Equal(t, "new value", h1)
-
-		h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{})
-		assert.NoError(t, err)
-		assert.Equal(t, "world", h2)
 	})
 }