Unable to use HCL identifiers with "." in function call in block attribute

[picatz]

While investigating the following syntax:

example "something arbitrary" {
   result = repository.file("main.go").example
}

I ran into the following error parsing it:

Error: Missing newline after argument

  on issue.hcl line 2, in example "something arbitrary":
   2:    result = repository.file("main.go").example

An argument definition must end with a newline.

issue.hcl:2,28-29: Missing newline after argument; An argument definition must end with a newline.

Seems related to this line in (*hclsyntax.parser).finishParsingBodyAttribute:

hcl/hclsyntax/parser.go

Line 260 in 67270ba

detail := "An argument definition must end with a newline."

Based on my reading of the spec's "function call" and "identifier" definitions, this seems like it could be possible, but currently ins't implemented? Or is this something that isn't allowed purposefully? It's not clear to me how traversals should work in this case.

[apparentlymart]

Hi @picatz,

For historical reasons (dating back to before HCL 2, in HIL), the function and variable namespaces are entirely separate. The function namespace is just a flat lookup table, so it doesn't currently have any sense of hierarchy, and so dots are not valid as part of function names.

You can see over in #535 a prototype of a hierarchical namespace for functions, but it remains distinct from the variable namespace and I don't think merging them will be straightforward because existing HCL-based languages such as the Terraform language currently allow the same name to exist in both namespaces and rely on syntax rules to resolve the ambiguity.

At the very least though I think there's an opportunity for a better error message in this situation: it's never valid for an open parenthesis ( to immediately follow a traversal, but right now I think it's falling out into the general codepath looking for continuation operators (like repository.file + foo, for example) and since ( isn't ever valid as a continuation it assumes that this was intended to be the end of the argument and that there's a missing newline to mark it.

We could potentially add a rule to the traversal parser where if it finds a ( after more than one step of traversal it'll generate a specialized error that looks more like the "no such function" error message, reporting that repository.file isn't a valid function to call.

I'm not sure what repository.file was intended to mean in this hypothetical language, but I think for today's HCL you'd need to find a different way to express that meaning. A simple answer would be to name the function repository_file instead, which would be valid syntax, but I'm not sure if you had some other meaning in mind here beyond just writing a function name with a dot in it.

[picatz]

I spent a little bit of time investigating a POC. Though, to be clear, we would certainly want to have a design document before doing this realistically. Thankfully, I was able to get what I need working in the hclsyntax package, and introduced a hcl.TraverseFunction type:

The main change needed to occur in (*hclsyntax.parser).parseExpressionTraversals, which didn't handle the hclsyntax.TokenOParen case:

hcl/hclsyntax/parser.go

Line 630 in 67270ba

switch next.Type {

--- a/hclsyntax/parser.go
+++ b/hclsyntax/parser.go
@@ -906,7 +906,89 @@ Traversal:
                                        }
                                }
                        }
+               case TokenOParen:
+                       oParen := p.Read() // eat open paren
 
+                       p.PushIncludeNewlines(false)
+
+                       expr, diags := p.ParseExpression()
+                       if diags.HasErrors() {
+                               // attempt to place the peeker after our closing paren
+                               // before we return, so that the next parser has some
+                               // chance of finding a valid expression.
+                               p.recover(TokenCParen)
+                               p.PopIncludeNewlines()
+                               return expr, diags
+                       }
+
+                       close := p.Peek()
+                       if close.Type != TokenCParen {
+                               diags = append(diags, &hcl.Diagnostic{
+                                       Severity: hcl.DiagError,
+                                       Summary:  "Unbalanced parentheses",
+                                       Detail:   "Expected a closing parenthesis to terminate the expression.",
+                                       Subject:  &close.Range,
+                                       Context:  hcl.RangeBetween(oParen.Range, close.Range).Ptr(),
+                               })
+
+                               p.setRecovery()
+                               return expr, diags
+                       }
+
+                       cParen := p.Read() // eat closing paren
+                       p.PopIncludeNewlines()
+
+                       // Our parser's already taken care of the precedence effect of the
+                       // parentheses by considering them to be a kind of "term", but we
+                       // still need to include the parentheses in our AST so we can give
+                       // an accurate representation of the source range that includes the
+                       // // open and closing parentheses.
+                       // expr = &ParenthesesExpr{
+                       //      Expression: expr,
+                       //      SrcRange:   hcl.RangeBetween(oParen.Range, cParen.Range),
+                       // }
+
+                       exprValue, diags := expr.Value(nil)
+                       if diags.HasErrors() {
+                               // attempt to place the peeker after our closing paren
+                               // before we return, so that the next parser has some
+                               // chance of finding a valid expression.
+                               p.recover(TokenCParen)
+                               p.PopIncludeNewlines()
+                               return expr, diags
+                       }
+
+                       rng := hcl.RangeBetween(oParen.Range, cParen.Range)
+
+                       // get name of function from last traversal
+                       scopTraversalExpr, ok := ret.(*ScopeTraversalExpr)
+                       if !ok {
+                               panic("invalid parser state") // TODO: return diags
+                       }
+
+                       // get length of traversal
+                       x := len(scopTraversalExpr.Traversal)
+
+                       // get the last element in the traversal (function name)
+                       e := scopTraversalExpr.Traversal[x-1]
+
+                       // remove the last element from the traversal, because we will re-add it
+                       // as its "proper" type
+                       y := scopTraversalExpr.Traversal[:len(scopTraversalExpr.Traversal)-1]
+
+                       scopTraversalExpr.Traversal = y
+
+                       ta, ok := e.(hcl.TraverseAttr)
+                       if !ok {
+                               panic("invalid parser state") // TODO: return diags
+                       }
+
+                       step := hcl.TraverseFunction{
+                               Name:     ta.Name,
+                               Params:   []cty.Value{exprValue},
+                               SrcRange: rng,
+                       }
+                       ret = makeRelativeTraversal(ret, step, rng)

--- a/traversal.go
+++ b/traversal.go
@@ -291,3 +291,19 @@ func (tn TraverseSplat) TraversalStep(val cty.Value) (cty.Value, Diagnostics) {
 func (tn TraverseSplat) SourceRange() Range {
        return tn.SrcRange
 }
+
+// TraverseFunction...
+type TraverseFunction struct {
+       isTraverser
+       Name     string
+       Params   []cty.Value
+       SrcRange Range
+}
+
+func (tf TraverseFunction) TraversalStep(val cty.Value) (cty.Value, Diagnostics) {
+       panic("TraverseFunction not yet implemented")
+}
+
+func (tf TraverseFunction) SourceRange() Range {
+       return tf.SrcRange
+}

So, when I parse the following:

example "something arbitrary" {
   result = repository.file("main.go").example
}

Ends up as:

issue.hcl:
                block: example: lables(1): ["something arbitrary"]
                        attr: "result": *hclsyntax.ScopeTraversalExpr (variables: 1)
                                Traversal: hcl.Traversal{
                                        hcl.TraverseRoot{
                                                isTraverser:hcl.isTraverser{}, 
                                                Name:"repository", 
                                                SrcRange:hcl.Range{...},
                                        }, 
                                        hcl.TraverseFunction{
                                                isTraverser:hcl.isTraverser{}, 
                                                Name:"file", 
                                                Params:[]cty.Value{cty.StringVal("main.go")}, 
                                                SrcRange:hcl.Range{...},
                                        }, 
                                        hcl.TraverseAttr{
                                                isTraverser:hcl.isTraverser{}, 
                                                Name:"example", 
                                                SrcRange:hcl.Range{...},
                                        },
                                }

This may not be its final form, but is useful progress in understanding (for myself) how this could work.

[picatz]

I'm not sure what repository.file was intended to mean in this hypothetical language, but I think for today's HCL you'd need to find a different way to express that meaning. A simple answer would be to name the function repository_file instead, which would be valid syntax, but I'm not sure if you had some other meaning in mind here beyond just writing a function name with a dot in it.

In my example repository would be an object with an attribute file that itself is a function that returns an object with attribute example.

For my specific use case, I just need the parsing to work, because I perform my own evaluation of the information. In essence, I'm using HCL as a substrate for the human readable/writable representation, but then convert it to another format. So, I just need the traversal information to be there so that I can use it.