From cc8b14cf458f0c3f83386a0a6e70515aa2b2c522 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 26 Feb 2018 08:38:35 -0800 Subject: [PATCH] hclsyntax: "null", "true", "false" AbsTraversalForExpr The contract for AbsTraversalForExpr calls for us to interpret an expression as if it were traversal syntax. Traversal syntax does not have the special keywords "null", "true" and "false", so we must interpret these as TraverseRoot rather than as literal values. Previously this wasn't working because the parser converted these to literals too early. To make this work properly, we implement AbsTraversalForExpr on literal expressions and effectively "undo" the parser's re-interpretation of these keywords to back out to the original keyword strings. We also rework how object keys are handled so that we wait until eval time to decide whether to interpret the key expression as an unquoted literal string. This allows us to properly support AbsTraversalForExpr on keys in object constructors, bypassing the string-interpretation behavior in that case. --- hcl/hclsyntax/expression.go | 113 ++++++++++++++++++++++ hcl/hclsyntax/expression_test.go | 32 ++++++ hcl/hclsyntax/expression_vars.go | 4 + hcl/hclsyntax/parser.go | 21 ++-- hcl/integrationtest/terraformlike_test.go | 81 +++++++++++++++- 5 files changed, 234 insertions(+), 17 deletions(-) diff --git a/hcl/hclsyntax/expression.go b/hcl/hclsyntax/expression.go index 1ededd9..db4b779 100644 --- a/hcl/hclsyntax/expression.go +++ b/hcl/hclsyntax/expression.go @@ -47,6 +47,51 @@ func (e *LiteralValueExpr) StartRange() hcl.Range { return e.SrcRange } +// Implementation for hcl.AbsTraversalForExpr. +func (e *LiteralValueExpr) AsTraversal() hcl.Traversal { + // This one's a little weird: the contract for AsTraversal is to interpret + // an expression as if it were traversal syntax, and traversal syntax + // doesn't have the special keywords "null", "true", and "false" so these + // are expected to be treated like variables in that case. + // Since our parser already turned them into LiteralValueExpr by the time + // we get here, we need to undo this and infer the name that would've + // originally led to our value. + // We don't do anything for any other values, since they don't overlap + // with traversal roots. + + if e.Val.IsNull() { + // In practice the parser only generates null values of the dynamic + // pseudo-type for literals, so we can safely assume that any null + // was orignally the keyword "null". + return hcl.Traversal{ + hcl.TraverseRoot{ + Name: "null", + SrcRange: e.SrcRange, + }, + } + } + + switch e.Val { + case cty.True: + return hcl.Traversal{ + hcl.TraverseRoot{ + Name: "true", + SrcRange: e.SrcRange, + }, + } + case cty.False: + return hcl.Traversal{ + hcl.TraverseRoot{ + Name: "false", + SrcRange: e.SrcRange, + }, + } + default: + // No traversal is possible for any other value. + return nil + } +} + // ScopeTraversalExpr is an Expression that retrieves a value from the scope // using a traversal. type ScopeTraversalExpr struct { @@ -102,6 +147,20 @@ func (e *RelativeTraversalExpr) StartRange() hcl.Range { return e.SrcRange } +// Implementation for hcl.AbsTraversalForExpr. +func (e *RelativeTraversalExpr) AsTraversal() hcl.Traversal { + // We can produce a traversal only if our source can. + st, diags := hcl.AbsTraversalForExpr(e.Source) + if diags.HasErrors() { + return nil + } + + ret := make(hcl.Traversal, len(st)+len(e.Traversal)) + copy(ret, st) + copy(ret[len(st):], e.Traversal) + return ret +} + // FunctionCallExpr is an Expression that calls a function from the EvalContext // and returns its result. type FunctionCallExpr struct { @@ -660,6 +719,60 @@ func (e *ObjectConsExpr) ExprMap() []hcl.KeyValuePair { return ret } +// ObjectConsKeyExpr is a special wrapper used only for ObjectConsExpr keys, +// which deals with the special case that a naked identifier in that position +// must be interpreted as a literal string rather than evaluated directly. +type ObjectConsKeyExpr struct { + Wrapped Expression +} + +func (e *ObjectConsKeyExpr) literalName() string { + // This is our logic for deciding whether to behave like a literal string. + // We lean on our AbsTraversalForExpr implementation here, which already + // deals with some awkward cases like the expression being the result + // of the keywords "null", "true" and "false" which we'd want to interpret + // as keys here too. + return hcl.ExprAsKeyword(e.Wrapped) +} + +func (e *ObjectConsKeyExpr) walkChildNodes(w internalWalkFunc) { + // We only treat our wrapped expression as a real expression if we're + // not going to interpret it as a literal. + if e.literalName() == "" { + e.Wrapped = w(e.Wrapped).(Expression) + } +} + +func (e *ObjectConsKeyExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { + if ln := e.literalName(); ln != "" { + return cty.StringVal(ln), nil + } + return e.Wrapped.Value(ctx) +} + +func (e *ObjectConsKeyExpr) Range() hcl.Range { + return e.Wrapped.Range() +} + +func (e *ObjectConsKeyExpr) StartRange() hcl.Range { + return e.Wrapped.StartRange() +} + +// Implementation for hcl.AbsTraversalForExpr. +func (e *ObjectConsKeyExpr) AsTraversal() hcl.Traversal { + // We can produce a traversal only if our wrappee can. + st, diags := hcl.AbsTraversalForExpr(e.Wrapped) + if diags.HasErrors() { + return nil + } + + return st +} + +func (e *ObjectConsKeyExpr) UnwrapExpression() Expression { + return e.Wrapped +} + // ForExpr represents iteration constructs: // // tuple = [for i, v in list: upper(v) if i > 2] diff --git a/hcl/hclsyntax/expression_test.go b/hcl/hclsyntax/expression_test.go index f192d3e..3d140c3 100644 --- a/hcl/hclsyntax/expression_test.go +++ b/hcl/hclsyntax/expression_test.go @@ -353,6 +353,38 @@ upper( }), 0, }, + { + `{true: "yes"}`, + nil, + cty.ObjectVal(map[string]cty.Value{ + "true": cty.StringVal("yes"), + }), + 0, + }, + { + `{false: "yes"}`, + nil, + cty.ObjectVal(map[string]cty.Value{ + "false": cty.StringVal("yes"), + }), + 0, + }, + { + `{null: "yes"}`, + nil, + cty.ObjectVal(map[string]cty.Value{ + "null": cty.StringVal("yes"), + }), + 0, + }, + { + `{15: "yes"}`, + nil, + cty.ObjectVal(map[string]cty.Value{ + "15": cty.StringVal("yes"), + }), + 0, + }, { `{"hello" = "world", "goodbye" = "cruel world"}`, nil, diff --git a/hcl/hclsyntax/expression_vars.go b/hcl/hclsyntax/expression_vars.go index c15d134..9177092 100755 --- a/hcl/hclsyntax/expression_vars.go +++ b/hcl/hclsyntax/expression_vars.go @@ -39,6 +39,10 @@ func (e *ObjectConsExpr) Variables() []hcl.Traversal { return Variables(e) } +func (e *ObjectConsKeyExpr) Variables() []hcl.Traversal { + return Variables(e) +} + func (e *RelativeTraversalExpr) Variables() []hcl.Traversal { return Variables(e) } diff --git a/hcl/hclsyntax/parser.go b/hcl/hclsyntax/parser.go index 726e09f..c35f944 100644 --- a/hcl/hclsyntax/parser.go +++ b/hcl/hclsyntax/parser.go @@ -1057,23 +1057,9 @@ func (p *parser) parseObjectCons() (Expression, hcl.Diagnostics) { break } - // As a special case, we allow the key to be a literal identifier. - // This means that a variable reference or function call can't appear - // directly as key expression, and must instead be wrapped in some - // disambiguation punctuation, like (var.a) = "b" or "${var.a}" = "b". var key Expression var keyDiags hcl.Diagnostics - if p.Peek().Type == TokenIdent { - nameTok := p.Read() - key = &LiteralValueExpr{ - Val: cty.StringVal(string(nameTok.Bytes)), - - SrcRange: nameTok.Range, - } - } else { - key, keyDiags = p.ParseExpression() - } - + key, keyDiags = p.ParseExpression() diags = append(diags, keyDiags...) if p.recovery && keyDiags.HasErrors() { @@ -1084,6 +1070,11 @@ func (p *parser) parseObjectCons() (Expression, hcl.Diagnostics) { break } + // We wrap up the key expression in a special wrapper that deals + // with our special case that naked identifiers as object keys + // are interpreted as literal strings. + key = &ObjectConsKeyExpr{Wrapped: key} + next = p.Peek() if next.Type != TokenEqual && next.Type != TokenColon { if !p.recovery { diff --git a/hcl/integrationtest/terraformlike_test.go b/hcl/integrationtest/terraformlike_test.go index 78b23fb..c42a4d0 100644 --- a/hcl/integrationtest/terraformlike_test.go +++ b/hcl/integrationtest/terraformlike_test.go @@ -46,9 +46,14 @@ func TestTerraformLike(t *testing.T) { Config hcl.Body `hcl:",remain"` DependsOn hcl.Expression `hcl:"depends_on,attr"` } + type Module struct { + Name string `hcl:"name,label"` + Providers hcl.Expression `hcl:"providers"` + } type Root struct { Variables []*Variable `hcl:"variable,block"` Resources []*Resource `hcl:"resource,block"` + Modules []*Module `hcl:"module,block"` } instanceDecode := &hcldec.ObjectSpec{ "image_id": &hcldec.AttrSpec{ @@ -276,6 +281,65 @@ func TestTerraformLike(t *testing.T) { t.Errorf("wrong depends_on traversal RootName %#v; want %#v", got, want) } }) + + t.Run("module", func(t *testing.T) { + if got, want := len(root.Modules), 1; got != want { + t.Fatalf("wrong number of Modules %d; want %d", got, want) + } + mod := root.Modules[0] + if got, want := mod.Name, "foo"; got != want { + t.Errorf("wrong module name %q; want %q", got, want) + } + + pExpr := mod.Providers + pairs, diags := hcl.ExprMap(pExpr) + if len(diags) != 0 { + t.Errorf("unexpected diagnostics extracting providers") + for _, diag := range diags { + t.Logf("- %s", diag) + } + } + if got, want := len(pairs), 1; got != want { + t.Fatalf("wrong number of key/value pairs in providers %d; want %d", got, want) + } + + pair := pairs[0] + kt, diags := hcl.AbsTraversalForExpr(pair.Key) + if len(diags) != 0 { + t.Errorf("unexpected diagnostics extracting providers key %#v", pair.Key) + for _, diag := range diags { + t.Logf("- %s", diag) + } + } + vt, diags := hcl.AbsTraversalForExpr(pair.Value) + if len(diags) != 0 { + t.Errorf("unexpected diagnostics extracting providers value %#v", pair.Value) + for _, diag := range diags { + t.Logf("- %s", diag) + } + } + + if got, want := len(kt), 1; got != want { + t.Fatalf("wrong number of key traversal steps %d; want %d", got, want) + } + if got, want := len(vt), 2; got != want { + t.Fatalf("wrong number of value traversal steps %d; want %d", got, want) + } + + if got, want := kt.RootName(), "null"; got != want { + t.Errorf("wrong number key traversal root %s; want %s", got, want) + } + if got, want := vt.RootName(), "null"; got != want { + t.Errorf("wrong number value traversal root %s; want %s", got, want) + } + if at, ok := vt[1].(hcl.TraverseAttr); ok { + if got, want := at.Name, "foo"; got != want { + t.Errorf("wrong number value traversal attribute name %s; want %s", got, want) + } + } else { + t.Errorf("wrong value traversal [1] type %T; want hcl.TraverseAttr", vt[1]) + } + }) }) } } @@ -290,8 +354,8 @@ resource "happycloud_instance" "test" { image_id = var.image_id tags = { - "Name" = "foo" - "${"Environment"}" = "prod" + "Name" = "foo" + "${"Environment"}" = "prod" } depends_on = [ @@ -320,6 +384,12 @@ resource "happycloud_security_group" "private" { } } +module "foo" { + providers = { + null = null.foo + } +} + ` const terraformLikeJSON = ` @@ -367,6 +437,13 @@ const terraformLikeJSON = ` } } } + }, + "module": { + "foo": { + "providers": { + "null": "null.foo" + } + } } } `