Repository: smithy-lang/smithy (java)

Issue:
Duplicated enum entries in the smithy model
It seems that it is possible to have duplicated enum key-pair entries in the smithy model.

Following snippet passes the validation:
```smithy
intEnum FaceCard {
    JACK = 1
    JACK = 1
}
```

The documentation does not say anything about this, there is only a constraint for values to be unique:
> Enums do not support aliasing; all values MUST be unique. 

I think that is fine, as during the model assembly these two will be flatten into a single entry, but wanted to double check if this is indeed an intended behavior. 

Maybe a `ValidationEvent(severity: NOTE)` should be emitted? 

Pull request description:
Fail when (int)enum conflicts are detected
Closes #2110

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


Patch:
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java
index 7b87c6ec3..562c07e8a 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java
@@ -668,6 +668,7 @@ final class IdlModelLoader {
         tokenizer.expect(IdlToken.LBRACE);
         tokenizer.next();
         tokenizer.skipWs();
+        Set<String> parsedMemberNames = new HashSet<>();
 
         while (tokenizer.getCurrentToken() != IdlToken.EOF && tokenizer.getCurrentToken() != IdlToken.RBRACE) {
             List<IdlTraitParser.Result> memberTraits = IdlTraitParser
@@ -676,6 +677,10 @@ final class IdlModelLoader {
             tokenizer.expect(IdlToken.IDENTIFIER);
             String memberName = internString(tokenizer.getCurrentTokenLexeme());
 
+            if (!parsedMemberNames.add(memberName)) {
+                throw syntax(id, "Found conflicting member name, `" + memberName + "`");
+            }
+
             MemberShape.Builder memberBuilder = MemberShape.builder()
                     .id(id.withMember(memberName))
                     .source(memberLocation)
diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/BracketFormatter.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/BracketFormatter.java
index dfb992fdd..68c494ff8 100644
--- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/BracketFormatter.java
+++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/BracketFormatter.java
@@ -32,6 +32,7 @@ final class BracketFormatter {
     private Doc close = Formatter.RBRACE;
     private Collection<Doc> children;
     private boolean forceLineBreaks;
+    private boolean forceInline = false;
 
     static Function<TreeCursor, Stream<Doc>> extractor(
             Function<TreeCursor, Doc> visitor,
@@ -91,8 +92,8 @@ final class BracketFormatter {
         return false;
     }
 
-    BracketFormatter forceLineBreaks(boolean forceLineBreaks) {
-        this.forceLineBreaks = forceLineBreaks;
+    BracketFormatter forceLineBreaks() {
+        forceLineBreaks = true;
         return this;
     }
 
@@ -103,15 +104,39 @@ final class BracketFormatter {
         return this;
     }
 
+    // Don't force line breaks and don't indent inside brackets
+    BracketFormatter forceInline() {
+        forceInline = true;
+        forceLineBreaks = false;
+        return this;
+    }
+
     Doc write() {
         SmithyBuilder.requiredState("open", open);
         SmithyBuilder.requiredState("close", close);
         SmithyBuilder.requiredState("children", children);
         if (forceLineBreaks) {
             return FormatVisitor.renderBlock(open, close, Doc.intersperse(Doc.line(), children));
+        }
+
+        int indent = forceInline ? 0 : 4;
+        Doc lineBreakDoc = getBracketLineBreakDoc();
+        return Doc.intersperse(Formatter.LINE_OR_COMMA, children).bracket(indent, lineBreakDoc, open, close);
+    }
+
+    private Doc getBracketLineBreakDoc() {
+        Doc lineDoc;
+        if (!children.isEmpty() && open == Formatter.LBRACE) {
+            // Make flattened objects have space padding inside the braces
+            lineDoc = Doc.lineOrSpace();
+        } else if (children.size() > 1) {
+            // Multiple children may be separated on to different lines
+            lineDoc = Doc.lineOrEmpty();
         } else {
-            return Doc.intersperse(Formatter.LINE_OR_COMMA, children).bracket(4, Doc.lineOrEmpty(), open, close);
+            // Only one child or less doesn't need line separation
+            lineDoc = Doc.empty();
         }
+        return lineDoc;
     }
 
     private static final class Extractor implements Function<TreeCursor, Stream<Doc>> {
diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FormatVisitor.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FormatVisitor.java
index 59d2995ff..2df51058e 100644
--- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FormatVisitor.java
+++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FormatVisitor.java
@@ -271,8 +271,7 @@ final class FormatVisitor {
                                         .close(Formatter.RBRACKET)
                                         .extractChildren(cursor, BracketFormatter.extractByType(TreeType.SHAPE_ID,
                                                                                                 this::visit))
-                                        .detectHardLines(cursor)
-                                        .forceLineBreaks(true) // always put each error on separate lines.
+                                        .forceLineBreaks() // always put each error on separate lines.
                                         .write());
             }
 
@@ -327,30 +326,46 @@ final class FormatVisitor {
                             }))
                             .detectHardLines(cursor)
                             .write();
-                } else {
-                    if (cursor.getFirstChild(TreeType.TRAIT_NODE) != null) {
-                        // Check the inner trait node for hard line breaks rather than the wrapper.
-                        TreeCursor traitNode = cursor
-                                .getFirstChild(TreeType.TRAIT_NODE)
-                                .getFirstChild(TreeType.NODE_VALUE)
-                                .getFirstChild(); // The actual node value.
-                        return new BracketFormatter()
-                                .open(Formatter.LPAREN)
-                                .close(Formatter.RPAREN)
-                                .extractChildren(cursor, BracketFormatter.extractor(this::visit, child -> {
-                                    if (child.getTree().getType() == TreeType.TRAIT_NODE) {
-                                        // Split WS and NODE_VALUE so that they appear on different lines.
-                                        return child.getChildrenByType(TreeType.NODE_VALUE, TreeType.WS).stream();
-                                    } else {
-                                        return Stream.empty();
-                                    }
-                                }))
-                                .detectHardLines(traitNode)
-                                .write();
+                } else if (cursor.getFirstChild(TreeType.TRAIT_NODE) != null) {
+                    TreeCursor traitNode = cursor.getFirstChild(TreeType.TRAIT_NODE);
+                    // Check the inner trait node for hard line breaks rather than the wrapper.
+                    TreeCursor actualTraitNodeValue = traitNode
+                            .getFirstChild(TreeType.NODE_VALUE)
+                            .getFirstChild(); // The actual node value.
+
+                    BracketFormatter formatter = new BracketFormatter()
+                            .open(Formatter.LPAREN)
+                            .close(Formatter.RPAREN)
+                            .extractChildren(cursor, BracketFormatter.extractor(this::visit, child -> {
+                                if (child.getTree().getType() == TreeType.TRAIT_NODE) {
+                                    // Split WS and NODE_VALUE so that they appear on different lines.
+                                    return child.getChildrenByType(TreeType.NODE_VALUE, TreeType.WS).stream();
+                                } else {
+                                    return Stream.empty();
+                                }
+                            }));
+
+                    // TraitBody may have leading comments and TraitNode may have trailing comments
+                    // which both require line break
+                    TreeCursor bodyWs = cursor.getFirstChild(TreeType.WS);
+                    TreeCursor traitNodeWs = traitNode.getFirstChild(TreeType.WS);
+                    if ((bodyWs != null && !bodyWs.findChildrenByType(TreeType.COMMENT).isEmpty())
+                            || (traitNodeWs != null && !traitNodeWs.findChildrenByType(TreeType.COMMENT).isEmpty())) {
+                        // Need to line break if there's a leading comment in the trait body no matter what
+                        formatter.forceLineBreaks();
+                    } else if (actualTraitNodeValue.getTree().getType() == TreeType.NODE_ARRAY
+                            || actualTraitNodeValue.getTree().getType() == TreeType.NODE_OBJECT) {
+                        // Just inline arrays and objects in trait body
+                        formatter.forceInline();
                     } else {
-                        // If the trait node is empty, remove the empty parentheses.
-                        return Doc.text("");
+                        // Any other values can use normal detection
+                        formatter.detectHardLines(cursor);
                     }
+
+                    return formatter.write();
+                } else {
+                    // If the trait node is empty, remove the empty parentheses.
+                    return Doc.text("");
                 }
             }
 
@@ -400,11 +415,17 @@ final class FormatVisitor {
             }
 
             case NODE_OBJECT: {
-                return new BracketFormatter()
+                BracketFormatter formatter = new BracketFormatter()
                         .extractChildren(cursor, BracketFormatter.extractByType(TreeType.NODE_OBJECT_KVP,
-                                                                                this::visit))
-                        .detectHardLines(cursor)
-                        .write();
+                                                                                this::visit));
+                if (cursor.getParent().getParent().getTree().getType() == TreeType.NODE_ARRAY)  {
+                    // Always break objects inside arrays if not empty
+                    formatter.forceLineBreaksIfNotEmpty();
+                } else {
+                    formatter.detectHardLines(cursor);
+                }
+                return formatter.write();
+
             }
 
             case NODE_OBJECT_KVP: {


Test patch:
diff --git a/smithy-aws-protocol-tests/model/awsJson1_0/defaults.smithy b/smithy-aws-protocol-tests/model/awsJson1_0/defaults.smithy
index fbac5fe08..182c36d65 100644
--- a/smithy-aws-protocol-tests/model/awsJson1_0/defaults.smithy
+++ b/smithy-aws-protocol-tests/model/awsJson1_0/defaults.smithy
@@ -37,7 +37,16 @@ apply OperationWithDefaults @httpRequestTests([
                     "defaultDouble": 1.0,
                     "defaultMap": {},
                     "defaultEnum": "FOO",
-                    "defaultIntEnum": 1
+                    "defaultIntEnum": 1,
+                    "emptyString": "",
+                    "falseBoolean": false,
+                    "emptyBlob": "",
+                    "zeroByte": 0,
+                    "zeroShort": 0,
+                    "zeroInteger": 0,
+                    "zeroLong": 0,
+                    "zeroFloat": 0.0,
+                    "zeroDouble": 0.0
                 }
             }"""
         params: {
@@ -91,6 +100,15 @@ apply OperationWithDefaults @httpRequestTests([
                 defaultMap: {name: "Jack"}
                 defaultEnum: "BAR"
                 defaultIntEnum: 2
+                emptyString: "foo"
+                falseBoolean: true
+                emptyBlob: "hi"
+                zeroByte: 1
+                zeroShort: 1
+                zeroInteger: 1
+                zeroLong: 1
+                zeroFloat: 1.0
+                zeroDouble: 1.0
             }
         }
         body: """
@@ -114,7 +132,16 @@ apply OperationWithDefaults @httpRequestTests([
                     "defaultDouble": 2.0,
                     "defaultMap": {"name": "Jack"},
                     "defaultEnum": "BAR",
-                    "defaultIntEnum": 2
+                    "defaultIntEnum": 2,
+                    "emptyString": "foo",
+                    "falseBoolean": true,
+                    "emptyBlob": "aGk=",
+                    "zeroByte": 1,
+                    "zeroShort": 1,
+                    "zeroInteger": 1,
+                    "zeroLong": 1,
+                    "zeroFloat": 1.0,
+                    "zeroDouble": 1.0
                 }
             }"""
     }
@@ -152,8 +179,56 @@ apply OperationWithDefaults @httpRequestTests([
                 defaultMap: {}
                 defaultEnum: "FOO"
                 defaultIntEnum: 1
+                emptyString: ""
+                falseBoolean: false
+                emptyBlob: ""
+                zeroByte: 0
+                zeroShort: 0
+                zeroInteger: 0
+                zeroLong: 0
+                zeroFloat: 0.0
+                zeroDouble: 0.0
             },
             topLevelDefault: "hi"
+            otherTopLevelDefault: 0
+        }
+    }
+    {
+        id: "AwsJson10ClientUsesExplicitlyProvidedValuesInTopLevel"
+        documentation: "Any time a value is provided for a member in the top level of input, it is used, regardless of if its the default."
+        appliesTo: "client"
+        tags: ["defaults"]
+        protocol: awsJson1_0
+        method: "POST"
+        bodyMediaType: "application/json"
+        uri: "/"
+        headers: {"Content-Type": "application/x-amz-json-1.0"}
+        body: """
+            {
+                "topLevelDefault": "hi",
+                "otherTopLevelDefault": 0
+            }"""
+        params: {
+            topLevelDefault: "hi"
+            otherTopLevelDefault: 0
+        }
+    }
+    {
+        id: "AwsJson10ClientIgnoresNonTopLevelDefaultsOnMembersWithClientOptional"
+        documentation: "Typically, non top-level members would have defaults filled in, but if they have the clientOptional trait, the defaults should be ignored."
+        appliesTo: "client"
+        tags: ["defaults"]
+        protocol: awsJson1_0
+        method: "POST"
+        bodyMediaType: "application/json"
+        uri: "/"
+        headers: {"Content-Type": "application/x-amz-json-1.0"}
+        body: """
+            {
+                "clientOptionalDefaults": {}
+            }"""
+        params: {
+            clientOptionalDefaults: {}
         }
     }
 ])
@@ -188,6 +263,15 @@ apply OperationWithDefaults @httpResponseTests([
             defaultMap: {}
             defaultEnum: "FOO"
             defaultIntEnum: 1
+            emptyString: ""
+            falseBoolean: false
+            emptyBlob: ""
+            zeroByte: 0
+            zeroShort: 0
+            zeroInteger: 0
+            zeroLong: 0
+            zeroFloat: 0.0
+            zeroDouble: 0.0
         }
     }
     {
@@ -219,7 +303,16 @@ apply OperationWithDefaults @httpResponseTests([
                 "defaultDouble": 2.0,
                 "defaultMap": {"name": "Jack"},
                 "defaultEnum": "BAR",
-                "defaultIntEnum": 2
+                "defaultIntEnum": 2,
+                "emptyString": "foo",
+                "falseBoolean": true,
+                "emptyBlob": "aGk=",
+                "zeroByte": 1,
+                "zeroShort": 1,
+                "zeroInteger": 1,
+                "zeroLong": 1,
+                "zeroFloat": 1.0,
+                "zeroDouble": 1.0
             }"""
         params: {
             defaultString: "bye"
@@ -241,6 +334,15 @@ apply OperationWithDefaults @httpResponseTests([
             defaultMap: {name: "Jack"}
             defaultEnum: "BAR"
             defaultIntEnum: 2
+            emptyString: "foo"
+            falseBoolean: true
+            emptyBlob: "hi"
+            zeroByte: 1
+            zeroShort: 1
+            zeroInteger: 1
+            zeroLong: 1
+            zeroFloat: 1.0
+            zeroDouble: 1.0
         }
     }
     {
@@ -271,7 +373,16 @@ apply OperationWithDefaults @httpResponseTests([
                 "defaultDouble": 1.0,
                 "defaultMap": {},
                 "defaultEnum": "FOO",
-                "defaultIntEnum": 1
+                "defaultIntEnum": 1,
+                "emptyString": "",
+                "falseBoolean": false,
+                "emptyBlob": "",
+                "zeroByte": 0,
+                "zeroShort": 0,
+                "zeroInteger": 0,
+                "zeroLong": 0,
+                "zeroFloat": 0.0,
+                "zeroDouble": 0.0
             }"""
         params: {}
     }
@@ -280,8 +391,9 @@ apply OperationWithDefaults @httpResponseTests([
 operation OperationWithDefaults {
     input := {
         defaults: Defaults
-
+        clientOptionalDefaults: ClientOptionalDefaults
         topLevelDefault: String = "hi" // Client should ignore default values in input shape
+        otherTopLevelDefault: Integer = 0
     }
 
     output := with [DefaultsMixin] {}
@@ -289,6 +401,11 @@ operation OperationWithDefaults {
 
 structure Defaults with [DefaultsMixin] {}
 
+structure ClientOptionalDefaults {
+    @clientOptional
+    member: Integer = 0
+}
+
 @mixin
 structure DefaultsMixin {
     defaultString: String = "hi"
@@ -310,6 +427,15 @@ structure DefaultsMixin {
     defaultMap: TestStringMap = {}
     defaultEnum: TestEnum = "FOO"
     defaultIntEnum: TestIntEnum = 1
+    emptyString: String = ""
+    falseBoolean: Boolean = false
+    emptyBlob: Blob = ""
+    zeroByte: Byte = 0
+    zeroShort: Short = 0
+    zeroInteger: Integer = 0
+    zeroLong: Long = 0
+    zeroFloat: Float = 0.0
+    zeroDouble: Double = 0.0
 }
 
 list TestStringList {
diff --git a/smithy-aws-protocol-tests/model/restJson1/http-query.smithy b/smithy-aws-protocol-tests/model/restJson1/http-query.smithy
index dea2c2a7f..995b18904 100644
--- a/smithy-aws-protocol-tests/model/restJson1/http-query.smithy
+++ b/smithy-aws-protocol-tests/model/restJson1/http-query.smithy
@@ -218,6 +218,22 @@ apply AllQueryStringTypes @httpRequestTests([
             }
         }
     },
+    {
+        id: "RestJsonZeroAndFalseQueryValues"
+        documentation: "Query values of 0 and false are serialized"
+        protocol: restJson1
+        method: "GET"
+        uri: "/AllQueryStringTypesInput"
+        body: ""
+        queryParams: [
+            "Integer=0"
+            "Boolean=false"
+        ]
+        params: {
+            queryInteger: 0
+            queryBoolean: false
+        }
+    }
 ])
 
 @suppress(["HttpQueryParamsTrait"])
diff --git a/smithy-aws-protocol-tests/model/restXml/http-query.smithy b/smithy-aws-protocol-tests/model/restXml/http-query.smithy
index 98f57de8e..017003b41 100644
--- a/smithy-aws-protocol-tests/model/restXml/http-query.smithy
+++ b/smithy-aws-protocol-tests/model/restXml/http-query.smithy
@@ -180,6 +180,22 @@ apply AllQueryStringTypes @httpRequestTests([
             queryDouble: "-Infinity",
         }
     },
+    {
+        id: "RestXmlZeroAndFalseQueryValues"
+        documentation: "Query values of 0 and false are serialized"
+        protocol: restXml
+        method: "GET"
+        uri: "/AllQueryStringTypesInput"
+        body: ""
+        queryParams: [
+            "Integer=0"
+            "Boolean=false"
+        ]
+        params: {
+            queryInteger: 0
+            queryBoolean: false
+        }
+    }
 ])
 
 @suppress(["HttpQueryParamsTrait"])
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/enum-with-duplicates.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/enum-with-duplicates.smithy
new file mode 100644
index 000000000..950969b08
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/enum-with-duplicates.smithy
@@ -0,0 +1,9 @@
+// smithy.example#Foo: Syntax error at line 8, column 5: Found conflicting member name, `BAR`
+$version: "2.0"
+
+namespace smithy.example
+
+enum Foo {
+    BAR = "bar"
+    BAR = "bar"
+}
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/intEnum-with-duplicates.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/intEnum-with-duplicates.smithy
new file mode 100644
index 000000000..178d28e94
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/enums/intEnum-with-duplicates.smithy
@@ -0,0 +1,9 @@
+// smithy.example#Foo: Syntax error at line 8, column 5: Found conflicting member name, `BAR`
+$version: "2.0"
+
+namespace smithy.example
+
+intEnum Foo {
+    BAR = 10
+    BAR = 10
+}
diff --git a/smithy-syntax/src/test/java/software/amazon/smithy/syntax/ParseAndFormatTest.java b/smithy-syntax/src/test/java/software/amazon/smithy/syntax/ParseAndFormatTest.java
index e97c6dc60..3eb26da41 100644
--- a/smithy-syntax/src/test/java/software/amazon/smithy/syntax/ParseAndFormatTest.java
+++ b/smithy-syntax/src/test/java/software/amazon/smithy/syntax/ParseAndFormatTest.java
@@ -14,6 +14,7 @@ import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.IdlTokenizer;
+import software.amazon.smithy.model.loader.ModelAssembler;
 import software.amazon.smithy.utils.IoUtils;
 
 // A parameterized test that finds models in corpus, parses them, skipping files that end with ".formatted.smithy".
@@ -32,9 +33,19 @@ public class ParseAndFormatTest {
         }
 
         // Ensure that the tests can be parsed by smithy-model too.
-        Model.assembler().addImport(filename).disableValidation().assemble().unwrap();
+        Model.assembler()
+                .addImport(filename)
+                .putProperty(ModelAssembler.ALLOW_UNKNOWN_TRAITS, true)
+                .disableValidation()
+                .assemble()
+                .unwrap();
         if (!formattedFile.equals(filename)) {
-            Model.assembler().addImport(formattedFile).disableValidation().assemble().unwrap();
+            Model.assembler()
+                    .addImport(formattedFile)
+                    .putProperty(ModelAssembler.ALLOW_UNKNOWN_TRAITS, true)
+                    .disableValidation()
+                    .assemble()
+                    .unwrap();
         }
 
         String model = IoUtils.readUtf8File(filename);
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.formatted.smithy
new file mode 100644
index 000000000..2a32fbdb5
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.formatted.smithy
@@ -0,0 +1,15 @@
+$version: "2.0"
+
+namespace smithy.example
+
+@examples([
+    {
+        title: "Foo"
+        documentation: "Foo"
+    }
+    {
+        title: "Bar"
+        documentation: "Bar"
+    }
+])
+operation Foo {}
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.smithy
new file mode 100644
index 000000000..768b31d0b
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/array-of-objects.smithy
@@ -0,0 +1,17 @@
+$version: "2.0"
+
+namespace smithy.example
+
+@examples(
+    [
+        {
+            title: "Foo"
+            documentation: "Foo"
+        }
+        {
+            title: "Bar"
+            documentation: "Bar"
+        }
+    ]
+)
+operation Foo {}
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.formatted.smithy
new file mode 100644
index 000000000..4f79946e1
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.formatted.smithy
@@ -0,0 +1,192 @@
+$version: "2.0"
+
+// Single line
+metadata a = { a: "b" }
+
+// Multiline
+metadata b = {
+    a: "foobarbazbux"
+    b: "foobarbazbux"
+    c: "foobarbazbux"
+    d: "foobarbazbux"
+    e: "foobarbazbux"
+    f: "foobarbazbux"
+}
+
+// Leading and trailing WS
+metadata c = {
+    // Foo
+    a: "b"
+}
+
+metadata d = {
+    a: "b"
+    // Foo
+}
+
+// Parent is array
+metadata e = [
+    {
+        foo: "bar"
+    }
+]
+
+// Line breaking values
+metadata f = [
+    """
+    foo
+    """
+]
+
+metadata g = [
+    "abc
+def"
+]
+
+metadata h = {
+    a: [
+        {
+            foo: "bar"
+        }
+    ]
+}
+
+namespace smithy.example
+
+// TraitStructure
+// Single line
+@externalDocumentation(a: "http://foo.com", c: "http://foo.com")
+structure A {}
+
+// Multi-line
+@externalDocumentation(
+    a: "http://foo.com"
+    b: "http://foo.com"
+    c: "http://foo.com"
+    d: "http://foo.com"
+    e: "http://foo.com"
+    f: "http://foo.com"
+    g: "http://foo.com"
+    h: "http://foo.com"
+    i: "http://foo.com"
+    j: "http://foo.com"
+)
+structure B {}
+
+// Leading WS
+@externalDocumentation(
+    // Foo
+    a: "http://foo.com"
+)
+structure C {}
+
+// Trailing WS
+@externalDocumentation(
+    a: "http://foo.com"
+    // Foo
+)
+structure D {}
+
+// Object values
+@foo(
+    a: { bar: "baz" }
+)
+structure E {}
+
+// Array values
+@foo(
+    a: ["bar", "baz"]
+)
+structure F {}
+
+// TraitNode
+// Single line
+@foo(["foo", "bar"])
+structure G {}
+
+@foo({ foo: "bar", baz: "foo" })
+structure H {}
+
+// Multiline
+@foo([
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+])
+structure I {}
+
+@foo({
+    a: "foobarbaz"
+    b: "foobarbaz"
+    c: "foobarbaz"
+    d: "foobarbaz"
+    e: "foobarbaz"
+    f: "foobarbaz"
+    g: "foobarbaz"
+    h: "foobarbaz"
+    i: "foobarbaz"
+})
+structure J {}
+
+// Leading WS
+@foo(
+    // Foo
+    ["a"]
+)
+structure K {}
+
+// Trailing WS
+@foo(
+    ["a"]
+    // Foo
+)
+structure L {}
+
+// Line breaking NodeValues
+@foo(
+    """
+    foo
+    """
+)
+structure M {}
+
+@foo(
+    "a
+bc"
+)
+structure N {}
+
+// Nested comments
+@foo([
+    "abc"
+    // Foo
+    "def"
+    // Bar
+])
+structure O {}
+
+// Nested objects
+@foo([
+    {}
+])
+structure P {}
+
+@foo([
+    {
+        foo: "bar"
+    }
+])
+structure Q {}
+
+@foo([
+    [
+        []
+    ]
+])
+structure R {}
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.smithy
new file mode 100644
index 000000000..ec25f1878
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/brackets.smithy
@@ -0,0 +1,184 @@
+$version: "2.0"
+
+// Single line
+metadata a = { a: "b" }
+
+// Multiline
+metadata b = {
+    a: "foobarbazbux"
+    b: "foobarbazbux"
+    c: "foobarbazbux"
+    d: "foobarbazbux"
+    e: "foobarbazbux"
+    f: "foobarbazbux"
+}
+
+// Leading and trailing WS
+metadata c = {
+    // Foo
+    a: "b"
+}
+
+metadata d = {
+    a: "b" // Foo
+}
+
+// Parent is array
+metadata e = [
+    {
+        foo: "bar"
+    }
+]
+
+// Line breaking values
+metadata f = [
+    """
+    foo
+    """
+]
+
+metadata g = [
+    "abc
+def"
+]
+
+metadata h = {
+    a: [
+        {
+            foo: "bar"
+        }
+    ]
+
+}
+
+namespace smithy.example
+
+// TraitStructure
+// Single line
+@externalDocumentation(a: "http://foo.com", c: "http://foo.com")
+structure A {}
+
+// Multi-line
+@externalDocumentation(
+    a: "http://foo.com"
+    b: "http://foo.com"
+    c: "http://foo.com"
+    d: "http://foo.com"
+    e: "http://foo.com"
+    f: "http://foo.com"
+    g: "http://foo.com"
+    h: "http://foo.com"
+    i: "http://foo.com"
+    j: "http://foo.com"
+)
+structure B {}
+
+// Leading WS
+@externalDocumentation(
+    // Foo
+    a: "http://foo.com"
+)
+structure C {}
+
+// Trailing WS
+@externalDocumentation(
+    a: "http://foo.com" // Foo
+)
+structure D {}
+
+// Object values
+@foo(
+    a: { bar: "baz" }
+)
+structure E {}
+
+// Array values
+@foo(
+    a: ["bar", "baz"]
+)
+structure F {}
+
+// TraitNode
+// Single line
+@foo(["foo", "bar"])
+structure G {}
+
+@foo({ foo: "bar", baz: "foo" })
+structure H {}
+
+// Multiline
+@foo([
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+    "foobarbaz"
+])
+structure I {}
+
+@foo({
+    a: "foobarbaz"
+    b: "foobarbaz"
+    c: "foobarbaz"
+    d: "foobarbaz"
+    e: "foobarbaz"
+    f: "foobarbaz"
+    g: "foobarbaz"
+    h: "foobarbaz"
+    i: "foobarbaz"
+})
+structure J {}
+
+// Leading WS
+@foo(
+    // Foo
+    [
+        "a"
+    ]
+)
+structure K {}
+
+// Trailing WS
+@foo(
+    [
+        "a"
+    ] // Foo
+)
+structure L {}
+
+// Line breaking NodeValues
+@foo(
+    """
+    foo
+    """
+)
+structure M {}
+
+@foo(
+    "a
+bc"
+)
+structure N {}
+
+// Nested comments
+@foo([
+    "abc" // Foo
+    "def" // Bar
+])
+structure O {}
+
+// Nested objects
+@foo([{}])
+structure P {}
+
+@foo([{
+    foo: "bar"
+      }])
+structure Q {}
+
+@foo([[[]]])
+structure R {}
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes-line-break.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes-line-break.formatted.smithy
index f4aac3766..76a25c8f7 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes-line-break.formatted.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes-line-break.formatted.smithy
@@ -30,7 +30,7 @@ operation GetTime2 {
 
 @http(method: "X", uri: "/foo", code: 200)
 resource Sprocket2 {
-    identifiers: {username: String, id: String, otherId: String}
+    identifiers: { username: String, id: String, otherId: String }
 }
 
 @error("client")
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes.smithy
index 585e574ef..ce49d2b3a 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/entity-shapes.smithy
@@ -33,12 +33,12 @@ operation GetTime2 {
 }
 
 resource Sprocket1 {
-    identifiers: {username: String}
+    identifiers: { username: String }
 }
 
 @http(method: "X", uri: "/foo", code: 200)
 resource Sprocket2 {
-    identifiers: {username: String, id: String, otherId: String}
+    identifiers: { username: String, id: String, otherId: String }
     collectionOperations: [
         SomeOperation
     ]
@@ -54,7 +54,7 @@ structure SomeOperationFoo {}
 
 @http(method: "X", uri: "/foo3", code: 200)
 resource Sprocket3 {
-    identifiers: {username: String, id: String, otherId: String}
+    identifiers: { username: String, id: String, otherId: String }
     // It's empty, so on a single line.
     collectionOperations: []
 }
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.formatted.smithy
new file mode 100644
index 000000000..5c55a0567
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.formatted.smithy
@@ -0,0 +1,24 @@
+$version: "2.0"
+
+metadata foo = {
+    bar: ["baz", "foo", "bar", "baz"]
+    baz: {
+        foo: ["bar"]
+    }
+}
+
+metadata bar = [
+    {
+        foo: [
+            {
+                bar: ["baz", "foo"]
+                baz: ["foo", "bar"]
+            }
+            {
+                foo: "bar"
+            }
+        ]
+    }
+]
+
+namespace smithy.example
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.smithy
new file mode 100644
index 000000000..405c2933f
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/nested-arrays-and-objects.smithy
@@ -0,0 +1,7 @@
+$version: "2.0"
+
+metadata foo = {bar: ["baz", "foo", "bar", "baz"], baz: {foo: ["bar"]}}
+
+metadata bar = [{foo: [{bar: ["baz", "foo"], baz: ["foo", "bar"]}, {foo: "bar"}]}]
+
+namespace smithy.example
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/section-statement-blank-lines.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/section-statement-blank-lines.formatted.smithy
index ff3b73ab8..a5bf0f95a 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/section-statement-blank-lines.formatted.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/section-statement-blank-lines.formatted.smithy
@@ -2,7 +2,7 @@
 // newline.
 $version: "2.0"
 $foo: 100
-$baz: {abc: 123}
+$baz: { abc: 123 }
 
 $bar: {
     aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1"
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.formatted.smithy
new file mode 100644
index 000000000..01e6f7860
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.formatted.smithy
@@ -0,0 +1,9 @@
+$version: "2.0"
+
+metadata foo = [
+    {
+        bar: { baz: "2" }
+    }
+]
+
+namespace smithy.example
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.smithy
new file mode 100644
index 000000000..7bd08106e
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/single-line-object.smithy
@@ -0,0 +1,9 @@
+$version: "2.0"
+
+metadata foo = [
+    {
+        bar: {baz: "2"}
+    }
+]
+
+namespace smithy.example
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.formatted.smithy
index 8fa2be8e6..899acc3ee 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.formatted.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.formatted.smithy
@@ -24,3 +24,11 @@ string Bar
     """
 )
 string Baz
+
+@documentation(
+    """
+    This is the documentation for Bux.
+    Lorem ipsum dolor.
+    """
+)
+string Bux
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.smithy
index 99efba0d8..834b3579b 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/trait-textblock.smithy
@@ -24,3 +24,9 @@ Lorem ipsum dolor.
 """
 )
 string Baz
+
+@documentation("""
+    This is the documentation for Bux.
+    Lorem ipsum dolor.
+""")
+string Bux
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/traits-lots-of-tags.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/traits-lots-of-tags.formatted.smithy
new file mode 100644
index 000000000..29b9d4ded
--- /dev/null
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/traits-lots-of-tags.formatted.smithy
@@ -0,0 +1,20 @@
+$version: "2.0"
+
+namespace smithy.example
+
+@tags([
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+    "abcdefghijklmnopqrstuvwxyz"
+])
+string Foo