Repository: smithy-lang/smithy
Pull request #1951 (2023-08-25 18:34:16)

Support removing unused mixins
*Description of changes:*

[FlattenAndRemoveMixins](https://github.com/smithy-lang/smithy/blob/main/smithy-model/src/main/java/software/amazon/smithy/model/transform/FlattenAndRemoveMixins.java) only removes mixins if it flattened 1 or more shapes. 

This can cause mixins to leak into downstream code generators (when we really want them inlined).

Specifically, if a Model has mixins which are *all* unused, it will trigger this behavior. But if at least 1 mixin was flattened into a shape, all mixins will be removed (including unused ones). This can happen in shared Smithy model packages that are meant to be used by multiple downstream services.


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/transform/FlattenAndRemoveMixins.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/FlattenAndRemoveMixins.java
index 5aa3cfa5d..ad790d78c 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/transform/FlattenAndRemoveMixins.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/FlattenAndRemoveMixins.java
@@ -38,7 +38,7 @@ final class FlattenAndRemoveMixins {
             }
         }
 
-        if (!updatedShapes.isEmpty()) {
+        if (!updatedShapes.isEmpty() || !toRemove.isEmpty()) {
             Model.Builder builder = model.toBuilder();
             updatedShapes.forEach(builder::addShape);
             // Don't use the removeShapes transform because that would further mutate shapes and remove the things


Test patch:
diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/transform/ModelTransformerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/transform/ModelTransformerTest.java
index 3e54ff288..ee267e7b4 100644
--- a/smithy-model/src/test/java/software/amazon/smithy/model/transform/ModelTransformerTest.java
+++ b/smithy-model/src/test/java/software/amazon/smithy/model/transform/ModelTransformerTest.java
@@ -145,6 +145,36 @@ public class ModelTransformerTest {
                    Matchers.equalTo(concrete.toBuilder().flattenMixins().build()));
     }
 
+    @Test
+    public void canFilterAndRemoveMixinsWhenUnusedMixinsArePresent() {
+        ModelTransformer transformer = ModelTransformer.create();
+        Model.Builder builder = Model.builder();
+        StringShape string = StringShape.builder().id("smithy.example#String").build();
+        StructureShape mixin1 = StructureShape.builder()
+                .id("smithy.example#Mixin1")
+                .addTrait(MixinTrait.builder().build())
+                .addMember("a", string.getId())
+                .build();
+        StructureShape mixin2 = StructureShape.builder()
+                .id("smithy.example#Mixin2")
+                .addMember("b", string.getId())
+                .addTrait(MixinTrait.builder().build())
+                .build();
+        StructureShape mixin3 = StructureShape.builder()
+                .id("smithy.example#Mixin3")
+                .addMember("c", string.getId())
+                .addTrait(MixinTrait.builder().build())
+                .addMixin(mixin2)
+                .build();
+        builder.addShapes(mixin1, mixin2, mixin3);
+        Model model = builder.build();
+        Model result = transformer.flattenAndRemoveMixins(model);
+
+        assertThat(result.toSet(), Matchers.not(Matchers.hasItem(mixin1)));
+        assertThat(result.toSet(), Matchers.not(Matchers.hasItem(mixin2)));
+        assertThat(result.toSet(), Matchers.not(Matchers.hasItem(mixin3)));
+    }
+
     @ParameterizedTest
     @MethodSource("flattenShapesData")
     public void flattenShapes(String name) {