Repository: opentofu/opentofu
File: internal/tofu/transform_module_expansion.go (Go)

Code before:
		if err != nil {
			return err
		}
	}

	// Now go through and connect all nodes to their respective module closers.
	// This is done all at once here, because orphaned modules were already
	// 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() {
		switch v.(type) {
		case GraphNodeDestroyer:
			// Destroy nodes can only be ordered relative to other resource
			// instances.
			continue
		case *nodeCloseModule:
			// a module closer cannot connect to itself
			continue
		case *nodeExpandCheck, *nodeReportCheck, *nodeCheckAssert:
			// Check nodes should not get edges added to.
			// This prevents cycles when a module with checks is referenced
			// via depends_on by another module.
			continue
		}

		// Also skip data sources nested inside check blocks for the same reason as above.
		if t.isNestedCheckDataSource(v) {
			continue
		}

		// any node that executes within the scope of a module should be a
		// GraphNodeModulePath
		pather, ok := v.(GraphNodeModulePath)
		if !ok {
			continue
		}
		if closer, ok := t.closers[pather.ModulePath().String()]; ok {
			// 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))
		}
	}

	// Modules implicitly depend on their child modules, so connect closers to
	// other which contain their path.
	for _, c := range t.closers {
		// For a closer c with address ["module.foo", "module.bar", "module.baz"],
		// we'll look up all potential parent modules:
		//
		// - t.closers["module.foo"]

Review comment (suggestion):
```suggestion
		case *nodeExpandCheck, *nodeReportCheck, *nodeCheckAssert:
			// Check-related nodes are not module-close dependencies because
			// they don't produce any values that could potentially contribute
			// to a module's output values, and skipping these edges avoids
			// dependency cycles when a module containing checks is used
			// in a depends_on in the parent module.
			continue
		}
```

This is just a slightly more elaborate comment to try to capture the reason why we think it's okay to skip adding these dependencies, in addition to the reason why we _need_ to skip adding these dependencies. We've had some recent experience elsewhere with trying to understand why certain special cases are present in graph construction, so I hope this'll be helpful context for a future maintainer someday. 😀 


Code after:
		if err != nil {
			return err
		}
	}

	// Now go through and connect all nodes to their respective module closers.
	// This is done all at once here, because orphaned modules were already
	// 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() {
		switch v.(type) {
		case GraphNodeDestroyer:
			// Destroy nodes can only be ordered relative to other resource
			// instances.
			continue
		case *nodeCloseModule:
			// a module closer cannot connect to itself
			continue
		case *nodeExpandCheck, *nodeReportCheck, *nodeCheckAssert:
			// Check-related nodes are not module-close dependencies because
			// they don't produce any values that could potentially contribute
			// to a module's output values, and skipping these edges avoids
			// dependency cycles when a module containing checks is used
			// in a depends_on in the parent module.
			continue
		}

		// Also skip data sources nested inside check blocks for the same reason as above.
		if t.isNestedCheckDataSource(v) {
			continue
		}

		// any node that executes within the scope of a module should be a
		// GraphNodeModulePath
		pather, ok := v.(GraphNodeModulePath)
		if !ok {
			continue
		}
		if closer, ok := t.closers[pather.ModulePath().String()]; ok {
			// 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))
		}
	}

	// Modules implicitly depend on their child modules, so connect closers to
	// other which contain their path.
	for _, c := range t.closers {
		// For a closer c with address ["module.foo", "module.bar", "module.baz"],
		// we'll look up all potential parent modules: