configs: Specialized warning for single-interpolation object keys

We have an existing warning message to encourage moving away from the old
0.11-and-earlier style of redundantly wrapping standalone expressions in
templates, but due to the special rules for object keys the warning
message was giving misleading advice in that context: a user following the
advice as given would then encounter an error about the object key being
ambiguous.

To account for that, this introduces a special alternative version of the
warning just for that particular position, directing the user to replace
the template interpolation markers with parenthesis instead. That will
then get the same result as the former interpolation sequence, rather than
producing the ambiguity error.

configs/compat_shim.go

@@ -147,22 +147,81 @@ func warnForDeprecatedInterpolationsInExpr(expr hcl.Expression) hcl.Diagnostics
 		return nil
 	}
 
-	return hclsyntax.VisitAll(node, func(n hclsyntax.Node) hcl.Diagnostics {
-		e, ok := n.(*hclsyntax.TemplateWrapExpr)
-		if !ok {
-			// We're only interested in TemplateWrapExpr, because that's how
-			// the HCL native syntax parser represents the case of a template
-			// that consists entirely of a single interpolation expression, which
-			// is therefore subject to the special case of passing through the
-			// inner value without conversion to string.
-			return nil
-		}
-
-		return hcl.Diagnostics{&hcl.Diagnostic{
-			Severity: hcl.DiagWarning,
-			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.",
-			Subject:  e.Range().Ptr(),
-		}}
-	})
+	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

@@ -0,0 +1,61 @@
+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
+}