Merge pull request #27835 from hashicorp/pselle/warn-interpolation

Remove interpolation-only warning

Author: Pam Selle

command/testdata/validate-invalid/incorrectmodulename/main.tf

@@ -2,5 +2,5 @@ module "super#module" {
 }
 
 module "super" {
-  source = "${var.modulename}"
+  source = var.modulename
 }

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
         }
       }
     },

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
     }
 }

command/testdata/validate-invalid/missing_var/output.json

@@ -1,26 +1,8 @@
 {
   "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.",
-      "range": {
-        "filename": "testdata/validate-invalid/missing_var/main.tf",
-        "start": {
-          "line": 6,
-          "column": 21,
-          "byte": 117
-        },
-        "end": {
-          "line": 6,
-          "column": 41,
-          "byte": 137
-        }
-      }
-    },
     {
       "severity": "error",
       "summary": "Reference to undeclared input variable",
@@ -29,13 +11,13 @@
         "filename": "testdata/validate-invalid/missing_var/main.tf",
         "start": {
           "line": 6,
-          "column": 24,
-          "byte": 120
+          "column": 21,
+          "byte": 117
         },
         "end": {
           "line": 6,
-          "column": 39,
-          "byte": 135
+          "column": 36,
+          "byte": 132
         }
       }
     }

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]
-}

configs/compat_shim_test.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,

configs/module_call.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,

configs/named_values.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...)
 

configs/provider.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