From d71f0c6149e64b14554f4a36a8273bd6b6234074 Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Thu, 25 Mar 2021 15:40:54 -0400
Subject: [PATCH] cli: Fix fmt output for multi-line value exprs

The formatter for value expressions which use legacy interpolation
syntax was previously behaving incorrectly with some multi-line
expressions. Any HCL expression which requires parenthesis to be allowed
to span multiple lines could be skip those parens if already inside
string interpolation (`"${}"`).

When removing string interpolation, we now check for a resulting
multi-line expression, and conservatively ensure that it starts and ends
with parenthesis. These may be redundant, as not all expressions require
parens to permit spanning multiple lines, but at least it will be valid
output.
---
 command/fmt.go                      | 36 ++++++++++++++++++++++++++++-
 command/testdata/fmt/general_in.tf  |  9 ++++++++
 command/testdata/fmt/general_out.tf |  9 ++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/command/fmt.go b/command/fmt.go
index 09c7ea740..d133e307b 100644
--- a/command/fmt.go
+++ b/command/fmt.go
@@ -366,7 +366,41 @@ func (c *FmtCommand) formatValueExpr(tokens hclwrite.Tokens) hclwrite.Tokens {
 	// "${
 	//    foo
 	// }"
-	return c.trimNewlines(inside)
+	trimmed := c.trimNewlines(inside)
+
+	// Finally, we check if the unwrapped expression is on multiple lines. If
+	// so, we ensure that it is surrounded by parenthesis to make sure that it
+	// parses correctly after unwrapping. This may be redundant in some cases,
+	// but is required for at least multi-line ternary expressions.
+	isMultiLine := false
+	hasLeadingParen := false
+	hasTrailingParen := false
+	for i, token := range trimmed {
+		switch {
+		case i == 0 && token.Type == hclsyntax.TokenOParen:
+			hasLeadingParen = true
+		case token.Type == hclsyntax.TokenNewline:
+			isMultiLine = true
+		case i == len(trimmed)-1 && token.Type == hclsyntax.TokenCParen:
+			hasTrailingParen = true
+		}
+	}
+	if isMultiLine && !(hasLeadingParen && hasTrailingParen) {
+		wrapped := make(hclwrite.Tokens, 0, len(trimmed)+2)
+		wrapped = append(wrapped, &hclwrite.Token{
+			Type:  hclsyntax.TokenOParen,
+			Bytes: []byte("("),
+		})
+		wrapped = append(wrapped, trimmed...)
+		wrapped = append(wrapped, &hclwrite.Token{
+			Type:  hclsyntax.TokenCParen,
+			Bytes: []byte(")"),
+		})
+
+		return wrapped
+	}
+
+	return trimmed
 }
 
 func (c *FmtCommand) formatTypeExpr(tokens hclwrite.Tokens) hclwrite.Tokens {
diff --git a/command/testdata/fmt/general_in.tf b/command/testdata/fmt/general_in.tf
index 0ee143731..94db1893f 100644
--- a/command/testdata/fmt/general_in.tf
+++ b/command/testdata/fmt/general_in.tf
@@ -42,3 +42,12 @@ resource "foo_instance" /* ... */ "baz" {
 
   provider "" {
 }
+
+locals {
+  name = "${contains(["foo"], var.my_var) ? "${var.my_var}-bar" :
+    contains(["baz"], var.my_var) ? "baz-${var.my_var}" :
+  file("ERROR: unsupported type ${var.my_var}")}"
+  wrapped = "${(var.my_var == null ? 1 :
+    var.your_var == null ? 2 :
+  3)}"
+}
diff --git a/command/testdata/fmt/general_out.tf b/command/testdata/fmt/general_out.tf
index 974646ebd..1fe6b5b3c 100644
--- a/command/testdata/fmt/general_out.tf
+++ b/command/testdata/fmt/general_out.tf
@@ -42,3 +42,12 @@ resource "foo_instance" "baz" {
 
 provider "" {
 }
+
+locals {
+  name = (contains(["foo"], var.my_var) ? "${var.my_var}-bar" :
+    contains(["baz"], var.my_var) ? "baz-${var.my_var}" :
+  file("ERROR: unsupported type ${var.my_var}"))
+  wrapped = (var.my_var == null ? 1 :
+    var.your_var == null ? 2 :
+  3)
+}