From c367d8421e3b9d7c31951c6392d8c0ca35fcc7c2 Mon Sep 17 00:00:00 2001
From: Kazuma Watanabe <watassbass@gmail.com>
Date: Mon, 24 Oct 2022 14:40:28 +0000
Subject: [PATCH] Fix incorrect circular reference detection

---
 terraform/evaluator.go      | 47 ++++++++++++++++++++-----------------
 terraform/evaluator_test.go | 14 ++++++++++-
 tflint/runner.go            |  2 +-
 3 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/terraform/evaluator.go b/terraform/evaluator.go
index 114f16920..58fbb725f 100644
--- a/terraform/evaluator.go
+++ b/terraform/evaluator.go
@@ -20,22 +20,22 @@ type ContextMeta struct {
 	OriginalWorkingDir string
 }
 
-type CallGraph struct {
-	edges map[string]addrs.Reference
+type CallStack struct {
+	addrs map[string]addrs.Reference
 	stack []string
 }
 
-func NewCallGraph() *CallGraph {
-	return &CallGraph{
-		edges: make(map[string]addrs.Reference),
+func NewCallStack() *CallStack {
+	return &CallStack{
+		addrs: make(map[string]addrs.Reference),
 		stack: make([]string, 0),
 	}
 }
 
-func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
+func (g *CallStack) Push(addr addrs.Reference) hcl.Diagnostics {
 	g.stack = append(g.stack, addr.Subject.String())
 
-	if _, exists := g.edges[addr.Subject.String()]; exists {
+	if _, exists := g.addrs[addr.Subject.String()]; exists {
 		return hcl.Diagnostics{
 			{
 				Severity: hcl.DiagError,
@@ -45,20 +45,30 @@ func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
 			},
 		}
 	}
-	g.edges[addr.Subject.String()] = addr
+	g.addrs[addr.Subject.String()] = addr
 	return hcl.Diagnostics{}
 }
 
-func (g *CallGraph) String() string {
+func (g *CallStack) Pop() {
+	if g.Empty() {
+		panic("cannot pop from empty stack")
+	}
+
+	addr := g.stack[len(g.stack)-1]
+	g.stack = g.stack[:len(g.stack)-1]
+	delete(g.addrs, addr)
+}
+
+func (g *CallStack) String() string {
 	return strings.Join(g.stack, " -> ")
 }
 
-func (g *CallGraph) Empty() bool {
+func (g *CallStack) Empty() bool {
 	return len(g.stack) == 0
 }
 
-func (g *CallGraph) Clear() {
-	g.edges = make(map[string]addrs.Reference)
+func (g *CallStack) Clear() {
+	g.addrs = make(map[string]addrs.Reference)
 	g.stack = make([]string, 0)
 }
 
@@ -67,7 +77,7 @@ type Evaluator struct {
 	ModulePath     addrs.ModuleInstance
 	Config         *Config
 	VariableValues map[string]map[string]cty.Value
-	CallGraph      *CallGraph
+	CallStack      *CallStack
 }
 
 func (e *Evaluator) EvaluateExpr(expr hcl.Expression, wantType cty.Type, keyData InstanceKeyEvalData) (cty.Value, hcl.Diagnostics) {
@@ -298,9 +308,8 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
 		return cty.DynamicVal, diags
 	}
 
-	entry := d.Evaluator.CallGraph.Empty()
-	// Build a callgraph for circular reference detection only when getting a local value.
-	if diags := d.Evaluator.CallGraph.Add(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
+	// Build a call stack for circular reference detection only when getting a local value.
+	if diags := d.Evaluator.CallStack.Push(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
 		return cty.UnknownVal(cty.DynamicPseudoType), diags
 	}
 
@@ -308,11 +317,7 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
 	// that depend on instance keys, such as `count.*` and `each.*`.
 	val, diags := d.Evaluator.EvaluateExpr(config.Expr, cty.DynamicPseudoType, EvalDataForNoInstanceKey)
 
-	// Since we build a callgraph for each local value,
-	// we clear the callgraph when the local value is finally resolved.
-	if entry {
-		d.Evaluator.CallGraph.Clear()
-	}
+	d.Evaluator.CallStack.Pop()
 	return val, diags
 }
 
diff --git a/terraform/evaluator_test.go b/terraform/evaluator_test.go
index fe8a3ca27..750754b54 100644
--- a/terraform/evaluator_test.go
+++ b/terraform/evaluator_test.go
@@ -586,6 +586,18 @@ locals {
 				return diags.Error() != `main.tf:4,9-18: circular reference found; local.foo -> local.bar -> local.foo`
 			},
 		},
+		{
+			name: "nested multiple local values",
+			config: `
+locals {
+  foo = "foo"
+  bar = [local.foo, local.foo]
+}`,
+			expr:     expr(`local.bar`),
+			ty:       cty.List(cty.String),
+			want:     `cty.ListVal([]cty.Value{cty.StringVal("foo"), cty.StringVal("foo")})`,
+			errCheck: neverHappend,
+		},
 		{
 			name:     "count.index in non-counted context",
 			expr:     expr(`count.index`),
@@ -666,7 +678,7 @@ locals {
 				ModulePath:     config.Path.UnkeyedInstanceShim(),
 				Config:         config,
 				VariableValues: variableValues,
-				CallGraph:      NewCallGraph(),
+				CallStack:      NewCallStack(),
 			}
 
 			got, diags := evaluator.EvaluateExpr(test.expr, test.ty, test.keyData)
diff --git a/tflint/runner.go b/tflint/runner.go
index eec464085..78ec9616a 100644
--- a/tflint/runner.go
+++ b/tflint/runner.go
@@ -53,7 +53,7 @@ func NewRunner(c *Config, ants map[string]Annotations, cfg *terraform.Config, va
 		ModulePath:     cfg.Path.UnkeyedInstanceShim(),
 		Config:         cfg.Root,
 		VariableValues: variableValues,
-		CallGraph:      terraform.NewCallGraph(),
+		CallStack:      terraform.NewCallStack(),
 	}
 
 	runner := &Runner{