fix: disambiguate variable defaults with "empty" values from undefined

- Variable struct and json representation now have an explicit `required` field to disambiguate between a variable with a default value which may either be `null` or the variable type's zero value
hashicorp · Nov 15, 2019 · 0054cba · 0054cba
1 parent da6960d
commit 0054cba
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 21 deletions.
diff --git a/main.go b/main.go
@@ -99,7 +99,7 @@ Provider Requirements:
 
 ## Input Variables
 {{- range .Variables }}
-* {{ tt .Name }}{{ if .Default }} (default {{ json .Default | tt }}){{else}} (required){{end}}
+* {{ tt .Name }}{{ if .Required }} (required){{else}} (default {{ json .Default | tt }}){{end}}
 {{- if .Description}}: {{ .Description }}{{ end }}
 {{- end}}{{end}}
 

diff --git a/tfconfig/load_hcl.go b/tfconfig/load_hcl.go
@@ -7,8 +7,8 @@ import (
 
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 
-	"github.com/hashicorp/hcl/v2/gohcl"
 	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/gohcl"
 	"github.com/hashicorp/hcl/v2/hclparse"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 )
@@ -148,8 +148,10 @@ func loadModule(dir string) (*Module, Diagnostics) {
 							// guaranteed valid by ctyjson.Marshal.
 							panic(fmt.Errorf("failed to re-parse default value from JSON: %s", err))
 						}
-						v.Default = &Default{Value: def}
+						v.Default = def
 					}
+				} else {
+					v.Required = true
 				}
 
 			case "output":

diff --git a/tfconfig/load_legacy.go b/tfconfig/load_legacy.go
@@ -101,7 +101,8 @@ func loadModuleLegacyHCL(dir string) (*Module, Diagnostics) {
 					Name:        name,
 					Type:        block.Type,
 					Description: block.Description,
-					Default:     &Default{Value: block.Default},
+					Default:     block.Default,
+					Required:    block.Default == nil,
 					Pos:         sourcePosLegacyHCL(item.Pos(), filename),
 				}
 				if _, exists := mod.Variables[name]; exists {

diff --git a/tfconfig/test-fixtures/basics-json/basics-json.out.json b/tfconfig/test-fixtures/basics-json/basics-json.out.json
@@ -9,6 +9,7 @@
     "A": {
       "name": "A",
       "default": "A default",
+      "required": false,
       "pos": {
         "filename": "test-fixtures/basics-json/basics.tf.json",
         "line": 3
@@ -17,6 +18,8 @@
     "B": {
       "name": "B",
       "description": "The B variable",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "test-fixtures/basics-json/basics.tf.json",
         "line": 6

diff --git a/tfconfig/test-fixtures/basics/basics.out.json b/tfconfig/test-fixtures/basics/basics.out.json
@@ -9,6 +9,7 @@
     "A": {
       "name": "A",
       "default": "A default",
+      "required": false,
       "pos": {
         "filename": "test-fixtures/basics/basics.tf",
         "line": 1
@@ -17,6 +18,8 @@
     "B": {
       "name": "B",
       "description": "The B variable",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "test-fixtures/basics/basics.tf",
         "line": 5

diff --git a/tfconfig/test-fixtures/for-expression/for-expression.out.json b/tfconfig/test-fixtures/for-expression/for-expression.out.json
@@ -11,28 +11,31 @@
                 "one",
                 "two",
                 "three"
-            ]
+            ],
+            "required": false
         },
         "enabled": {
             "name": "enabled",
             "pos": {
                 "filename": "test-fixtures/for-expression/for-expression.tf",
                 "line": 4
             },
-            "default": true
+            "default": true,
+            "required": false
         },
         "retention_days": {
             "name": "retention_days",
             "pos": {
                 "filename": "test-fixtures/for-expression/for-expression.tf",
                 "line": 7
             },
-            "default": 7
+            "default": 7,
+            "required": false
         }
     },
     "required_providers": {},
     "outputs": {},
     "managed_resources": {},
     "data_resources": {},
     "module_calls": {}
-}
+}
diff --git a/tfconfig/test-fixtures/invalid-braces/invalid-braces.out.json b/tfconfig/test-fixtures/invalid-braces/invalid-braces.out.json
@@ -4,6 +4,7 @@
         "foo": {
             "name": "foo",
             "default": "123",
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/invalid-braces/invalid-braces.tf",
                 "line": 5

diff --git a/tfconfig/test-fixtures/legacy-block-labels/legacy-block-labels.out.json b/tfconfig/test-fixtures/legacy-block-labels/legacy-block-labels.out.json
@@ -14,6 +14,7 @@
             "name": "foo",
             "description": "foo description",
             "default": "foo default",
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/legacy-block-labels/legacy-block-labels.tf",
                 "line": 29

diff --git a/tfconfig/test-fixtures/overrides/overrides.out.json b/tfconfig/test-fixtures/overrides/overrides.out.json
@@ -9,6 +9,8 @@
     "A": {
       "name": "A",
       "description": "The A variable OVERRIDDEN",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "test-fixtures/overrides/overrides_override.tf",
         "line": 1
@@ -17,6 +19,8 @@
     "B": {
       "name": "B",
       "description": "The B variable",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "test-fixtures/overrides/overrides.tf",
         "line": 5
@@ -25,6 +29,8 @@
     "C": {
       "name": "C",
       "description": "An entirely new variable C",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "test-fixtures/overrides/overrides_override.tf",
         "line": 5

diff --git a/tfconfig/test-fixtures/type-conversions/type-conversions.out.json b/tfconfig/test-fixtures/type-conversions/type-conversions.out.json
@@ -13,6 +13,8 @@
             "name": "foo",
             "type": "true",
             "description": "true",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/type-conversions/type-conversions.tf",
                 "line": 1

diff --git a/tfconfig/test-fixtures/type-errors/type-errors.out.json b/tfconfig/test-fixtures/type-errors/type-errors.out.json
@@ -85,6 +85,8 @@
         "foo": {
             "name": "foo",
             "type": "{\"what\":\"the\"}",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/type-errors/type-errors.tf",
                 "line": 1

diff --git a/tfconfig/test-fixtures/variable-types/variable-types.out.json b/tfconfig/test-fixtures/variable-types/variable-types.out.json
@@ -6,6 +6,8 @@
     "variables": {
         "primitive": {
             "name": "primitive",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 1
@@ -14,6 +16,8 @@
         "list": {
             "name": "list",
             "type": "list(string)",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 4
@@ -22,6 +26,8 @@
         "list_json": {
             "name": "list_json",
             "type": "list(string)",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf.json",
                 "line": 3
@@ -30,6 +36,8 @@
         "map": {
             "name": "map",
             "type": "map",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 8
@@ -39,6 +47,7 @@
             "name": "string_default_empty",
             "type": "string",
             "default": "",
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 14
@@ -48,6 +57,7 @@
             "name": "string_default_null",
             "type": "string",
             "default": null,
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 19
@@ -57,6 +67,7 @@
             "name": "list_default_empty",
             "type": "list(string)",
             "default": [],
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 24
@@ -66,6 +77,7 @@
             "name": "object_default_empty",
             "type": "object({})",
             "default": {},
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 29
@@ -75,6 +87,7 @@
             "name": "number_default_zero",
             "type": "number",
             "default": 0,
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 34
@@ -84,6 +97,7 @@
             "name": "bool_default_false",
             "type": "bool",
             "default": false,
+            "required": false,
             "pos": {
                 "filename": "test-fixtures/variable-types/variable-types.tf",
                 "line": 39

diff --git a/tfconfig/variable.go b/tfconfig/variable.go
@@ -1,13 +1,5 @@
 package tfconfig
 
-import (
-	"encoding/json"
-)
-
-type Default struct {
-	Value interface{}
-}
-
 // Variable represents a single variable from a Terraform module.
 type Variable struct {
 	Name        string `json:"name"`
@@ -18,11 +10,8 @@ type Variable struct {
 	// the native Go type system. The conversion from the value given in
 	// configuration may be slightly lossy. Only values that can be
 	// serialized by json.Marshal will be included here.
-	Default *Default `json:"default,omitempty"`
+	Default  interface{} `json:"default"`
+	Required bool        `json:"required"`
 
 	Pos SourcePos `json:"pos"`
 }
-
-func (d *Default) MarshalJSON() ([]byte, error) {
-	return json.Marshal(&d.Value)
-}