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

hclsyntax: Explicit AST node for parentheses #426

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

apparentlymart
Copy link
Contributor

So far the expression parentheses syntax has been handled entirely in the parser and has been totally invisible in the AST. That's fine for typical expression evaluation, but over the years it's led to a few quirky behaviors in less common situations where we've assumed that all expressions are covered by the AST itself or by the source ranges that the AST captures.

In particular, hclwrite assumes that all expressions will have source ranges that cover their tokens, and it generates an incorrect physical syntax tree when the AST doesn't uphold that.

After resisting through a few other similar bugs, this commit finally introduces an explicit AST node for parentheses, which makes the parentheses explicit in the AST and captures the larger source range that includes the TokenOParen and the TokenCParen.

This means that parentheses will now be visible as a distinct node when walking the AST, as reflected in the updated tests here. That may cause downstream applications that traverse the tree to exhibit different behaviors but we're not considering that as a "breaking change" because the Walk function doesn't make any guarantees about the specific AST shape.

This is the HCL part of the fix for hashicorp/terraform#27040.

So far the expression parentheses syntax has been handled entirely in the
parser and has been totally invisible in the AST. That's fine for typical
expression evaluation, but over the years it's led to a few quirky
behaviors in less common situations where we've assumed that all
expressions are covered by the AST itself or by the source ranges that the
AST captures.

In particular, hclwrite assumes that all expressions will have source
ranges that cover their tokens, and it generates an incorrect physical
syntax tree when the AST doesn't uphold that.

After resisting through a few other similar bugs, this commit finally
introduces an explicit AST node for parentheses, which makes the
parentheses explicit in the AST and captures the larger source range that
includes the TokenOParen and the TokenCParen.

This means that parentheses will now be visible as a distinct node when
walking the AST, as reflected in the updated tests here. That may cause
downstream applications that traverse the tree to exhibit different
behaviors but we're not considering that as a "breaking change" because
the Walk function doesn't make any guarantees about the specific AST
shape.
@mitchellh
Copy link
Contributor

Makes sense, I've always had to have paren AST nodes for the toy (and not toy :P) languages I wrote.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug hclwrite syntax/native v2 Relates to the v2 line of releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants