core: Static-validate resource references against schemas

In the initial move to HCL2 we started relying only on full expression
evaluation to catch attribute errors, but that's not sufficient for
resource attributes in practice because during validation we can't know
yet whether a resource reference evaluates to a single object or to a
list of objects (if count is set).

To address this, here we reinstate some static validation of resource
references by analyzing directly the reference objects, disregarding any
instance index if present, and produce errors if the remaining subsequent
traversal steps do not correspond to items within the resource type
schema.

This also allows us to produce some more specialized error messages for
certain situations. In particular, we can recognize a reference like
aws_instance.foo.count, which in 0.11 and prior was a weird special case
for determining the count value of a resource block, and offer a helpful
error showing the new length(aws_instance.foo) usage pattern.

This eventually delegates to the static traversal validation logic that
was added to the configschema package in a previous commit, which also
includes some specialized error messages that distinguish between
attributes and block types in the schema so that the errors relate more
directly to constructs the user can see in the configuration.

In future we could potentially move more of the checks from the dynamic
schema construction step to the static validation step, but resources
are the reference type that most needs this immediately due to the
ambiguity caused by the instance indexing syntax. We can safely refactor
other reference types to be statically validated in later releases.

This is verified by two pre-existing context validate tests which we
temporarily disabled during earlier work (now re-enabled) and also by a
new validate test aimed specifically at the special case for the "count"
attribute.

lang/data.go

@@ -20,6 +20,8 @@ import (
 // cases where it's not possible to even determine a suitable result type,
 // cty.DynamicVal is returned along with errors describing the problem.
 type Data interface {
+	StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics
+
 	GetCountAttr(addrs.CountAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetResourceInstance(addrs.ResourceInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)

lang/data_test.go

@@ -18,6 +18,10 @@ type dataForTests struct {
 
 var _ Data = &dataForTests{}
 
+func (d *dataForTests) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics {
+	return nil // does nothing in this stub implementation
+}
+
 func (d *dataForTests) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	return d.CountAttrs[addr.Name], nil
 }

lang/eval.go

@@ -52,6 +52,11 @@ func (s *Scope) EvalBlock(body hcl.Body, schema *configschema.Block) (cty.Value,
 
 	ctx, ctxDiags := s.EvalContext(refs)
 	diags = diags.Append(ctxDiags)
+	if diags.HasErrors() {
+		// We'll stop early if we found problems in the references, because
+		// it's likely evaluation will produce redundant copies of the same errors.
+		return cty.UnknownVal(schema.ImpliedType()), diags
+	}
 
 	val, evalDiags := hcldec.Decode(body, spec, ctx)
 	diags = diags.Append(evalDiags)
@@ -74,6 +79,11 @@ func (s *Scope) EvalExpr(expr hcl.Expression, wantType cty.Type) (cty.Value, tfd
 
 	ctx, ctxDiags := s.EvalContext(refs)
 	diags = diags.Append(ctxDiags)
+	if diags.HasErrors() {
+		// We'll stop early if we found problems in the references, because
+		// it's likely evaluation will produce redundant copies of the same errors.
+		return cty.UnknownVal(wantType), diags
+	}
 
 	val, evalDiags := expr.Value(ctx)
 	diags = diags.Append(evalDiags)
@@ -155,6 +165,14 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 		return ctx, diags
 	}
 
+	// First we'll do static validation of the references. This catches things
+	// early that might otherwise not get caught due to unknown values being
+	// present in the scope during planning.
+	if staticDiags := s.Data.StaticValidateReferences(refs, selfAddr); staticDiags.HasErrors() {
+		diags = diags.Append(staticDiags)
+		return ctx, diags
+	}
+
 	// The reference set we are given has not been de-duped, and so there can
 	// be redundant requests in it for two reasons:
 	//  - The same item is referenced multiple times

repl/session_test.go

@@ -84,7 +84,7 @@ func TestSession_basicState(t *testing.T) {
 				{
 					Input:         "test_instance.bar.id",
 					Error:         true,
-					ErrorContains: `A resource "test_instance" "bar" has not been declared`,
+					ErrorContains: `A managed resource "test_instance" "bar" has not been declared`,
 				},
 			},
 		})
@@ -176,6 +176,8 @@ func TestSession_stateless(t *testing.T) {
 }
 
 func testSession(t *testing.T, test testSessionTest) {
+	t.Helper()
+
 	p := &terraform.MockProvider{}
 	p.GetSchemaReturn = &terraform.ProviderSchema{
 		ResourceTypes: map[string]*configschema.Block{

terraform/context_validate_test.go

@@ -1149,12 +1149,6 @@ func TestContext2Validate_PlanGraphBuilder(t *testing.T) {
 	}
 }
 
-// FIXME: these 2 tests should be caught in validation.
-// Since the evaluator can't determine if the resource contains a count during
-// validation, any refereces are currently returned as unknown. We need a
-// static validation of a reference to determine if it's possible within a
-// particular schema.
-/*
 func TestContext2Validate_invalidOutput(t *testing.T) {
 	m := testModuleInline(t, map[string]string{
 		"main.tf": `
@@ -1177,10 +1171,13 @@ output "out" {
 
 	diags := ctx.Validate()
 	if !diags.HasErrors() {
-		// Should get this error:
-		// Unsupported attribute: This object does not have an attribute named "missing"
 		t.Fatal("succeeded; want errors")
 	}
+	// Should get this error:
+	// Unsupported attribute: This object does not have an attribute named "missing"
+	if got, want := diags.Err().Error(), "Unsupported attribute"; strings.Index(got, want) == -1 {
+		t.Fatalf("wrong error:\ngot:  %s\nwant: message containing %q", got, want)
+	}
 }
 
 func TestContext2Validate_invalidModuleOutput(t *testing.T) {
@@ -1213,9 +1210,42 @@ resource "aws_instance" "foo" {
 
 	diags := ctx.Validate()
 	if !diags.HasErrors() {
-		// Should get this error:
-		// Unsupported attribute: This object does not have an attribute named "missing"
 		t.Fatal("succeeded; want errors")
 	}
+	// Should get this error:
+	// Unsupported attribute: This object does not have an attribute named "missing"
+	if got, want := diags.Err().Error(), "Unsupported attribute"; strings.Index(got, want) == -1 {
+		t.Fatalf("wrong error:\ngot:  %s\nwant: message containing %q", got, want)
+	}
+}
+
+func TestContext2Validate_legacyResourceCount(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "aws_instance" "test" {}
+
+output "out" {
+  value = aws_instance.test.count
+}`,
+	})
+
+	p := testProvider("aws")
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+	})
+
+	diags := ctx.Validate()
+	if !diags.HasErrors() {
+		t.Fatal("succeeded; want errors")
+	}
+	// Should get this error:
+	// Invalid resource count attribute: The special "count" attribute is no longer supported after Terraform v0.12. Instead, use length(aws_instance.test) to count resource instances.
+	if got, want := diags.Err().Error(), "Invalid resource count attribute:"; strings.Index(got, want) == -1 {
+		t.Fatalf("wrong error:\ngot:  %s\nwant: message containing %q", got, want)
+	}
 }
-*/

terraform/evaluate_valid.go

@@ -0,0 +1,172 @@
+package terraform
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/hcl2/hcl"
+
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/configs/configschema"
+	"github.com/hashicorp/terraform/tfdiags"
+)
+
+// StaticValidateReferences checks the given references against schemas and
+// other statically-checkable rules, producing error diagnostics if any
+// problems are found.
+//
+// If this method returns errors for a particular reference then evaluating
+// that reference is likely to generate a very similar error, so callers should
+// not run this method and then also evaluate the source expression(s) and
+// merge the two sets of diagnostics together, since this will result in
+// confusing redundant errors.
+//
+// This method can find more errors than can be found by evaluating an
+// expression with a partially-populated scope, since it checks the referenced
+// names directly against the schema rather than relying on evaluation errors.
+//
+// The result may include warning diagnostics if, for example, deprecated
+// features are referenced.
+func (d *evaluationStateData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+	for _, ref := range refs {
+		moreDiags := d.staticValidateReference(ref, self)
+		diags = diags.Append(moreDiags)
+	}
+	return diags
+}
+
+func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics {
+	modCfg := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
+	if modCfg == nil {
+		// This is a bug in the caller rather than a problem with the
+		// reference, but rather than crashing out here in an unhelpful way
+		// we'll just ignore it and trust a different layer to catch it.
+		return nil
+	}
+
+	if ref.Subject == addrs.Self {
+		// The "self" address is a special alias for the address given as
+		// our self parameter here, if present.
+		if self == nil {
+			var diags tfdiags.Diagnostics
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  `Invalid "self" reference`,
+				// This detail message mentions some current practice that
+				// this codepath doesn't really "know about". If the "self"
+				// object starts being supported in more contexts later then
+				// we'll need to adjust this message.
+				Detail:  `The "self" object is not available in this context. This object can be used only in resource provisioner and connection blocks.`,
+				Subject: ref.SourceRange.ToHCL().Ptr(),
+			})
+			return diags
+		}
+
+		synthRef := *ref // shallow copy
+		synthRef.Subject = self
+		ref = &synthRef
+	}
+
+	switch addr := ref.Subject.(type) {
+
+	// For static validation we validate both resource and resource instance references the same way, disregarding the index
+	case addrs.Resource:
+		return d.staticValidateResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)
+	case addrs.ResourceInstance:
+		return d.staticValidateResourceReference(modCfg, addr.ContainingResource(), ref.Remaining, ref.SourceRange)
+
+	default:
+		// Anything else we'll just permit through without any static validation
+		// and let it be caught during dynamic evaluation, in evaluate.go .
+		return nil
+	}
+}
+
+func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
+	var modeAdjective string
+	switch addr.Mode {
+	case addrs.ManagedResourceMode:
+		modeAdjective = "managed"
+	case addrs.DataResourceMode:
+		modeAdjective = "data"
+	default:
+		// should never happen
+		modeAdjective = "<invalid-mode>"
+	}
+
+	cfg := modCfg.Module.ResourceByAddr(addr)
+	if cfg == nil {
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Reference to undeclared resource`,
+			Detail:   fmt.Sprintf(`A %s resource %q %q has not been declared in %s`, modeAdjective, addr.Type, addr.Name, moduleConfigDisplayAddr(modCfg.Path)),
+			Subject:  rng.ToHCL().Ptr(),
+		})
+		return diags
+	}
+
+	// Normally accessing this directly is wrong because it doesn't take into
+	// account provider inheritance, etc but it's okay here because we're only
+	// paying attention to the type anyway.
+	providerType := cfg.ProviderConfigAddr().Type
+	var schema *configschema.Block
+	switch addr.Mode {
+	case addrs.ManagedResourceMode:
+		schema = d.Evaluator.Schemas.ResourceTypeConfig(providerType, addr.Type)
+	case addrs.DataResourceMode:
+		schema = d.Evaluator.Schemas.DataSourceConfig(providerType, addr.Type)
+	}
+
+	if schema == nil {
+		// Prior validation should've taken care of a resource block with an
+		// unsupported type, so we should never get here but we'll handle it
+		// here anyway for robustness.
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Invalid resource type`,
+			Detail:   fmt.Sprintf(`A %s resource type %q is not supported by provider %q.`, modeAdjective, addr.Type, providerType),
+			Subject:  rng.ToHCL().Ptr(),
+		})
+		return diags
+	}
+
+	// As a special case we'll detect attempts to access an attribute called
+	// "count" and produce a special error for it, since versions of Terraform
+	// prior to v0.12 offered this as a weird special case that we can no
+	// longer support.
+	if len(remain) > 0 {
+		if step, ok := remain[0].(hcl.TraverseAttr); ok && step.Name == "count" {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  `Invalid resource count attribute`,
+				Detail:   fmt.Sprintf(`The special "count" attribute is no longer supported after Terraform v0.12. Instead, use length(%s) to count resource instances.`, addr),
+				Subject:  rng.ToHCL().Ptr(),
+			})
+			return diags
+		}
+	}
+
+	// If we got this far then we'll try to validate the remaining traversal
+	// steps against our schema.
+	moreDiags := schema.StaticValidateTraversal(remain)
+	diags = diags.Append(moreDiags)
+
+	return diags
+}
+
+// moduleConfigDisplayAddr returns a string describing the given module
+// address that is appropriate for returning to users in situations where the
+// root module is possible. Specifically, it returns "the root module" if the
+// root module instance is given, or a string representation of the module
+// address otherwise.
+func moduleConfigDisplayAddr(addr addrs.Module) string {
+	switch {
+	case addr.IsRoot():
+		return "the root module"
+	default:
+		return addr.String()
+	}
+}