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

@@ -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]

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,

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)
 }

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 {

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"
+      }
+    }
   }
 }
 `