From aeadb8ca9037806d67f34e2d0e81753bab1f4fbb Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 10 Apr 2020 16:52:47 -0400
Subject: [PATCH 01/12] start with a failing test

---
 terraform/context_plan_test.go | 44 ++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go
index cf2c3e021..7a97f7b23 100644
--- a/terraform/context_plan_test.go
+++ b/terraform/context_plan_test.go
@@ -5832,3 +5832,47 @@ resource "aws_instance" "foo" {
 		t.Errorf("missing %s change for %s", action, res)
 	}
 }
+
+func TestContext2Plan_indexInVar(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "a" {
+  count = 1
+  source = "./mod"
+  in = "test"
+}
+
+module "b" {
+  count = 1
+  source = "./mod"
+  in = length(module.a)
+}
+`,
+		"mod/main.tf": `
+resource "aws_instance" "foo" {
+  foo = var.in
+}
+
+variable "in" {
+}
+
+output"out" {
+  value = aws_instance.foo.id
+}
+`,
+	})
+
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+		},
+	})
+
+	_, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+}

From 600d4c3e1f05e314419c10e1206fe471aadf2b32 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Sun, 12 Apr 2020 10:43:41 -0400
Subject: [PATCH 02/12] eval Data needs to operate on whole modules

In order to be able to use module values, and handle operations like
possibly invalid module indexes in conditional statements, whole modules
must always be returned during evaluation.
---
 lang/data.go      |   2 +-
 lang/data_test.go |   2 +-
 lang/eval.go      | 121 +++++-----------------------------------------
 lang/eval_test.go |  18 +++++++
 4 files changed, 32 insertions(+), 111 deletions(-)

diff --git a/lang/data.go b/lang/data.go
index 27ee17028..e4e227972 100644
--- a/lang/data.go
+++ b/lang/data.go
@@ -26,7 +26,7 @@ type Data interface {
 	GetForEachAttr(addrs.ForEachAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetResource(addrs.Resource, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
-	GetModuleInstance(addrs.ModuleCallInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
+	GetModule(addrs.ModuleCall, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetModuleInstanceOutput(addrs.AbsModuleCallOutput, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetPathAttr(addrs.PathAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetTerraformAttr(addrs.TerraformAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
diff --git a/lang/data_test.go b/lang/data_test.go
index ba825bd4d..4d9e7b0ee 100644
--- a/lang/data_test.go
+++ b/lang/data_test.go
@@ -43,7 +43,7 @@ func (d *dataForTests) GetLocalValue(addr addrs.LocalValue, rng tfdiags.SourceRa
 	return d.LocalValues[addr.Name], nil
 }
 
-func (d *dataForTests) GetModuleInstance(addr addrs.ModuleCallInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d *dataForTests) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	return d.Modules[addr.String()], nil
 }
 
diff --git a/lang/eval.go b/lang/eval.go
index 0ddee410a..8e3422787 100644
--- a/lang/eval.go
+++ b/lang/eval.go
@@ -2,8 +2,6 @@ package lang
 
 import (
 	"fmt"
-	"log"
-	"strconv"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/ext/dynblock"
@@ -196,7 +194,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 	// that's redundant in the process of populating our values map.
 	dataResources := map[string]map[string]cty.Value{}
 	managedResources := map[string]map[string]cty.Value{}
-	wholeModules := map[string]map[addrs.InstanceKey]cty.Value{}
+	wholeModules := map[string]cty.Value{}
 	moduleOutputs := map[string]map[addrs.InstanceKey]map[string]cty.Value{}
 	inputVariables := map[string]cty.Value{}
 	localValues := map[string]cty.Value{}
@@ -258,9 +256,14 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 
 		// This type switch must cover all of the "Referenceable" implementations
 		// in package addrs, however we are removing the possibility of
-		// ResourceInstance beforehand.
-		if addr, ok := rawSubj.(addrs.ResourceInstance); ok {
+		// Instances beforehand.
+		switch addr := rawSubj.(type) {
+		case addrs.ResourceInstance:
 			rawSubj = addr.ContainingResource()
+		case addrs.ModuleCallInstance:
+			rawSubj = addr.Call
+		case addrs.AbsModuleCallOutput:
+			rawSubj = addr.Call.Call
 		}
 
 		switch subj := rawSubj.(type) {
@@ -284,14 +287,10 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 			}
 			into[r.Type][r.Name] = val
 
-		case addrs.ModuleCallInstance:
-			val, valDiags := normalizeRefValue(s.Data.GetModuleInstance(subj, rng))
+		case addrs.ModuleCall:
+			val, valDiags := normalizeRefValue(s.Data.GetModule(subj, rng))
 			diags = diags.Append(valDiags)
-
-			if wholeModules[subj.Call.Name] == nil {
-				wholeModules[subj.Call.Name] = make(map[addrs.InstanceKey]cty.Value)
-			}
-			wholeModules[subj.Call.Name][subj.Key] = val
+			wholeModules[subj.Name] = val
 
 		case addrs.AbsModuleCallOutput:
 			val, valDiags := normalizeRefValue(s.Data.GetModuleInstanceOutput(subj, rng))
@@ -347,7 +346,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 		vals[k] = v
 	}
 	vals["data"] = cty.ObjectVal(buildResourceObjects(dataResources))
-	vals["module"] = cty.ObjectVal(buildModuleObjects(wholeModules, moduleOutputs))
+	vals["module"] = cty.ObjectVal(wholeModules)
 	vals["var"] = cty.ObjectVal(inputVariables)
 	vals["local"] = cty.ObjectVal(localValues)
 	vals["path"] = cty.ObjectVal(pathAttrs)
@@ -369,102 +368,6 @@ func buildResourceObjects(resources map[string]map[string]cty.Value) map[string]
 	return vals
 }
 
-func buildModuleObjects(wholeModules map[string]map[addrs.InstanceKey]cty.Value, moduleOutputs map[string]map[addrs.InstanceKey]map[string]cty.Value) map[string]cty.Value {
-	vals := make(map[string]cty.Value)
-
-	for name, keys := range wholeModules {
-		vals[name] = buildInstanceObjects(keys)
-	}
-
-	for name, keys := range moduleOutputs {
-		if _, exists := wholeModules[name]; exists {
-			// If we also have a whole module value for this name then we'll
-			// skip this since the individual outputs are embedded in that result.
-			continue
-		}
-
-		// The shape of this collection isn't compatible with buildInstanceObjects,
-		// but rather than replicating most of the buildInstanceObjects logic
-		// here we'll instead first transform the structure to be what that
-		// function expects and then use it. This is a little wasteful, but
-		// we do not expect this these maps to be large and so the extra work
-		// here should not hurt too much.
-		flattened := make(map[addrs.InstanceKey]cty.Value, len(keys))
-		for k, vals := range keys {
-			flattened[k] = cty.ObjectVal(vals)
-		}
-		vals[name] = buildInstanceObjects(flattened)
-	}
-
-	return vals
-}
-
-func buildInstanceObjects(keys map[addrs.InstanceKey]cty.Value) cty.Value {
-	if val, exists := keys[addrs.NoKey]; exists {
-		// If present, a "no key" value supersedes all other values,
-		// since they should be embedded inside it.
-		return val
-	}
-
-	// If we only have individual values then we need to construct
-	// either a list or a map, depending on what sort of keys we
-	// have.
-	haveInt := false
-	haveString := false
-	maxInt := 0
-
-	for k := range keys {
-		switch tk := k.(type) {
-		case addrs.IntKey:
-			haveInt = true
-			if int(tk) > maxInt {
-				maxInt = int(tk)
-			}
-		case addrs.StringKey:
-			haveString = true
-		}
-	}
-
-	// We should either have ints or strings and not both, but
-	// if we have both then we'll prefer strings and let the
-	// language interpreter try to convert the int keys into
-	// strings in a map.
-	switch {
-	case haveString:
-		vals := make(map[string]cty.Value)
-		for k, v := range keys {
-			switch tk := k.(type) {
-			case addrs.StringKey:
-				vals[string(tk)] = v
-			case addrs.IntKey:
-				sk := strconv.Itoa(int(tk))
-				vals[sk] = v
-			}
-		}
-		return cty.ObjectVal(vals)
-	case haveInt:
-		// We'll make a tuple that is long enough for our maximum
-		// index value. It doesn't matter if we end up shorter than
-		// the number of instances because if length(...) were
-		// being evaluated we would've got a NoKey reference and
-		// thus not ended up in this codepath at all.
-		vals := make([]cty.Value, maxInt+1)
-		for i := range vals {
-			if v, exists := keys[addrs.IntKey(i)]; exists {
-				vals[i] = v
-			} else {
-				// Just a placeholder, since nothing will access this anyway
-				vals[i] = cty.DynamicVal
-			}
-		}
-		return cty.TupleVal(vals)
-	default:
-		// Should never happen because there are no other key types.
-		log.Printf("[ERROR] strange makeInstanceObjects call with no supported key types")
-		return cty.EmptyObjectVal
-	}
-}
-
 func normalizeRefValue(val cty.Value, diags tfdiags.Diagnostics) (cty.Value, tfdiags.Diagnostics) {
 	if diags.HasErrors() {
 		// If there are errors then we will force an unknown result so that
diff --git a/lang/eval_test.go b/lang/eval_test.go
index fc6a3c22a..f9ee894d6 100644
--- a/lang/eval_test.go
+++ b/lang/eval_test.go
@@ -178,6 +178,22 @@ func TestScopeEvalContext(t *testing.T) {
 				}),
 			},
 		},
+		{
+			// at this level, all instance references return the entire resource
+			`null_resource.each["each1"].attr`,
+			map[string]cty.Value{
+				"null_resource": cty.ObjectVal(map[string]cty.Value{
+					"each": cty.ObjectVal(map[string]cty.Value{
+						"each0": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each0"),
+						}),
+						"each1": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each1"),
+						}),
+					}),
+				}),
+			},
+		},
 		{
 			`foo(null_resource.multi, null_resource.multi[1])`,
 			map[string]cty.Value{
@@ -216,11 +232,13 @@ func TestScopeEvalContext(t *testing.T) {
 				}),
 			},
 		},
+		// any module reference returns the entire module
 		{
 			`module.foo.output1`,
 			map[string]cty.Value{
 				"module": cty.ObjectVal(map[string]cty.Value{
 					"foo": cty.ObjectVal(map[string]cty.Value{
+						"output0": cty.StringVal("bar0"),
 						"output1": cty.StringVal("bar1"),
 					}),
 				}),

From a7507e140d229744b9a71e20f991241600f47e3e Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Sun, 12 Apr 2020 11:26:44 -0400
Subject: [PATCH 03/12] parse module references as whole modules

Module references, like resource references, need to always return the
and object containing all instances in order to handle modules as single
values, and to postpone index evaluation to when the expression as whole
is evaluated.
---
 addrs/parse_ref.go      | 17 ++++++++---------
 addrs/parse_ref_test.go |  6 ++----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/addrs/parse_ref.go b/addrs/parse_ref.go
index cd8b114f6..eac77f306 100644
--- a/addrs/parse_ref.go
+++ b/addrs/parse_ref.go
@@ -120,10 +120,9 @@ func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 			return nil, diags
 		}
 
-		// A traversal starting with "module" can either be a reference to
-		// an entire module instance or to a single output from a module
-		// instance, depending on what we find after this introducer.
-
+		// A traversal starting with "module" can either be a reference to an
+		// entire module, or to a single output from a module instance,
+		// depending on what we find after this introducer.
 		callInstance := ModuleCallInstance{
 			Call: ModuleCall{
 				Name: callName,
@@ -132,12 +131,12 @@ func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 		}
 
 		if len(remain) == 0 {
-			// Reference to an entire module instance. Might alternatively
-			// be a reference to a collection of instances of a particular
-			// module, but the caller will need to deal with that ambiguity
-			// since we don't have enough context here.
+			// Reference to an entire module. Might alternatively be a
+			// reference to a single instance of a particular module, but the
+			// caller will need to deal with that ambiguity since we don't have
+			// enough context here.
 			return &Reference{
-				Subject:     callInstance,
+				Subject:     callInstance.Call,
 				SourceRange: tfdiags.SourceRangeFromHCL(callRange),
 				Remaining:   remain,
 			}, diags
diff --git a/addrs/parse_ref_test.go b/addrs/parse_ref_test.go
index be1eb244e..233bf8685 100644
--- a/addrs/parse_ref_test.go
+++ b/addrs/parse_ref_test.go
@@ -281,10 +281,8 @@ func TestParseRef(t *testing.T) {
 		{
 			`module.foo`,
 			&Reference{
-				Subject: ModuleCallInstance{
-					Call: ModuleCall{
-						Name: "foo",
-					},
+				Subject: ModuleCall{
+					Name: "foo",
 				},
 				SourceRange: tfdiags.SourceRange{
 					Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},

From 2490e6c84b843a7bf865b5416c4db7437ab561e9 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Sun, 12 Apr 2020 11:29:21 -0400
Subject: [PATCH 04/12] provide a method to get all modules changes

Since modules need to be evaluated as whole objects, yet the outputs are
all handled individually, we need a method to collect and return all
output changes for a module from the plan, including all known
module instances.
---
 plans/changes.go      | 25 +++++++++++++++++++++++++
 plans/changes_sync.go | 17 +++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/plans/changes.go b/plans/changes.go
index d7e0dcdb8..1350181ff 100644
--- a/plans/changes.go
+++ b/plans/changes.go
@@ -81,6 +81,31 @@ func (c *Changes) OutputValue(addr addrs.AbsOutputValue) *OutputChangeSrc {
 	return nil
 }
 
+// OutputValues returns planned changes for all outputs for all module
+// instances that reside in the parent path.  Returns nil if no changes are
+// planned.
+func (c *Changes) OutputValues(parent addrs.ModuleInstance, module addrs.ModuleCall) []*OutputChangeSrc {
+	var res []*OutputChangeSrc
+
+	for _, oc := range c.Outputs {
+		changeMod, changeCall := oc.Addr.Module.Call()
+		// this does not reside on our parent instance path
+		if !changeMod.Equal(parent) {
+			continue
+		}
+
+		// this is not the module you're looking for
+		if changeCall.Name != module.Name {
+			continue
+		}
+
+		res = append(res, oc)
+
+	}
+
+	return res
+}
+
 // SyncWrapper returns a wrapper object around the receiver that can be used
 // to make certain changes to the receiver in a concurrency-safe way, as long
 // as all callers share the same wrapper object.
diff --git a/plans/changes_sync.go b/plans/changes_sync.go
index 6b4ff98ff..56323e0e9 100644
--- a/plans/changes_sync.go
+++ b/plans/changes_sync.go
@@ -123,6 +123,23 @@ func (cs *ChangesSync) GetOutputChange(addr addrs.AbsOutputValue) *OutputChangeS
 	return cs.changes.OutputValue(addr)
 }
 
+// GetOutputChanges searches the set of output changes for any that reside in
+// module instances beneath the given module. If no changes exist, nil
+// is returned.
+//
+// The returned objects are a deep copy of the change recorded in the plan, so
+// callers may mutate them although it's generally better (less confusing) to
+// treat planned changes as immutable after they've been initially constructed.
+func (cs *ChangesSync) GetOutputChanges(parent addrs.ModuleInstance, module addrs.ModuleCall) []*OutputChangeSrc {
+	if cs == nil {
+		panic("GetOutputChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	return cs.changes.OutputValues(parent, module)
+}
+
 // RemoveOutputChange searches the set of output value changes for one matching
 // the given address, and removes it from the set if it exists.
 func (cs *ChangesSync) RemoveOutputChange(addr addrs.AbsOutputValue) {

From 323d9fb69fe14b7a35fcbe3a37d995a4c79b1fbb Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 13 Apr 2020 16:21:09 -0400
Subject: [PATCH 05/12] plans fix

---
 plans/changes.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/plans/changes.go b/plans/changes.go
index 1350181ff..d7c86e6c2 100644
--- a/plans/changes.go
+++ b/plans/changes.go
@@ -88,6 +88,11 @@ func (c *Changes) OutputValues(parent addrs.ModuleInstance, module addrs.ModuleC
 	var res []*OutputChangeSrc
 
 	for _, oc := range c.Outputs {
+		// we can't evaluate root module outputs
+		if oc.Addr.Module.Equal(addrs.RootModuleInstance) {
+			continue
+		}
+
 		changeMod, changeCall := oc.Addr.Module.Call()
 		// this does not reside on our parent instance path
 		if !changeMod.Equal(parent) {

From 27cc2aeb9cd18f2897498412dbadaea2d244066c Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 13 Apr 2020 16:23:24 -0400
Subject: [PATCH 06/12] change evaluation to use whole modules

The evaluationStateData needs the change to the GetModule method to work
with the new evaluator. This is using a deep copy of module instances,
which we will clean up after some changes to the states package.
---
 terraform/evaluate.go | 180 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 155 insertions(+), 25 deletions(-)

diff --git a/terraform/evaluate.go b/terraform/evaluate.go
index 7dcea4729..b009e109e 100644
--- a/terraform/evaluate.go
+++ b/terraform/evaluate.go
@@ -336,54 +336,184 @@ func (d *evaluationStateData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.S
 	return val, diags
 }
 
-func (d *evaluationStateData) GetModuleInstance(addr addrs.ModuleCallInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	// Output results live in the module that declares them, which is one of
 	// the child module instances of our current module path.
-	moduleAddr := addr.ModuleInstance(d.ModulePath)
+	moduleAddr := d.ModulePath.Module().Child(addr.Name)
+
+	parentCfg := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
+	callConfig, ok := parentCfg.Module.ModuleCalls[addr.Name]
+	if !ok {
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Reference to undeclared module`,
+			Detail:   fmt.Sprintf(`The configuration contains no %s.`, moduleAddr),
+			Subject:  rng.ToHCL().Ptr(),
+		})
+		return cty.DynamicVal, diags
+	}
 
 	// We'll consult the configuration to see what output names we are
 	// expecting, so we can ensure the resulting object is of the expected
 	// type even if our data is incomplete for some reason.
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(moduleAddr)
+	moduleConfig := d.Evaluator.Config.Descendent(moduleAddr)
 	if moduleConfig == nil {
-		// should never happen, since this should've been caught during
-		// static validation.
+		// should never happen, since we have a valid module call above, this
+		// should be caught during static validation.
 		panic(fmt.Sprintf("output value read from %s, which has no configuration", moduleAddr))
 	}
 	outputConfigs := moduleConfig.Module.Outputs
 
-	vals := map[string]cty.Value{}
-	for n := range outputConfigs {
-		addr := addrs.OutputValue{Name: n}.Absolute(moduleAddr)
+	// Collect all the relevant outputs that current exist in the state.
+	// We know the instance path up to this point, and the child module name,
+	// so we only need to store these by instance key.
+	stateMap := map[addrs.InstanceKey]map[string]cty.Value{}
+	for _, m := range d.Evaluator.State.ModuleInstances(moduleAddr) {
+		// skip module instances that aren't a child of our particular parent
+		// module instance.
+		if !d.ModulePath.Equal(m.Addr.Parent()) {
+			continue
+		}
+
+		_, callInstance := m.Addr.CallInstance()
+		instance, ok := stateMap[callInstance.Key]
+		if !ok {
+			instance = map[string]cty.Value{}
+			stateMap[callInstance.Key] = instance
+		}
+
+		for name, output := range m.OutputValues {
+			instance[name] = output.Value
+		}
+	}
+
+	// Get all changes that reside for this module call within our path.
+	// The change contains the full addr, so we can key these with strings.
+	changesMap := map[addrs.InstanceKey]map[string]*plans.OutputChangeSrc{}
+	for _, change := range d.Evaluator.Changes.GetOutputChanges(d.ModulePath, addr) {
+		_, callInstance := change.Addr.Module.CallInstance()
+		instance, ok := changesMap[callInstance.Key]
+		if !ok {
+			instance = map[string]*plans.OutputChangeSrc{}
+			changesMap[callInstance.Key] = instance
+		}
+
+		instance[change.Addr.OutputValue.Name] = change
+	}
+
+	// Build up all the module objects, creating a map of values for each
+	// module instance.
+	moduleInstances := map[addrs.InstanceKey]map[string]cty.Value{}
+
+	// the structure is based on the configuration, so iterate through all the
+	// defined outputs, and add any instance state or changes we find.
+	for _, cfg := range outputConfigs {
+		// get all instance output for this path from the state
+		for key, states := range stateMap {
+			outputState, ok := states[cfg.Name]
+			if !ok {
+				continue
+			}
+
+			instance, ok := moduleInstances[key]
+			if !ok {
+				instance = map[string]cty.Value{}
+				moduleInstances[key] = instance
+			}
+
+			instance[cfg.Name] = outputState
+		}
+
+		// any pending changes override the state state values
+		for key, changes := range changesMap {
+			changeSrc, ok := changes[cfg.Name]
+			if !ok {
+				continue
+			}
+
+			instance, ok := moduleInstances[key]
+			if !ok {
+				instance = map[string]cty.Value{}
+				moduleInstances[key] = instance
+			}
 
-		// If a pending change is present in our current changeset then its value
-		// takes priority over what's in state. (It will usually be the same but
-		// will differ if the new value is unknown during planning.)
-		if changeSrc := d.Evaluator.Changes.GetOutputChange(addr); changeSrc != nil {
 			change, err := changeSrc.Decode()
 			if err != nil {
 				// This should happen only if someone has tampered with a plan
 				// file, so we won't bother with a pretty error for it.
 				diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", addr, err))
-				vals[n] = cty.DynamicVal
+				instance[cfg.Name] = cty.DynamicVal
 				continue
 			}
-			// We care only about the "after" value, which is the value this output
-			// will take on after the plan is applied.
-			vals[n] = change.After
-		} else {
-			os := d.Evaluator.State.OutputValue(addr)
-			if os == nil {
-				// Not evaluated yet?
-				vals[n] = cty.DynamicVal
-				continue
-			}
-			vals[n] = os.Value
+
+			instance[cfg.Name] = change.After
 		}
 	}
-	return cty.ObjectVal(vals), diags
+
+	// ensure all defined outputs names are present in the module value, even
+	// if they are not known yet.
+	for _, instance := range moduleInstances {
+		for configKey := range outputConfigs {
+			if _, ok := instance[configKey]; !ok {
+				instance[configKey] = cty.DynamicVal
+			}
+		}
+	}
+
+	// compile the outputs into the correct value type for the each mode
+	switch {
+	case callConfig.Count != nil:
+		vals := make([]cty.Value, len(moduleInstances))
+		for key, instance := range moduleInstances {
+			intKey, ok := key.(addrs.IntKey)
+			if !ok {
+				// old key from state which is being dropped
+				continue
+			}
+
+			vals[int(intKey)] = cty.ObjectVal(instance)
+		}
+
+		// we shouldn't have any holes, but insert real values just in case,
+		// while trimming off any extra values that may have there from old
+		// entries.
+		last := 0
+		for i, v := range vals {
+			if v.IsNull() {
+				vals[i] = cty.DynamicVal
+				continue
+			}
+			last = i
+		}
+		vals = vals[:last+1]
+		return cty.TupleVal(vals), diags
+
+	case callConfig.ForEach != nil:
+		vals := make(map[string]cty.Value)
+		for key, instance := range moduleInstances {
+			strKey, ok := key.(addrs.StringKey)
+			if !ok {
+				continue
+			}
+
+			vals[string(strKey)] = cty.ObjectVal(instance)
+		}
+		return cty.ObjectVal(vals), diags
+
+	default:
+		val, ok := moduleInstances[addrs.NoKey]
+		if !ok {
+			// create the object is there wasn't one known
+			val = map[string]cty.Value{}
+			for k := range outputConfigs {
+				val[k] = cty.DynamicVal
+			}
+		}
+
+		return cty.ObjectVal(val), diags
+	}
 }
 
 func (d *evaluationStateData) GetModuleInstanceOutput(addr addrs.AbsModuleCallOutput, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {

From e9eb8e04cc4aa75a7793f5e8543f373c68d74832 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 13 Apr 2020 16:37:59 -0400
Subject: [PATCH 07/12] add AbsOutputAddrs to state outputs

We need all module instance outputs to build the objects for evaluation,
but there is no need to copy all the resource instances along with that.
This allows us to only return the output states, with enough information
to connect them with their module instances.
---
 states/module.go             |  6 ++++++
 states/output_value.go       |  2 ++
 states/state_deepcopy.go     |  1 +
 states/state_test.go         | 16 ++++++++++++++++
 states/statefile/version4.go |  8 +++++++-
 5 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/states/module.go b/states/module.go
index acce10129..ddfada9ae 100644
--- a/states/module.go
+++ b/states/module.go
@@ -259,6 +259,12 @@ func (ms *Module) maybeRestoreResourceInstanceDeposed(addr addrs.ResourceInstanc
 // existing value of the same name.
 func (ms *Module) SetOutputValue(name string, value cty.Value, sensitive bool) *OutputValue {
 	os := &OutputValue{
+		Addr: addrs.AbsOutputValue{
+			Module: ms.Addr,
+			OutputValue: addrs.OutputValue{
+				Name: name,
+			},
+		},
 		Value:     value,
 		Sensitive: sensitive,
 	}
diff --git a/states/output_value.go b/states/output_value.go
index d232b76d4..268420cf4 100644
--- a/states/output_value.go
+++ b/states/output_value.go
@@ -1,6 +1,7 @@
 package states
 
 import (
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -9,6 +10,7 @@ import (
 // It is not valid to mutate an OutputValue object once it has been created.
 // Instead, create an entirely new OutputValue to replace the previous one.
 type OutputValue struct {
+	Addr      addrs.AbsOutputValue
 	Value     cty.Value
 	Sensitive bool
 }
diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go
index 1c5aee0a9..817e1c19d 100644
--- a/states/state_deepcopy.go
+++ b/states/state_deepcopy.go
@@ -226,6 +226,7 @@ func (os *OutputValue) DeepCopy() *OutputValue {
 	}
 
 	return &OutputValue{
+		Addr:      os.Addr,
 		Value:     os.Value,
 		Sensitive: os.Sensitive,
 	}
diff --git a/states/state_test.go b/states/state_test.go
index 2a38a9c53..8fe191d57 100644
--- a/states/state_test.go
+++ b/states/state_test.go
@@ -53,10 +53,20 @@ func TestState(t *testing.T) {
 				},
 				OutputValues: map[string]*OutputValue{
 					"bar": {
+						Addr: addrs.AbsOutputValue{
+							OutputValue: addrs.OutputValue{
+								Name: "bar",
+							},
+						},
 						Value:     cty.StringVal("bar value"),
 						Sensitive: false,
 					},
 					"secret": {
+						Addr: addrs.AbsOutputValue{
+							OutputValue: addrs.OutputValue{
+								Name: "secret",
+							},
+						},
 						Value:     cty.StringVal("secret value"),
 						Sensitive: true,
 					},
@@ -92,6 +102,12 @@ func TestState(t *testing.T) {
 				LocalValues: map[string]cty.Value{},
 				OutputValues: map[string]*OutputValue{
 					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("child", addrs.NoKey),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
 						Value:     cty.StringVal("hawaiian"),
 						Sensitive: false,
 					},
diff --git a/states/statefile/version4.go b/states/statefile/version4.go
index c49599d82..0cb0ae9b0 100644
--- a/states/statefile/version4.go
+++ b/states/statefile/version4.go
@@ -281,7 +281,13 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
 	{
 		rootModule := state.RootModule()
 		for name, fos := range sV4.RootOutputs {
-			os := &states.OutputValue{}
+			os := &states.OutputValue{
+				Addr: addrs.AbsOutputValue{
+					OutputValue: addrs.OutputValue{
+						Name: name,
+					},
+				},
+			}
 			os.Sensitive = fos.Sensitive
 
 			ty, err := ctyjson.UnmarshalType([]byte(fos.ValueTypeRaw))

From 2c4c027a9777339867de706786f54d687773debd Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 13 Apr 2020 17:59:09 -0400
Subject: [PATCH 08/12] Add ModuleOutputs method to states

In order to efficiently build the module objects for evaluation, we need
to collect the outputs from a set of module instances. The ModuleOutputs
method will return a copy of the state outputs, while not requiring the
unnecessary copying of each entire module.
---
 states/state.go      | 29 ++++++++++++++++++++++
 states/state_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++
 states/sync.go       | 12 ++++++++++
 3 files changed, 98 insertions(+)

diff --git a/states/state.go b/states/state.go
index ed77eb5f0..6d0cec148 100644
--- a/states/state.go
+++ b/states/state.go
@@ -82,6 +82,35 @@ func (s *State) ModuleInstances(addr addrs.Module) []*Module {
 	return ms
 }
 
+// ModuleOutputs returns all outputs for the given module call under the
+// parentAddr instance.
+func (s *State) ModuleOutputs(parentAddr addrs.ModuleInstance, module addrs.ModuleCall) []*OutputValue {
+	var os []*OutputValue
+	for _, m := range s.Modules {
+		// can't get outputs from the root module
+		if m.Addr.IsRoot() {
+			continue
+		}
+
+		parent, call := m.Addr.Call()
+		// make sure this is a descendent in the correct path
+		if !parentAddr.Equal(parent) {
+			continue
+		}
+
+		// and check if this is the correct child
+		if call.Name != module.Name {
+			continue
+		}
+
+		for _, o := range m.OutputValues {
+			os = append(os, o)
+		}
+	}
+
+	return os
+}
+
 // RemoveModule removes the module with the given address from the state,
 // unless it is the root module. The root module cannot be deleted, and so
 // this method will panic if that is attempted.
diff --git a/states/state_test.go b/states/state_test.go
index 8fe191d57..cd57757cb 100644
--- a/states/state_test.go
+++ b/states/state_test.go
@@ -43,6 +43,10 @@ func TestState(t *testing.T) {
 
 	childModule := state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
 	childModule.SetOutputValue("pizza", cty.StringVal("hawaiian"), false)
+	multiModA := state.EnsureModule(addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")))
+	multiModA.SetOutputValue("pizza", cty.StringVal("cheese"), false)
+	multiModB := state.EnsureModule(addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")))
+	multiModB.SetOutputValue("pizza", cty.StringVal("sausage"), false)
 
 	want := &State{
 		Modules: map[string]*Module{
@@ -114,6 +118,40 @@ func TestState(t *testing.T) {
 				},
 				Resources: map[string]*Resource{},
 			},
+			`module.multi["a"]`: {
+				Addr:        addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")),
+				LocalValues: map[string]cty.Value{},
+				OutputValues: map[string]*OutputValue{
+					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
+						Value:     cty.StringVal("cheese"),
+						Sensitive: false,
+					},
+				},
+				Resources: map[string]*Resource{},
+			},
+			`module.multi["b"]`: {
+				Addr:        addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")),
+				LocalValues: map[string]cty.Value{},
+				OutputValues: map[string]*OutputValue{
+					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
+						Value:     cty.StringVal("sausage"),
+						Sensitive: false,
+					},
+				},
+				Resources: map[string]*Resource{},
+			},
 		},
 	}
 
@@ -133,6 +171,25 @@ func TestState(t *testing.T) {
 	for _, problem := range deep.Equal(state, want) {
 		t.Error(problem)
 	}
+
+	expectedOutputs := map[string]string{
+		`module.multi["a"].output.pizza`: "cheese",
+		`module.multi["b"].output.pizza`: "sausage",
+	}
+
+	for _, o := range state.ModuleOutputs(addrs.RootModuleInstance, addrs.ModuleCall{Name: "multi"}) {
+		addr := o.Addr.String()
+		expected := expectedOutputs[addr]
+		delete(expectedOutputs, addr)
+
+		if expected != o.Value.AsString() {
+			t.Fatalf("expected %q:%q, got %q", addr, expected, o.Value.AsString())
+		}
+	}
+
+	for addr, o := range expectedOutputs {
+		t.Fatalf("missing output %q:%q", addr, o)
+	}
 }
 
 func TestStateDeepCopy(t *testing.T) {
diff --git a/states/sync.go b/states/sync.go
index af391e55f..4b82c69fb 100644
--- a/states/sync.go
+++ b/states/sync.go
@@ -48,6 +48,18 @@ func (s *SyncState) Module(addr addrs.ModuleInstance) *Module {
 	return ret
 }
 
+// ModuleOutputs returns the set of OutputValues that matches the given path.
+func (s *SyncState) ModuleOutputs(parentAddr addrs.ModuleInstance, module addrs.ModuleCall) []*OutputValue {
+	s.lock.RLock()
+	defer s.lock.RUnlock()
+	var os []*OutputValue
+
+	for _, o := range s.state.ModuleOutputs(parentAddr, module) {
+		os = append(os, o.DeepCopy())
+	}
+	return os
+}
+
 // RemoveModule removes the entire state for the given module, taking with
 // it any resources associated with the module. This should generally be
 // called only for modules whose resources have all been destroyed, but

From ad069b7416b8d66970c322d8e69f169a9d4a9acc Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 13 Apr 2020 18:17:08 -0400
Subject: [PATCH 09/12] update evaluation to use state ModuleOutputs

This way we don't need the extra copy of the entire module.
---
 terraform/evaluate.go | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/terraform/evaluate.go b/terraform/evaluate.go
index b009e109e..1199d387f 100644
--- a/terraform/evaluate.go
+++ b/terraform/evaluate.go
@@ -370,23 +370,15 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 	// We know the instance path up to this point, and the child module name,
 	// so we only need to store these by instance key.
 	stateMap := map[addrs.InstanceKey]map[string]cty.Value{}
-	for _, m := range d.Evaluator.State.ModuleInstances(moduleAddr) {
-		// skip module instances that aren't a child of our particular parent
-		// module instance.
-		if !d.ModulePath.Equal(m.Addr.Parent()) {
-			continue
-		}
-
-		_, callInstance := m.Addr.CallInstance()
+	for _, output := range d.Evaluator.State.ModuleOutputs(d.ModulePath, addr) {
+		_, callInstance := output.Addr.Module.CallInstance()
 		instance, ok := stateMap[callInstance.Key]
 		if !ok {
 			instance = map[string]cty.Value{}
 			stateMap[callInstance.Key] = instance
 		}
 
-		for name, output := range m.OutputValues {
-			instance[name] = output.Value
-		}
+		instance[output.Addr.OutputValue.Name] = output.Value
 	}
 
 	// Get all changes that reside for this module call within our path.
@@ -414,7 +406,9 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 		for key, states := range stateMap {
 			outputState, ok := states[cfg.Name]
 			if !ok {
-				continue
+				// we'll take this chance to insert any missing values that are
+				// defined in the config
+				outputState = cty.DynamicVal
 			}
 
 			instance, ok := moduleInstances[key]

From 42cee86ee2bff817a96bd7416879f4ddfa7938d2 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 14 Apr 2020 13:36:55 -0400
Subject: [PATCH 10/12] remove GetModuleInstanceOutput

There is no codepath that can use this any longer, since we need to
evaluate the modules as whole objects.

This means we're going to have to live for now with invalid module
output references returning "object" errors rather that "module".
---
 lang/data.go          |  1 -
 lang/eval.go          | 15 ---------
 terraform/evaluate.go | 74 ++-----------------------------------------
 3 files changed, 2 insertions(+), 88 deletions(-)

diff --git a/lang/data.go b/lang/data.go
index e4e227972..a47a2a32d 100644
--- a/lang/data.go
+++ b/lang/data.go
@@ -27,7 +27,6 @@ type Data interface {
 	GetResource(addrs.Resource, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetModule(addrs.ModuleCall, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
-	GetModuleInstanceOutput(addrs.AbsModuleCallOutput, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetPathAttr(addrs.PathAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetTerraformAttr(addrs.TerraformAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetInputVariable(addrs.InputVariable, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
diff --git a/lang/eval.go b/lang/eval.go
index 8e3422787..db5a15a24 100644
--- a/lang/eval.go
+++ b/lang/eval.go
@@ -195,7 +195,6 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 	dataResources := map[string]map[string]cty.Value{}
 	managedResources := map[string]map[string]cty.Value{}
 	wholeModules := map[string]cty.Value{}
-	moduleOutputs := map[string]map[addrs.InstanceKey]map[string]cty.Value{}
 	inputVariables := map[string]cty.Value{}
 	localValues := map[string]cty.Value{}
 	pathAttrs := map[string]cty.Value{}
@@ -292,20 +291,6 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 			diags = diags.Append(valDiags)
 			wholeModules[subj.Name] = val
 
-		case addrs.AbsModuleCallOutput:
-			val, valDiags := normalizeRefValue(s.Data.GetModuleInstanceOutput(subj, rng))
-			diags = diags.Append(valDiags)
-
-			callName := subj.Call.Call.Name
-			callKey := subj.Call.Key
-			if moduleOutputs[callName] == nil {
-				moduleOutputs[callName] = make(map[addrs.InstanceKey]map[string]cty.Value)
-			}
-			if moduleOutputs[callName][callKey] == nil {
-				moduleOutputs[callName][callKey] = make(map[string]cty.Value)
-			}
-			moduleOutputs[callName][callKey][subj.Name] = val
-
 		case addrs.InputVariable:
 			val, valDiags := normalizeRefValue(s.Data.GetInputVariable(subj, rng))
 			diags = diags.Append(valDiags)
diff --git a/terraform/evaluate.go b/terraform/evaluate.go
index 1199d387f..d508541a4 100644
--- a/terraform/evaluate.go
+++ b/terraform/evaluate.go
@@ -471,8 +471,8 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 		}
 
 		// we shouldn't have any holes, but insert real values just in case,
-		// while trimming off any extra values that may have there from old
-		// entries.
+		// while trimming off any extra values that we may have from guessing
+		// the length via the state instances.
 		last := 0
 		for i, v := range vals {
 			if v.IsNull() {
@@ -510,76 +510,6 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 	}
 }
 
-func (d *evaluationStateData) GetModuleInstanceOutput(addr addrs.AbsModuleCallOutput, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-
-	// Output results live in the module that declares them, which is one of
-	// the child module instances of our current module path.
-	absAddr := addr.AbsOutputValue(d.ModulePath)
-	moduleAddr := absAddr.Module
-
-	// First we'll consult the configuration to see if an output of this
-	// name is declared at all.
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(moduleAddr)
-	if moduleConfig == nil {
-		// this doesn't happen in normal circumstances due to our validation
-		// pass, but it can turn up in some unusual situations, like in the
-		// "terraform console" repl where arbitrary expressions can be
-		// evaluated.
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  `Reference to undeclared module`,
-			Detail:   fmt.Sprintf(`The configuration contains no %s.`, moduleAddr),
-			Subject:  rng.ToHCL().Ptr(),
-		})
-		return cty.DynamicVal, diags
-	}
-
-	config := moduleConfig.Module.Outputs[addr.Name]
-	if config == nil {
-		var suggestions []string
-		for k := range moduleConfig.Module.Outputs {
-			suggestions = append(suggestions, k)
-		}
-		suggestion := nameSuggestion(addr.Name, suggestions)
-		if suggestion != "" {
-			suggestion = fmt.Sprintf(" Did you mean %q?", suggestion)
-		}
-
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  `Reference to undeclared output value`,
-			Detail:   fmt.Sprintf(`An output value with the name %q has not been declared in %s.%s`, addr.Name, moduleDisplayAddr(moduleAddr), suggestion),
-			Subject:  rng.ToHCL().Ptr(),
-		})
-		return cty.DynamicVal, diags
-	}
-
-	// If a pending change is present in our current changeset then its value
-	// takes priority over what's in state. (It will usually be the same but
-	// will differ if the new value is unknown during planning.)
-	if changeSrc := d.Evaluator.Changes.GetOutputChange(absAddr); changeSrc != nil {
-		change, err := changeSrc.Decode()
-		if err != nil {
-			// This should happen only if someone has tampered with a plan
-			// file, so we won't bother with a pretty error for it.
-			diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", absAddr, err))
-			return cty.DynamicVal, diags
-		}
-		// We care only about the "after" value, which is the value this output
-		// will take on after the plan is applied.
-		return change.After, diags
-	}
-
-	os := d.Evaluator.State.OutputValue(absAddr)
-	if os == nil {
-		// Not evaluated yet?
-		return cty.DynamicVal, diags
-	}
-
-	return os.Value, diags
-}
-
 func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 	switch addr.Name {

From d152d13beaf00b474b844a79e5a17591c311801f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 14 Apr 2020 14:50:43 -0400
Subject: [PATCH 11/12] fix error output in repl test

The module error is unfortunately less specific at the moment, but
change the error text here to match.
---
 repl/session_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repl/session_test.go b/repl/session_test.go
index 59545c278..27eb10567 100644
--- a/repl/session_test.go
+++ b/repl/session_test.go
@@ -125,7 +125,7 @@ func TestSession_basicState(t *testing.T) {
 				{
 					Input:         "module.module.foo",
 					Error:         true,
-					ErrorContains: `An output value with the name "foo" has not been declared in module.module`,
+					ErrorContains: `Unsupported attribute: This object does not have an attribute named "foo"`,
 				},
 			},
 		})

From 92837e6296a8b4d2da0665f5a1e8ab35d580778f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 17 Apr 2020 12:36:17 -0400
Subject: [PATCH 12/12] return unknown module expansions during validate

There is no expansion during validation, so in order for module
references to work we need to ensure that the returned values are
unknown.
---
 terraform/context_validate_test.go | 10 +++++
 terraform/evaluate.go              | 60 +++++++++++++++++-------------
 2 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go
index 178f29174..495cad6ca 100644
--- a/terraform/context_validate_test.go
+++ b/terraform/context_validate_test.go
@@ -1421,6 +1421,7 @@ module "mod1" {
 module "mod2" {
   for_each = module.mod1
   source = "./mod"
+  input = module.mod1["a"].out
 }
 
 module "mod3" {
@@ -1432,6 +1433,15 @@ module "mod3" {
 resource "aws_instance" "foo" {
 }
 
+output "out" {
+  value = 1
+}
+
+variable "input" {
+  type = number
+  default = 0
+}
+
 module "nested" {
   count = 2
   source = "./nested"
diff --git a/terraform/evaluate.go b/terraform/evaluate.go
index d508541a4..bddac5412 100644
--- a/terraform/evaluate.go
+++ b/terraform/evaluate.go
@@ -406,9 +406,7 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 		for key, states := range stateMap {
 			outputState, ok := states[cfg.Name]
 			if !ok {
-				// we'll take this chance to insert any missing values that are
-				// defined in the config
-				outputState = cty.DynamicVal
+				continue
 			}
 
 			instance, ok := moduleInstances[key]
@@ -446,15 +444,7 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 		}
 	}
 
-	// ensure all defined outputs names are present in the module value, even
-	// if they are not known yet.
-	for _, instance := range moduleInstances {
-		for configKey := range outputConfigs {
-			if _, ok := instance[configKey]; !ok {
-				instance[configKey] = cty.DynamicVal
-			}
-		}
-	}
+	var ret cty.Value
 
 	// compile the outputs into the correct value type for the each mode
 	switch {
@@ -470,19 +460,23 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 			vals[int(intKey)] = cty.ObjectVal(instance)
 		}
 
-		// we shouldn't have any holes, but insert real values just in case,
-		// while trimming off any extra values that we may have from guessing
-		// the length via the state instances.
-		last := 0
-		for i, v := range vals {
-			if v.IsNull() {
-				vals[i] = cty.DynamicVal
-				continue
+		if len(vals) > 0 {
+			// we shouldn't have any holes, but insert real values just in case,
+			// while trimming off any extra values that we may have from guessing
+			// the length via the state instances.
+			last := 0
+			for i, v := range vals {
+				if v.IsNull() {
+					vals[i] = cty.DynamicVal
+					continue
+				}
+				last = i
 			}
-			last = i
+			vals = vals[:last+1]
+			ret = cty.ListVal(vals)
+		} else {
+			ret = cty.ListValEmpty(cty.DynamicPseudoType)
 		}
-		vals = vals[:last+1]
-		return cty.TupleVal(vals), diags
 
 	case callConfig.ForEach != nil:
 		vals := make(map[string]cty.Value)
@@ -494,20 +488,34 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
 
 			vals[string(strKey)] = cty.ObjectVal(instance)
 		}
-		return cty.ObjectVal(vals), diags
+
+		if len(vals) > 0 {
+			ret = cty.MapVal(vals)
+		} else {
+			ret = cty.MapValEmpty(cty.DynamicPseudoType)
+		}
 
 	default:
 		val, ok := moduleInstances[addrs.NoKey]
 		if !ok {
-			// create the object is there wasn't one known
+			// create the object if there wasn't one known
 			val = map[string]cty.Value{}
 			for k := range outputConfigs {
 				val[k] = cty.DynamicVal
 			}
 		}
 
-		return cty.ObjectVal(val), diags
+		ret = cty.ObjectVal(val)
 	}
+
+	// The module won't be expanded during validation, so we need to return an
+	// unknown value. This will ensure the types looks correct, since we built
+	// the objects based on the configuration.
+	if d.Operation == walkValidate {
+		return cty.UnknownVal(ret.Type()), diags
+	}
+
+	return ret, diags
 }
 
 func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {