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

@@ -140,9 +140,7 @@ func main() {
 	}
 
 	calls, globals := args.Parse(pflag.Args()...)
-	for name, value := range globals {
+	e.Taskfile.Vars.Merge(globals)
-		e.Taskfile.Vars[name] = value
-	}
 
 	ctx := context.Background()
 	if !watch {

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)
-			name, value := splitVar((arg))
+			calls[len(calls)-1].Vars.Set(name, taskfile.Var{Static: value})
-			calls[len(calls)-1].Vars[name] = taskfile.Var{Static: value}
 		}
 	}
 

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,41 +29,53 @@ func TestArgs(t *testing.T) {
 			ExpectedCalls: []taskfile.Call{
 				{
 					Task: "task-a",
-					Vars: taskfile.Vars{
+					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{
+					Vars: &taskfile.Vars{
+						Keys: []string{"BAR", "BAZ"},
+						Mapping: map[string]taskfile.Var{
 							"BAR": taskfile.Var{Static: "baz"},
 							"BAZ": taskfile.Var{Static: "foo"},
 						},
 					},
 				},
 			},
+		},
 		{
 			Args: []string{"task-a", "CONTENT=with some spaces"},
 			ExpectedCalls: []taskfile.Call{
 				{
 					Task: "task-a",
-					Vars: taskfile.Vars{
+					Vars: &taskfile.Vars{
+						Keys: []string{"CONTENT"},
+						Mapping: map[string]taskfile.Var{
 							"CONTENT": taskfile.Var{Static: "with some spaces"},
 						},
 					},
 				},
 			},
+		},
 		{
 			Args: []string{"FOO=bar", "task-a", "task-b"},
 			ExpectedCalls: []taskfile.Call{
 				{Task: "task-a"},
 				{Task: "task-b"},
 			},
-			ExpectedGlobals: taskfile.Vars{
+			ExpectedGlobals: &taskfile.Vars{
+				Keys: []string{"FOO"},
+				Mapping: map[string]taskfile.Var{
 					"FOO": {Static: "bar"},
 				},
 			},
+		},
 		{
 			Args: nil,
 			ExpectedCalls: []taskfile.Call{
@@ -81,11 +93,14 @@ func TestArgs(t *testing.T) {
 			ExpectedCalls: []taskfile.Call{
 				{Task: "default"},
 			},
-			ExpectedGlobals: taskfile.Vars{
+			ExpectedGlobals: &taskfile.Vars{
+				Keys: []string{"FOO", "BAR"},
+				Mapping: map[string]taskfile.Var{
 					"FOO": {Static: "bar"},
 					"BAR": {Static: "baz"},
 				},
 			},
+		},
 	}
 
 	for i, test := range tests {

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)
 }

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 {
+func GetEnviron() *taskfile.Vars {
-	var (
+	m := &taskfile.Vars{}
-		env = os.Environ()
+	for _, e := range os.Environ() {
-		m   = make(taskfile.Vars, len(env))
-	)
-
-	for _, e := range env {
 		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
 }

internal/compiler/v2/compiler_v2.go

@@ -19,8 +19,8 @@ var _ compiler.Compiler = &CompilerV2{}
 type CompilerV2 struct {
 	Dir string
 
-	Taskvars     taskfile.Vars
+	Taskvars     *taskfile.Vars
-	TaskfileVars 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}
+	vr.vars.Set("TASK", taskfile.Var{Static: t.Task})
-	for _, vars := range []taskfile.Vars{c.Taskvars, c.TaskfileVars, call.Vars, t.Vars} {
+	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()
 }
 

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
 }

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

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)
+		t1.Env = &Vars{}
-	}
-	for k, v := range t2.Env {
-		t1.Env[k] = v
 	}
+	t1.Vars.Merge(t2.Vars)
+	t1.Env.Merge(t2.Env)
 
 	if t1.Tasks == nil {
 		t1.Tasks = make(Tasks)

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) {
+func Taskvars(dir string) (*taskfile.Vars, error) {
-	vars := make(taskfile.Vars)
+	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
 		}
-
+		vars.Merge(osVars)
-		if vars == nil {
-			vars = osVars
-		} else {
-			for k, v := range osVars {
-				vars[k] = v
-			}
-		}
 	}
 
 	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)
 }

internal/taskfile/task.go

@@ -19,8 +19,8 @@ type Task struct {
 	Status        []string
 	Preconditions []*Precondition
 	Dir           string
-	Vars          Vars
+	Vars          *Vars
-	Env           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
+		Vars          *Vars
-		Env           Vars
+		Env           *Vars
 		Silent        bool
 		Method        string
 		Prefix        string

internal/taskfile/taskfile.go

@@ -7,8 +7,8 @@ type Taskfile struct {
 	Output     string
 	Method     string
 	Includes   IncludedTaskfiles
-	Vars       Vars
+	Vars       *Vars
-	Env        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
+		Vars       *Vars
-		Env        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
 }

internal/taskfile/taskfile_test.go

@@ -33,9 +33,12 @@ vars:
 		{
 			yamlTaskCall,
 			&taskfile.Cmd{},
-			&taskfile.Cmd{Task: "another-task", Vars: taskfile.Vars{
+			&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{
+			&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"},
+				},
 			}},
 		},
 	}

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{}) {
+func (vs *Vars) ToCacheMap() (m map[string]interface{}) {
-	m = make(map[string]interface{}, len(vs))
+	m = make(map[string]interface{}, len(vs.Keys))
-	for k, v := range vs {
+	vs.Range(func(k string, v Var) error {
 		if v.Sh != "" {
 			// Dynamic variable is not yet resolved; trigger
 			// <no value> 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
 }
 

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 {
+func (r *Templater) ReplaceVars(vars *taskfile.Vars) *taskfile.Vars {
-	if r.err != nil || len(vars) == 0 {
+	if r.err != nil || vars == nil || len(vars.Keys) == 0 {
 		return nil
 	}
 
-	new := make(taskfile.Vars, len(vars))
+	var new taskfile.Vars
-	for k, v := range vars {
+	vars.Range(func(k string, v taskfile.Var) error {
-		new[k] = taskfile.Var{
+		new.Set(k, taskfile.Var{
 			Static: r.Replace(v.Static),
 			Live:   v.Live,
 			Sh:     r.Replace(v.Sh),
-		}
+		})
-	}
+		return nil
-	return new
+	})
+
+	return &new
 }
 
 func (r *Templater) Err() error {

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

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))
+	new.Env = &taskfile.Vars{}
-	for k, v := range r.ReplaceVars(e.Taskfile.Env) {
+	new.Env.Merge(r.ReplaceVars(e.Taskfile.Env))
-		new.Env[k] = v
+	new.Env.Merge(r.ReplaceVars(origTask.Env))
-	}
+	err = new.Env.Range(func(k string, v taskfile.Var) error {
-	for k, v := range r.ReplaceVars(origTask.Env) {
-		new.Env[k] = v
-	}
-	for k, v := range new.Env {
 		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