hclsyntax: Explicit AST node for parentheses

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.

hclsyntax/expression.go

@@ -27,6 +27,32 @@ type Expression interface {
 // Assert that Expression implements hcl.Expression
 var assertExprImplExpr hcl.Expression = Expression(nil)
 
+// ParenthesesExpr represents an expression written in grouping
+// parentheses.
+//
+// The parser takes care of the precedence effect of the parentheses, so the
+// only purpose of this separate expression node is to capture the source range
+// of the parentheses themselves, rather than the source range of the
+// expression within. All of the other expression operations just pass through
+// to the underlying expression.
+type ParenthesesExpr struct {
+	Expression
+	SrcRange hcl.Range
+}
+
+var _ hcl.Expression = (*ParenthesesExpr)(nil)
+
+func (e *ParenthesesExpr) Range() hcl.Range {
+	return e.SrcRange
+}
+
+func (e *ParenthesesExpr) walkChildNodes(w internalWalkFunc) {
+	// We override the walkChildNodes from the embedded Expression to
+	// ensure that both the parentheses _and_ the content are visible
+	// in a walk.
+	w(e.Expression)
+}
+
 // LiteralValueExpr is an expression that just always returns a given value.
 type LiteralValueExpr struct {
 	Val      cty.Value

hclsyntax/parser.go

@@ -911,7 +911,7 @@ func (p *parser) parseExpressionTerm() (Expression, hcl.Diagnostics) {
 
 	switch start.Type {
 	case TokenOParen:
-		p.Read() // eat open paren
+		oParen := p.Read() // eat open paren
 
 		p.PushIncludeNewlines(false)
 
@@ -937,9 +937,19 @@ func (p *parser) parseExpressionTerm() (Expression, hcl.Diagnostics) {
 			p.setRecovery()
 		}
 
-		p.Read() // eat closing paren
+		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),
+		}
+
 		return expr, diags
 
 	case TokenNumberLit:

hclsyntax/structure_at_pos_test.go

@@ -282,7 +282,7 @@ func TestOutermostExprAtPos(t *testing.T) {
 		"parens": {
 			`a = (1 + 1)`,
 			hcl.Pos{Byte: 6},
-			`1 + 1`, // The parser trims the parens off, so they aren't considered as part of the expression :(
+			`(1 + 1)`,
 		},
 		"tuple cons": {
 			`a = [1, 2, 3]`,

hclsyntax/walk_test.go

@@ -45,12 +45,14 @@ func TestWalk(t *testing.T) {
 		{
 			`(1 + 1)`,
 			[]testWalkCall{
+				{testWalkEnter, "*hclsyntax.ParenthesesExpr"},
 				{testWalkEnter, "*hclsyntax.BinaryOpExpr"},
 				{testWalkEnter, "*hclsyntax.LiteralValueExpr"},
 				{testWalkExit, "*hclsyntax.LiteralValueExpr"},
 				{testWalkEnter, "*hclsyntax.LiteralValueExpr"},
 				{testWalkExit, "*hclsyntax.LiteralValueExpr"},
 				{testWalkExit, "*hclsyntax.BinaryOpExpr"},
+				{testWalkExit, "*hclsyntax.ParenthesesExpr"},
 			},
 		},
 		{

hclwrite/parser_test.go

@@ -189,6 +189,70 @@ func TestParse(t *testing.T) {
 				},
 			},
 		},
+		{
+			"a = (\n  1 + 2\n)\nb = 3\n",
+			TestTreeNode{
+				Type: "Body",
+				Children: []TestTreeNode{
+					{
+						Type: "Attribute",
+						Children: []TestTreeNode{
+							{Type: "comments"},
+							{
+								Type: "identifier",
+								Val:  "a",
+							},
+							{
+								Type: "Tokens",
+								Val:  " =",
+							},
+							{
+								Type: "Expression",
+								Children: []TestTreeNode{
+									{
+										Type: "Tokens",
+										Val:  " (\n  1 + 2\n)",
+									},
+								},
+							},
+							{Type: "comments"},
+							{
+								Type: "Tokens",
+								Val:  "\n",
+							},
+						},
+					},
+					{
+						Type: "Attribute",
+						Children: []TestTreeNode{
+							{Type: "comments"},
+							{
+								Type: "identifier",
+								Val:  "b",
+							},
+							{
+								Type: "Tokens",
+								Val:  " =",
+							},
+							{
+								Type: "Expression",
+								Children: []TestTreeNode{
+									{
+										Type: "Tokens",
+										Val:  " 3",
+									},
+								},
+							},
+							{Type: "comments"},
+							{
+								Type: "Tokens",
+								Val:  "\n",
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			"b {}\n",
 			TestTreeNode{