Fix NPE on trait with implicit null (smithy-lang#1942)

alextwoods · Sep 15, 2023 · ec31537 · ec31537
1 parent cfbc4be
commit ec31537
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 18 deletions.
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
@@ -328,24 +328,29 @@ Doc visit(TreeCursor cursor) {
                             .detectHardLines(cursor)
                             .write();
                 } else {
-                    // 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();
+                    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 the trait node is empty, remove the empty parentheses.
+                        return Doc.text("");
+                    }
                 }
             }
 

diff --git a/...c/test/resources/software/amazon/smithy/syntax/formatter/null-trait-body.formatted.smithy b/...c/test/resources/software/amazon/smithy/syntax/formatter/null-trait-body.formatted.smithy
@@ -0,0 +1,17 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure Struct {
+    @default
+    a: String
+
+    @default
+    b: String
+
+    @default
+    c: String
+
+    @default(null)
+    d: String
+}
diff --git a/...-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/null-trait-body.smithy b/...-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/null-trait-body.smithy
@@ -0,0 +1,17 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure Struct {
+    @default
+    a: String
+
+    @default()
+    b: String
+
+    @default( )
+    c: String
+
+    @default(null)
+    d: String
+}