Skip to content

Commit ced535c

Browse files
committed
Fix incorrect circular reference detection
1 parent f978a90 commit ced535c

File tree

3 files changed

+39
-22
lines changed

3 files changed

+39
-22
lines changed

terraform/evaluator.go

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,22 @@ type ContextMeta struct {
2020
OriginalWorkingDir string
2121
}
2222

23-
type CallGraph struct {
24-
edges map[string]addrs.Reference
23+
type CallStack struct {
24+
addrs map[string]addrs.Reference
2525
stack []string
2626
}
2727

28-
func NewCallGraph() *CallGraph {
29-
return &CallGraph{
30-
edges: make(map[string]addrs.Reference),
28+
func NewCallStack() *CallStack {
29+
return &CallStack{
30+
addrs: make(map[string]addrs.Reference),
3131
stack: make([]string, 0),
3232
}
3333
}
3434

35-
func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
35+
func (g *CallStack) Push(addr addrs.Reference) hcl.Diagnostics {
3636
g.stack = append(g.stack, addr.Subject.String())
3737

38-
if _, exists := g.edges[addr.Subject.String()]; exists {
38+
if _, exists := g.addrs[addr.Subject.String()]; exists {
3939
return hcl.Diagnostics{
4040
{
4141
Severity: hcl.DiagError,
@@ -45,20 +45,30 @@ func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
4545
},
4646
}
4747
}
48-
g.edges[addr.Subject.String()] = addr
48+
g.addrs[addr.Subject.String()] = addr
4949
return hcl.Diagnostics{}
5050
}
5151

52-
func (g *CallGraph) String() string {
52+
func (g *CallStack) Pop() {
53+
if g.Empty() {
54+
panic("cannot pop from empty call graph")
55+
}
56+
57+
addr := g.stack[len(g.stack)-1]
58+
g.stack = g.stack[:len(g.stack)-1]
59+
delete(g.addrs, addr)
60+
}
61+
62+
func (g *CallStack) String() string {
5363
return strings.Join(g.stack, " -> ")
5464
}
5565

56-
func (g *CallGraph) Empty() bool {
66+
func (g *CallStack) Empty() bool {
5767
return len(g.stack) == 0
5868
}
5969

60-
func (g *CallGraph) Clear() {
61-
g.edges = make(map[string]addrs.Reference)
70+
func (g *CallStack) Clear() {
71+
g.addrs = make(map[string]addrs.Reference)
6272
g.stack = make([]string, 0)
6373
}
6474

@@ -67,7 +77,7 @@ type Evaluator struct {
6777
ModulePath addrs.ModuleInstance
6878
Config *Config
6979
VariableValues map[string]map[string]cty.Value
70-
CallGraph *CallGraph
80+
CallStack *CallStack
7181
}
7282

7383
func (e *Evaluator) EvaluateExpr(expr hcl.Expression, wantType cty.Type, keyData InstanceKeyEvalData) (cty.Value, hcl.Diagnostics) {
@@ -298,21 +308,16 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
298308
return cty.DynamicVal, diags
299309
}
300310

301-
entry := d.Evaluator.CallGraph.Empty()
302311
// Build a callgraph for circular reference detection only when getting a local value.
303-
if diags := d.Evaluator.CallGraph.Add(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
312+
if diags := d.Evaluator.CallStack.Push(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
304313
return cty.UnknownVal(cty.DynamicPseudoType), diags
305314
}
306315

307316
// Always use EvalDataForNoInstanceKey because local values cannot use expressions
308317
// that depend on instance keys, such as `count.*` and `each.*`.
309318
val, diags := d.Evaluator.EvaluateExpr(config.Expr, cty.DynamicPseudoType, EvalDataForNoInstanceKey)
310319

311-
// Since we build a callgraph for each local value,
312-
// we clear the callgraph when the local value is finally resolved.
313-
if entry {
314-
d.Evaluator.CallGraph.Clear()
315-
}
320+
d.Evaluator.CallStack.Pop()
316321
return val, diags
317322
}
318323

terraform/evaluator_test.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,18 @@ locals {
586586
return diags.Error() != `main.tf:4,9-18: circular reference found; local.foo -> local.bar -> local.foo`
587587
},
588588
},
589+
{
590+
name: "nested multiple local values",
591+
config: `
592+
locals {
593+
foo = "foo"
594+
bar = [local.foo, local.foo]
595+
}`,
596+
expr: expr(`local.bar`),
597+
ty: cty.List(cty.String),
598+
want: `cty.ListVal([]cty.Value{cty.StringVal("foo"), cty.StringVal("foo")})`,
599+
errCheck: neverHappend,
600+
},
589601
{
590602
name: "count.index in non-counted context",
591603
expr: expr(`count.index`),
@@ -666,7 +678,7 @@ locals {
666678
ModulePath: config.Path.UnkeyedInstanceShim(),
667679
Config: config,
668680
VariableValues: variableValues,
669-
CallGraph: NewCallGraph(),
681+
CallStack: NewCallStack(),
670682
}
671683

672684
got, diags := evaluator.EvaluateExpr(test.expr, test.ty, test.keyData)

tflint/runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func NewRunner(c *Config, ants map[string]Annotations, cfg *terraform.Config, va
5353
ModulePath: cfg.Path.UnkeyedInstanceShim(),
5454
Config: cfg.Root,
5555
VariableValues: variableValues,
56-
CallGraph: terraform.NewCallGraph(),
56+
CallStack: terraform.NewCallStack(),
5757
}
5858

5959
runner := &Runner{

0 commit comments

Comments
 (0)