Repository: smithy-lang/smithy (java)

Issue:
IDL Model serializer writes documentation twice
Hi! My team noticed that when you construct a `documentation` trait generically, using `DynamicTrait`, it'll get serialized to the model twice: as `///`, and as a normal `@documentation` trait. There's probably some special-case in the serializer which renders `///` but forgets to remove `@documentation`.

A reproduction in scala-cli, with outputs included:

```scala

//> using dep software.amazon.smithy:smithy-model:1.54.0
//> using option -Wunused:all
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.shapes.SmithyIdlModelSerializer
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.traits.DynamicTrait

import scala.jdk.CollectionConverters.*

import util.chaining.*

val m = Model
  .builder()
  .addShape(
    StructureShape
      .builder()
      .tap {
        _.id(ShapeId.from("demo#Unit"))
      }
      .tap(
        _.addTrait(new DynamicTrait(ShapeId.from("smithy.api#documentation"), Node.from("hello")))
      )
      .build()
  )
  .build()
// m: Model = software.amazon.smithy.model.Model@b3143cd9

val ss =
  SmithyIdlModelSerializer
    .builder
    .build
    .serialize(m)
    .asScala
    .head
    ._2
// ss: String = $version: "2.0"
// 
// namespace demo
// 
/// hello
// @documentation("hello")
// structure Unit {}
```



Pull request:
Fix documentation being duplicated with dynamic doc trait
#### Background
* resolve #2529 

#### Links
* Links to additional context, if necessary
* Issue #, if applicable (see [here](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) for a list of keywords to use for linking issues)

---
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/shapes/SmithyIdlModelSerializer.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java
index 6f1db2603..d84d6a73e 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java
@@ -707,7 +707,7 @@ public final class SmithyIdlModelSerializer {
 
             traits.values()
                     .stream()
-                    .filter(trait -> noSpecialDocsSyntax || !(trait instanceof DocumentationTrait))
+                    .filter(trait -> noSpecialDocsSyntax || !trait.toShapeId().equals(DocumentationTrait.ID))
                     // The default and enumValue traits are serialized using the assignment syntactic sugar.
                     .filter(trait -> {
                         if (trait instanceof EnumValueTrait) {


Test patch:
diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
index 6086104b1..70ebb2848 100644
--- a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
+++ b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
@@ -13,6 +13,8 @@ import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
@@ -29,9 +31,11 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
+import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.StringNode;
 import software.amazon.smithy.model.traits.DefaultTrait;
 import software.amazon.smithy.model.traits.DocumentationTrait;
+import software.amazon.smithy.model.traits.DynamicTrait;
 import software.amazon.smithy.model.traits.RequiredTrait;
 import software.amazon.smithy.model.traits.synthetic.OriginalShapeIdTrait;
 import software.amazon.smithy.utils.IoUtils;
@@ -134,7 +138,7 @@ public class SmithyIdlModelSerializerTest {
                 .assemble()
                 .unwrap();
         SmithyIdlModelSerializer serializer = SmithyIdlModelSerializer.builder()
-                .traitFilter(trait -> !(trait instanceof DocumentationTrait))
+                .traitFilter(trait -> !trait.toShapeId().equals(DocumentationTrait.ID))
                 .build();
         Map<Path, String> serialized = serializer.serialize(model);
         for (String output : serialized.values()) {
@@ -345,4 +349,24 @@ public class SmithyIdlModelSerializerTest {
                 .replace("\r\n", "\n");
         assertEquals(expected, modelResult);
     }
+
+    @Test
+    public void docsNotDuplicateDocsForDynamicDocTrait() {
+        Model model = Model.builder()
+                .addShape(StructureShape.builder()
+                        .id("test#test")
+                        .addTrait(
+                                new DynamicTrait(
+                                        ShapeId.from("smithy.api#documentation"),
+                                        Node.from("hello")))
+                        .build())
+                .build();
+        Map<Path, String> serialized = SmithyIdlModelSerializer.builder().build().serialize(model);
+        String modelResult = serialized.values().iterator().next().replace("\r\n", "\n");
+
+        // IDL serializer will serialize docs with the special syntax, so we can check that it does not also add
+        // the doc trait itself
+        assertFalse(modelResult.contains("@documentation"));
+        assertTrue(modelResult.contains("/// hello"));
+    }
 }