Fix filterSuppressions handling of members

`builder.addShape` does not allow adding member shapes
directly. Instead use ReplaceShapes which handles replacing member
shapes too.

Author: gosar

smithy-build/src/main/java/software/amazon/smithy/build/transforms/FilterSuppressions.java

@@ -32,6 +32,7 @@
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.traits.SuppressTrait;
+import software.amazon.smithy.model.transform.ModelTransformer;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.suppressions.Suppression;
@@ -217,15 +218,14 @@ protected Model transformWithConfig(TransformContext context, Config config) {
         }
 
         Model model = context.getModel();
-        Model.Builder builder = model.toBuilder();
         Set<String> removedValidators = getRemovedValidators(context, config);
         List<ValidationEvent> suppressedEvents = context.getOriginalModelValidationEvents().stream()
                 .filter(event -> event.getSeverity() == Severity.SUPPRESSED)
                 .filter(event -> !removedValidators.contains(event.getId()))
                 .collect(Collectors.toList());
-        filterSuppressionTraits(model, builder, config, suppressedEvents);
-        filterMetadata(model, builder, config, suppressedEvents, removedValidators);
-        return builder.build();
+        model = filterSuppressionTraits(model, config, suppressedEvents, context.getTransformer());
+        model = filterMetadata(model, config, suppressedEvents, removedValidators);
+        return model;
     }
 
     private Set<String> getRemovedValidators(TransformContext context, Config config) {
@@ -271,12 +271,14 @@ private Set<String> getValidatorNames(Model model) {
         return metadataSuppressions;
     }
 
-    private void filterSuppressionTraits(
+    private Model filterSuppressionTraits(
             Model model,
-            Model.Builder builder,
             Config config,
-            List<ValidationEvent> suppressedEvents
+            List<ValidationEvent> suppressedEvents,
+            ModelTransformer transformer
     ) {
+
+        List<Shape> replacementShapes = new ArrayList<>();
         // First filter and '@suppress' traits that didn't suppress anything.
         for (Shape shape : model.getShapesWithTrait(SuppressTrait.class)) {
             SuppressTrait trait = shape.expectTrait(SuppressTrait.class);
@@ -293,25 +295,24 @@ private void filterSuppressionTraits(
             }
 
             if (allowed.isEmpty()) {
-                builder.addShape(Shape.shapeToBuilder(shape).removeTrait(SuppressTrait.ID).build());
+                replacementShapes.add(Shape.shapeToBuilder(shape).removeTrait(SuppressTrait.ID).build());
             } else if (!allowed.equals(trait.getValues())) {
                 trait = trait.toBuilder().values(allowed).build();
-                builder.addShape(Shape.shapeToBuilder(shape).addTrait(trait).build());
+                replacementShapes.add(Shape.shapeToBuilder(shape).addTrait(trait).build());
             }
         }
+        return transformer.replaceShapes(model, replacementShapes);
     }
 
-    private void filterMetadata(
+    private Model filterMetadata(
             Model model,
-            Model.Builder builder,
             Config config,
             List<ValidationEvent> suppressedEvents,
             Set<String> removedValidators
     ) {
         // Next remove metadata suppressions that didn't suppress anything.
         ArrayNode suppressionsNode = model.getMetadata()
                 .getOrDefault("suppressions", Node.arrayNode()).expectArrayNode();
-        builder.removeMetadataProperty("suppressions");
         List<ObjectNode> updatedMetadataSuppressions = new ArrayList<>();
 
         for (Node suppressionNode : suppressionsNode) {
@@ -339,9 +340,16 @@ && isAllowed(namespace, config.getNamespaceAllowList(), config.getNamespaceDenyL
             }
         }
 
+        ArrayNode updatedMetadataSuppressionsNode = Node.fromNodes(updatedMetadataSuppressions);
+        if (suppressionsNode.equals(updatedMetadataSuppressionsNode)) {
+            return model;
+        }
+        Model.Builder builder = model.toBuilder();
+        builder.removeMetadataProperty("suppressions");
         if (!updatedMetadataSuppressions.isEmpty()) {
-            builder.putMetadataProperty("suppressions", Node.fromNodes(updatedMetadataSuppressions));
+            builder.putMetadataProperty("suppressions", updatedMetadataSuppressionsNode);
         }
+        return builder.build();
     }
 
     private boolean isAllowed(String value, Collection<String> allowList, Collection<String> denyList) {

smithy-build/src/test/java/software/amazon/smithy/build/transforms/FilterSuppressionsTest.java

@@ -64,7 +64,9 @@ public void cannotSetBothNamespaceAllowAndDenyList() {
         "namespaces,namespaceDenyList",
         "namespaces,filterWithTopLevelImports",
         "namespaces,filterWithProjectionImports",
-        "namespaces,detectsValidatorRemoval"
+        "namespaces,detectsValidatorRemoval",
+        "namespaces,unchanged",
+        "noSuppressions,removeUnused"
     })
     public void runTransformTests(String modelFile, String testName) throws Exception {
         Model model = Model.assembler()

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.unchanged.json

@@ -0,0 +1,15 @@
+{
+    "version": "1.0",
+    "projections": {
+        "foo": {
+            "transforms": [
+                {
+                    "name": "filterSuppressions",
+                    "args": {
+                        "namespaceDenyList": ["smithy.nonexistent"]
+                    }
+                }
+            ]
+        }
+    }
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.unchanged.smithy

@@ -0,0 +1,39 @@
+$version: "1.0"
+
+metadata validators = [
+    {
+        name: "EmitEachSelector",
+        id: "Test",
+        severity: "WARNING",
+        configuration: {
+            selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])"
+        }
+    }
+]
+
+metadata suppressions = [
+    {
+        id: "Test",
+        reason: "reason...",
+        namespace: "smithy.example"
+    },
+    {
+        id: "Lorem",
+        namespace: "smithy.foo"
+    },
+    {
+        id: "Test",
+        namespace: "smithy.example.nested",
+        reason: "reason..."
+    },
+    {
+        id: "Ipsum",
+        namespace: "*"
+    }
+]
+
+namespace smithy.example
+
+structure Foo {}
+
+structure Baz {}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/noSuppressions.removeUnused.json

@@ -0,0 +1,15 @@
+{
+    "version": "1.0",
+    "projections": {
+        "foo": {
+            "transforms": [
+                {
+                    "name": "filterSuppressions",
+                    "args": {
+                        "removeUnused": true
+                    }
+                }
+            ]
+        }
+    }
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/noSuppressions.removeUnused.smithy

@@ -0,0 +1,24 @@
+$version: "1.0"
+
+metadata validators = [
+    {
+        name: "EmitEachSelector",
+        id: "Test",
+        severity: "WARNING",
+        configuration: {
+            selector: ":not([id='smithy.example#NoMatches'])"
+        }
+    }
+]
+
+metadata suppressions = []
+
+namespace smithy.example
+
+structure Foo {
+    foo: String
+}
+
+structure Baz {
+    baz: String
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/noSuppressions.smithy

@@ -0,0 +1,24 @@
+$version: "1.0"
+
+metadata validators = [
+    {
+        name: "EmitEachSelector",
+        id: "Test",
+        severity: "WARNING",
+        configuration: {
+            selector: ":not([id='smithy.example#NoMatches'])"
+        }
+    }
+]
+
+metadata suppressions = []
+
+namespace smithy.example
+
+structure Foo {
+    foo: String
+}
+
+structure Baz {
+    baz: String
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.smithy

@@ -6,7 +6,7 @@ metadata validators = [
         id: "Test",
         severity: "WARNING",
         configuration: {
-            selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])"
+            selector: ":not([id='smithy.example#NoMatches'])"
         }
     }
 ]
@@ -17,6 +17,11 @@ namespace smithy.example
 string NoMatches
 
 @suppress(["Test"])
-structure Foo {}
+structure Foo {
+    @suppress(["Test"])
+    foo: String
+}
 
-structure Baz {}
+structure Baz {
+    baz:String
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.smithy

@@ -6,7 +6,7 @@ metadata validators = [
         id: "Test",
         severity: "WARNING",
         configuration: {
-            selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])"
+            selector: ":not([id='smithy.example#NoMatches'])"
         }
     }
 ]
@@ -17,7 +17,13 @@ namespace smithy.example
 string NoMatches
 
 @suppress(["NeverUsed"])
-structure Foo {}
+structure Foo {
+    @suppress(["NeverUsed"])
+    foo: String
+}
 
 @suppress(["NeverUsed"])
-structure Baz {}
+structure Baz {
+    @suppress(["NeverUsed"])
+    baz:String
+}

smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.smithy

@@ -6,7 +6,7 @@ metadata validators = [
         id: "Test",
         severity: "WARNING",
         configuration: {
-            selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])"
+            selector: ":not([id='smithy.example#NoMatches'])"
         }
     }
 ]
@@ -16,6 +16,11 @@ namespace smithy.example
 string NoMatches
 
 @suppress(["Test"])
-structure Foo {}
+structure Foo {
+    @suppress(["Test"])
+    foo: String
+}
 
-structure Baz {}
+structure Baz {
+    baz: String
+}