hcl/hclsyntax: Better handling of naked attribute keys with dots

To make things read better in the normal case, we treat naked identifiers
in the place of object keys as literal strings containing the identifier
text rather than as references. However, this had a couple sub-optimal
implications:

- If a user would try to create a key containing a period, the evaluator
  would see that it wasn't a valid keyword and try to resolve it as a
  normal scope traversal, causing a confusing error that didn't align
  with the user's intent.

- In the rarer case where the attempted key contains a period followed by
  a digit, the parser would trip over what seems to be an unexpected
  identifier following the colon and produce, again, a confusing error
  that doesn't align with what the user intended.

To address the first of these problems, it is now invalid to use a naked
traversal with more than one step as an object key, which allows us to
produce a targeted error message that directs the user to either put the
expression in parentheses to force interpretation as a scope traversal
or in quotes to force interpretation as a literal.

The second problem can't be addressed exactly due to it being a parser
problem, but we improve the situation slightly here by adding an extra
hint to the parse error message in this case so that a user making this
mistake might understand better how the error relates to what they were
trying to express.
This commit is contained in:
Martin Atkins 2018-12-11 16:48:31 -08:00
parent 17ab3729e7
commit 7ace05a3be
3 changed files with 74 additions and 3 deletions

View File

@ -797,6 +797,26 @@ func (e *ObjectConsKeyExpr) walkChildNodes(w internalWalkFunc) {
}
func (e *ObjectConsKeyExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
// Because we accept a naked identifier as a literal key rather than a
// reference, it's confusing to accept a traversal containing periods
// here since we can't tell if the user intends to create a key with
// periods or actually reference something. To avoid confusing downstream
// errors we'll just prohibit a naked multi-step traversal here and
// require the user to state their intent more clearly.
// (This is handled at evaluation time rather than parse time because
// an application using static analysis _can_ accept a naked multi-step
// traversal here, if desired.)
if travExpr, isTraversal := e.Wrapped.(*ScopeTraversalExpr); isTraversal && len(travExpr.Traversal) > 1 {
var diags hcl.Diagnostics
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Ambiguous attribute key",
Detail: "If this expression is intended to be a reference, wrap it in parentheses. If it's instead intended as a literal name containing periods, wrap it in quotes to create a string literal.",
Subject: e.Range().Ptr(),
})
return cty.DynamicVal, diags
}
if ln := e.literalName(); ln != "" {
return cty.StringVal(ln), nil
}

View File

@ -409,6 +409,40 @@ upper(
cty.DynamicVal,
1, // Incorrect key type; Can't use this value as a key: string required
},
{
`{"centos_7.2_ap-south-1" = "ami-abc123"}`,
nil,
cty.ObjectVal(map[string]cty.Value{
"centos_7.2_ap-south-1": cty.StringVal("ami-abc123"),
}),
0,
},
{
// This is syntactically valid (it's similar to foo["bar"])
// but is rejected during evaluation to force the user to be explicit
// about which of the following interpretations they mean:
// -{(foo.bar) = "baz"}
// -{"foo.bar" = "baz"}
// naked traversals as keys are allowed when analyzing an expression
// statically so an application can define object-syntax-based
// language constructs with looser requirements, but we reject
// this during normal expression evaluation.
`{foo.bar = "ami-abc123"}`,
nil,
cty.DynamicVal,
1, // Ambiguous attribute key; If this expression is intended to be a reference, wrap it in parentheses. If it's instead intended as a literal name containing periods, wrap it in quotes to create a string literal.
},
{
// This is a weird variant of the above where a period is followed
// by a digit, causing the parser to interpret it as an index
// operator using the legacy HIL/Terraform index syntax.
// This one _does_ fail parsing, causing it to be subject to
// parser recovery behavior.
`{centos_7.2_ap-south-1 = "ami-abc123"}`,
nil,
cty.EmptyObjectVal, // (due to parser recovery behavior)
1, // Missing key/value separator; Expected an equals sign ("=") to mark the beginning of the attribute value. If you intended to given an attribute name containing periods or spaces, write the name in quotes to create a string literal.
},
{
`{"hello" = "world", "goodbye" = "cruel world"}`,
nil,

View File

@ -273,7 +273,7 @@ Token:
return &Block{
Type: blockType,
Labels: labels,
Body: &Body{
Body: &Body{
SrcRange: ident.Range,
EndRange: ident.Range,
},
@ -1135,7 +1135,8 @@ func (p *parser) parseObjectCons() (Expression, hcl.Diagnostics) {
next = p.Peek()
if next.Type != TokenEqual && next.Type != TokenColon {
if !p.recovery {
if next.Type == TokenNewline || next.Type == TokenComma {
switch next.Type {
case TokenNewline, TokenComma:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing attribute value",
@ -1143,7 +1144,23 @@ func (p *parser) parseObjectCons() (Expression, hcl.Diagnostics) {
Subject: &next.Range,
Context: hcl.RangeBetween(open.Range, next.Range).Ptr(),
})
} else {
case TokenIdent:
// Although this might just be a plain old missing equals
// sign before a reference, one way to get here is to try
// to write an attribute name containing a period followed
// by a digit, which was valid in HCL1, like this:
// foo1.2_bar = "baz"
// We can't know exactly what the user intended here, but
// we'll augment our message with an extra hint in this case
// in case it is helpful.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing key/value separator",
Detail: "Expected an equals sign (\"=\") to mark the beginning of the attribute value. If you intended to given an attribute name containing periods or spaces, write the name in quotes to create a string literal.",
Subject: &next.Range,
Context: hcl.RangeBetween(open.Range, next.Range).Ptr(),
})
default:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing key/value separator",