core: Fix destroy when modules vars are used in resource counts

A new problem was introduced by the prior fixes for destroy
interpolation messages when resources depend on module variables with
a _count_ attribute, this makes the variable crucial for properly
building the graph - even in destroys. So removing all module variables
from the graph as noops was overzealous.

By borrowing the logic in `DestroyEdgeInclude` we are able to determine
if we need to keep a given module variable relatively easily.

I'd like to overhaul the `Destroy: true` implementation so that it does
not depend on config at all, but I want to continue for now with the
targeted fixes that we can backport into the 0.6 series.

terraform/context_apply_test.go

@@ -2622,6 +2622,90 @@ module.child:
 	}
 }
 
+func TestContext2Apply_destroyWithModuleVariableAndCount(t *testing.T) {
+	m := testModule(t, "apply-destroy-mod-var-and-count")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	var state *State
+	var err error
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Module: m,
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		if _, err := ctx.Plan(); err != nil {
+			t.Fatalf("plan err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("apply err: %s", err)
+		}
+	}
+
+	h := new(HookRecordApplyOrder)
+	h.Active = true
+
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Destroy: true,
+			Module:  m,
+			State:   state,
+			Hooks:   []Hook{h},
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		plan, err := ctx.Plan()
+		if err != nil {
+			t.Fatalf("destroy plan err: %s", err)
+		}
+
+		var buf bytes.Buffer
+		if err := WritePlan(plan, &buf); err != nil {
+			t.Fatalf("plan write err: %s", err)
+		}
+
+		planFromFile, err := ReadPlan(&buf)
+		if err != nil {
+			t.Fatalf("plan read err: %s", err)
+		}
+
+		ctx, err = planFromFile.Context(&ContextOpts{
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("destroy apply err: %s", err)
+		}
+	}
+
+	//Test that things were destroyed
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(`
+<no state>
+module.child:
+  <no state>
+		`)
+	if actual != expected {
+		t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
+	}
+}
+
 func TestContext2Apply_destroyOutputs(t *testing.T) {
 	m := testModule(t, "apply-destroy-outputs")
 	h := new(HookRecordApplyOrder)

terraform/graph_config_node_variable.go

@@ -94,9 +94,19 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool {
 	// the flat node's implementation of Path() below.
 	modDiff := opts.Diff.ModuleByPath(n.ModulePath)
 
-	// If we're destroying, we have no need of variables.
+	// If we're destroying, we have no need of variables unless they are depended
+	// on by the count of a resource.
 	if modDiff != nil && modDiff.Destroy {
-		log.Printf("[DEBUG] Destroy diff, treating variable as a noop")
+		for _, v := range opts.Graph.UpEdges(opts.Vertex).List() {
+			// Here we borrow the implementation of DestroyEdgeInclude, whose logic
+			// and semantics are exactly what we want here.
+			if n.DestroyEdgeInclude(v) {
+				log.Printf("[DEBUG] Variable has destroy edge from %s, not a noop",
+					dag.VertexName(v))
+				return false
+			}
+		}
+		log.Printf("[DEBUG] Variable has no included destroy edges: noop!")
 		return true
 	}
 

terraform/test-fixtures/apply-destroy-mod-var-and-count/child/main.tf

@@ -0,0 +1,5 @@
+variable "mod_count" { }
+
+resource "aws_instance" "foo" {
+  count = "${var.mod_count}"
+}

terraform/test-fixtures/apply-destroy-mod-var-and-count/main.tf

@@ -0,0 +1,4 @@
+module "child" {
+  source    = "./child"
+  mod_count = "3"
+}