dag: fix ReverseDepthFirstWalk when nodes remove themselves

The report in #7378 led us into a deep rabbit hole that turned out to
expose a bug in the graph walk implementation being used by the
`NoopTransformer`. The problem ended up being when two nodes in a single
dependency chain both reported `Noop() -> true` and needed to be
removed. This was breaking the walk and preventing the second node from
ever being visited.

Fixes #7378

dag/dag.go

@@ -332,12 +332,7 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) er
 		}
 		seen[current.Vertex] = struct{}{}
 
-		// Visit the current node
+		// Add next set of targets in a consistent order.
-		if err := f(current.Vertex, current.Depth); err != nil {
-			return err
-		}
-
-		// Visit targets of this in a consistent order.
 		targets := AsVertexList(g.UpEdges(current.Vertex))
 		sort.Sort(byVertexName(targets))
 		for _, t := range targets {
@@ -346,6 +341,11 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) er
 				Depth:  current.Depth + 1,
 			})
 		}
+
+		// Visit the current node
+		if err := f(current.Vertex, current.Depth); err != nil {
+			return err
+		}
 	}
 
 	return nil

dag/dag_test.go

@@ -260,6 +260,33 @@ func TestAcyclicGraphWalk_error(t *testing.T) {
 	t.Fatalf("bad: %#v", visits)
 }
 
+func TestAcyclicGraph_ReverseDepthFirstWalk_WithRemoval(t *testing.T) {
+	var g AcyclicGraph
+	g.Add(1)
+	g.Add(2)
+	g.Add(3)
+	g.Connect(BasicEdge(3, 2))
+	g.Connect(BasicEdge(2, 1))
+
+	var visits []Vertex
+	var lock sync.Mutex
+	err := g.ReverseDepthFirstWalk([]Vertex{1}, func(v Vertex, d int) error {
+		lock.Lock()
+		defer lock.Unlock()
+		visits = append(visits, v)
+		g.Remove(v)
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := []Vertex{1, 2, 3}
+	if !reflect.DeepEqual(visits, expected) {
+		t.Fatalf("expected: %#v, got: %#v", expected, visits)
+	}
+}
+
 const testGraphTransReductionStr = `
 1
   2

terraform/context_apply_test.go

@@ -4873,6 +4873,8 @@ func TestContext2Apply_destroyNestedModuleWithAttrsReferencingResource(t *testin
 	expected := strings.TrimSpace(`
 <no state>
 module.middle:
+  <no state>
+module.middle.bottom:
   <no state>
 		`)
 	if actual != expected {

terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf

@@ -1 +1,5 @@
-variable "bottom_param" {}
+variable bottom_param {}
+
+resource "null_resource" "bottom" {
+  value = "${var.bottom_param}"
+}

terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf

@@ -1,8 +1,10 @@
-variable "param" {}
+variable param {}
-
-resource "null_resource" "n" {}
 
 module "bottom" {
   source       = "./bottom"
   bottom_param = "${var.param}"
 }
+
+resource "null_resource" "middle" {
+  value = "${var.param}"
+}