requiresInstanceExpansion and instanceExpander

create interfaces that nodes can implement to declare whether they
expand into instances of some sort, using the instances.Expander, and/or
whether use the instances.Expander to find instances.

included is a rough transformer implementation to remove these nodes
from the apply graph.

terraform/graph_builder_apply.go

@@ -165,9 +165,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 
 		// Prune unreferenced values, which may have interpolations that can't
 		// be resolved.
-		&PruneUnusedValuesTransformer{
-			Destroy: b.Destroy,
-		},
+		&pruneUnusedExpanderTransformer{},
 
 		// Add the node to fix the state count boundaries
 		&CountBoundaryTransformer{

terraform/instance_expanders.go

@@ -0,0 +1,19 @@
+package terraform
+
+import "github.com/hashicorp/terraform/addrs"
+
+// instanceExpander is implemented by nodes that causes instances to be
+// registered in the instances.Expander.
+// This is used to determine during apply whether a node is required to be in
+// the graph, by checking if it has any requiresInstanceExpansion dependents.
+// This prevents unnecessary nodes from being evaluated, and if the module is
+// being removed, we may not be able to evaluate the expansion at all.
+type instanceExpander interface {
+	expandsInstances() addrs.Module
+}
+
+// requiresInstanceExpansion is implemented by nodes that require their address
+// be previously registered in the instances.Expander in order to evaluate.
+type requiresInstanceExpansion interface {
+	requiresExpansion()
+}

terraform/node_local.go

@@ -23,8 +23,16 @@ var (
 	_ GraphNodeReferencer        = (*nodeExpandLocal)(nil)
 	_ GraphNodeDynamicExpandable = (*nodeExpandLocal)(nil)
 	_ graphNodeTemporaryValue    = (*nodeExpandLocal)(nil)
+	_ requiresInstanceExpansion  = (*nodeExpandLocal)(nil)
 )
 
+func (n *nodeExpandLocal) expandsInstances() addrs.Module {
+	return n.Module
+}
+
+// requiresInstanceExpansion implementation
+func (n *nodeExpandLocal) requiresExpansion() {}
+
 // graphNodeTemporaryValue
 func (n *nodeExpandLocal) temporaryValue() bool {
 	return true

terraform/node_module_expand.go

@@ -31,8 +31,21 @@ var (
 	_ RemovableIfNotTargeted = (*nodeExpandModule)(nil)
 	_ GraphNodeEvalable      = (*nodeExpandModule)(nil)
 	_ GraphNodeReferencer    = (*nodeExpandModule)(nil)
+
+	// modules both record their expansion, and require expansion from their
+	// parent modules.
+	_ requiresInstanceExpansion = (*nodeExpandModule)(nil)
+	_ instanceExpander          = (*nodeExpandModule)(nil)
 )
 
+// requiresInstanceExpansion implementation
+func (n *nodeExpandModule) requiresExpansion() {}
+
+// instanceExander implementation
+func (n *nodeExpandModule) expandsInstances() addrs.Module {
+	return n.Addr
+}
+
 func (n *nodeExpandModule) Name() string {
 	return n.Addr.String() + " (expand)"
 }
@@ -125,8 +138,7 @@ var (
 )
 
 func (n *nodeCloseModule) ModulePath() addrs.Module {
-	mod, _ := n.Addr.Call()
-	return mod
+	return n.Addr
 }
 
 func (n *nodeCloseModule) ReferenceableAddrs() []addrs.Referenceable {

terraform/node_module_variable.go

@@ -27,8 +27,16 @@ var (
 	_ GraphNodeReferencer        = (*nodeExpandModuleVariable)(nil)
 	_ graphNodeTemporaryValue    = (*nodeExpandModuleVariable)(nil)
 	_ RemovableIfNotTargeted     = (*nodeExpandModuleVariable)(nil)
+	_ requiresInstanceExpansion  = (*nodeExpandModuleVariable)(nil)
 )
 
+// requiresInstanceExpansion implementation
+func (n *nodeExpandModuleVariable) requiresExpansion() {}
+
+func (n *nodeExpandModuleVariable) expandsInstances() addrs.Module {
+	return n.Module
+}
+
 func (n *nodeExpandModuleVariable) temporaryValue() bool {
 	return true
 }

terraform/node_output.go

@@ -23,9 +23,16 @@ var (
 	_ GraphNodeReferenceable     = (*nodeExpandOutput)(nil)
 	_ GraphNodeReferencer        = (*nodeExpandOutput)(nil)
 	_ GraphNodeDynamicExpandable = (*nodeExpandOutput)(nil)
-	_ graphNodeTemporaryValue    = (*NodeApplyableOutput)(nil)
+	_ graphNodeTemporaryValue    = (*nodeExpandOutput)(nil)
+	_ requiresInstanceExpansion  = (*nodeExpandOutput)(nil)
 )
 
+func (n *nodeExpandOutput) expandsInstances() addrs.Module {
+	return n.Module
+}
+
+func (m *nodeExpandOutput) requiresExpansion() {}
+
 func (n *nodeExpandOutput) temporaryValue() bool {
 	// this must always be evaluated if it is a root module output
 	return !n.Module.IsRoot()

terraform/node_resource_apply.go

@@ -22,8 +22,20 @@ var (
 	_ GraphNodeReferencer           = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeConfigResource       = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeAttachResourceConfig = (*nodeExpandApplyableResource)(nil)
+
+	// Resource both expand instances and require module path expansion.
+	_ requiresInstanceExpansion = (*nodeExpandApplyableResource)(nil)
+	_ instanceExpander          = (*nodeExpandApplyableResource)(nil)
 )
 
+// requiresInstanceExpansion implementation
+func (n *nodeExpandApplyableResource) requiresExpansion() {}
+
+// instanceExpander implementation
+func (n *nodeExpandApplyableResource) expandsInstances() addrs.Module {
+	return n.ModulePath()
+}
+
 func (n *nodeExpandApplyableResource) References() []*addrs.Reference {
 	return (&NodeApplyableResource{NodeAbstractResource: n.NodeAbstractResource}).References()
 }

terraform/node_resource_apply_instance.go

@@ -37,8 +37,12 @@ var (
 	_ GraphNodeDeposer            = (*NodeApplyableResourceInstance)(nil)
 	_ GraphNodeEvalable           = (*NodeApplyableResourceInstance)(nil)
 	_ GraphNodeAttachDependencies = (*NodeApplyableResourceInstance)(nil)
+	_ requiresInstanceExpansion   = (*NodeApplyableResourceInstance)(nil)
 )
 
+// requiresInstanceExpansion implementation
+func (n *NodeApplyableResourceInstance) requiresExpansion() {}
+
 // GraphNodeAttachDestroyer
 func (n *NodeApplyableResourceInstance) AttachDestroyNode(d GraphNodeDestroyerCBD) {
 	n.destroyNode = d

terraform/transform_destroy_edge.go

@@ -2,6 +2,7 @@ package terraform
 
 import (
 	"log"
+	"sort"
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/states"
@@ -163,46 +164,129 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error {
 		}
 	}
 
-	return t.pruneResources(g)
-}
-
-// If there are only destroy instances for a particular resource, there's no
-// reason for the resource node to prepare the state. Remove Resource nodes so
-// that they don't fail by trying to evaluate a resource that is only being
-// destroyed along with its dependencies.
-func (t *DestroyEdgeTransformer) pruneResources(g *Graph) error {
-	for _, v := range g.Vertices() {
-		n, ok := v.(*nodeExpandApplyableResource)
-		if !ok {
-			continue
-		}
-
-		// if there are only destroy dependencies, we don't need this node
-		descendents, err := g.Descendents(n)
-		if err != nil {
-			return err
-		}
-
-		nonDestroyInstanceFound := false
-		for _, v := range descendents {
-			if _, ok := v.(*NodeApplyableResourceInstance); ok {
-				nonDestroyInstanceFound = true
-				break
-			}
-		}
-
-		if nonDestroyInstanceFound {
-			continue
-		}
-
-		// connect all the through-edges, then delete the node
-		for _, d := range g.DownEdges(n) {
-			for _, u := range g.UpEdges(n) {
-				g.Connect(dag.BasicEdge(u, d))
-			}
-		}
-		log.Printf("DestroyEdgeTransformer: pruning unused resource node %s", dag.VertexName(n))
-		g.Remove(n)
-	}
 	return nil
 }
+
+// Nodes that register instances in the instances.Expander are not needed
+// during apply if there are no instances that will lookup the expansion. This
+// is the case when a module tree is removed or during a full destroy, and we
+// may not be able to evaluate the expansion expression.
+type pruneUnusedExpanderTransformer struct {
+}
+
+func (t *pruneUnusedExpanderTransformer) Transform(g *Graph) error {
+	// We need a reverse depth first walk of modules, but it needs to be
+	// recursive so that we can process the lead modules first.
+
+	// collect all nodes into their containing module
+	type mod struct {
+		addr  addrs.Module
+		nodes []dag.Vertex
+	}
+
+	// first collect the nodes into their respective modules
+	moduleMap := make(map[string]*mod)
+	for _, v := range g.Vertices() {
+		var path addrs.Module
+		switch v := v.(type) {
+		case instanceExpander:
+			path = v.expandsInstances()
+
+		case graphNodeModuleCloser:
+			// module closers are connected like module calls, and report
+			// their parent module address
+			path = v.CloseModule()
+
+		case GraphNodeModulePath:
+			path = v.ModulePath()
+		}
+		m, ok := moduleMap[path.String()]
+		if !ok {
+			m = &mod{}
+			moduleMap[path.String()] = m
+		}
+
+		m.addr = path
+		m.nodes = append(m.nodes, v)
+	}
+
+	// now we need to restructure the modules so we can sort them
+	var modules []*mod
+
+	for _, mod := range moduleMap {
+		modules = append(modules, mod)
+	}
+
+	// Sort them by path length, longest first, so that we process the deepest
+	// modules first.  The order of modules at the same tree level doesn't
+	// matter, we just need to ensure that child modules are processed before
+	// parent modules.
+	sort.Slice(modules, func(i, j int) bool {
+		return len(modules[i].addr) > len(modules[j].addr)
+	})
+
+	for _, module := range modules {
+		t.removeUnused(module.nodes, g)
+	}
+
+	return nil
+}
+
+func (t *pruneUnusedExpanderTransformer) removeUnused(nodes []dag.Vertex, g *Graph) {
+	// since we have no defined structure within the module, just cycle through
+	// the nodes until there are no more removals
+	removed := true
+	for {
+		if !removed {
+			return
+		}
+		removed = false
+
+		last := len(nodes) - 1
+
+	NEXT:
+		for i := 0; i < len(nodes); i++ {
+			n := nodes[i]
+			switch n.(type) {
+			case graphNodeTemporaryValue:
+				if n, ok := n.(GraphNodeModulePath); ok {
+					// root outputs always have a dependency on remote state
+					if n.ModulePath().IsRoot() {
+						continue NEXT
+					}
+				}
+				for _, vv := range g.UpEdges(n) {
+					if _, ok := vv.(GraphNodeReferencer); ok {
+						continue NEXT
+					}
+				}
+
+			case instanceExpander:
+				for _, vv := range g.UpEdges(n) {
+					if _, ok := vv.(requiresInstanceExpansion); ok {
+						continue NEXT
+					}
+				}
+
+			default:
+				continue NEXT
+			}
+
+			removed = true
+
+			//// connect through edges
+			//for _, d := range g.DownEdges(n) {
+			//    for _, u := range g.UpEdges(n) {
+			//        g.Connect(dag.BasicEdge(u, d))
+			//    }
+			//}
+
+			g.Remove(n)
+
+			// remove the node from our iteration as well
+			nodes[i], nodes[last] = nodes[last], nodes[i]
+			nodes = nodes[:last]
+			last--
+		}
+	}
+}

terraform/transform_module_expansion.go

@@ -42,6 +42,10 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error {
 	// handled by the RemovedModuleTransformer, and those module closers are in
 	// the graph already, and need to be connected to their parent closers.
 	for _, v := range g.Vertices() {
+		// skip closers so they don't attach to themselves
+		if _, ok := v.(graphNodeModuleCloser); ok {
+			continue
+		}
 		// any node that executes within the scope of a module should be a
 		// GraphNodeModulePath
 		pather, ok := v.(GraphNodeModulePath)
@@ -49,7 +53,7 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error {
 			continue
 		}
 		if closer, ok := t.closers[pather.ModulePath().String()]; ok {
-			// The module root depends on each child resource instance, since
+			// The module closer depends on each child resource instance, since
 			// during apply the module expansion will complete before the
 			// individual instances are applied.
 			g.Connect(dag.BasicEdge(closer, v))

terraform/transform_reference.go

@@ -259,7 +259,6 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error {
 // values reference a resource that is no longer in the state the interpolation
 // could fail.
 type PruneUnusedValuesTransformer struct {
-	Destroy bool
 }
 
 func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
@@ -282,7 +281,7 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
 			// any referencers in the dependents means we need to keep this
 			// value for evaluation
 			removable := true
-			for _, d := range dependants.List() {
+			for _, d := range dependants {
 				if _, ok := d.(GraphNodeReferencer); ok {
 					removable = false
 					break