From 8f7807684a7dd48043ef4c8636a62ff466be62c8 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Fri, 19 Feb 2021 10:39:49 -0500 Subject: [PATCH] Upgrade to quoted keywords to error The warning about deprecation is upgraded to an error --- .../validate-valid/with-tfvars-file/main.tf | 2 +- configs/named_values.go | 36 +++++++++---------- .../error-files/variable_type_quoted.tf | 11 ++++++ .../warning-files/variable_type_quoted.tf | 11 ------ .../main.tf | 6 ++-- .../amodule/main.tf | 4 +-- .../apply-map-var-through-module/main.tf | 20 +++++------ .../child/child.tf | 4 +-- .../input-submodule-count/mod/submod/main.tf | 4 +-- terraform/testdata/issue-7824/main.tf | 2 +- .../inner/main.tf | 10 +++--- .../plan-module-variable-from-splat/main.tf | 2 +- .../mod/main.tf | 6 ++-- .../inner/main.tf | 12 +++---- .../plan-module-wrong-var-type-nested/main.tf | 2 +- .../middle/main.tf | 16 ++++----- .../plan-module-wrong-var-type/inner/main.tf | 4 +-- .../plan-module-wrong-var-type/main.tf | 10 +++--- .../main.tf | 4 +-- terraform/testdata/vars-basic/main.tf | 12 +++---- terraform/variables_test.go | 12 +++---- 21 files changed, 95 insertions(+), 95 deletions(-) create mode 100644 configs/testdata/error-files/variable_type_quoted.tf delete mode 100644 configs/testdata/warning-files/variable_type_quoted.tf diff --git a/command/testdata/validate-valid/with-tfvars-file/main.tf b/command/testdata/validate-valid/with-tfvars-file/main.tf index 86a19b8fb..b318fbf0c 100644 --- a/command/testdata/validate-valid/with-tfvars-file/main.tf +++ b/command/testdata/validate-valid/with-tfvars-file/main.tf @@ -1,4 +1,4 @@ variable "var_without_default" { - type = "string" + type = string } diff --git a/configs/named_values.go b/configs/named_values.go index 3520484b2..61aa2f927 100644 --- a/configs/named_values.go +++ b/configs/named_values.go @@ -149,12 +149,12 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl.Diagnostics) { if exprIsNativeQuotedString(expr) { - // Here we're accepting the pre-0.12 form of variable type argument where - // the string values "string", "list" and "map" are accepted has a hint - // about the type used primarily for deciding how to parse values - // given on the command line and in environment variables. + // If a user provides the pre-0.12 form of variable type argument where + // the string values "string", "list" and "map" are accepted, we + // provide an error to point the user towards using the type system + // correctly has a hint. // Only the native syntax ends up in this codepath; we handle the - // JSON syntax (which is, of course, quoted even in the new format) + // JSON syntax (which is, of course, quoted within the type system) // in the normal codepath below. val, diags := expr.Value(nil) if diags.HasErrors() { @@ -164,33 +164,33 @@ func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl switch str { case "string": diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Quoted type constraints are deprecated", - Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. To silence this warning, remove the quotes around \"string\".", + Severity: hcl.DiagError, + Summary: "Invalid quoted type constraints", + Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"string\".", Subject: expr.Range().Ptr(), }) - return cty.String, VariableParseLiteral, diags + return cty.DynamicPseudoType, VariableParseLiteral, diags case "list": diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Quoted type constraints are deprecated", - Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. To silence this warning, remove the quotes around \"list\" and write list(string) instead to explicitly indicate that the list elements are strings.", + Severity: hcl.DiagError, + Summary: "Invalid quoted type constraints", + Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"list\" and write list(string) instead to explicitly indicate that the list elements are strings.", Subject: expr.Range().Ptr(), }) - return cty.List(cty.DynamicPseudoType), VariableParseHCL, diags + return cty.DynamicPseudoType, VariableParseHCL, diags case "map": diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Quoted type constraints are deprecated", - Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. To silence this warning, remove the quotes around \"map\" and write map(string) instead to explicitly indicate that the map elements are strings.", + Severity: hcl.DiagError, + Summary: "Invalid quoted type constraints", + Detail: "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"map\" and write map(string) instead to explicitly indicate that the map elements are strings.", Subject: expr.Range().Ptr(), }) - return cty.Map(cty.DynamicPseudoType), VariableParseHCL, diags + return cty.DynamicPseudoType, VariableParseHCL, diags default: return cty.DynamicPseudoType, VariableParseHCL, hcl.Diagnostics{{ Severity: hcl.DiagError, Summary: "Invalid legacy variable type hint", - Detail: `The legacy variable type hint form, using a quoted string, allows only the values "string", "list", and "map". To provide a full type expression, remove the surrounding quotes and give the type expression directly.`, + Detail: `To provide a full type expression, remove the surrounding quotes and give the type expression directly.`, Subject: expr.Range().Ptr(), }} } diff --git a/configs/testdata/error-files/variable_type_quoted.tf b/configs/testdata/error-files/variable_type_quoted.tf new file mode 100644 index 000000000..2292ce154 --- /dev/null +++ b/configs/testdata/error-files/variable_type_quoted.tf @@ -0,0 +1,11 @@ +variable "bad_string" { + type = "string" # ERROR: Invalid quoted type constraints +} + +variable "bad_map" { + type = "map" # ERROR: Invalid quoted type constraints +} + +variable "bad_list" { + type = "list" # ERROR: Invalid quoted type constraints +} diff --git a/configs/testdata/warning-files/variable_type_quoted.tf b/configs/testdata/warning-files/variable_type_quoted.tf deleted file mode 100644 index 9201ba62e..000000000 --- a/configs/testdata/warning-files/variable_type_quoted.tf +++ /dev/null @@ -1,11 +0,0 @@ -variable "bad_string" { - type = "string" # WARNING: Quoted type constraints are deprecated -} - -variable "bad_map" { - type = "map" # WARNING: Quoted type constraints are deprecated -} - -variable "bad_list" { - type = "list" # WARNING: Quoted type constraints are deprecated -} diff --git a/terraform/testdata/apply-destroy-mod-var-and-count-nested/main.tf b/terraform/testdata/apply-destroy-mod-var-and-count-nested/main.tf index 676ddc79b..58600cdb9 100644 --- a/terraform/testdata/apply-destroy-mod-var-and-count-nested/main.tf +++ b/terraform/testdata/apply-destroy-mod-var-and-count-nested/main.tf @@ -1,9 +1,9 @@ variable "mod_count_root" { - type = "string" + type = string default = "3" } module "child" { - source = "./child" - mod_count_child = "${var.mod_count_root}" + source = "./child" + mod_count_child = var.mod_count_root } diff --git a/terraform/testdata/apply-map-var-through-module/amodule/main.tf b/terraform/testdata/apply-map-var-through-module/amodule/main.tf index ce2cc8c78..a5284966e 100644 --- a/terraform/testdata/apply-map-var-through-module/amodule/main.tf +++ b/terraform/testdata/apply-map-var-through-module/amodule/main.tf @@ -1,9 +1,9 @@ variable "amis" { - type = "map" + type = map(string) } resource "null_resource" "noop" {} output "amis_out" { - value = "${var.amis}" + value = var.amis } diff --git a/terraform/testdata/apply-map-var-through-module/main.tf b/terraform/testdata/apply-map-var-through-module/main.tf index 991a0ecf6..4cec4a678 100644 --- a/terraform/testdata/apply-map-var-through-module/main.tf +++ b/terraform/testdata/apply-map-var-through-module/main.tf @@ -1,19 +1,19 @@ variable "amis_in" { - type = "map" - default = { - "us-west-1" = "ami-123456" - "us-west-2" = "ami-456789" - "eu-west-1" = "ami-789012" - "eu-west-2" = "ami-989484" - } + type = map(string) + default = { + "us-west-1" = "ami-123456" + "us-west-2" = "ami-456789" + "eu-west-1" = "ami-789012" + "eu-west-2" = "ami-989484" + } } module "test" { - source = "./amodule" + source = "./amodule" - amis = "${var.amis_in}" + amis = var.amis_in } output "amis_from_module" { - value = "${module.test.amis_out}" + value = module.test.amis_out } diff --git a/terraform/testdata/apply-multi-var-comprehensive/child/child.tf b/terraform/testdata/apply-multi-var-comprehensive/child/child.tf index 5d925d93b..8fe7df7c2 100644 --- a/terraform/testdata/apply-multi-var-comprehensive/child/child.tf +++ b/terraform/testdata/apply-multi-var-comprehensive/child/child.tf @@ -2,11 +2,11 @@ variable "num" { } variable "source_ids" { - type = "list" + type = list(string) } variable "source_names" { - type = "list" + type = list(string) } resource "test_thing" "multi_count_var" { diff --git a/terraform/testdata/input-submodule-count/mod/submod/main.tf b/terraform/testdata/input-submodule-count/mod/submod/main.tf index c0c8d15af..732ce43b1 100644 --- a/terraform/testdata/input-submodule-count/mod/submod/main.tf +++ b/terraform/testdata/input-submodule-count/mod/submod/main.tf @@ -1,7 +1,7 @@ variable "list" { - type = "list" + type = list(string) } resource "aws_instance" "bar" { - count = "${var.list[0]}" + count = var.list[0] } diff --git a/terraform/testdata/issue-7824/main.tf b/terraform/testdata/issue-7824/main.tf index 835254b65..ec76bc392 100644 --- a/terraform/testdata/issue-7824/main.tf +++ b/terraform/testdata/issue-7824/main.tf @@ -1,5 +1,5 @@ variable "test" { - type = "map" + type = map(string) default = { "test" = "1" } diff --git a/terraform/testdata/plan-module-var-with-default-value/inner/main.tf b/terraform/testdata/plan-module-var-with-default-value/inner/main.tf index 8a089655a..5b5cf6cdf 100644 --- a/terraform/testdata/plan-module-var-with-default-value/inner/main.tf +++ b/terraform/testdata/plan-module-var-with-default-value/inner/main.tf @@ -1,12 +1,12 @@ variable "im_a_string" { - type = "string" + type = string } variable "service_region_ami" { - type = "map" - default = { - us-east-1 = "ami-e4c9db8e" - } + type = map(string) + default = { + us-east-1 = "ami-e4c9db8e" + } } resource "null_resource" "noop" {} diff --git a/terraform/testdata/plan-module-variable-from-splat/main.tf b/terraform/testdata/plan-module-variable-from-splat/main.tf index 7212325ec..be900a3c4 100644 --- a/terraform/testdata/plan-module-variable-from-splat/main.tf +++ b/terraform/testdata/plan-module-variable-from-splat/main.tf @@ -5,5 +5,5 @@ module "mod1" { module "mod2" { source = "./mod" - param = ["${module.mod1.out_from_splat[0]}"] + param = [module.mod1.out_from_splat[0]] } diff --git a/terraform/testdata/plan-module-variable-from-splat/mod/main.tf b/terraform/testdata/plan-module-variable-from-splat/mod/main.tf index 28d9175d2..66127d36b 100644 --- a/terraform/testdata/plan-module-variable-from-splat/mod/main.tf +++ b/terraform/testdata/plan-module-variable-from-splat/mod/main.tf @@ -1,12 +1,12 @@ variable "param" { - type = "list" + type = list(string) } resource "aws_instance" "test" { - count = "2" + count = "2" thing = "doesnt" } output "out_from_splat" { - value = ["${aws_instance.test.*.thing}"] + value = aws_instance.test.*.thing } diff --git a/terraform/testdata/plan-module-wrong-var-type-nested/inner/main.tf b/terraform/testdata/plan-module-wrong-var-type-nested/inner/main.tf index 88995119d..dabe507fe 100644 --- a/terraform/testdata/plan-module-wrong-var-type-nested/inner/main.tf +++ b/terraform/testdata/plan-module-wrong-var-type-nested/inner/main.tf @@ -1,13 +1,13 @@ variable "inner_in" { - type = "map" - default = { - us-west-1 = "ami-12345" - us-west-2 = "ami-67890" - } + type = map(string) + default = { + us-west-1 = "ami-12345" + us-west-2 = "ami-67890" + } } resource "null_resource" "inner_noop" {} output "inner_out" { - value = "${lookup(var.inner_in, "us-west-1")}" + value = lookup(var.inner_in, "us-west-1") } diff --git a/terraform/testdata/plan-module-wrong-var-type-nested/main.tf b/terraform/testdata/plan-module-wrong-var-type-nested/main.tf index fe63fc5f8..8f9fdcc56 100644 --- a/terraform/testdata/plan-module-wrong-var-type-nested/main.tf +++ b/terraform/testdata/plan-module-wrong-var-type-nested/main.tf @@ -1,3 +1,3 @@ module "middle" { - source = "./middle" + source = "./middle" } diff --git a/terraform/testdata/plan-module-wrong-var-type-nested/middle/main.tf b/terraform/testdata/plan-module-wrong-var-type-nested/middle/main.tf index 1e8235761..eb989fe93 100644 --- a/terraform/testdata/plan-module-wrong-var-type-nested/middle/main.tf +++ b/terraform/testdata/plan-module-wrong-var-type-nested/middle/main.tf @@ -1,19 +1,19 @@ variable "middle_in" { - type = "map" - default = { - eu-west-1 = "ami-12345" - eu-west-2 = "ami-67890" - } + type = map(string) + default = { + eu-west-1 = "ami-12345" + eu-west-2 = "ami-67890" + } } module "inner" { - source = "../inner" + source = "../inner" - inner_in = "hello" + inner_in = "hello" } resource "null_resource" "middle_noop" {} output "middle_out" { - value = "${lookup(var.middle_in, "us-west-1")}" + value = lookup(var.middle_in, "us-west-1") } diff --git a/terraform/testdata/plan-module-wrong-var-type/inner/main.tf b/terraform/testdata/plan-module-wrong-var-type/inner/main.tf index c36aef10c..7782d1b84 100644 --- a/terraform/testdata/plan-module-wrong-var-type/inner/main.tf +++ b/terraform/testdata/plan-module-wrong-var-type/inner/main.tf @@ -1,5 +1,5 @@ variable "map_in" { - type = "map" + type = map(string) default = { us-west-1 = "ami-12345" @@ -9,5 +9,5 @@ variable "map_in" { // We have to reference it so it isn't pruned output "output" { - value = "${var.map_in}" + value = var.map_in } diff --git a/terraform/testdata/plan-module-wrong-var-type/main.tf b/terraform/testdata/plan-module-wrong-var-type/main.tf index 4fc7f8a7c..5a39cd5d5 100644 --- a/terraform/testdata/plan-module-wrong-var-type/main.tf +++ b/terraform/testdata/plan-module-wrong-var-type/main.tf @@ -1,10 +1,10 @@ variable "input" { - type = "string" - default = "hello world" + type = string + default = "hello world" } module "test" { - source = "./inner" + source = "./inner" - map_in = "${var.input}" -} + map_in = var.input +} diff --git a/terraform/testdata/validate-var-no-default-explicit-type/main.tf b/terraform/testdata/validate-var-no-default-explicit-type/main.tf index 6bd2bdd5e..5953eab4d 100644 --- a/terraform/testdata/validate-var-no-default-explicit-type/main.tf +++ b/terraform/testdata/validate-var-no-default-explicit-type/main.tf @@ -1,5 +1,5 @@ variable "maybe_a_map" { - type = "map" + type = map(string) - // No default + // No default } diff --git a/terraform/testdata/vars-basic/main.tf b/terraform/testdata/vars-basic/main.tf index 66fa77a8b..af3ba5cc6 100644 --- a/terraform/testdata/vars-basic/main.tf +++ b/terraform/testdata/vars-basic/main.tf @@ -1,14 +1,14 @@ variable "a" { - default = "foo" - type = "string" + default = "foo" + type = string } variable "b" { - default = [] - type = "list" + default = [] + type = list(string) } variable "c" { - default = {} - type = "map" + default = {} + type = map(string) } diff --git a/terraform/variables_test.go b/terraform/variables_test.go index 3ac112ebe..320a6740d 100644 --- a/terraform/variables_test.go +++ b/terraform/variables_test.go @@ -30,21 +30,21 @@ func TestVariables(t *testing.T) { }, }, "b": &InputValue{ - Value: cty.ListValEmpty(cty.DynamicPseudoType), + Value: cty.ListValEmpty(cty.String), SourceType: ValueFromConfig, SourceRange: tfdiags.SourceRange{ Filename: "testdata/vars-basic/main.tf", - Start: tfdiags.SourcePos{Line: 6, Column: 1, Byte: 58}, - End: tfdiags.SourcePos{Line: 6, Column: 13, Byte: 70}, + Start: tfdiags.SourcePos{Line: 6, Column: 1, Byte: 55}, + End: tfdiags.SourcePos{Line: 6, Column: 13, Byte: 67}, }, }, "c": &InputValue{ - Value: cty.MapValEmpty(cty.DynamicPseudoType), + Value: cty.MapValEmpty(cty.String), SourceType: ValueFromConfig, SourceRange: tfdiags.SourceRange{ Filename: "testdata/vars-basic/main.tf", - Start: tfdiags.SourcePos{Line: 11, Column: 1, Byte: 111}, - End: tfdiags.SourcePos{Line: 11, Column: 13, Byte: 123}, + Start: tfdiags.SourcePos{Line: 11, Column: 1, Byte: 113}, + End: tfdiags.SourcePos{Line: 11, Column: 13, Byte: 125}, }, }, },