terraform: Further generalization of the EvalContext scope handling

This is a continuation of some restructuring started in earlier commits:
    641837d
    42f012b
    079021d

In those earlier commits my goal was to minimize breaking changes to the
EvalContext API to churn existing code as little as possible.

However, the way I achieved that was to handle fully-expanded modules and
partial-expanded modules as two completely separate codepaths, which means
that all other code must statically decide which of the two cases it's
dealing with even though there's not really any reason the EvalContext
implementation couldn't support that being decided dynamically.

For the "deferred actions" work, in a future commit we'll need to
introduce a new graph node type representing partial-expanded resources
which must decide dynamically whether it's in a fully-expanded module
scope or a partial-expanded module scope, because the treatment is quite
different for a fully-expanded module containing a resource with unknown
expansion vs. a resource that's inside a module that hasn't been expanded
itself.

This is therefore the logical conclusion of the previous work, making
"eval context scope" the primary way that we talk about where an eval
context is doing its work, with a helper function for the common case
where that's a fully-expanded ModuleInstance.

In particular, this includes a third variation of the family of interfaces
that have method Path indicating the scope that the node should be
evaluated in, which is allowed to return any non-nil evalContextScope
chosen dynamically at runtime. The existing interfaces that statically
return addrs.ModuleInstance and addrs.PartialExpandedModule remain as
statically-typed alternatives for the common case, since the need to
decide dynamically is (for now) limited only to nodes representing
addrs.PartialExpandedResource addresses.

This has unfortunately now churned a little more code than I originally
wanted to, but the helper functions and the retaining of the
statically-typed node-scope-discovery interfaces has kept it as small as
possible.

(This also continues our effort to gradually make the implementation
details of package terraform be unexported, at the expense of a little
inconsistency in the short term where these new fields/types are mixed
in with exported fields/types. Hopefully we'll continue on this quest
over time and eventually reach things being consistently unexported.)

internal/terraform/context_eval.go

@@ -116,6 +116,6 @@ func evalScopeFromGraphWalk(walker *ContextGraphWalker, moduleAddr addrs.ModuleI
 	// just to get hold of an EvalContext for it. ContextGraphWalker
 	// caches its contexts, so we should get hold of the context that was
 	// previously used for evaluation here, unless we skipped walking.
-	evalCtx := walker.EnterPath(moduleAddr)
+	evalCtx := walker.enterScope(evalContextModuleInstance{Addr: moduleAddr})
 	return evalCtx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey)
 }

internal/terraform/eval_context.go

@@ -190,11 +190,13 @@ type EvalContext interface {
 	// this execution.
 	Overrides() *mocking.Overrides
 
-	// WithPath returns a copy of the context with the internal path set to the
-	// path argument.
-	WithPath(path addrs.ModuleInstance) EvalContext
+	// withScope derives a new EvalContext that has all of the same global
+	// context, but a new evaluation scope.
+	withScope(scope evalContextScope) EvalContext
+}
 
-	// WithPartialExpandedPath returns a copy of the context with the internal
-	// path set to the path argument.
-	WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext
+func evalContextForModuleInstance(baseCtx EvalContext, addr addrs.ModuleInstance) EvalContext {
+	return baseCtx.withScope(evalContextModuleInstance{
+		Addr: addr,
+	})
 }

internal/terraform/eval_context_builtin.go

@@ -86,19 +86,9 @@ type BuiltinEvalContext struct {
 // BuiltinEvalContext implements EvalContext
 var _ EvalContext = (*BuiltinEvalContext)(nil)
 
-func (ctx *BuiltinEvalContext) WithPath(path addrs.ModuleInstance) EvalContext {
+func (ctx *BuiltinEvalContext) withScope(scope evalContextScope) EvalContext {
 	newCtx := *ctx
-	newCtx.scope = evalContextModuleInstance{
-		Addr: path,
-	}
-	return &newCtx
-}
-
-func (ctx *BuiltinEvalContext) WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext {
-	newCtx := *ctx
-	newCtx.scope = evalContextPartialExpandedModule{
-		Addr: path,
-	}
+	newCtx.scope = scope
 	return &newCtx
 }
 

internal/terraform/eval_context_builtin_test.go

@@ -20,12 +20,12 @@ func TestBuiltinEvalContextProviderInput(t *testing.T) {
 	cache := make(map[string]map[string]cty.Value)
 
 	ctx1 := testBuiltinEvalContext(t)
-	ctx1 = ctx1.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext)
+	ctx1 = ctx1.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance}).(*BuiltinEvalContext)
 	ctx1.ProviderInputConfig = cache
 	ctx1.ProviderLock = &lock
 
 	ctx2 := testBuiltinEvalContext(t)
-	ctx2 = ctx2.WithPath(addrs.RootModuleInstance.Child("child", addrs.NoKey)).(*BuiltinEvalContext)
+	ctx2 = ctx2.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey)}).(*BuiltinEvalContext)
 	ctx2.ProviderInputConfig = cache
 	ctx2.ProviderLock = &lock
 
@@ -61,7 +61,7 @@ func TestBuildingEvalContextInitProvider(t *testing.T) {
 	testP := &MockProvider{}
 
 	ctx := testBuiltinEvalContext(t)
-	ctx = ctx.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext)
+	ctx = ctx.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance}).(*BuiltinEvalContext)
 	ctx.ProviderLock = &lock
 	ctx.ProviderCache = make(map[string]providers.Interface)
 	ctx.Plugins = newContextPlugins(map[addrs.Provider]providers.Factory{

internal/terraform/eval_context_mock.go

@@ -118,7 +118,7 @@ type MockEvalContext struct {
 	EvaluationScopeScope   *lang.Scope
 
 	PathCalled bool
-	PathPath   addrs.ModuleInstance
+	Scope      evalContextScope
 
 	LanguageExperimentsActive experiments.Set
 
@@ -326,23 +326,18 @@ func (c *MockEvalContext) EvaluationScope(self addrs.Referenceable, source addrs
 	return c.EvaluationScopeScope
 }
 
-func (c *MockEvalContext) WithPath(path addrs.ModuleInstance) EvalContext {
+func (c *MockEvalContext) withScope(scope evalContextScope) EvalContext {
 	newC := *c
-	newC.PathPath = path
+	newC.Scope = scope
 	return &newC
 }
 
-func (c *MockEvalContext) WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext {
-	// This is not yet implemented as a mock, because we've not yet had any
-	// need for it. If we end up needing to test this behavior in isolation
-	// somewhere then we'll need to figure out how to fit it in here without
-	// upsetting too many existing tests that rely on the PathPath field.
-	panic("WithPartialExpandedPath not implemented for MockEvalContext")
-}
-
 func (c *MockEvalContext) Path() addrs.ModuleInstance {
 	c.PathCalled = true
-	return c.PathPath
+	// This intentionally panics if scope isn't a module instance; callers
+	// should use this only for an eval context that's working in a
+	// fully-expanded module instance.
+	return c.Scope.(evalContextModuleInstance).Addr
 }
 
 func (c *MockEvalContext) LanguageExperimentActive(experiment experiments.Experiment) bool {

internal/terraform/eval_context_scope.go

@@ -5,6 +5,7 @@ package terraform
 
 import (
 	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/collections"
 )
 
 // evalContextScope represents the scope that an [EvalContext] (or rather,
@@ -26,13 +27,17 @@ import (
 //     a common known prefix, in situations where a module call has an unknown
 //     value for its count or for_each argument.
 type evalContextScope interface {
+	collections.UniqueKeyer[evalContextScope]
+
 	// evalContextScopeModule returns the static module address of whatever
 	// fully- or partially-expanded module instance address this scope is
 	// associated with.
 	//
 	// A "global" evaluation context is a nil [evalContextScope], and so
 	// this method will panic for that scope.
 	evalContextScopeModule() addrs.Module
+
+	String() string
 }
 
 // evalContextGlobal is the nil [evalContextScope] used to represent an
@@ -49,6 +54,16 @@ func (s evalContextModuleInstance) evalContextScopeModule() addrs.Module {
 	return s.Addr.Module()
 }
 
+func (s evalContextModuleInstance) String() string {
+	return s.Addr.String()
+}
+
+func (s evalContextModuleInstance) UniqueKey() collections.UniqueKey[evalContextScope] {
+	return evalContextScopeUniqueKey{
+		k: s.Addr.UniqueKey(),
+	}
+}
+
 // evalContextPartialExpandedModule is an [evalContextScope] associated with
 // an unbounded set of possible module instances that share a common known
 // address prefix.
@@ -59,3 +74,20 @@ type evalContextPartialExpandedModule struct {
 func (s evalContextPartialExpandedModule) evalContextScopeModule() addrs.Module {
 	return s.Addr.Module()
 }
+
+func (s evalContextPartialExpandedModule) String() string {
+	return s.Addr.String()
+}
+
+func (s evalContextPartialExpandedModule) UniqueKey() collections.UniqueKey[evalContextScope] {
+	return evalContextScopeUniqueKey{
+		k: s.Addr.UniqueKey(),
+	}
+}
+
+type evalContextScopeUniqueKey struct {
+	k addrs.UniqueKey
+}
+
+// IsUniqueKey implements collections.UniqueKey.
+func (evalContextScopeUniqueKey) IsUniqueKey(evalContextScope) {}

internal/terraform/eval_import.go

@@ -21,7 +21,7 @@ func evaluateImportIdExpression(expr hcl.Expression, ctx EvalContext, keyData in
 
 	// import blocks only exist in the root module, and must be evaluated in
 	// that context.
-	ctx = ctx.WithPath(addrs.RootModuleInstance)
+	ctx = evalContextForModuleInstance(ctx, addrs.RootModuleInstance)
 
 	if expr == nil {
 		return "", diags.Append(&hcl.Diagnostic{

internal/terraform/graph.go

@@ -76,21 +76,33 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics {
 
 		// vertexCtx is the context that we use when evaluating. This
 		// is normally the global context but can be overridden
-		// with a GraphNodeModuleInstance or GraphNodePartialExpandedModule
-		// impl. (The two interfaces are intentionally mutually-exclusive by
-		// both having the same method name but with different signatures,
-		// since a node can't belong to two different contexts at once.)
+		// with either a GraphNodeModuleInstance, GraphNodePartialExpandedModule,
+		// or graphNodeEvalContextScope implementation. (These interfaces are
+		// all intentionally mutually-exclusive by having the same method
+		// name but different signatures, since a node can only belong to
+		// one context at a time.)
 		vertexCtx := ctx
-		if pn, ok := v.(GraphNodeModuleInstance); ok {
+		if pn, ok := v.(graphNodeEvalContextScope); ok {
+			scope := pn.Path()
+			log.Printf("[TRACE] vertex %q: belongs to %s", dag.VertexName(v), scope)
+			vertexCtx = walker.enterScope(scope)
+			defer walker.exitScope(scope)
+		} else if pn, ok := v.(GraphNodeModuleInstance); ok {
 			moduleAddr := pn.Path() // An addrs.ModuleInstance
 			log.Printf("[TRACE] vertex %q: belongs to %s", dag.VertexName(v), moduleAddr)
-			vertexCtx = walker.EnterPath(moduleAddr)
-			defer walker.ExitPath(pn.Path())
+			scope := evalContextModuleInstance{
+				Addr: moduleAddr,
+			}
+			vertexCtx = walker.enterScope(scope)
+			defer walker.exitScope(scope)
 		} else if pn, ok := v.(GraphNodePartialExpandedModule); ok {
 			moduleAddr := pn.Path() // An addrs.PartialExpandedModule
 			log.Printf("[TRACE] vertex %q: belongs to all of %s", dag.VertexName(v), moduleAddr)
-			vertexCtx = walker.EnterPartialExpandedPath(pn.Path())
-			defer walker.ExitPartialExpandedPath(pn.Path())
+			scope := evalContextPartialExpandedModule{
+				Addr: moduleAddr,
+			}
+			vertexCtx = walker.enterScope(scope)
+			defer walker.exitScope(scope)
 		} else {
 			log.Printf("[TRACE] vertex %q: does not belong to any module instance", dag.VertexName(v))
 		}

internal/terraform/graph_interface_subgraph.go

@@ -36,3 +36,17 @@ type GraphNodeModulePath interface {
 type GraphNodePartialExpandedModule interface {
 	Path() addrs.PartialExpandedModule
 }
+
+// graphNodeEvalContextScope is essentially a combination of
+// [GraphNodeModuleInstance] and [GraphNodePartialExpandedModule] for when
+// the decision between the two must be made dynamically.
+//
+// When a graph node implements this interface, the [EvalContext] passed
+// to its DynamicExpand and/or Execute method will be associated with whatever
+// scope is returned by method Path.
+type graphNodeEvalContextScope interface {
+	// Path must return a _non-nil_ evalContextScope value, which therefore
+	// describes either a fully-expanded module instance address or a
+	// partial-expanded module address.
+	Path() evalContextScope
+}

internal/terraform/graph_walk.go

@@ -4,18 +4,15 @@
 package terraform
 
 import (
-	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
 // GraphWalker is an interface that can be implemented that when used
 // with Graph.Walk will invoke the given callbacks under certain events.
 type GraphWalker interface {
 	EvalContext() EvalContext
-	EnterPath(addrs.ModuleInstance) EvalContext
-	ExitPath(addrs.ModuleInstance)
-	EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext
-	ExitPartialExpandedPath(addrs.PartialExpandedModule)
+	enterScope(evalContextScope) EvalContext
+	exitScope(evalContextScope)
 	Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics
 }
 
@@ -24,11 +21,7 @@ type GraphWalker interface {
 // implementing all the required functions.
 type NullGraphWalker struct{}
 
-func (NullGraphWalker) EvalContext() EvalContext                   { return new(MockEvalContext) }
-func (NullGraphWalker) EnterPath(addrs.ModuleInstance) EvalContext { return new(MockEvalContext) }
-func (NullGraphWalker) ExitPath(addrs.ModuleInstance)              {}
-func (NullGraphWalker) EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext {
-	return new(MockEvalContext)
-}
-func (NullGraphWalker) ExitPartialExpandedPath(addrs.PartialExpandedModule)          {}
+func (NullGraphWalker) EvalContext() EvalContext                                     { return new(MockEvalContext) }
+func (NullGraphWalker) enterScope(evalContextScope) EvalContext                      { return new(MockEvalContext) }
+func (NullGraphWalker) exitScope(evalContextScope)                                   {}
 func (NullGraphWalker) Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics { return nil }

internal/terraform/graph_walk_context.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/checks"
+	"github.com/hashicorp/terraform/internal/collections"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/instances"
@@ -51,7 +52,7 @@ type ContextGraphWalker struct {
 	NonFatalDiagnostics tfdiags.Diagnostics
 
 	once                sync.Once
-	contexts            map[string]*BuiltinEvalContext
+	contexts            collections.Map[evalContextScope, *BuiltinEvalContext]
 	contextLock         sync.Mutex
 	providerCache       map[string]providers.Interface
 	providerFuncCache   map[string]providers.Interface
@@ -65,33 +66,23 @@ type ContextGraphWalker struct {
 
 var _ GraphWalker = (*ContextGraphWalker)(nil)
 
-func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext {
-	w.contextLock.Lock()
-	defer w.contextLock.Unlock()
-
-	// If we already have a context for this path cached, use that
-	key := path.String()
-	if ctx, ok := w.contexts[key]; ok {
-		return ctx
+// enterScope provides an EvalContext associated with the given scope.
+func (w *ContextGraphWalker) enterScope(scope evalContextScope) EvalContext {
+	if scope == nil {
+		// Just want a global EvalContext then, presumably.
+		return w.EvalContext()
 	}
 
-	ctx := w.EvalContext().WithPath(path)
-	w.contexts[key] = ctx.(*BuiltinEvalContext)
-	return ctx
-}
-
-func (w *ContextGraphWalker) EnterPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext {
 	w.contextLock.Lock()
 	defer w.contextLock.Unlock()
 
-	// If we already have a context for this path cached, use that
-	key := path.String()
-	if ctx, ok := w.contexts[key]; ok {
+	// We might already have a context for this scope.
+	if ctx, ok := w.contexts.GetOk(scope); ok {
 		return ctx
 	}
 
-	ctx := w.EvalContext().WithPartialExpandedPath(path)
-	w.contexts[key] = ctx.(*BuiltinEvalContext)
+	ctx := w.EvalContext().withScope(scope).(*BuiltinEvalContext)
+	w.contexts.Put(scope, ctx)
 	return ctx
 }
 
@@ -142,7 +133,7 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
 }
 
 func (w *ContextGraphWalker) init() {
-	w.contexts = make(map[string]*BuiltinEvalContext)
+	w.contexts = collections.NewMap[evalContextScope, *BuiltinEvalContext]()
 	w.providerCache = make(map[string]providers.Interface)
 	w.providerFuncCache = make(map[string]providers.Interface)
 	w.providerSchemas = make(map[string]providers.ProviderSchema)