core: Document that TransformRoot must produce coalescable node

We use a non-pointer value for this particular node, which means that
there can never be two root nodes in the same graph: the graph
implementation will just coalesce them together when a second one is added.

Our resource expansion code is relying on that coalescing so that it can
subsume together multiple graphs for different modules instances into a
single mega-graph with all instances across all module instances, with
any root nodes coalescing together to produce a single root.

This also updates one of the context tests that exercises resource
expansion so that it will generate multiple resource instance nodes per
module and thus potentially have multiple roots to coalesce together.
However, we aren't currently explicitly validating the return values from
DynamicExpand and so this test doesn't actually fail if the coalescing
doesn't happen. We may choose to validate the DynamicExpand result in a
later commit in order to make it more obvious if future modifications fail
to uphold this invariant.

Author: apparentlymart

internal/terraform/context_plan2_test.go

@@ -453,7 +453,21 @@ func TestContext2Plan_resourceChecksInExpandedModule(t *testing.T) {
 				a = "a"
 			}
 
-			resource "test" "test" {
+			resource "test" "test1" {
+				lifecycle {
+					postcondition {
+						# It doesn't matter what this checks as long as it
+						# passes, because if we don't handle expansion properly
+						# then we'll crash before we even get to evaluating this.
+						condition = local.a == local.a
+						error_message = "Postcondition failed."
+					}
+				}
+			}
+
+			resource "test" "test2" {
+				count = 2
+
 				lifecycle {
 					postcondition {
 						# It doesn't matter what this checks as long as it
@@ -478,8 +492,12 @@ func TestContext2Plan_resourceChecksInExpandedModule(t *testing.T) {
 	assertNoErrors(t, diags)
 
 	resourceInsts := []addrs.AbsResourceInstance{
-		mustResourceInstanceAddr("module.child[0].test.test"),
-		mustResourceInstanceAddr("module.child[1].test.test"),
+		mustResourceInstanceAddr("module.child[0].test.test1"),
+		mustResourceInstanceAddr("module.child[0].test.test2[0]"),
+		mustResourceInstanceAddr("module.child[0].test.test2[1]"),
+		mustResourceInstanceAddr("module.child[1].test.test1"),
+		mustResourceInstanceAddr("module.child[1].test.test2[0]"),
+		mustResourceInstanceAddr("module.child[1].test.test2[1]"),
 	}
 
 	for _, instAddr := range resourceInsts {

internal/terraform/transform_root.go

@@ -15,11 +15,21 @@ func (t *RootTransformer) Transform(g *Graph) error {
 		return nil
 	}
 
-	// Add a root
+	// We intentionally add a graphNodeRoot value -- rather than a pointer to
+	// one -- so that all root nodes will coalesce together if two graphs
+	// are merged. Each distinct node value can only be in a graph once,
+	// so adding another graphNodeRoot value to the same graph later will
+	// be a no-op and all of the edges from root nodes will coalesce together
+	// under Graph.Subsume.
+	//
+	// It's important to retain this coalescing guarantee under future
+	// maintenence.
 	var root graphNodeRoot
 	g.Add(root)
 
-	// Connect the root to all the edges that need it
+	// We initially make the root node depend on every node except itself.
+	// If the caller subsequently runs transitive reduction on the graph then
+	// it's typical for some of these edges to then be removed.
 	for _, v := range g.Vertices() {
 		if v == root {
 			continue