From d015458327fd7c89fc4ad4b1f1d59e4e4f3b1ac9 Mon Sep 17 00:00:00 2001 From: Walter Scott Date: Tue, 5 Aug 2025 11:45:24 -0500 Subject: [PATCH] feat: add recursive HCL variable resolution --- compiler.go | 65 +++------ hcl_e2e_test.go | 6 + internal/hclext/resolve.go | 211 ++++++++++++++++++++++++++++ taskfile/hcl_recursive_vars_test.go | 59 ++++++++ testdata/HCLE2ETest/Taskfile.hcl | 9 +- variables.go | 36 ++--- 6 files changed, 314 insertions(+), 72 deletions(-) create mode 100644 internal/hclext/resolve.go create mode 100644 taskfile/hcl_recursive_vars_test.go diff --git a/compiler.go b/compiler.go index 9c59636f80..5e9bdd3853 100644 --- a/compiler.go +++ b/compiler.go @@ -48,6 +48,7 @@ func (c *Compiler) FastGetVariables(t *ast.Task, call *Call) (*ast.Vars, error) func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (*ast.Vars, error) { result := env.GetEnviron() evaluator := hclext.NewHCLEvaluator(result, result, nil) + hasHCL := false specialVars, err := c.getSpecialVars(t, call) if err != nil { return nil, err @@ -60,62 +61,30 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* getRangeFunc := func(dir string) func(k string, v ast.Var) error { return func(k string, v ast.Var) error { if v.Expr != nil || v.ShExpr != nil { - if v.Expr != nil { - val, err := evaluator.EvalString(v.Expr) - if err != nil { - return err - } - result.Set(k, ast.Var{Value: val}) - evaluator.SetVar(k, val) - return nil - } - if v.ShExpr != nil { - if !evaluateShVars { - result.Set(k, ast.Var{Value: ""}) - evaluator.SetVar(k, "") - return nil - } - cmd, err := evaluator.EvalString(v.ShExpr) - if err != nil { - return err - } - static, err := c.HandleDynamicVar(ast.Var{Sh: &cmd}, dir, env.GetFromVars(result)) - if err != nil { - return err - } - result.Set(k, ast.Var{Value: static}) - evaluator.SetVar(k, static) - return nil - } + result.Set(k, v) + hasHCL = true + return nil } - cache := &templater.Cache{Vars: result} - // Replace values newVar := templater.ReplaceVar(v, cache) - // If the variable should not be evaluated, but is nil, set it to an empty string - // This stops empty interface errors when using the templater to replace values later if !evaluateShVars && newVar.Value == nil { result.Set(k, ast.Var{Value: ""}) evaluator.SetVar(k, "") return nil } - // If the variable should not be evaluated and it is set, we can set it and return if !evaluateShVars { result.Set(k, ast.Var{Value: newVar.Value}) evaluator.SetVar(k, fmt.Sprint(newVar.Value)) return nil } - // Now we can check for errors since we've handled all the cases when we don't want to evaluate if err := cache.Err(); err != nil { return err } - // If the variable is already set, we can set it and return if newVar.Value != nil || newVar.Sh == nil { result.Set(k, ast.Var{Value: newVar.Value}) evaluator.SetVar(k, fmt.Sprint(newVar.Value)) return nil } - // If the variable is dynamic, we need to resolve it first static, err := c.HandleDynamicVar(newVar, dir, env.GetFromVars(result)) if err != nil { return err @@ -163,19 +132,27 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* } } - if t == nil || call == nil { - return result, nil - } - - for k, v := range call.Vars.All() { - if err := rangeFunc(k, v); err != nil { - return nil, err + if t != nil && call != nil { + for k, v := range call.Vars.All() { + if err := rangeFunc(k, v); err != nil { + return nil, err + } + } + for k, v := range t.Vars.All() { + if err := taskRangeFunc(k, v); err != nil { + return nil, err + } } } - for k, v := range t.Vars.All() { - if err := taskRangeFunc(k, v); err != nil { + + if hasHCL { + resVars, resEnv, err := hclext.NewResolver(result, result, nil).Resolve() + if err != nil { return nil, err } + result = ast.NewVars() + result.Merge(resEnv, nil) + result.Merge(resVars, nil) } return result, nil diff --git a/hcl_e2e_test.go b/hcl_e2e_test.go index ef57639078..e11aea7158 100644 --- a/hcl_e2e_test.go +++ b/hcl_e2e_test.go @@ -32,4 +32,10 @@ func TestHCLE2E(t *testing.T) { if idx == -1 || idx+5 >= len(output) || output[idx+5] == '\n' { t.Fatalf("missing path output: %s", output) } + if !strings.Contains(output, "GREET=HELLO, BOB!") { + t.Fatalf("missing greet output: %s", output) + } + if !strings.Contains(output, "EXT=base-ext") { + t.Fatalf("missing ext output: %s", output) + } } diff --git a/internal/hclext/resolve.go b/internal/hclext/resolve.go new file mode 100644 index 0000000000..a86054235f --- /dev/null +++ b/internal/hclext/resolve.go @@ -0,0 +1,211 @@ +package hclext + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + + "github.com/go-task/task/v3/taskfile/ast" +) + +// Resolver evaluates HCL expressions for vars and env allowing recursive references. +// It resolves variables on demand, detecting cycles and caching results. +type Resolver struct { + vars *ast.Vars + env *ast.Vars + runner TaskRunner + varCache map[string]string + envCache map[string]string + varStack map[string]bool + envStack map[string]bool +} + +// NewResolver creates a new Resolver. +func NewResolver(vars, env *ast.Vars, runner TaskRunner) *Resolver { + r := &Resolver{ + vars: vars, + env: env, + runner: runner, + varCache: map[string]string{}, + envCache: map[string]string{}, + varStack: map[string]bool{}, + envStack: map[string]bool{}, + } + if vars != nil { + for k, v := range vars.All() { + if v.Expr == nil { + if v.Value != nil { + r.varCache[k] = fmt.Sprint(v.Value) + } + } + } + } + if env != nil { + for k, v := range env.All() { + if v.Expr == nil { + if v.Value != nil { + r.envCache[k] = fmt.Sprint(v.Value) + } + } + } + } + return r +} + +// Resolve evaluates all expressions and returns new vars and env with values set. +func (r *Resolver) Resolve() (*ast.Vars, *ast.Vars, error) { + if r.vars != nil { + for k := range r.vars.All() { + if _, ok := r.varCache[k]; !ok { + if _, err := r.resolveVar(k); err != nil { + return nil, nil, err + } + } + } + } + if r.env != nil { + for k := range r.env.All() { + if _, ok := r.envCache[k]; !ok { + if _, err := r.resolveEnv(k); err != nil { + return nil, nil, err + } + } + } + } + vars := ast.NewVars() + for k, v := range r.varCache { + vars.Set(k, ast.Var{Value: v}) + } + env := ast.NewVars() + for k, v := range r.envCache { + env.Set(k, ast.Var{Value: v}) + } + return vars, env, nil +} + +func (r *Resolver) resolveVar(name string) (string, error) { + if v, ok := r.varCache[name]; ok { + return v, nil + } + if r.varStack[name] { + return "", fmt.Errorf("cyclic variable reference for %s", name) + } + if r.vars == nil { + return "", fmt.Errorf("undefined variable %s", name) + } + v, ok := r.vars.Get(name) + if !ok { + return "", fmt.Errorf("undefined variable %s", name) + } + if v.Expr == nil { + val := fmt.Sprint(v.Value) + r.varCache[name] = val + return val, nil + } + r.varStack[name] = true + defer delete(r.varStack, name) + depsVars, depsEnv := findDeps(v.Expr) + for dv := range depsVars { + if _, err := r.resolveVar(dv); err != nil { + return "", err + } + } + for de := range depsEnv { + if _, err := r.resolveEnv(de); err != nil { + return "", err + } + } + eval := NewHCLEvaluator(varsFromCache(r.varCache), envFromCache(r.envCache), r.runner) + val, err := eval.EvalString(v.Expr) + if err != nil { + return "", err + } + r.varCache[name] = val + return val, nil +} + +func (r *Resolver) resolveEnv(name string) (string, error) { + if v, ok := r.envCache[name]; ok { + return v, nil + } + if r.envStack[name] { + return "", fmt.Errorf("cyclic env reference for %s", name) + } + if r.env != nil { + if v, ok := r.env.Get(name); ok { + if v.Expr == nil { + val := fmt.Sprint(v.Value) + r.envCache[name] = val + return val, nil + } + r.envStack[name] = true + defer delete(r.envStack, name) + depsVars, depsEnv := findDeps(v.Expr) + for dv := range depsVars { + if _, err := r.resolveVar(dv); err != nil { + return "", err + } + } + for de := range depsEnv { + if _, err := r.resolveEnv(de); err != nil { + return "", err + } + } + eval := NewHCLEvaluator(varsFromCache(r.varCache), envFromCache(r.envCache), r.runner) + val, err := eval.EvalString(v.Expr) + if err != nil { + return "", err + } + r.envCache[name] = val + return val, nil + } + } + // Not defined; return empty string + r.envCache[name] = "" + return "", nil +} + +func varsFromCache(m map[string]string) *ast.Vars { + vs := ast.NewVars() + for k, v := range m { + vs.Set(k, ast.Var{Value: v}) + } + return vs +} + +func envFromCache(m map[string]string) *ast.Vars { + vs := ast.NewVars() + for k, v := range m { + vs.Set(k, ast.Var{Value: v}) + } + return vs +} + +func findDeps(expr hcl.Expression) (vars map[string]struct{}, env map[string]struct{}) { + vars = map[string]struct{}{} + env = map[string]struct{}{} + if expr == nil { + return + } + for _, tr := range expr.Variables() { + if len(tr) != 2 { + continue + } + root := tr.RootName() + attr, ok := tr[1].(hcl.TraverseAttr) + if !ok { + continue + } + switch root { + case "vars": + vars[attr.Name] = struct{}{} + case "env": + env[attr.Name] = struct{}{} + } + } + return +} + +// Helper to satisfy linter for unused imports +var _ = cty.String diff --git a/taskfile/hcl_recursive_vars_test.go b/taskfile/hcl_recursive_vars_test.go new file mode 100644 index 0000000000..858327a2a0 --- /dev/null +++ b/taskfile/hcl_recursive_vars_test.go @@ -0,0 +1,59 @@ +package taskfile + +import ( + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/stretchr/testify/require" + + "github.com/go-task/task/v3/internal/env" + "github.com/go-task/task/v3/internal/hclext" + "github.com/go-task/task/v3/taskfile/ast" +) + +func parseExpr(t *testing.T, s string) hcl.Expression { + t.Helper() + expr, diags := hclsyntax.ParseExpression([]byte(s), "test.hcl", hcl.InitialPos) + require.False(t, diags.HasErrors()) + return expr +} + +func TestRecursiveVars(t *testing.T) { + vars := ast.NewVars() + vars.Set("GREETING", ast.Var{Expr: parseExpr(t, `"Hello, ${vars.NAME}!"`)}) + vars.Set("NAME", ast.Var{Expr: parseExpr(t, `"BOB"`)}) + vars.Set("UPPER_GREETING", ast.Var{Expr: parseExpr(t, `upper(vars.GREETING)`)}) + + resolver := hclext.NewResolver(vars, env.GetEnviron(), nil) + resolved, _, err := resolver.Resolve() + require.NoError(t, err) + + g, _ := resolved.Get("GREETING") + require.Equal(t, "Hello, BOB!", g.Value) + u, _ := resolved.Get("UPPER_GREETING") + require.Equal(t, "HELLO, BOB!", u.Value) +} + +func TestOrderIndependence(t *testing.T) { + vars := ast.NewVars() + vars.Set("FINAL", ast.Var{Expr: parseExpr(t, `upper(vars.INTERMEDIATE)`)}) + vars.Set("INTERMEDIATE", ast.Var{Expr: parseExpr(t, `"${vars.BASE} + ok"`)}) + vars.Set("BASE", ast.Var{Expr: parseExpr(t, `"yup"`)}) + + resolver := hclext.NewResolver(vars, env.GetEnviron(), nil) + resolved, _, err := resolver.Resolve() + require.NoError(t, err) + + v, _ := resolved.Get("FINAL") + require.Equal(t, "YUP + OK", v.Value) +} + +func TestCyclicReference(t *testing.T) { + vars := ast.NewVars() + vars.Set("LOOP", ast.Var{Expr: parseExpr(t, `"${vars.LOOP}"`)}) + + resolver := hclext.NewResolver(vars, env.GetEnviron(), nil) + _, _, err := resolver.Resolve() + require.Error(t, err) +} diff --git a/testdata/HCLE2ETest/Taskfile.hcl b/testdata/HCLE2ETest/Taskfile.hcl index 7689dade0e..8e14b25500 100644 --- a/testdata/HCLE2ETest/Taskfile.hcl +++ b/testdata/HCLE2ETest/Taskfile.hcl @@ -2,9 +2,14 @@ version = "3" vars { ORIGINAL = "foo" + NAME = "BOB" + GREETING = "Hello, ${vars.NAME}!" + UPPER_GREETING = upper(vars.GREETING) } env { + EXTENDED = "${env.BASE}-ext" + BASE = "base" PATH_COPY = env("PATH") } @@ -24,6 +29,8 @@ task "all" { ] cmds = [ "echo FINAL ${vars.ORIGINAL}", - "echo PATH=${env.PATH_COPY}" + "echo PATH=${env.PATH_COPY}", + "echo GREET=${vars.UPPER_GREETING}", + "echo EXT=${env.EXTENDED}" ] } diff --git a/variables.go b/variables.go index 457033782e..d9ac1ae56b 100644 --- a/variables.go +++ b/variables.go @@ -110,34 +110,16 @@ func (e *Executor) compiledTask(call *Call, evaluateShVars bool) (*ast.Task, err new.Env = ast.NewVars() if origTask.IsHCL { - evalTemp := hclext.NewHCLEvaluator(vars, env.GetEnviron(), e.callTask) - evaluated := ast.NewVars() - for k, v := range e.Taskfile.Env.All() { - if v.Expr != nil { - val, err := evalTemp.EvalString(v.Expr) - if err != nil { - return nil, err - } - evaluated.Set(k, ast.Var{Value: val}) - } else { - evaluated.Set(k, v) - } - } - new.Env.Merge(evaluated, nil) - new.Env.Merge(templater.ReplaceVars(dotenvEnvs, cache), nil) - evaluated = ast.NewVars() - for k, v := range origTask.Env.All() { - if v.Expr != nil { - val, err := evalTemp.EvalString(v.Expr) - if err != nil { - return nil, err - } - evaluated.Set(k, ast.Var{Value: val}) - } else { - evaluated.Set(k, v) - } + envDefs := env.GetEnviron() + envDefs.Merge(e.Taskfile.Env, nil) + envDefs.Merge(templater.ReplaceVars(dotenvEnvs, cache), nil) + envDefs.Merge(origTask.Env, nil) + resolver := hclext.NewResolver(vars, envDefs, e.callTask) + _, resolvedEnv, err := resolver.Resolve() + if err != nil { + return nil, err } - new.Env.Merge(evaluated, nil) + new.Env = resolvedEnv } else { new.Env.Merge(templater.ReplaceVars(e.Taskfile.Env, cache), nil) new.Env.Merge(templater.ReplaceVars(dotenvEnvs, cache), nil)