diff --git a/command/testdata/validate-invalid/incorrectmodulename/main.tf b/command/testdata/validate-invalid/incorrectmodulename/main.tf index e58d0adeb..45509406c 100644 --- a/command/testdata/validate-invalid/incorrectmodulename/main.tf +++ b/command/testdata/validate-invalid/incorrectmodulename/main.tf @@ -2,5 +2,5 @@ module "super#module" { } module "super" { - source = "${var.modulename}" + source = var.modulename } diff --git a/command/testdata/validate-invalid/incorrectmodulename/output.json b/command/testdata/validate-invalid/incorrectmodulename/output.json index 6fb8b2cad..ee1679c6c 100644 --- a/command/testdata/validate-invalid/incorrectmodulename/output.json +++ b/command/testdata/validate-invalid/incorrectmodulename/output.json @@ -1,7 +1,7 @@ { "valid": false, "error_count": 6, - "warning_count": 1, + "warning_count": 0, "diagnostics": [ { "severity": "error", @@ -40,9 +40,9 @@ } }, { - "severity": "warning", - "summary": "Interpolation-only expressions are deprecated", - "detail": "Terraform 0.11 and earlier required all non-constant expressions to be provided via interpolation syntax, but this pattern is now deprecated. To silence this warning, remove the \"${ sequence from the start and the }\" sequence from the end of this expression, leaving just the inner expression.\n\nTemplate interpolation syntax is still used to construct strings from expressions when the template includes multiple interpolation sequences or a mixture of literal strings and interpolations. This deprecation applies only to templates that consist entirely of a single interpolation sequence.", + "severity": "error", + "summary": "Variables not allowed", + "detail": "Variables may not be used here.", "range": { "filename": "testdata/validate-invalid/incorrectmodulename/main.tf", "start": { @@ -51,27 +51,9 @@ "byte": 55 }, "end": { - "line": 5, - "column": 31, - "byte": 74 - } - } - }, - { - "severity": "error", - "summary": "Variables not allowed", - "detail": "Variables may not be used here.", - "range": { - "filename": "testdata/validate-invalid/incorrectmodulename/main.tf", - "start": { "line": 5, "column": 15, "byte": 58 - }, - "end": { - "line": 5, - "column": 18, - "byte": 61 } } }, @@ -88,8 +70,8 @@ }, "end": { "line": 5, - "column": 31, - "byte": 74 + "column": 26, + "byte": 69 } } }, diff --git a/command/testdata/validate-invalid/missing_var/main.tf b/command/testdata/validate-invalid/missing_var/main.tf index 385828cb9..37a77555e 100644 --- a/command/testdata/validate-invalid/missing_var/main.tf +++ b/command/testdata/validate-invalid/missing_var/main.tf @@ -3,6 +3,6 @@ resource "test_instance" "foo" { network_interface { device_index = 0 - description = "${var.description}" + description = var.description } } diff --git a/command/testdata/validate-invalid/missing_var/output.json b/command/testdata/validate-invalid/missing_var/output.json index 677e5f634..cd9bae6b8 100644 --- a/command/testdata/validate-invalid/missing_var/output.json +++ b/command/testdata/validate-invalid/missing_var/output.json @@ -1,12 +1,12 @@ { "valid": false, "error_count": 1, - "warning_count": 1, + "warning_count": 0, "diagnostics": [ { - "severity": "warning", - "summary": "Interpolation-only expressions are deprecated", - "detail": "Terraform 0.11 and earlier required all non-constant expressions to be provided via interpolation syntax, but this pattern is now deprecated. To silence this warning, remove the \"${ sequence from the start and the }\" sequence from the end of this expression, leaving just the inner expression.\n\nTemplate interpolation syntax is still used to construct strings from expressions when the template includes multiple interpolation sequences or a mixture of literal strings and interpolations. This deprecation applies only to templates that consist entirely of a single interpolation sequence.", + "severity": "error", + "summary": "Reference to undeclared input variable", + "detail": "An input variable with the name \"description\" has not been declared. This variable can be declared with a variable \"description\" {} block.", "range": { "filename": "testdata/validate-invalid/missing_var/main.tf", "start": { @@ -16,26 +16,8 @@ }, "end": { "line": 6, - "column": 41, - "byte": 137 - } - } - }, - { - "severity": "error", - "summary": "Reference to undeclared input variable", - "detail": "An input variable with the name \"description\" has not been declared. This variable can be declared with a variable \"description\" {} block.", - "range": { - "filename": "testdata/validate-invalid/missing_var/main.tf", - "start": { - "line": 6, - "column": 24, - "byte": 120 - }, - "end": { - "line": 6, - "column": 39, - "byte": 135 + "column": 36, + "byte": 132 } } } diff --git a/configs/compat_shim.go b/configs/compat_shim.go index 47f512b50..79360201e 100644 --- a/configs/compat_shim.go +++ b/configs/compat_shim.go @@ -107,121 +107,3 @@ func shimIsIgnoreChangesStar(expr hcl.Expression) bool { } return val.AsString() == "*" } - -// warnForDeprecatedInterpolations returns warning diagnostics if the given -// body can be proven to contain attributes whose expressions are native -// syntax expressions consisting entirely of a single template interpolation, -// which is a deprecated way to include a non-literal value in configuration. -// -// This is a best-effort sort of thing which relies on the physical HCL native -// syntax AST, so it might not catch everything. The main goal is to catch the -// "obvious" cases in order to help spread awareness that this old form is -// deprecated, when folks copy it from older examples they've found on the -// internet that were written for Terraform 0.11 or earlier. -func warnForDeprecatedInterpolationsInBody(body hcl.Body) hcl.Diagnostics { - var diags hcl.Diagnostics - - nativeBody, ok := body.(*hclsyntax.Body) - if !ok { - // If it's not native syntax then we've nothing to do here. - return diags - } - - for _, attr := range nativeBody.Attributes { - moreDiags := warnForDeprecatedInterpolationsInExpr(attr.Expr) - diags = append(diags, moreDiags...) - } - - for _, block := range nativeBody.Blocks { - // We'll also go hunting in nested blocks - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - } - - return diags -} - -func warnForDeprecatedInterpolationsInExpr(expr hcl.Expression) hcl.Diagnostics { - node, ok := expr.(hclsyntax.Node) - if !ok { - return nil - } - - walker := warnForDeprecatedInterpolationsWalker{ - // create some capacity so that we can deal with simple expressions - // without any further allocation during our walk. - contextStack: make([]warnForDeprecatedInterpolationsContext, 0, 16), - } - return hclsyntax.Walk(node, &walker) -} - -// warnForDeprecatedInterpolationsWalker is an implementation of -// hclsyntax.Walker that we use to generate deprecation warnings for template -// expressions that consist entirely of a single interpolation directive. -// That's always redundant in Terraform v0.12 and later, but tends to show up -// when people work from examples written for Terraform v0.11 or earlier. -type warnForDeprecatedInterpolationsWalker struct { - contextStack []warnForDeprecatedInterpolationsContext -} - -var _ hclsyntax.Walker = (*warnForDeprecatedInterpolationsWalker)(nil) - -type warnForDeprecatedInterpolationsContext int - -const ( - warnForDeprecatedInterpolationsNormal warnForDeprecatedInterpolationsContext = 0 - warnForDeprecatedInterpolationsObjKey warnForDeprecatedInterpolationsContext = 1 -) - -func (w *warnForDeprecatedInterpolationsWalker) Enter(node hclsyntax.Node) hcl.Diagnostics { - var diags hcl.Diagnostics - - context := warnForDeprecatedInterpolationsNormal - switch node := node.(type) { - case *hclsyntax.ObjectConsKeyExpr: - context = warnForDeprecatedInterpolationsObjKey - case *hclsyntax.TemplateWrapExpr: - // hclsyntax.TemplateWrapExpr is a special node type used by HCL only - // for the situation where a template is just a single interpolation, - // so we don't need to do anything further to distinguish that - // situation. ("normal" templates are *hclsyntax.TemplateExpr.) - - const summary = "Interpolation-only expressions are deprecated" - switch w.currentContext() { - case warnForDeprecatedInterpolationsObjKey: - // This case requires a different resolution in order to retain - // the same meaning, so we have a different detail message for - // it. - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: summary, - Detail: "Terraform 0.11 and earlier required all non-constant expressions to be provided via interpolation syntax, but this pattern is now deprecated.\n\nTo silence this warning, replace the \"${ opening sequence and the }\" closing sequence with opening and closing parentheses respectively. Parentheses are needed here to mark this as an expression to be evaluated, rather than as a literal string key.\n\nTemplate interpolation syntax is still used to construct strings from expressions when the template includes multiple interpolation sequences or a mixture of literal strings and interpolations. This deprecation applies only to templates that consist entirely of a single interpolation sequence.", - Subject: node.Range().Ptr(), - }) - default: - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: summary, - Detail: "Terraform 0.11 and earlier required all non-constant expressions to be provided via interpolation syntax, but this pattern is now deprecated. To silence this warning, remove the \"${ sequence from the start and the }\" sequence from the end of this expression, leaving just the inner expression.\n\nTemplate interpolation syntax is still used to construct strings from expressions when the template includes multiple interpolation sequences or a mixture of literal strings and interpolations. This deprecation applies only to templates that consist entirely of a single interpolation sequence.", - Subject: node.Range().Ptr(), - }) - } - } - - // Note the context of the current node for when we potentially visit - // child nodes. - w.contextStack = append(w.contextStack, context) - return diags -} - -func (w *warnForDeprecatedInterpolationsWalker) Exit(node hclsyntax.Node) hcl.Diagnostics { - w.contextStack = w.contextStack[:len(w.contextStack)-1] - return nil -} - -func (w *warnForDeprecatedInterpolationsWalker) currentContext() warnForDeprecatedInterpolationsContext { - if len(w.contextStack) == 0 { - return warnForDeprecatedInterpolationsNormal - } - return w.contextStack[len(w.contextStack)-1] -} diff --git a/configs/compat_shim_test.go b/configs/compat_shim_test.go deleted file mode 100644 index f6068bce9..000000000 --- a/configs/compat_shim_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package configs - -import ( - "strings" - "testing" - - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" -) - -func TestWarnForDeprecatedInterpolationsInExpr(t *testing.T) { - tests := []struct { - Expr string - WantSubstr string - }{ - { - `"${foo}"`, - "leaving just the inner expression", - }, - { - `{"${foo}" = 1}`, - // Special message for object key expressions, because just - // removing the interpolation markers would change the meaning - // in that context. - "opening and closing parentheses respectively", - }, - { - `{upper("${foo}") = 1}`, - // But no special message if the template is just descended from an - // object key, because the special interpretation applies only to - // a naked reference in te object key position. - "leaving just the inner expression", - }, - } - - for _, test := range tests { - t.Run(test.Expr, func(t *testing.T) { - expr, diags := hclsyntax.ParseExpression([]byte(test.Expr), "", hcl.InitialPos) - if diags.HasErrors() { - t.Fatalf("parse error: %s", diags.Error()) - } - - diags = warnForDeprecatedInterpolationsInExpr(expr) - if !diagWarningsContainSubstring(diags, test.WantSubstr) { - t.Errorf("wrong warning message\nwant detail substring: %s\ngot: %s", test.WantSubstr, diags.Error()) - } - }) - } -} - -func diagWarningsContainSubstring(diags hcl.Diagnostics, want string) bool { - for _, diag := range diags { - if diag.Severity != hcl.DiagWarning { - continue - } - if strings.Contains(diag.Detail, want) { - return true - } - } - return false -} diff --git a/configs/module_call.go b/configs/module_call.go index eefb3f2cb..6d1678ce9 100644 --- a/configs/module_call.go +++ b/configs/module_call.go @@ -33,11 +33,6 @@ type ModuleCall struct { func decodeModuleBlock(block *hcl.Block, override bool) (*ModuleCall, hcl.Diagnostics) { var diags hcl.Diagnostics - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - mc := &ModuleCall{ Name: block.Labels[0], DeclRange: block.DefRange, diff --git a/configs/named_values.go b/configs/named_values.go index 375d1e288..3520484b2 100644 --- a/configs/named_values.go +++ b/configs/named_values.go @@ -453,11 +453,6 @@ func decodeOutputBlock(block *hcl.Block, override bool) (*Output, hcl.Diagnostic schema = schemaForOverrides(schema) } - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - content, moreDiags := block.Body.Content(schema) diags = append(diags, moreDiags...) @@ -522,11 +517,6 @@ func decodeLocalsBlock(block *hcl.Block) ([]*Local, hcl.Diagnostics) { }) } - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. - moreDiags := warnForDeprecatedInterpolationsInExpr(attr.Expr) - diags = append(diags, moreDiags...) - locals = append(locals, &Local{ Name: name, Expr: attr.Expr, diff --git a/configs/provider.go b/configs/provider.go index 8317cc830..7fd39b174 100644 --- a/configs/provider.go +++ b/configs/provider.go @@ -30,13 +30,6 @@ type Provider struct { func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { var diags hcl.Diagnostics - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. We do this up front here because - // then we can also catch instances inside special blocks like "connection", - // before PartialContent extracts them. - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - content, config, moreDiags := block.Body.PartialContent(providerBlockSchema) diags = append(diags, moreDiags...) diff --git a/configs/resource.go b/configs/resource.go index a6946854d..30fb87d3b 100644 --- a/configs/resource.go +++ b/configs/resource.go @@ -92,13 +92,6 @@ func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) { Managed: &ManagedResource{}, } - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. We do this up front here because - // then we can also catch instances inside special blocks like "connection", - // before PartialContent extracts them. - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - content, remain, moreDiags := block.Body.PartialContent(resourceBlockSchema) diags = append(diags, moreDiags...) r.Config = remain @@ -303,11 +296,6 @@ func decodeDataBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) { TypeRange: block.LabelRanges[0], } - // Produce deprecation messages for any pre-0.12-style - // single-interpolation-only expressions. - moreDiags := warnForDeprecatedInterpolationsInBody(block.Body) - diags = append(diags, moreDiags...) - content, remain, moreDiags := block.Body.PartialContent(dataBlockSchema) diags = append(diags, moreDiags...) r.Config = remain diff --git a/configs/testdata/warning-files/redundant_interp.tf b/configs/testdata/warning-files/redundant_interp.tf deleted file mode 100644 index d1a3522d9..000000000 --- a/configs/testdata/warning-files/redundant_interp.tf +++ /dev/null @@ -1,55 +0,0 @@ -# It's redundant to write an expression that is just a single template -# interpolation with another expression inside, like "${foo}", but it -# was required before Terraform v0.12 and so there are lots of existing -# examples out there using that style. -# -# We are generating warnings for that situation in order to guide those -# who are following old examples toward the new idiom. - -variable "triggers" { - type = "map" # WARNING: Quoted type constraints are deprecated -} - -provider "null" { - foo = "${var.triggers["foo"]}" # WARNING: Interpolation-only expressions are deprecated -} - -resource "null_resource" "a" { - triggers = "${var.triggers}" # WARNING: Interpolation-only expressions are deprecated - - connection { - type = "ssh" - host = "${var.triggers["host"]}" # WARNING: Interpolation-only expressions are deprecated - } - - provisioner "local-exec" { - single = "${var.triggers["greeting"]}" # WARNING: Interpolation-only expressions are deprecated - - # No warning for this one, because there's more than just one interpolation - # in the template. - template = " ${var.triggers["greeting"]} " - - wrapped = ["${var.triggers["greeting"]}"] # WARNING: Interpolation-only expressions are deprecated - } -} - -module "foo" { - source = "./foo" - foo = "${var.foo}" # WARNING: Interpolation-only expressions are deprecated -} - -data "null_data_source" "b" { - inputs = { - host = "${var.triggers["host"]}" # WARNING: Interpolation-only expressions are deprecated - } - - has_computed_default = "${var.foo}" # WARNING: Interpolation-only expressions are deprecated -} - -output "output" { - value = "${var.foo}" # WARNING: Interpolation-only expressions are deprecated -} - -locals { - foo = "${var.foo}" # WARNING: Interpolation-only expressions are deprecated -}