From e4bcd3c44895ebbd17f3bc3de587f312c47aab2e Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Fri, 18 Jul 2014 17:48:30 -0700
Subject: [PATCH] config: Variables is now a slice

---
 config/config.go        |  9 +++++++--
 config/loader_libucl.go |  9 ++++++---
 config/loader_test.go   | 32 +++++++++++++++-----------------
 config/merge.go         | 15 +++++++++++----
 4 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/config/config.go b/config/config.go
index bcde29439..51b0deabf 100644
--- a/config/config.go
+++ b/config/config.go
@@ -15,7 +15,7 @@ import (
 type Config struct {
 	ProviderConfigs map[string]*ProviderConfig
 	Resources       []*Resource
-	Variables       map[string]*Variable
+	Variables       []*Variable
 	Outputs         map[string]*Output
 
 	// The fields below can be filled in by loaders for validation
@@ -51,6 +51,7 @@ type Provisioner struct {
 
 // Variable is a variable defined within the configuration.
 type Variable struct {
+	Name        string
 	Default     string
 	Description string
 	defaultSet  bool
@@ -124,6 +125,10 @@ func (c *Config) Validate() error {
 	}
 
 	vars := c.allVariables()
+	varMap := make(map[string]*Variable)
+	for _, v := range c.Variables {
+		varMap[v.Name] = v
+	}
 
 	// Check for references to user variables that do not actually
 	// exist and record those errors.
@@ -134,7 +139,7 @@ func (c *Config) Validate() error {
 				continue
 			}
 
-			if _, ok := c.Variables[uv.Name]; !ok {
+			if _, ok := varMap[uv.Name]; !ok {
 				errs = append(errs, fmt.Errorf(
 					"%s: unknown variable referenced: %s",
 					source,
diff --git a/config/loader_libucl.go b/config/loader_libucl.go
index 246a1cf67..15bceca41 100644
--- a/config/loader_libucl.go
+++ b/config/loader_libucl.go
@@ -45,8 +45,10 @@ func (t *libuclConfigurable) Config() (*Config, error) {
 
 	// Start building up the actual configuration. We start with
 	// variables.
+	// TODO(mitchellh): Make function like loadVariablesLibucl so that
+	// duplicates aren't overriden
 	config := new(Config)
-	config.Variables = make(map[string]*Variable)
+	config.Variables = make([]*Variable, 0, len(rawConfig.Variable))
 	for k, v := range rawConfig.Variable {
 		defaultSet := false
 		for _, f := range v.Fields {
@@ -56,11 +58,12 @@ func (t *libuclConfigurable) Config() (*Config, error) {
 			}
 		}
 
-		config.Variables[k] = &Variable{
+		config.Variables = append(config.Variables, &Variable{
+			Name:        k,
 			Default:     v.Default,
 			Description: v.Description,
 			defaultSet:  defaultSet,
-		}
+		})
 	}
 
 	// Build the provider configs
diff --git a/config/loader_test.go b/config/loader_test.go
index d51da23ee..c05e55843 100644
--- a/config/loader_test.go
+++ b/config/loader_test.go
@@ -116,16 +116,6 @@ func TestLoad_variables(t *testing.T) {
 	if actual != strings.TrimSpace(variablesVariablesStr) {
 		t.Fatalf("bad:\n%s", actual)
 	}
-
-	if !c.Variables["foo"].Required() {
-		t.Fatal("foo should be required")
-	}
-	if c.Variables["bar"].Required() {
-		t.Fatal("bar should not be required")
-	}
-	if c.Variables["baz"].Required() {
-		t.Fatal("baz should not be required")
-	}
 }
 
 func TestLoadDir_basic(t *testing.T) {
@@ -384,16 +374,18 @@ func resourcesStr(rs []*Resource) string {
 
 // This helper turns a variables field into a deterministic
 // string value for comparison in tests.
-func variablesStr(vs map[string]*Variable) string {
+func variablesStr(vs []*Variable) string {
 	result := ""
 	ks := make([]string, 0, len(vs))
-	for k, _ := range vs {
-		ks = append(ks, k)
+	m := make(map[string]*Variable)
+	for _, v := range vs {
+		ks = append(ks, v.Name)
+		m[v.Name] = v
 	}
 	sort.Strings(ks)
 
 	for _, k := range ks {
-		v := vs[k]
+		v := m[k]
 
 		if v.Default == "" {
 			v.Default = "<>"
@@ -402,9 +394,15 @@ func variablesStr(vs map[string]*Variable) string {
 			v.Description = "<>"
 		}
 
+		required := ""
+		if v.Required() {
+			required = " (required)"
+		}
+
 		result += fmt.Sprintf(
-			"%s\n  %s\n  %s\n",
+			"%s%s\n  %s\n  %s\n",
 			k,
+			required,
 			v.Default,
 			v.Description)
 	}
@@ -497,7 +495,7 @@ aws_security_group[web] (x1)
 `
 
 const importVariablesStr = `
-bar
+bar (required)
   <>
   <>
 foo
@@ -538,7 +536,7 @@ bar
 baz
   foo
   <>
-foo
+foo (required)
   <>
   <>
 `
diff --git a/config/merge.go b/config/merge.go
index be5bb39af..3d7a54794 100644
--- a/config/merge.go
+++ b/config/merge.go
@@ -26,9 +26,13 @@ func Merge(c1, c2 *Config) (*Config, error) {
 
 	// Merge variables: Variable merging is quite simple. Set fields in
 	// later set variables override those earlier.
-	c.Variables = c1.Variables
-	for k, v2 := range c2.Variables {
-		v1, ok := c.Variables[k]
+	c.Variables = make([]*Variable, 0, len(c1.Variables)+len(c2.Variables))
+	varMap := make(map[string]*Variable)
+	for _, v := range c1.Variables {
+		varMap[v.Name] = v
+	}
+	for _, v2 := range c2.Variables {
+		v1, ok := varMap[v2.Name]
 		if ok {
 			if v2.Default == "" {
 				v2.Default = v1.Default
@@ -38,7 +42,10 @@ func Merge(c1, c2 *Config) (*Config, error) {
 			}
 		}
 
-		c.Variables[k] = v2
+		varMap[v2.Name] = v2
+	}
+	for _, v := range varMap {
+		c.Variables = append(c.Variables, v)
 	}
 
 	// Merge outputs: If they collide, just take the latest one for now. In