From 6ed30f1addf85dcf015e3586008e3f9bdf59041d Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sun, 29 Mar 2020 16:54:59 -0300 Subject: [PATCH] Refactor variables: Keep order of declaration This shouldn't have any behavior changes for now. This is a code refactor that should allow us to do further improvements on how variables are handled, specially regarding respecting the declaration order in Taskfiles, which should make it easier for the users. Initial work on #218 --- cmd/task/task.go | 4 +- internal/args/args.go | 16 +++--- internal/args/args_test.go | 41 +++++++++----- internal/compiler/compiler.go | 2 +- internal/compiler/env.go | 12 ++--- internal/compiler/v2/compiler_v2.go | 23 ++++---- internal/taskfile/call.go | 2 +- internal/taskfile/cmd.go | 8 +-- internal/taskfile/merge.go | 13 ++--- internal/taskfile/read/taskvars.go | 17 ++---- internal/taskfile/task.go | 8 +-- internal/taskfile/taskfile.go | 11 ++-- internal/taskfile/taskfile_test.go | 18 ++++--- internal/taskfile/var.go | 84 ++++++++++++++++++++++++++--- internal/templater/templater.go | 20 +++---- task.go | 2 +- variables.go | 22 ++++---- 17 files changed, 188 insertions(+), 115 deletions(-) diff --git a/cmd/task/task.go b/cmd/task/task.go index d99db44d..dde290c2 100644 --- a/cmd/task/task.go +++ b/cmd/task/task.go @@ -140,9 +140,7 @@ func main() { } calls, globals := args.Parse(pflag.Args()...) - for name, value := range globals { - e.Taskfile.Vars[name] = value - } + e.Taskfile.Vars.Merge(globals) ctx := context.Background() if !watch { diff --git a/internal/args/args.go b/internal/args/args.go index acea9c29..e157db32 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -7,9 +7,9 @@ import ( ) // Parse parses command line argument: tasks and vars of each task -func Parse(args ...string) ([]taskfile.Call, taskfile.Vars) { +func Parse(args ...string) ([]taskfile.Call, *taskfile.Vars) { var calls []taskfile.Call - var globals taskfile.Vars + var globals *taskfile.Vars for _, arg := range args { if !strings.Contains(arg, "=") { @@ -19,18 +19,16 @@ func Parse(args ...string) ([]taskfile.Call, taskfile.Vars) { if len(calls) < 1 { if globals == nil { - globals = taskfile.Vars{} + globals = &taskfile.Vars{} } - name, value := splitVar(arg) - globals[name] = taskfile.Var{Static: value} + globals.Set(name, taskfile.Var{Static: value}) } else { if calls[len(calls)-1].Vars == nil { - calls[len(calls)-1].Vars = make(taskfile.Vars) + calls[len(calls)-1].Vars = &taskfile.Vars{} } - - name, value := splitVar((arg)) - calls[len(calls)-1].Vars[name] = taskfile.Var{Static: value} + name, value := splitVar(arg) + calls[len(calls)-1].Vars.Set(name, taskfile.Var{Static: value}) } } diff --git a/internal/args/args_test.go b/internal/args/args_test.go index 8dc96e4c..779b37f9 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -14,7 +14,7 @@ func TestArgs(t *testing.T) { tests := []struct { Args []string ExpectedCalls []taskfile.Call - ExpectedGlobals taskfile.Vars + ExpectedGlobals *taskfile.Vars }{ { Args: []string{"task-a", "task-b", "task-c"}, @@ -29,16 +29,22 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ { Task: "task-a", - Vars: taskfile.Vars{ - "FOO": taskfile.Var{Static: "bar"}, + Vars: &taskfile.Vars{ + Keys: []string{"FOO"}, + Mapping: map[string]taskfile.Var{ + "FOO": taskfile.Var{Static: "bar"}, + }, }, }, {Task: "task-b"}, { Task: "task-c", - Vars: taskfile.Vars{ - "BAR": taskfile.Var{Static: "baz"}, - "BAZ": taskfile.Var{Static: "foo"}, + Vars: &taskfile.Vars{ + Keys: []string{"BAR", "BAZ"}, + Mapping: map[string]taskfile.Var{ + "BAR": taskfile.Var{Static: "baz"}, + "BAZ": taskfile.Var{Static: "foo"}, + }, }, }, }, @@ -48,8 +54,11 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ { Task: "task-a", - Vars: taskfile.Vars{ - "CONTENT": taskfile.Var{Static: "with some spaces"}, + Vars: &taskfile.Vars{ + Keys: []string{"CONTENT"}, + Mapping: map[string]taskfile.Var{ + "CONTENT": taskfile.Var{Static: "with some spaces"}, + }, }, }, }, @@ -60,8 +69,11 @@ func TestArgs(t *testing.T) { {Task: "task-a"}, {Task: "task-b"}, }, - ExpectedGlobals: taskfile.Vars{ - "FOO": {Static: "bar"}, + ExpectedGlobals: &taskfile.Vars{ + Keys: []string{"FOO"}, + Mapping: map[string]taskfile.Var{ + "FOO": {Static: "bar"}, + }, }, }, { @@ -81,9 +93,12 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ {Task: "default"}, }, - ExpectedGlobals: taskfile.Vars{ - "FOO": {Static: "bar"}, - "BAR": {Static: "baz"}, + ExpectedGlobals: &taskfile.Vars{ + Keys: []string{"FOO", "BAR"}, + Mapping: map[string]taskfile.Var{ + "FOO": {Static: "bar"}, + "BAR": {Static: "baz"}, + }, }, }, } diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index aa31d893..75dcb3ee 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -7,6 +7,6 @@ import ( // Compiler handles compilation of a task before its execution. // E.g. variable merger, template processing, etc. type Compiler interface { - GetVariables(t *taskfile.Task, call taskfile.Call) (taskfile.Vars, error) + GetVariables(t *taskfile.Task, call taskfile.Call) (*taskfile.Vars, error) HandleDynamicVar(v taskfile.Var) (string, error) } diff --git a/internal/compiler/env.go b/internal/compiler/env.go index 4a9ecf4c..283577cc 100644 --- a/internal/compiler/env.go +++ b/internal/compiler/env.go @@ -9,16 +9,12 @@ import ( // GetEnviron the all return all environment variables encapsulated on a // taskfile.Vars -func GetEnviron() taskfile.Vars { - var ( - env = os.Environ() - m = make(taskfile.Vars, len(env)) - ) - - for _, e := range env { +func GetEnviron() *taskfile.Vars { + m := &taskfile.Vars{} + for _, e := range os.Environ() { keyVal := strings.SplitN(e, "=", 2) key, val := keyVal[0], keyVal[1] - m[key] = taskfile.Var{Static: val} + m.Set(key, taskfile.Var{Static: val}) } return m } diff --git a/internal/compiler/v2/compiler_v2.go b/internal/compiler/v2/compiler_v2.go index f42b5fbe..6bcbfaf2 100644 --- a/internal/compiler/v2/compiler_v2.go +++ b/internal/compiler/v2/compiler_v2.go @@ -19,8 +19,8 @@ var _ compiler.Compiler = &CompilerV2{} type CompilerV2 struct { Dir string - Taskvars taskfile.Vars - TaskfileVars taskfile.Vars + Taskvars *taskfile.Vars + TaskfileVars *taskfile.Vars Expansions int @@ -36,10 +36,10 @@ type CompilerV2 struct { // 3. Taskfile variables // 4. Taskvars file variables // 5. Environment variables -func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (taskfile.Vars, error) { +func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (*taskfile.Vars, error) { vr := varResolver{c: c, vars: compiler.GetEnviron()} - vr.vars["TASK"] = taskfile.Var{Static: t.Task} - for _, vars := range []taskfile.Vars{c.Taskvars, c.TaskfileVars, call.Vars, t.Vars} { + vr.vars.Set("TASK", taskfile.Var{Static: t.Task}) + for _, vars := range []*taskfile.Vars{c.Taskvars, c.TaskfileVars, call.Vars, t.Vars} { for i := 0; i < c.Expansions; i++ { vr.merge(vars) } @@ -49,16 +49,16 @@ func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (taskfil type varResolver struct { c *CompilerV2 - vars taskfile.Vars + vars *taskfile.Vars err error } -func (vr *varResolver) merge(vars taskfile.Vars) { +func (vr *varResolver) merge(vars *taskfile.Vars) { if vr.err != nil { return } tr := templater.Templater{Vars: vr.vars} - for k, v := range vars { + vars.Range(func(k string, v taskfile.Var) error { v = taskfile.Var{ Static: tr.Replace(v.Static), Sh: tr.Replace(v.Sh), @@ -66,10 +66,11 @@ func (vr *varResolver) merge(vars taskfile.Vars) { static, err := vr.c.HandleDynamicVar(v) if err != nil { vr.err = err - return + return err } - vr.vars[k] = taskfile.Var{Static: static} - } + vr.vars.Set(k, taskfile.Var{Static: static}) + return nil + }) vr.err = tr.Err() } diff --git a/internal/taskfile/call.go b/internal/taskfile/call.go index eec41031..c1ca1083 100644 --- a/internal/taskfile/call.go +++ b/internal/taskfile/call.go @@ -3,5 +3,5 @@ package taskfile // Call is the parameters to a task call type Call struct { Task string - Vars Vars + Vars *Vars } diff --git a/internal/taskfile/cmd.go b/internal/taskfile/cmd.go index ea267811..c992dd30 100644 --- a/internal/taskfile/cmd.go +++ b/internal/taskfile/cmd.go @@ -10,14 +10,14 @@ type Cmd struct { Cmd string Silent bool Task string - Vars Vars + Vars *Vars IgnoreError bool } // Dep is a task dependency type Dep struct { Task string - Vars Vars + Vars *Vars } var ( @@ -51,7 +51,7 @@ func (c *Cmd) UnmarshalYAML(unmarshal func(interface{}) error) error { } var taskCall struct { Task string - Vars Vars + Vars *Vars } if err := unmarshal(&taskCall); err == nil { c.Task = taskCall.Task @@ -70,7 +70,7 @@ func (d *Dep) UnmarshalYAML(unmarshal func(interface{}) error) error { } var taskCall struct { Task string - Vars Vars + Vars *Vars } if err := unmarshal(&taskCall); err == nil { d.Task = taskCall.Task diff --git a/internal/taskfile/merge.go b/internal/taskfile/merge.go index d350e4b9..24b3ebb0 100644 --- a/internal/taskfile/merge.go +++ b/internal/taskfile/merge.go @@ -29,18 +29,13 @@ func Merge(t1, t2 *Taskfile, namespaces ...string) error { } if t1.Vars == nil { - t1.Vars = make(Vars) + t1.Vars = &Vars{} } - for k, v := range t2.Vars { - t1.Vars[k] = v - } - if t1.Env == nil { - t1.Env = make(Vars) - } - for k, v := range t2.Env { - t1.Env[k] = v + t1.Env = &Vars{} } + t1.Vars.Merge(t2.Vars) + t1.Env.Merge(t2.Env) if t1.Tasks == nil { t1.Tasks = make(Tasks) diff --git a/internal/taskfile/read/taskvars.go b/internal/taskfile/read/taskvars.go index f664cf04..cc5bcaa8 100644 --- a/internal/taskfile/read/taskvars.go +++ b/internal/taskfile/read/taskvars.go @@ -12,8 +12,8 @@ import ( ) // Taskvars reads a Taskvars for a given directory -func Taskvars(dir string) (taskfile.Vars, error) { - vars := make(taskfile.Vars) +func Taskvars(dir string) (*taskfile.Vars, error) { + vars := &taskfile.Vars{} path := filepath.Join(dir, "Taskvars.yml") if _, err := os.Stat(path); err == nil { @@ -29,24 +29,17 @@ func Taskvars(dir string) (taskfile.Vars, error) { if err != nil { return nil, err } - - if vars == nil { - vars = osVars - } else { - for k, v := range osVars { - vars[k] = v - } - } + vars.Merge(osVars) } return vars, nil } -func readTaskvars(file string) (taskfile.Vars, error) { +func readTaskvars(file string) (*taskfile.Vars, error) { f, err := os.Open(file) if err != nil { return nil, err } var vars taskfile.Vars - return vars, yaml.NewDecoder(f).Decode(&vars) + return &vars, yaml.NewDecoder(f).Decode(&vars) } diff --git a/internal/taskfile/task.go b/internal/taskfile/task.go index f00ee947..d39999cf 100644 --- a/internal/taskfile/task.go +++ b/internal/taskfile/task.go @@ -19,8 +19,8 @@ type Task struct { Status []string Preconditions []*Precondition Dir string - Vars Vars - Env Vars + Vars *Vars + Env *Vars Silent bool Method string Prefix string @@ -55,8 +55,8 @@ func (t *Task) UnmarshalYAML(unmarshal func(interface{}) error) error { Status []string Preconditions []*Precondition Dir string - Vars Vars - Env Vars + Vars *Vars + Env *Vars Silent bool Method string Prefix string diff --git a/internal/taskfile/taskfile.go b/internal/taskfile/taskfile.go index bcb7653c..b8cf5da9 100644 --- a/internal/taskfile/taskfile.go +++ b/internal/taskfile/taskfile.go @@ -7,8 +7,8 @@ type Taskfile struct { Output string Method string Includes IncludedTaskfiles - Vars Vars - Env Vars + Vars *Vars + Env *Vars Tasks Tasks Silent bool } @@ -21,8 +21,8 @@ func (tf *Taskfile) UnmarshalYAML(unmarshal func(interface{}) error) error { Output string Method string Includes IncludedTaskfiles - Vars Vars - Env Vars + Vars *Vars + Env *Vars Tasks Tasks Silent bool } @@ -41,8 +41,5 @@ func (tf *Taskfile) UnmarshalYAML(unmarshal func(interface{}) error) error { if tf.Expansions <= 0 { tf.Expansions = 2 } - if tf.Vars == nil { - tf.Vars = make(Vars) - } return nil } diff --git a/internal/taskfile/taskfile_test.go b/internal/taskfile/taskfile_test.go index 9b229a07..fcdd4bb1 100644 --- a/internal/taskfile/taskfile_test.go +++ b/internal/taskfile/taskfile_test.go @@ -33,9 +33,12 @@ vars: { yamlTaskCall, &taskfile.Cmd{}, - &taskfile.Cmd{Task: "another-task", Vars: taskfile.Vars{ - "PARAM1": taskfile.Var{Static: "VALUE1"}, - "PARAM2": taskfile.Var{Static: "VALUE2"}, + &taskfile.Cmd{Task: "another-task", Vars: &taskfile.Vars{ + Keys: []string{"PARAM1", "PARAM2"}, + Mapping: map[string]taskfile.Var{ + "PARAM1": taskfile.Var{Static: "VALUE1"}, + "PARAM2": taskfile.Var{Static: "VALUE2"}, + }, }}, }, { @@ -46,9 +49,12 @@ vars: { yamlTaskCall, &taskfile.Dep{}, - &taskfile.Dep{Task: "another-task", Vars: taskfile.Vars{ - "PARAM1": taskfile.Var{Static: "VALUE1"}, - "PARAM2": taskfile.Var{Static: "VALUE2"}, + &taskfile.Dep{Task: "another-task", Vars: &taskfile.Vars{ + Keys: []string{"PARAM1", "PARAM2"}, + Mapping: map[string]taskfile.Var{ + "PARAM1": taskfile.Var{Static: "VALUE1"}, + "PARAM2": taskfile.Var{Static: "VALUE2"}, + }, }}, }, } diff --git a/internal/taskfile/var.go b/internal/taskfile/var.go index 225aed1e..99946f8a 100644 --- a/internal/taskfile/var.go +++ b/internal/taskfile/var.go @@ -3,6 +3,8 @@ package taskfile import ( "errors" "strings" + + "gopkg.in/yaml.v3" ) var ( @@ -11,17 +13,86 @@ var ( ) // Vars is a string[string] variables map. -type Vars map[string]Var +type Vars struct { + Keys []string + Mapping map[string]Var +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (vs *Vars) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return errors.New("task: vars is not a map") + } + + // NOTE(@andreynering): on this style of custom unmarsheling, + // even number contains the keys, while odd numbers contains + // the values. + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + valueNode := node.Content[i+1] + + var v Var + if err := valueNode.Decode(&v); err != nil { + return err + } + vs.Set(keyNode.Value, v) + } + return nil +} + +// Merge merges the given Vars into the caller one +func (vs *Vars) Merge(other *Vars) { + other.Range(func(key string, value Var) error { + vs.Set(key, value) + return nil + }) +} + +// Set sets a value to a given key +func (vs *Vars) Set(key string, value Var) { + if vs == nil { + vs = &Vars{} + } + if vs.Mapping == nil { + vs.Mapping = make(map[string]Var, 1) + } + if !strSliceContains(vs.Keys, key) { + vs.Keys = append(vs.Keys, key) + } + vs.Mapping[key] = value +} + +func strSliceContains(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + return false +} + +// Range allows you to loop into the vars in its right order +func (vs *Vars) Range(yield func(key string, value Var) error) error { + if vs == nil { + return nil + } + for _, k := range vs.Keys { + if err := yield(k, vs.Mapping[k]); err != nil { + return err + } + } + return nil +} // ToCacheMap converts Vars to a map containing only the static // variables -func (vs Vars) ToCacheMap() (m map[string]interface{}) { - m = make(map[string]interface{}, len(vs)) - for k, v := range vs { +func (vs *Vars) ToCacheMap() (m map[string]interface{}) { + m = make(map[string]interface{}, len(vs.Keys)) + vs.Range(func(k string, v Var) error { if v.Sh != "" { // Dynamic variable is not yet resolved; trigger // to be used in templates. - continue + return nil } if v.Live != nil { @@ -29,7 +100,8 @@ func (vs Vars) ToCacheMap() (m map[string]interface{}) { } else { m[k] = v.Static } - } + return nil + }) return } diff --git a/internal/templater/templater.go b/internal/templater/templater.go index b395070a..b905b1ba 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -12,7 +12,7 @@ import ( // happen will be assigned to r.err, and consecutive calls to funcs will just // return the zero value. type Templater struct { - Vars taskfile.Vars + Vars *taskfile.Vars cacheMap map[string]interface{} err error @@ -57,20 +57,22 @@ func (r *Templater) ReplaceSlice(strs []string) []string { return new } -func (r *Templater) ReplaceVars(vars taskfile.Vars) taskfile.Vars { - if r.err != nil || len(vars) == 0 { +func (r *Templater) ReplaceVars(vars *taskfile.Vars) *taskfile.Vars { + if r.err != nil || vars == nil || len(vars.Keys) == 0 { return nil } - new := make(taskfile.Vars, len(vars)) - for k, v := range vars { - new[k] = taskfile.Var{ + var new taskfile.Vars + vars.Range(func(k string, v taskfile.Var) error { + new.Set(k, taskfile.Var{ Static: r.Replace(v.Static), Live: v.Live, Sh: r.Replace(v.Sh), - } - } - return new + }) + return nil + }) + + return &new } func (r *Templater) Err() error { diff --git a/task.go b/task.go index be6740ca..0e3eb327 100644 --- a/task.go +++ b/task.go @@ -52,7 +52,7 @@ type Executor struct { Output output.Output OutputStyle string - taskvars taskfile.Vars + taskvars *taskfile.Vars taskCallCount map[string]*int32 mkdirMutexMap map[string]*sync.Mutex diff --git a/variables.go b/variables.go index d3160171..a2ef97d8 100644 --- a/variables.go +++ b/variables.go @@ -50,19 +50,19 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { new.Prefix = new.Task } - new.Env = make(taskfile.Vars, len(e.Taskfile.Env)+len(origTask.Env)) - for k, v := range r.ReplaceVars(e.Taskfile.Env) { - new.Env[k] = v - } - for k, v := range r.ReplaceVars(origTask.Env) { - new.Env[k] = v - } - for k, v := range new.Env { + new.Env = &taskfile.Vars{} + new.Env.Merge(r.ReplaceVars(e.Taskfile.Env)) + new.Env.Merge(r.ReplaceVars(origTask.Env)) + err = new.Env.Range(func(k string, v taskfile.Var) error { static, err := e.Compiler.HandleDynamicVar(v) if err != nil { - return nil, err + return err } - new.Env[k] = taskfile.Var{Static: static} + new.Env.Set(k, taskfile.Var{Static: static}) + return nil + }) + if err != nil { + return nil, err } if len(origTask.Cmds) > 0 { @@ -103,7 +103,7 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { if err != nil { return nil, err } - vars[strings.ToUpper(checker.Kind())] = taskfile.Var{Live: value} + vars.Set(strings.ToUpper(checker.Kind()), taskfile.Var{Live: value}) } // Adding new variables, requires us to refresh the templaters