Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additional function for parsing traversals with [*] keys #673

Merged
merged 2 commits into from Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 75 additions & 1 deletion hclsyntax/parse_traversal_test.go
Expand Up @@ -4,11 +4,13 @@
package hclsyntax

import (
"fmt"
"testing"

"github.com/go-test/deep"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/hcl/v2"
)

func TestParseTraversalAbs(t *testing.T) {
Expand Down Expand Up @@ -208,10 +210,61 @@ func TestParseTraversalAbs(t *testing.T) {
},
1, // extra junk after traversal
},

{
"foo[*]",
hcl.Traversal{
hcl.TraverseRoot{
Name: "foo",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
},
hcl.TraverseSplat{
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 4, Byte: 3},
End: hcl.Pos{Line: 1, Column: 7, Byte: 6},
},
},
},
0,
},
{
"foo.*", // Still not supporting this.
hcl.Traversal{
hcl.TraverseRoot{
Name: "foo",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
},
},
1,
},
{
"foo[*].bar", // Run this through the unsupported function.
hcl.Traversal{
hcl.TraverseRoot{
Name: "foo",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
},
},
1,
},
}

for _, test := range tests {
t.Run(test.src, func(t *testing.T) {
if test.src == "foo[*]" {
// Skip the test that introduces splat syntax.
t.Skip("skipping test for unsupported splat syntax")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand this right that we skip it here since we use ParseTraversalAbs in which case this is unsupported, but include it in the partial_ test cases since ParseTraversalPartial supports it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, exactly. Basically, I wanted to make sure that using the splat was still broken in the old implementation but worked in the new one. I figured the easiest way to do that was to just manually skip the relevant tests since there's only one of each branch that needs to be skipped.

I could complicate the whole function by adding in some additional metadata to the test cases, but thought it was overkill just for one case.

}

got, diags := ParseTraversalAbs([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1})
if len(diags) != test.diagCount {
for _, diag := range diags {
Expand All @@ -226,5 +279,26 @@ func TestParseTraversalAbs(t *testing.T) {
}
}
})

t.Run(fmt.Sprintf("partial_%s", test.src), func(t *testing.T) {
if test.src == "foo[*].bar" {
// Skip the test that's supposed to fail for splat syntax.
t.Skip("skipping test for unsupported splat syntax")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we test for this to error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this is a copy-paste error which makes the message wrong. This test case is expecting an error, which is not what we want here. So I skip this test case in the test branch that does support splats, otherwise the test will fail expecting an error when there isn't one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this shortly!

}

got, diags := ParseTraversalPartial([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1})
if len(diags) != test.diagCount {
for _, diag := range diags {
t.Logf(" - %s", diag.Error())
}
t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.diagCount)
}

if diff := deep.Equal(got, test.want); diff != nil {
for _, problem := range diff {
t.Error(problem)
}
}
})
}
}
51 changes: 50 additions & 1 deletion hclsyntax/parser_traversal.go
Expand Up @@ -4,15 +4,36 @@
package hclsyntax

import (
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/hcl/v2"
)

// ParseTraversalAbs parses an absolute traversal that is assumed to consume
// all of the remaining tokens in the peeker. The usual parser recovery
// behavior is not supported here because traversals are not expected to
// be parsed as part of a larger program.
func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) {
return p.parseTraversal(false)
}

// ParseTraversalPartial parses an absolute traversal that is permitted
// to contain splat ([*]) expressions. Only splat expressions within square
// brackets are permitted ([*]); splat expressions within attribute names are
// not permitted (.*).
//
// The meaning of partial here is that the traversal may be incomplete, in that
// any splat expression indicates reference to a potentially unknown number of
// elements.
//
// Traversals that include splats cannot be automatically traversed by HCL using
// the TraversalAbs or TraversalRel methods. Instead, the caller must handle
// the traversals manually.
func (p *parser) ParseTraversalPartial() (hcl.Traversal, hcl.Diagnostics) {
return p.parseTraversal(true)
}

func (p *parser) parseTraversal(allowSplats bool) (hcl.Traversal, hcl.Diagnostics) {
var ret hcl.Traversal
var diags hcl.Diagnostics

Expand Down Expand Up @@ -127,6 +148,34 @@ func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) {
return ret, diags
}

case TokenStar:
if allowSplats {

p.Read() // Eat the star.
close := p.Read()
if close.Type != TokenCBrack {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Unclosed index brackets",
Detail: "Index key must be followed by a closing bracket.",
Subject: &close.Range,
Context: hcl.RangeBetween(open.Range, close.Range).Ptr(),
})
}

ret = append(ret, hcl.TraverseSplat{
SrcRange: hcl.RangeBetween(open.Range, close.Range),
})

if diags.HasErrors() {
return ret, diags
}

continue
}

// Otherwise, return the error below for the star.
fallthrough
default:
if next.Type == TokenStar {
diags = append(diags, &hcl.Diagnostic{
Expand Down
31 changes: 31 additions & 0 deletions hclsyntax/public.go
Expand Up @@ -118,6 +118,37 @@ func ParseTraversalAbs(src []byte, filename string, start hcl.Pos) (hcl.Traversa
return expr, diags
}

// ParseTraversalPartial matches the behavior of ParseTraversalAbs except
// that it allows splat expressions ([*]) to appear in the traversal.
//
// The returned traversals are "partial" in that the splat expression indicates
// an unknown value for the index.
//
// Traversals that include splats cannot be automatically traversed by HCL using
// the TraversalAbs or TraversalRel methods. Instead, the caller must handle
// the traversals manually.
func ParseTraversalPartial(src []byte, filename string, start hcl.Pos) (hcl.Traversal, hcl.Diagnostics) {
tokens, diags := LexExpression(src, filename, start)
peeker := newPeeker(tokens, false)
parser := &parser{peeker: peeker}

// Bare traverals are always parsed in "ignore newlines" mode, as if
// they were wrapped in parentheses.
parser.PushIncludeNewlines(false)

expr, parseDiags := parser.ParseTraversalPartial()
diags = append(diags, parseDiags...)

parser.PopIncludeNewlines()

// Panic if the parser uses incorrect stack discipline with the peeker's
// newlines stack, since otherwise it will produce confusing downstream
// errors.
peeker.AssertEmptyIncludeNewlinesStack()

return expr, diags
}

// LexConfig performs lexical analysis on the given buffer, treating it as a
// whole HCL config file, and returns the resulting tokens.
//
Expand Down