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
jstewmon · May 18, 2020 · 27428fa · 27428fa
1 parent 36d6f40
commit 27428fa
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 6 deletions.
diff --git a/go.mod b/go.mod
@@ -12,3 +12,5 @@ require (
 	github.com/stretchr/testify v1.3.0 // indirect
 	github.com/zclconf/go-cty v1.1.0
 )
+
+go 1.13
diff --git a/tfconfig/load_hcl.go b/tfconfig/load_hcl.go
@@ -149,6 +149,8 @@ func loadModule(dir string) (*Module, Diagnostics) {
 						}
 						v.Default = def
 					}
+				} else {
+					v.Required = true
 				}
 
 			case "output":

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

diff --git a/tfconfig/markdown.go b/tfconfig/markdown.go
@@ -56,7 +56,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/testdata/basics-json/basics-json.out.json b/tfconfig/testdata/basics-json/basics-json.out.json
@@ -7,6 +7,7 @@
     "A": {
       "name": "A",
       "default": "A default",
+      "required": false,
       "pos": {
         "filename": "testdata/basics-json/basics.tf.json",
         "line": 3
@@ -15,6 +16,8 @@
     "B": {
       "name": "B",
       "description": "The B variable",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "testdata/basics-json/basics.tf.json",
         "line": 6

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

diff --git a/tfconfig/testdata/for-expression/for-expression.out.json b/tfconfig/testdata/for-expression/for-expression.out.json
@@ -11,28 +11,31 @@
                 "one",
                 "two",
                 "three"
-            ]
+            ],
+            "required": false
         },
         "enabled": {
             "name": "enabled",
             "pos": {
                 "filename": "testdata/for-expression/for-expression.tf",
                 "line": 4
             },
-            "default": true
+            "default": true,
+            "required": false
         },
         "retention_days": {
             "name": "retention_days",
             "pos": {
                 "filename": "testdata/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/testdata/invalid-braces/invalid-braces.out.json b/tfconfig/testdata/invalid-braces/invalid-braces.out.json
@@ -4,6 +4,7 @@
         "foo": {
             "name": "foo",
             "default": "123",
+            "required": false,
             "pos": {
                 "filename": "testdata/invalid-braces/invalid-braces.tf",
                 "line": 5

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

diff --git a/tfconfig/testdata/overrides/overrides.out.json b/tfconfig/testdata/overrides/overrides.out.json
@@ -7,6 +7,8 @@
     "A": {
       "name": "A",
       "description": "The A variable OVERRIDDEN",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "testdata/overrides/overrides_override.tf",
         "line": 1
@@ -15,6 +17,8 @@
     "B": {
       "name": "B",
       "description": "The B variable",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "testdata/overrides/overrides.tf",
         "line": 5
@@ -23,6 +27,8 @@
     "C": {
       "name": "C",
       "description": "An entirely new variable C",
+      "default": null,
+      "required": true,
       "pos": {
         "filename": "testdata/overrides/overrides_override.tf",
         "line": 5

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

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

diff --git a/tfconfig/testdata/variable-types/variable-types.out.json b/tfconfig/testdata/variable-types/variable-types.out.json
@@ -6,6 +6,8 @@
     "variables": {
         "primitive": {
             "name": "primitive",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "testdata/variable-types/variable-types.tf",
                 "line": 1
@@ -14,6 +16,8 @@
         "list": {
             "name": "list",
             "type": "list(string)",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "testdata/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": "testdata/variable-types/variable-types.tf.json",
                 "line": 3
@@ -30,10 +36,72 @@
         "map": {
             "name": "map",
             "type": "map",
+            "default": null,
+            "required": true,
             "pos": {
                 "filename": "testdata/variable-types/variable-types.tf",
                 "line": 8
             }
+        },
+        "string_default_empty": {
+            "name": "string_default_empty",
+            "type": "string",
+            "default": "",
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 14
+            }
+        },
+        "string_default_null": {
+            "name": "string_default_null",
+            "type": "string",
+            "default": null,
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 19
+            }
+        },
+        "list_default_empty": {
+            "name": "list_default_empty",
+            "type": "list(string)",
+            "default": [],
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 24
+            }
+        },
+        "object_default_empty": {
+            "name": "object_default_empty",
+            "type": "object({})",
+            "default": {},
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 29
+            }
+        },
+        "number_default_zero": {
+            "name": "number_default_zero",
+            "type": "number",
+            "default": 0,
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 34
+            }
+        },
+        "bool_default_false": {
+            "name": "bool_default_false",
+            "type": "bool",
+            "default": false,
+            "required": false,
+            "pos": {
+                "filename": "testdata/variable-types/variable-types.tf",
+                "line": 39
+            }
         }
     },
     "outputs": {},

diff --git a/tfconfig/testdata/variable-types/variable-types.out.md b/tfconfig/testdata/variable-types/variable-types.out.md
@@ -2,8 +2,14 @@
 # Module `testdata/variable-types`
 
 ## Input Variables
+* `bool_default_false` (default `false`)
 * `list` (required)
+* `list_default_empty` (default `[]`)
 * `list_json` (required)
 * `map` (required)
+* `number_default_zero` (default `0`)
+* `object_default_empty` (default `{}`)
 * `primitive` (required)
+* `string_default_empty` (default `""`)
+* `string_default_null` (default `null`)
 
diff --git a/tfconfig/testdata/variable-types/variable-types.tf b/tfconfig/testdata/variable-types/variable-types.tf
@@ -10,3 +10,33 @@ variable "map" {
   # with older configurations.
   type = "map"
 }
+
+variable "string_default_empty" {
+  type    = string
+  default = ""
+}
+
+variable "string_default_null" {
+  type    = string
+  default = null
+}
+
+variable "list_default_empty" {
+  type    = list(string)
+  default = []
+}
+
+variable "object_default_empty" {
+  type    = object({})
+  default = {}
+}
+
+variable "number_default_zero" {
+  type    = number
+  default = 0
+}
+
+variable "bool_default_false" {
+  type    = bool
+  default = false
+}
diff --git a/tfconfig/variable.go b/tfconfig/variable.go
@@ -10,7 +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 interface{} `json:"default,omitempty"`
+	Default  interface{} `json:"default"`
+	Required bool        `json:"required"`
 
 	Pos SourcePos `json:"pos"`
 }