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

Author: andreynering

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 {

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

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"},
+				},
 			},
 		},
 	}

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

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,27 +49,28 @@ 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),
 		}
 		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)
-	}
-	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)

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