Repository: smithy-lang/smithy
Pull request #2277 (2024-05-07 01:12:43)

Fix formatter to correctly convert invalid doc comments
#### Background
- Fixes an issue where a doc-comment above a trait-statement and member may get converted to a line-comment
- Updates the expected formatting of test case file (`.formatted.smithy`)
- Updates tests to use `assertEquals`, since it provides diff functionality in IDEs
- Caveat:
  - A trailing doc-comment won't get converted to a line-comment
    - TODO was added to handle this case, if it is possible 


#### Testing
- Ran the tests, no failures

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


milesziemer: I pushed a commit that fixes the regression, which is captured by what I added to the test case [here](https://github.com/smithy-lang/smithy/pull/2277/files#diff-5f0819eaa722d6e5033442a41baf6e785999ef798507bc0b2308207c814f970eR53-R57). The TODO [here](https://github.com/smithy-lang/smithy/pull/2277/files#diff-5f0819eaa722d6e5033442a41baf6e785999ef798507bc0b2308207c814f970eR38-R42) is a separate issue that occurs when the comment is after a default value assignment. We will need to follow up to address that.

Patch:
diff --git a/docs/source-2.0/additional-specs/protocols/smithy-rpc-v2.rst b/docs/source-2.0/additional-specs/protocols/smithy-rpc-v2.rst
index 2b2e4664b..40479383a 100644
--- a/docs/source-2.0/additional-specs/protocols/smithy-rpc-v2.rst
+++ b/docs/source-2.0/additional-specs/protocols/smithy-rpc-v2.rst
@@ -298,8 +298,8 @@ headers for responses:
       - Required
       - The value of ``rpc-v2-cbor``.
     * - ``Content-Type``
-      - Required with request bodies
-      - The value of ``application/cbor``. For event streaming requests, this
+      - Required with response bodies
+      - The value of ``application/cbor``. For event streaming responses, this
         is ``application/vnd.amazon.eventstream``.
     * - ``Content-Length``
       - Conditional
diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FixBadDocComments.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FixBadDocComments.java
index 7a62655cf..d4e180f6c 100644
--- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FixBadDocComments.java
+++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/FixBadDocComments.java
@@ -67,6 +67,11 @@ final class FixBadDocComments implements Function<TokenTree, TokenTree> {
             updateNestedChildren(cursor);
         }
 
+        // Doc comments should not appear in TRAIT_STATEMENTS.
+        for (TreeCursor cursor : shapeSection.findChildrenByType(TreeType.TRAIT_STATEMENTS)) {
+            updateNestedChildren(cursor);
+        }
+
         // Fix doc comments that come before apply statements.
         TreeCursor shapeStatements = shapeSection.getFirstChild(TreeType.SHAPE_STATEMENTS);
         if (shapeStatements != null) {
@@ -77,11 +82,15 @@ final class FixBadDocComments implements Function<TokenTree, TokenTree> {
                     updateNestedChildren(br);
                 }
             }
-
-            // Remove trailing doc comments in member bodies.
+            // Fix any trailing doc comments in shape bodies
             for (TreeCursor members : shapeStatements.findChildrenByType(TreeType.SHAPE_MEMBERS)) {
-                TreeCursor ws = members.getLastChild(TreeType.WS);
-                updateDirectChildren(ws);
+                TreeCursor closeBrace = members.getLastChild();
+                if (closeBrace != null) {
+                    TreeCursor possibleTrailingWs = closeBrace.getPreviousSibling();
+                    if (possibleTrailingWs != null && possibleTrailingWs.getTree().getType() == TreeType.WS) {
+                        updateDirectChildren(possibleTrailingWs);
+                    }
+                }
             }
         }
 


Test patch:
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 3eb26da41..e87105486 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
@@ -1,7 +1,6 @@
 package software.amazon.smithy.syntax;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -54,7 +53,7 @@ public class ParseAndFormatTest {
         String formatted = Formatter.format(tree, 120);
         String expected = IoUtils.readUtf8File(formattedFile);
 
-        assertThat(formatted, equalTo(expected));
+        assertEquals(expected, formatted);
     }
 
     public static List<Path> tests() throws Exception {
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.formatted.smithy
index b6984e10c..e7e8d0d29 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.formatted.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.formatted.smithy
@@ -23,19 +23,24 @@ use smithy.api#String
 
 /// 12 (keep)
 @deprecated
-/// 13 (change)
+// 13 (change)
 structure Foo {
     /// 14 (keep)
     @length(
         // 15 (change)
         min: 1
     )
-    /// 16 (change)
+    // 16 (change)
     @since("1.x")
-    /// 17 (TODO: change)
+    // 17 (change)
     bar: String
 
-    // 18 (change)
+    /// 17a (keep)
+    @length(min: 1)
+    baz: String = ""
+
+    /// 18 (TODO: Fix trailing comment after VALUE_ASSIGNMENT)
+
 }
 
 // 19 (change)
@@ -45,4 +50,10 @@ apply Foo @tags(["a"])
 list Baz {
     member: Integer
 }
-// 21 (change)
+
+structure Foo2 {
+    foo2: String
+
+    // 21 (change)
+}
+// 22 (change)
diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.smithy
index e16e23e1c..5aeb29841 100644
--- a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.smithy
+++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/fixes-invalid-doc-comments.smithy
@@ -32,9 +32,14 @@ structure Foo {
     )
     /// 16 (change)
     @since("1.x")
-    /// 17 (TODO: change)
+    /// 17 (change)
     bar: String
-    /// 18 (change)
+
+    /// 17a (keep)
+    @length(min: 1)
+    baz: String = ""
+
+    /// 18 (TODO: Fix trailing comment after VALUE_ASSIGNMENT)
 }
 
 /// 19 (change)
@@ -45,4 +50,10 @@ list Baz {
     member: Integer
 }
 
-/// 21 (change)
+structure Foo2 {
+    foo2: String
+
+    /// 21 (change)
+}
+
+/// 22 (change)