configs: Re-unify the ManagedResource and DataResource types

Initially the intent here was to tease these apart a little more since
they don't really share much behavior in common in core, but in practice
it'll take a lot of refactoring to tease apart these assumptions in core
right now and so we'll keep these things unified at the configuration
layer in the interests of minimizing disruption at the core layer.

The two types are still kept in separate maps to help reinforce the fact
that they are separate concepts with some behaviors in common, rather than
the same concept.

configs/module.go

@@ -21,8 +21,8 @@ type Module struct {
 
 	ModuleCalls map[string]*ModuleCall
 
-	ManagedResources map[string]*ManagedResource
-	DataResources    map[string]*DataResource
+	ManagedResources map[string]*Resource
+	DataResources    map[string]*Resource
 }
 
 // File describes the contents of a single configuration file.
@@ -49,8 +49,8 @@ type File struct {
 
 	ModuleCalls []*ModuleCall
 
-	ManagedResources []*ManagedResource
-	DataResources    []*DataResource
+	ManagedResources []*Resource
+	DataResources    []*Resource
 }
 
 // NewModule takes a list of primary files and a list of override files and
@@ -70,8 +70,8 @@ func NewModule(primaryFiles, overrideFiles []*File) (*Module, hcl.Diagnostics) {
 		Locals:               map[string]*Local{},
 		Outputs:              map[string]*Output{},
 		ModuleCalls:          map[string]*ModuleCall{},
-		ManagedResources:     map[string]*ManagedResource{},
-		DataResources:        map[string]*DataResource{},
+		ManagedResources:     map[string]*Resource{},
+		DataResources:        map[string]*Resource{},
 	}
 
 	for _, file := range primaryFiles {

configs/module_merge.go

@@ -3,6 +3,8 @@ package configs
 import (
 	"fmt"
 
+	"github.com/hashicorp/terraform/addrs"
+
 	"github.com/hashicorp/hcl2/hcl"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
@@ -188,75 +190,56 @@ func (mc *ModuleCall) merge(omc *ModuleCall) hcl.Diagnostics {
 	return diags
 }
 
-func (r *ManagedResource) merge(or *ManagedResource) hcl.Diagnostics {
+func (r *Resource) merge(or *Resource) hcl.Diagnostics {
 	var diags hcl.Diagnostics
 
-	if or.Connection != nil {
-		r.Connection = or.Connection
+	if r.Mode != or.Mode {
+		// This is always a programming error, since managed and data resources
+		// are kept in separate maps in the configuration structures.
+		panic(fmt.Errorf("can't merge %s into %s", or.Mode, r.Mode))
 	}
+
 	if or.Count != nil {
 		r.Count = or.Count
 	}
-	if or.CreateBeforeDestroySet {
-		r.CreateBeforeDestroy = or.CreateBeforeDestroy
-		r.CreateBeforeDestroySet = or.CreateBeforeDestroySet
-	}
 	if or.ForEach != nil {
 		r.ForEach = or.ForEach
 	}
-	if len(or.IgnoreChanges) != 0 {
-		r.IgnoreChanges = or.IgnoreChanges
-	}
-	if or.PreventDestroySet {
-		r.PreventDestroy = or.PreventDestroy
-		r.PreventDestroySet = or.PreventDestroySet
-	}
 	if or.ProviderConfigRef != nil {
 		r.ProviderConfigRef = or.ProviderConfigRef
 	}
-	if len(or.Provisioners) != 0 {
-		r.Provisioners = or.Provisioners
-	}
-
-	r.Config = MergeBodies(r.Config, or.Config)
-
-	// We don't allow depends_on to be overridden because that is likely to
-	// cause confusing misbehavior.
-	if len(r.DependsOn) != 0 {
-		diags = append(diags, &hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Unsupported override",
-			Detail:   "The depends_on argument may not be overridden.",
-			Subject:  r.DependsOn[0].SourceRange().Ptr(), // the first item is the closest range we have
-		})
-	}
+	if r.Mode == addrs.ManagedResourceMode {
+		// or.Managed is always non-nil for managed resource mode
 
-	return diags
-}
-
-func (r *DataResource) merge(or *DataResource) hcl.Diagnostics {
-	var diags hcl.Diagnostics
-
-	if or.Count != nil {
-		r.Count = or.Count
-	}
-	if or.ForEach != nil {
-		r.ForEach = or.ForEach
-	}
-	if or.ProviderConfigRef != nil {
-		r.ProviderConfigRef = or.ProviderConfigRef
+		if or.Managed.Connection != nil {
+			r.Managed.Connection = or.Managed.Connection
+		}
+		if or.Managed.CreateBeforeDestroySet {
+			r.Managed.CreateBeforeDestroy = or.Managed.CreateBeforeDestroy
+			r.Managed.CreateBeforeDestroySet = or.Managed.CreateBeforeDestroySet
+		}
+		if len(or.Managed.IgnoreChanges) != 0 {
+			r.Managed.IgnoreChanges = or.Managed.IgnoreChanges
+		}
+		if or.Managed.PreventDestroySet {
+			r.Managed.PreventDestroy = or.Managed.PreventDestroy
+			r.Managed.PreventDestroySet = or.Managed.PreventDestroySet
+		}
+		if len(or.Managed.Provisioners) != 0 {
+			r.Managed.Provisioners = or.Managed.Provisioners
+		}
 	}
 
 	r.Config = MergeBodies(r.Config, or.Config)
 
 	// We don't allow depends_on to be overridden because that is likely to
 	// cause confusing misbehavior.
-	if len(r.DependsOn) != 0 {
+	if len(or.DependsOn) != 0 {
 		diags = append(diags, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "Unsupported override",
 			Detail:   "The depends_on argument may not be overridden.",
-			Subject:  r.DependsOn[0].SourceRange().Ptr(), // the first item is the closest range we have
+			Subject:  or.DependsOn[0].SourceRange().Ptr(), // the first item is the closest range we have
 		})
 	}
 

configs/resource.go

@@ -4,13 +4,16 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/terraform/addrs"
+
 	"github.com/hashicorp/hcl2/gohcl"
 	"github.com/hashicorp/hcl2/hcl"
 	"github.com/hashicorp/hcl2/hcl/hclsyntax"
 )
 
-// ManagedResource represents a "resource" block in a module or file.
-type ManagedResource struct {
+// Resource represents a "resource" or "data" block in a module or file.
+type Resource struct {
+	Mode    addrs.ResourceMode
 	Name    string
 	Type    string
 	Config  hcl.Body
@@ -21,6 +24,17 @@ type ManagedResource struct {
 
 	DependsOn []hcl.Traversal
 
+	// Managed is populated only for Mode = addrs.ManagedResourceMode,
+	// containing the additional fields that apply to managed resources.
+	// For all other resource modes, this field is nil.
+	Managed *ManagedResource
+
+	DeclRange hcl.Range
+	TypeRange hcl.Range
+}
+
+// ManagedResource represents a "resource" block in a module or file.
+type ManagedResource struct {
 	Connection   *Connection
 	Provisioners []*Provisioner
 
@@ -31,20 +45,24 @@ type ManagedResource struct {
 
 	CreateBeforeDestroySet bool
 	PreventDestroySet      bool
-
-	DeclRange hcl.Range
-	TypeRange hcl.Range
 }
 
-func (r *ManagedResource) moduleUniqueKey() string {
-	return fmt.Sprintf("%s.%s", r.Name, r.Type)
+func (r *Resource) moduleUniqueKey() string {
+	switch r.Mode {
+	case addrs.ManagedResourceMode:
+		return fmt.Sprintf("%s.%s", r.Name, r.Type)
+	case addrs.DataResourceMode:
+		return fmt.Sprintf("data.%s.%s", r.Name, r.Type)
+	default:
+		panic(fmt.Errorf("Resource has invalid resource mode %s", r.Mode))
+	}
 }
 
 // ProviderConfigKey returns a string key for the provider configuration
 // that should be used for this resource. This function implements the
 // default behavior of extracting the type from the resource type name if
 // an explicit "provider" argument was not provided.
-func (r *ManagedResource) ProviderConfigKey() string {
+func (r *Resource) ProviderConfigKey() string {
 	if r.ProviderConfigRef == nil {
 		typeName := r.Type
 		if under := strings.Index(typeName, "_"); under != -1 {
@@ -56,12 +74,14 @@ func (r *ManagedResource) ProviderConfigKey() string {
 	return r.ProviderConfigRef.String()
 }
 
-func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
-	r := &ManagedResource{
+func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
+	r := &Resource{
+		Mode:      addrs.ManagedResourceMode,
 		Type:      block.Labels[0],
 		Name:      block.Labels[1],
 		DeclRange: block.DefRange,
 		TypeRange: block.LabelRanges[0],
+		Managed:   &ManagedResource{},
 	}
 
 	content, remain, diags := block.Body.PartialContent(resourceBlockSchema)
@@ -124,15 +144,15 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 			diags = append(diags, lcDiags...)
 
 			if attr, exists := lcContent.Attributes["create_before_destroy"]; exists {
-				valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.CreateBeforeDestroy)
+				valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.Managed.CreateBeforeDestroy)
 				diags = append(diags, valDiags...)
-				r.CreateBeforeDestroySet = true
+				r.Managed.CreateBeforeDestroySet = true
 			}
 
 			if attr, exists := lcContent.Attributes["prevent_destroy"]; exists {
-				valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.PreventDestroy)
+				valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.Managed.PreventDestroy)
 				diags = append(diags, valDiags...)
-				r.PreventDestroySet = true
+				r.Managed.PreventDestroySet = true
 			}
 
 			if attr, exists := lcContent.Attributes["ignore_changes"]; exists {
@@ -151,7 +171,7 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 
 				switch {
 				case kw == "all":
-					r.IgnoreAllChanges = true
+					r.Managed.IgnoreAllChanges = true
 				default:
 					exprs, listDiags := hcl.ExprList(attr.Expr)
 					diags = append(diags, listDiags...)
@@ -163,7 +183,7 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 						// our expr might be the literal string "*", which
 						// we accept as a deprecated way of saying "all".
 						if shimIsIgnoreChangesStar(expr) {
-							r.IgnoreAllChanges = true
+							r.Managed.IgnoreAllChanges = true
 							ignoreAllRange = expr.Range()
 							diags = append(diags, &hcl.Diagnostic{
 								Severity: hcl.DiagWarning,
@@ -180,11 +200,11 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 						traversal, travDiags := hcl.RelTraversalForExpr(expr)
 						diags = append(diags, travDiags...)
 						if len(traversal) != 0 {
-							r.IgnoreChanges = append(r.IgnoreChanges, traversal)
+							r.Managed.IgnoreChanges = append(r.Managed.IgnoreChanges, traversal)
 						}
 					}
 
-					if r.IgnoreAllChanges && len(r.IgnoreChanges) != 0 {
+					if r.Managed.IgnoreAllChanges && len(r.Managed.IgnoreChanges) != 0 {
 						diags = append(diags, &hcl.Diagnostic{
 							Severity: hcl.DiagError,
 							Summary:  "Invalid ignore_changes ruleset",
@@ -212,13 +232,13 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 
 			conn, connDiags := decodeConnectionBlock(block)
 			diags = append(diags, connDiags...)
-			r.Connection = conn
+			r.Managed.Connection = conn
 
 		case "provisioner":
 			pv, pvDiags := decodeProvisionerBlock(block)
 			diags = append(diags, pvDiags...)
 			if pv != nil {
-				r.Provisioners = append(r.Provisioners, pv)
+				r.Managed.Provisioners = append(r.Managed.Provisioners, pv)
 			}
 
 		default:
@@ -231,44 +251,9 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) {
 	return r, diags
 }
 
-// DataResource represents a "data" block in a module or file.
-type DataResource struct {
-	Name    string
-	Type    string
-	Config  hcl.Body
-	Count   hcl.Expression
-	ForEach hcl.Expression
-
-	ProviderConfigRef *ProviderConfigRef
-
-	DependsOn []hcl.Traversal
-
-	DeclRange hcl.Range
-	TypeRange hcl.Range
-}
-
-func (r *DataResource) moduleUniqueKey() string {
-	return fmt.Sprintf("data.%s.%s", r.Name, r.Type)
-}
-
-// ProviderConfigKey returns a string key for the provider configuration
-// that should be used for this resource. This function implements the
-// default behavior of extracting the type from the resource type name if
-// an explicit "provider" argument was not provided.
-func (r *DataResource) ProviderConfigKey() string {
-	if r.ProviderConfigRef == nil {
-		typeName := r.Type
-		if under := strings.Index(typeName, "_"); under != -1 {
-			return typeName[:under]
-		}
-		return typeName
-	}
-
-	return r.ProviderConfigRef.String()
-}
-
-func decodeDataBlock(block *hcl.Block) (*DataResource, hcl.Diagnostics) {
-	r := &DataResource{
+func decodeDataBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
+	r := &Resource{
+		Mode:      addrs.DataResourceMode,
 		Type:      block.Labels[0],
 		Name:      block.Labels[1],
 		DeclRange: block.DefRange,

terraform/resource_address.go

@@ -7,6 +7,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/hashicorp/terraform/addrs"
+
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/configs"
 )
@@ -109,7 +111,7 @@ func (r *ResourceAddress) WholeModuleAddress() *ResourceAddress {
 	}
 }
 
-// MatchesManagedResourceConfig returns true if the receiver matches the given
+// MatchesResourceConfig returns true if the receiver matches the given
 // configuration resource within the given _static_ module path. Note that
 // the module path in a resource address is a _dynamic_ module path, and
 // multiple dynamic resource paths may map to a single static path if
@@ -118,10 +120,21 @@ func (r *ResourceAddress) WholeModuleAddress() *ResourceAddress {
 // Since resource configuration blocks represent all of the instances of
 // a multi-instance resource, the index of the address (if any) is not
 // considered.
-func (r *ResourceAddress) MatchesManagedResourceConfig(path []string, rc *configs.ManagedResource) bool {
+func (r *ResourceAddress) MatchesResourceConfig(path addrs.Module, rc *configs.Resource) bool {
 	if r.HasResourceSpec() {
-		if r.Mode != config.ManagedResourceMode {
-			return false
+		// FIXME: Some ugliness while we are between worlds. Functionality
+		// in "addrs" should eventually replace this ResourceAddress idea
+		// completely, but for now we'll need to translate to the old
+		// way of representing resource modes.
+		switch r.Mode {
+		case config.ManagedResourceMode:
+			if rc.Mode != addrs.ManagedResourceMode {
+				return false
+			}
+		case config.DataResourceMode:
+			if rc.Mode != addrs.DataResourceMode {
+				return false
+			}
 		}
 		if r.Type != rc.Type || r.Name != rc.Name {
 			return false
@@ -137,7 +150,8 @@ func (r *ResourceAddress) MatchesManagedResourceConfig(path []string, rc *config
 	if len(path) == 0 {
 		path = nil
 	}
-	return reflect.DeepEqual(addrPath, path)
+	rawPath := []string(path)
+	return reflect.DeepEqual(addrPath, rawPath)
 }
 
 // stateId returns the ID that this resource should be entered with

terraform/resource_address_test.go

@@ -5,6 +5,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/hashicorp/terraform/addrs"
+
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/configs"
 )
@@ -1046,7 +1048,7 @@ func TestResourceAddressWholeModuleAddress(t *testing.T) {
 	}
 }
 
-func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
+func TestResourceAddressMatchesResourceConfig(t *testing.T) {
 	root := []string(nil)
 	child := []string{"child"}
 	grandchild := []string{"child", "grandchild"}
@@ -1055,7 +1057,7 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 	tests := []struct {
 		Addr       *ResourceAddress
 		ModulePath []string
-		Resource   *configs.ManagedResource
+		Resource   *configs.Resource
 		Want       bool
 	}{
 		{
@@ -1066,7 +1068,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			root,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1081,7 +1084,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			child,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1096,7 +1100,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			grandchild,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1108,7 +1113,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			child,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1120,7 +1126,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			grandchild,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1134,7 +1141,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			irrelevant,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1148,7 +1156,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			irrelevant,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "pizza",
 			},
@@ -1162,7 +1171,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			irrelevant,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "aws_instance",
 				Name: "baz",
 			},
@@ -1177,7 +1187,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			child,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1192,7 +1203,8 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 				Index: -1,
 			},
 			grandchild,
-			&configs.ManagedResource{
+			&configs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "null_resource",
 				Name: "baz",
 			},
@@ -1202,7 +1214,7 @@ func TestResourceAddressMatchesManagedResourceConfig(t *testing.T) {
 
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("%02d-%s", i, test.Addr), func(t *testing.T) {
-			got := test.Addr.MatchesManagedResourceConfig(test.ModulePath, test.Resource)
+			got := test.Addr.MatchesResourceConfig(test.ModulePath, test.Resource)
 			if got != test.Want {
 				t.Errorf(
 					"wrong result\naddr: %s\nmod:  %#v\nrsrc: %#v\ngot:  %#v\nwant: %#v",