Repository: aws-cloudformation/rain (go)

Issue:
rain fmt doesn't do round trip
rain fmt doesn't do a round trip with the `--unsorted` option.

Sample template:

```yaml
Parameters:
  RootId:
    Type: String

Resources:

  Sandbox:
    Type: AWS::Organizations::OrganizationalUnit
    Properties:
      ParentId: !Ref RootId
      Name: Sandbox
```

Command for round trip without `--unsorted`:

```bash
cat template.yaml | rain fmt --json | rain fmt
```

Result looks like YAML again (the keys remain unsorted in this case! Is that deliberate?).

```yaml
Parameters:
  RootId:
    Type: String

Resources:
  Sandbox:
    Type: AWS::Organizations::OrganizationalUnit
    Properties: {ParentId: !Ref "RootId", Name: Sandbox}
```

Command for round trip with `--unsorted`:

```bash
cat template.yaml | rain fmt --json --unsorted | rain fmt --unsorted
```

The result is technically YAML, but I was expecting it to be written on multiple lines!

```yaml
{Parameters: {RootId: {Type: String}}, Resources: {Sandbox: {Type: 'AWS::Organizations::OrganizationalUnit', Properties: {ParentId: !Ref "RootId", Name: Sandbox}}}}
```

Pull request description:
Fix formatting round trip - Style is always 0
When formatting to JSON and back to YAML, JSON styles were left in the YAML nodes, which caused inconsistent formatting.

This *might* be considered a breaking change, because one of the effects is to now format `!Sub` without double quotes, even if they were in the original.

Fixes #170


Patch:
diff --git a/cft/format/transform.go b/cft/format/transform.go
index 6f61892b..433b32f1 100644
--- a/cft/format/transform.go
+++ b/cft/format/transform.go
@@ -92,5 +92,8 @@ func formatNode(n *yaml.Node) *yaml.Node {
 		n.Content[i] = formatNode(child)
 	}
 
+	// Always set Style to 0 for consistent formatting
+	n.Style = 0
+
 	return n
 }


Test patch:
diff --git a/cft/format/format_test.go b/cft/format/format_test.go
index b5eafe67..c59d1919 100644
--- a/cft/format/format_test.go
+++ b/cft/format/format_test.go
@@ -107,7 +107,7 @@ Resources:
   Func1:
     Type: AWS::Lambda::Function
     Properties:
-      Role: !Sub "arn:aws:iam::${AWS::AccountID}:role/lambda-basic"
+      Role: !Sub arn:aws:iam::${AWS::AccountID}:role/lambda-basic
       Runtime: python3.7
       Handler: index.handler
       Code:
@@ -158,7 +158,7 @@ Resources:
   Func1:
     Type: AWS::Lambda::Function
     Properties:
-      Role: !Sub "arn:aws:iam::${AWS::AccountID}:role/lambda-basic"
+      Role: !Sub arn:aws:iam::${AWS::AccountID}:role/lambda-basic
       Runtime: python3.7
       Handler: index.handler
       Code:
@@ -439,19 +439,34 @@ func checkMultilineBlockHeaders(t *testing.T, s string, expected bool) {
 	}
 }
 
-func TestFormatDefault(t *testing.T) {
+func TestFormatYaml(t *testing.T) {
 	checkMatch(t, expectedYaml, format.Options{})
+}
+
+func TestFormatYamlUnsorted(t *testing.T) {
 	checkMatch(t, expectedYamlUnsorted, format.Options{
 		Unsorted: true,
 	})
+}
+
+func TestFormatJson(t *testing.T) {
 	checkMatch(t, expectedJson, format.Options{
 		JSON: true,
 	})
+}
+
+func TestFormatUnsortedJson(t *testing.T) {
 	checkMatch(t, expectedUnsortedJson, format.Options{
 		JSON:     true,
 		Unsorted: true,
 	})
+}
+
+func TestFormatMultiLineBlock(t *testing.T) {
 	checkMultilineBlockHeaders(t, correctMultilineBlockHeaders, true)
+}
+
+func TestFormatMultiLineBlockWrong(t *testing.T) {
 	checkMultilineBlockHeaders(t, wrongMultilineBlockHeaders, false)
 }
 
@@ -504,7 +519,10 @@ Resources:
   Topic:
     Type: AWS::SNS::Topic
     Properties:
-      TopicName: !FindInMap [EnvironmentMap, MappedParam, !Ref EnvironmentParam]
+      TopicName: !FindInMap
+        - EnvironmentMap
+        - MappedParam
+        - !Ref EnvironmentParam
 `
 
 	template, err := parse.String(yaml)
diff --git a/test/templates/fmttestinput.yaml b/test/templates/fmttestinput.yaml
new file mode 100644
index 00000000..581faeb2
--- /dev/null
+++ b/test/templates/fmttestinput.yaml
@@ -0,0 +1,61 @@
+Outputs:
+  Bucket1:
+    Value: !GetAtt Bucket1.Arn # Short GetAtt
+  Bucket2: # Bucket comment
+    Value:
+      Fn::GetAtt: # GetAtt comment
+        - Bucket2
+        - Arn # Arn comment
+
+Description: |
+  An example template for testing rain fmt command.
+
+# Multiline comment
+# starting at indent 0
+Resources:
+  Bucket2:
+    Properties:
+      BucketName: !Ref Name # Ref: comment
+    Type: "AWS::S3::Bucket"
+  Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Sub ${Bucket2}-newer
+  Func1:
+    Type: AWS::Lambda::Function
+    Properties:
+      Role: !Sub "arn:aws:iam::${AWS::AccountID}:role/lambda-basic"
+      Runtime: python3.7
+      Handler: index.handler
+      Code:
+        ZipFile: |
+          import boto3
+
+          def handler: 
+            """Example."""
+
+            print('hello')
+  Instance1:
+    Type: AWS::EC2::Instance
+    Properties:
+      UserData: !Base64
+        Fn::Sub:
+          - |
+            #!/bin/bash -xe
+            apt-get update
+
+            apt-get upgrade -y
+Rules:
+  Rule1:
+    RuleCondition: !Equals
+      - !Ref Environment
+      - test
+    Assertions:
+      - Assert:
+          Fn::Contains:
+            - - a1.medium
+            - !Ref InstanceType
+Parameters:
+  Name:
+    Type: String
+
diff --git a/test/templates/roundtrip.yaml b/test/templates/roundtrip.yaml
new file mode 100644
index 00000000..a8d4d4b7
--- /dev/null
+++ b/test/templates/roundtrip.yaml
@@ -0,0 +1,12 @@
+Parameters:
+  RootId:
+    Type: String
+
+Resources:
+
+  Sandbox:
+    Type: AWS::Organizations::OrganizationalUnit
+    Properties:
+      ParentId: !Ref RootId
+      Name: Sandbox
+