configs: Warn for deprecated interpolation and quoted type constraints

Following on from de652e2, this introduces deprecation warnings for
when an attribute value expression is a template with only a single
interpolation sequence, and for variable type constraints given in quotes.

As with the previous commit, we allowed these deprecated forms with no
warning for a few releases after v0.12.0 to ensure that folks who need to
write cross-compatible modules for a while during upgrading would be able
to do so, but we're now marking these as explicitly deprecated to guide
users towards the new idiomatic forms.

The "terraform 0.12upgrade" tool would've already updated configurations
to not hit these warnings for those who had pre-existing configurations
written for Terraform 0.11.

The main target audience for these warnings are newcomers to Terraform who
are learning from existing examples already published in various spots on
the wider internet that may be showing older Terraform syntax, since those
folks will not be running their configurations through the upgrade tool.
These warnings will hopefully guide them towards modern Terraform usage
during their initial experimentation, and thus reduce the chances of
inadvertently adopting the less-readable legacy usage patterns in
greenfield projects.

configs/compat_shim.go

@@ -107,3 +107,58 @@ 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 {
+	var diags hcl.Diagnostics
+
+	if _, ok := expr.(*hclsyntax.TemplateWrapExpr); !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 diags
+	}
+
+	diags = append(diags, &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 can still be 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:  expr.Range().Ptr(),
+	})
+
+	return diags
+}

configs/named_values.go

@@ -138,10 +138,28 @@ func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl
 		str := val.AsString()
 		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\".",
+				Subject:  expr.Range().Ptr(),
+			})
 			return cty.String, 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.",
+				Subject:  expr.Range().Ptr(),
+			})
 			return cty.List(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.",
+				Subject:  expr.Range().Ptr(),
+			})
 			return cty.Map(cty.DynamicPseudoType), VariableParseHCL, diags
 		default:
 			return cty.DynamicPseudoType, VariableParseHCL, hcl.Diagnostics{{

configs/provider.go

@@ -27,7 +27,17 @@ type Provider struct {
 }
 
 func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) {
-	content, config, diags := block.Body.PartialContent(providerBlockSchema)
+	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...)
 
 	provider := &Provider{
 		Name:      block.Labels[0],

configs/resource.go

@@ -76,6 +76,7 @@ func (r *Resource) ProviderConfigAddr() addrs.ProviderConfig {
 }
 
 func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
+	var diags hcl.Diagnostics
 	r := &Resource{
 		Mode:      addrs.ManagedResourceMode,
 		Type:      block.Labels[0],
@@ -85,7 +86,15 @@ func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
 		Managed:   &ManagedResource{},
 	}
 
-	content, remain, diags := block.Body.PartialContent(resourceBlockSchema)
+	// 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
 
 	if !hclsyntax.ValidIdentifier(r.Type) {

configs/testdata/valid-files/references.tf.json

@@ -0,0 +1,11 @@
+{
+  "//": "The purpose of this test file is to show that we can use template syntax unwrapping to provide complex expressions without generating the deprecation warnings we'd expect for native syntax.",
+  "resource": {
+    "null_resource": {
+      "baz": {
+        "//": "This particular use of template syntax is redundant, but we permit it because this is the documented way to use more complex expressions in JSON.",
+        "triggers": "${ {} }"
+      }
+    }
+  }
+}

configs/testdata/warning-files/redundant_interp.tf

@@ -0,0 +1,36 @@
+# 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"]} "
+
+    # No warning for this one, because it's embedded inside a more complex
+    # expression and our check is only for direct assignment to attributes.
+    wrapped = ["${var.triggers["greeting"]}"]
+  }
+}

configs/testdata/warning-files/variable_type_quoted.tf

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