stackeval: Improve coverage of namedPromiseReporter impls

The "namedPromiseReporter" interface is a helper we use to collect
user-friendly names for promises when we need to report a promise
self-reference error to the user.

We collect names for promises reactively on error, rather than proactively
in the normal course of work, because proactively maintaining a suitable
data structure for this cross-cutting concern would be both expensive and
complicated due to it being a cross-cutting concern.

This compromise therefore allows us to pay the cost of walking over all
of the promise-owning objects only in the exceptional case where we need
to report a self-reference error, but it does come at the expense of it
being possible for us to forget to implement this interface for certain
types and thus be unable to name a subset of the affected promises.

This commit implements reportNamedPromises for all promise-owning types
that are reachable from a stackeval.Main, although there is a notable gap
here where we can't actually report promises from the individual instances
of a multi-instance object (e.g. a component) because the promising API
doesn't let us peek into a promising.Once to see if it's resolved without
blocking until it becomes resolved. There are FIXME comments for that
limitation in all of the places where it's relevant, and so hopefully in a
future commit we'll devise a suitable way to perform a fallible
non-blocking peek of a promise so that we can report promises nested in its
result only if they are already present.

internal/stacks/stackruntime/internal/stackeval/component.go

@@ -347,3 +347,24 @@ func (c *Component) ApplySuccessful(ctx context.Context) bool {
 func (c *Component) tracingName() string {
 	return c.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (c *Component) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := c.Addr().String()
+	instsName := name + " instances"
+	forEachName := name + " for_each"
+	c.instances.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[map[addrs.InstanceKey]*ComponentInstance]]) {
+		cb(o.PromiseID(), instsName)
+	})
+	// FIXME: We should call reportNamedPromises on the individual
+	// ComponentInstance objects too, but promising.Once doesn't allow us
+	// to peek to see if the Once was already resolved without blocking on
+	// it, and we don't want to block on any promises in here.
+	// Without this, any promises belonging to the individual instances will
+	// not be named in a self-dependency error report, but since references
+	// to component instances are always indirect through the component this
+	// shouldn't be a big deal in most cases.
+	c.forEachValue.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), forEachName)
+	})
+}

internal/stacks/stackruntime/internal/stackeval/component_config.go

@@ -531,6 +531,12 @@ func (c *ComponentConfig) tracingName() string {
 	return c.Addr().String()
 }
 
+// reportNamedPromises implements namedPromiseReporter.
+func (c *ComponentConfig) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	cb(c.validate.PromiseID(), c.Addr().String())
+	cb(c.moduleTree.PromiseID(), c.Addr().String()+" modules")
+}
+
 // sourceBundleModuleWalker is an implementation of [configs.ModuleWalker]
 // that loads all modules from a single source bundle.
 type sourceBundleModuleWalker struct {

internal/stacks/stackruntime/internal/stackeval/component_instance.go

@@ -1488,3 +1488,8 @@ func (c *ComponentInstance) resourceTypeSchema(ctx context.Context, providerType
 func (c *ComponentInstance) tracingName() string {
 	return c.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (c *ComponentInstance) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	cb(c.moduleTreePlan.PromiseID(), c.Addr().String()+" plan")
+}

internal/stacks/stackruntime/internal/stackeval/diagnostics_test.go

@@ -0,0 +1,179 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package stackeval
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/zclconf/go-cty/cty"
+
+	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/collections"
+	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/plans"
+	"github.com/hashicorp/terraform/internal/promising"
+	"github.com/hashicorp/terraform/internal/providers"
+	providertest "github.com/hashicorp/terraform/internal/providers/testing"
+	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
+	"github.com/hashicorp/terraform/internal/stacks/stackstate"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
+
+func TestNamedPromisesPlan(t *testing.T) {
+	// The goal of this test is to make sure we retain namedPromiseReporter
+	// coverage over various important object types, so that we don't
+	// accidentally regress the quality of self-reference ("dependency cycle")
+	// errors under future maintenence.
+	//
+	// It isn't totally comprehensive over all implementations of
+	// namedPromiseReporter, but we do aim to cover the main cases that a
+	// typical stack configuration might hit.
+	//
+	// This is intentionally a test of the namedPromiseReporter implementations
+	// directly, rather than of the dependency-message-building logic built
+	// in terms of it, because the goal is for namedPromiseReporter to return
+	// everything and then the diagnostic reporter to cherry-pick only the
+	// subset of names it needs, and because this way we can get more test
+	// coverage without needing fixtures for every possible combination of
+	// self-references.
+
+	cfg := testStackConfig(t, "planning", "named_promises")
+
+	main := NewForPlanning(cfg, stackstate.NewState(), PlanOpts{
+		PlanningMode: plans.NormalMode,
+		InputVariableValues: map[stackaddrs.InputVariable]ExternalInputValue{
+			{Name: "in"}: ExternalInputValue{
+				Value: cty.StringVal("hello"),
+			},
+		},
+		ProviderFactories: ProviderFactories{
+			addrs.MustParseProviderSourceString("example.com/test/happycloud"): providers.FactoryFixed(
+				&providertest.MockProvider{
+					GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
+						Provider: providers.Schema{
+							Block: &configschema.Block{},
+						},
+						ResourceTypes: map[string]providers.Schema{
+							"happycloud_thingy": providers.Schema{
+								Block: &configschema.Block{},
+							},
+						},
+					},
+				},
+			),
+		},
+	})
+
+	// We don't actually really care about the plan here. We just want the
+	// side-effect of getting a bunch of promises created inside "main", which
+	// we'll then ask about below.
+	_, diags := testPlan(t, main)
+	assertNoDiagnostics(t, diags)
+
+	wantNames := collections.NewSetCmp[string](
+		// Component-related
+		`component.foo`,
+		`component.foo modules`,
+		`component.foo for_each`,
+		`component.foo instances`,
+
+		// Nested-stack-related
+		`stack.child collected outputs`,
+		`stack.child inputs`,
+		`stack.child for_each`,
+		`stack.child instances`,
+
+		// Provider-related
+		`example.com/test/happycloud schema`,
+		`provider["example.com/test/happycloud"].main`,
+		`provider["example.com/test/happycloud"].main for_each`,
+		`provider["example.com/test/happycloud"].main instances`,
+
+		// Output-value-related
+		`output.out value`,
+		`stack.child.output.out value`,
+		`output.out`,
+		`stack.child.output.out`,
+
+		// Input-variable-related
+		`var.in`,
+		`stack.child.var.in`,
+	)
+	gotNames := collections.NewSetCmp[string]()
+	ids := map[string]promising.PromiseID{}
+	main.reportNamedPromises(func(id promising.PromiseID, name string) {
+		gotNames.Add(name)
+		// We'll also remember the id associated with each name so that
+		// we can test the diagnostic message rendering below.
+		ids[name] = id
+		// NOTE: Some of the names get reused across both a config object
+		// and its associated dynamic object when there are no dynamic
+		// instance keys involved, and for those it's unspecified which
+		// promise ID will "win", but that's fine for our purposes here
+		// because we're only testing that some specific names get
+		// included into the error messages and so it doesn't matter which
+		// of the promise ids we use to achieve that.
+	})
+
+	if diff := cmp.Diff(wantNames, gotNames, collections.CmpOptions); diff != "" {
+		// If you're here because you've seen a failure where some of the
+		// wanted names seem to have vanished, and you weren't intentionally
+		// trying to remove them, check to make sure that the type that was
+		// supposed to report that name is still reachable indirectly from the
+		// Main.reportNamedPromises implementation.
+		t.Errorf("wrong promise names\n%s", diff)
+	}
+
+	// Since we're now holding all of the information required, let's also
+	// test that we can render some self-dependency diagnostic messages.
+	t.Run("diagnostics", func(t *testing.T) {
+		// For this we need to choose some specific promise ids to report.
+		// It doesn't matter which ones we use but we can only proceed if
+		// they were ones detected by the reportNamedPromises call earlier.
+		providerSchemaPromise := ids[`example.com/test/happycloud schema`]
+		stackCallInstancesPromise := ids[`stack.child instances`]
+		if providerSchemaPromise == promising.NoPromise || stackCallInstancesPromise == promising.NoPromise {
+			t.Fatalf("don't have the promise ids required to test diagnostic rendering")
+		}
+
+		t.Run("just one", func(t *testing.T) {
+			err := promising.ErrSelfDependent{stackCallInstancesPromise}
+			diag := taskSelfDependencyDiagnostic{
+				err:  err,
+				root: main,
+			}
+			got := diag.Description()
+			want := tfdiags.Description{
+				Summary: `Self-dependent item in configuration`,
+				Detail:  `The item "stack.child instances" depends on its own results, so there is no correct order of operations.`,
+			}
+			if diff := cmp.Diff(want, got); diff != "" {
+				t.Errorf("wrong diagnostic description\n%s", diff)
+			}
+		})
+		t.Run("multiple", func(t *testing.T) {
+			err := promising.ErrSelfDependent{
+				providerSchemaPromise,
+				stackCallInstancesPromise,
+			}
+			diag := taskSelfDependencyDiagnostic{
+				err:  err,
+				root: main,
+			}
+			got := diag.Description()
+			want := tfdiags.Description{
+				Summary: `Self-dependent items in configuration`,
+				Detail: `The following items in your configuration form a circular dependency chain through their references:
+  - example.com/test/happycloud schema
+  - stack.child instances
+
+Terraform uses references to decide a suitable order for performing operations, so configuration items may not refer to their own results either directly or indirectly.`,
+			}
+			if diff := cmp.Diff(want, got); diff != "" {
+				t.Errorf("wrong diagnostic description\n%s", diff)
+			}
+		})
+	})
+}

internal/stacks/stackruntime/internal/stackeval/input_variable.go

@@ -259,6 +259,14 @@ func (v *InputVariable) tracingName() string {
 	return v.Addr().String()
 }
 
+// reportNamedPromises implements namedPromiseReporter.
+func (v *InputVariable) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := v.Addr().String()
+	v.value.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), name)
+	})
+}
+
 // ExternalInputValue represents the value of an input variable provided
 // from outside the stack configuration.
 type ExternalInputValue struct {

internal/stacks/stackruntime/internal/stackeval/main.go

@@ -596,6 +596,12 @@ func (m *Main) reportNamedPromises(cb func(id promising.PromiseID, name string))
 	if m.mainStackConfig != nil {
 		m.mainStackConfig.reportNamedPromises(cb)
 	}
+	if m.mainStack != nil {
+		m.mainStack.reportNamedPromises(cb)
+	}
+	for _, pty := range m.providerTypes {
+		pty.reportNamedPromises(cb)
+	}
 }
 
 // availableProvisioners returns the table of provisioner factories that should

internal/stacks/stackruntime/internal/stackeval/output_value.go

@@ -190,3 +190,11 @@ func (v *OutputValue) CheckApply(ctx context.Context) ([]stackstate.AppliedChang
 func (v *OutputValue) tracingName() string {
 	return v.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (v *OutputValue) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := v.Addr().String()
+	v.resultValue.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), name)
+	})
+}

internal/stacks/stackruntime/internal/stackeval/provider.go

@@ -273,3 +273,24 @@ func (p *Provider) CheckApply(ctx context.Context) ([]stackstate.AppliedChange,
 func (p *Provider) tracingName() string {
 	return p.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (p *Provider) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := p.Addr().String()
+	forEachName := name + " for_each"
+	instsName := name + " instances"
+	p.forEachValue.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), forEachName)
+	})
+	p.instances.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[map[addrs.InstanceKey]*ProviderInstance]]) {
+		cb(o.PromiseID(), instsName)
+	})
+	// FIXME: We should call reportNamedPromises on the individual
+	// ProviderInstance objects too, but promising.Once doesn't allow us
+	// to peek to see if the Once was already resolved without blocking on
+	// it, and we don't want to block on any promises in here.
+	// Without this, any promises belonging to the individual instances will
+	// not be named in a self-dependency error report, but since references
+	// to provider instances are always indirect through the provider this
+	// shouldn't be a big deal in most cases.
+}

internal/stacks/stackruntime/internal/stackeval/provider_config.go

@@ -224,3 +224,8 @@ func (p *ProviderConfig) PlanChanges(ctx context.Context) ([]stackplan.PlannedCh
 func (p *ProviderConfig) tracingName() string {
 	return p.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (p *ProviderConfig) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	cb(p.providerArgs.PromiseID(), p.Addr().String())
+}

internal/stacks/stackruntime/internal/stackeval/provider_instance.go

@@ -308,6 +308,18 @@ func (p *ProviderInstance) tracingName() string {
 	return p.Addr().String()
 }
 
+// reportNamedPromises implements namedPromiseReporter.
+func (p *ProviderInstance) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := p.Addr().String()
+	clientName := name + " plugin client"
+	p.providerArgs.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), name)
+	})
+	p.client.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[providers.Interface]]) {
+		cb(o.PromiseID(), clientName)
+	})
+}
+
 // stubConfiguredProvider is a placeholder provider used when ConfigureProvider
 // on a real provider fails, so that callers can still receieve a usable client
 // that will just produce placeholder values from its operations.

internal/stacks/stackruntime/internal/stackeval/provider_type.go

@@ -70,3 +70,8 @@ func (pt *ProviderType) Schema(ctx context.Context) (providers.GetProviderSchema
 		return ret, nil
 	})
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (pt *ProviderType) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	cb(pt.schema.PromiseID(), pt.Addr().String()+" schema")
+}

internal/stacks/stackruntime/internal/stackeval/stack.go

@@ -16,6 +16,7 @@ import (
 	"github.com/hashicorp/terraform/internal/instances"
 	"github.com/hashicorp/terraform/internal/lang/marks"
 	"github.com/hashicorp/terraform/internal/plans"
+	"github.com/hashicorp/terraform/internal/promising"
 	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig/typeexpr"
@@ -640,3 +641,28 @@ func (s *Stack) tracingName() string {
 	}
 	return addr.String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (s *Stack) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	for _, child := range s.childStacks {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.inputVariables {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.outputValues {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.stackCalls {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.components {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.providers {
+		child.reportNamedPromises(cb)
+	}
+}

internal/stacks/stackruntime/internal/stackeval/stack_call.go

@@ -269,3 +269,24 @@ func (c *StackCall) CheckApply(ctx context.Context) ([]stackstate.AppliedChange,
 func (c *StackCall) tracingName() string {
 	return c.Addr().String()
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (c *StackCall) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	name := c.Addr().String()
+	instsName := name + " instances"
+	forEachName := name + " for_each"
+	c.instances.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[map[addrs.InstanceKey]*StackCallInstance]]) {
+		cb(o.PromiseID(), instsName)
+	})
+	// FIXME: We should call reportNamedPromises on the individual
+	// StackCallInstance objects too, but promising.Once doesn't allow us
+	// to peek to see if the Once was already resolved without blocking on
+	// it, and we don't want to block on any promises in here.
+	// Without this, any promises belonging to the individual instances will
+	// not be named in a self-dependency error report, but since references
+	// to stack call instances are always indirect through the stack call this
+	// shouldn't be a big deal in most cases.
+	c.forEachValue.Each(func(ep EvalPhase, o *promising.Once[withDiagnostics[cty.Value]]) {
+		cb(o.PromiseID(), forEachName)
+	})
+}