smithy-lang · mtdowling · Oct 27, 2022 · Oct 25, 2022
@@ -60,6 +60,11 @@ objects that are used to constrain a model. Each object in the
 
         If ``id`` is not specified, it will default to the ``name`` property of
         the validator definition.
+
+        IDs that contain dots (.) are hierarchical. For example, the ID
+        "Foo.Bar" contains the ID "Foo". Event ID hierarchies can be leveraged
+        to group validation events and allow more granular
+        :ref:`suppressions <suppression-definition>`.
     * - message
       - ``string``
       - Provides a custom message to use when emitting validation events. The
@@ -262,6 +267,53 @@ ID of ``OverlyBroadValidator``:
     ]
 
 
+Matching suppression event IDs
+==============================
+
+Both the :ref:`suppress-trait` and suppressions
+:ref:`defined in metadata <suppressions-metadata>` match events hierarchically
+based on dot (.) segments. For example, given a validation event ID of
+"Foo.Bar", and a suppression ID of "Foo", the suppression ID matches the
+event ID because "Foo.Bar" begins with the segment "Foo". However, a suppression
+ID of "Foo.Bar" does not match an event ID of "Foo" because "Foo.Bar" is more
+specific. Further, a suppression ID of "ABC" does not match an event ID of
+"ABC123" because "ABC" is not a segment of the event ID.
+
+.. list-table::
+    :header-rows: 1
+
+    * - Event ID
+      - Suppression ID
+      - Is match
+    * - ``Foo``
+      - ``Foo``
+      - Yes
+    * - ``Foo.Bar``
+      - ``Foo``
+      - Yes
+    * - ``Foo.Bar.Baz``
+      - ``Foo``
+      - Yes
+    * - ``Foo.``
+      - ``Foo.``
+      - Yes
+    * - ``Foo.``
+      - ``Foo``
+      - Yes
+    * - ``Foo``
+      - ``Foo.``
+      - No
+    * - ``Foosball``
+      - ``Foo``
+      - No
+    * - ``Foo``
+      - ``Foo.Bar``
+      - No
+    * - ``Abc.Foo.Bar``
+      - ``Foo.Bar``
+      - No
+
+
 -------------------
 Built-in validators
 -------------------

@@ -18,14 +18,14 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.StringJoiner;
 import java.util.stream.Stream;
 import software.amazon.smithy.diff.ChangedShape;
 import software.amazon.smithy.diff.Differences;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.NullableIndex;
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.traits.ClientOptionalTrait;
 import software.amazon.smithy.model.traits.DefaultTrait;
@@ -45,24 +45,23 @@ public class ChangedNullability extends AbstractDiffEvaluator {
     public List<ValidationEvent> evaluate(Differences differences) {
         NullableIndex oldIndex = NullableIndex.of(differences.getOldModel());
         NullableIndex newIndex = NullableIndex.of(differences.getNewModel());
-
         List<ValidationEvent> events = new ArrayList<>();
 
-        Stream<ChangedShape<MemberShape>> changed = Stream.concat(
+        Stream.concat(
             // Get members that changed.
             differences.changedShapes(MemberShape.class),
             // Get members of structures that added/removed the input trait.
             changedInputMembers(differences)
-        );
-
-        changed.map(change -> {
+        ).forEach(change -> {
             // If NullableIndex says the nullability of a member changed, then that's a breaking change.
             MemberShape oldShape = change.getOldShape();
             MemberShape newShape = change.getNewShape();
             boolean wasNullable = oldIndex.isMemberNullable(oldShape);
             boolean isNowNullable = newIndex.isMemberNullable(newShape);
-            return wasNullable == isNowNullable ? null : createError(differences, change, wasNullable);
-        }).filter(Objects::nonNull).forEach(events::add);
+            if (wasNullable != isNowNullable) {
+                createErrors(differences, change, wasNullable, events);
+            }
+        });
 
         return events;
     }
@@ -79,78 +78,92 @@ private Stream<ChangedShape<MemberShape>> changedInputMembers(Differences differ
                         .filter(Objects::nonNull));
     }
 
-    private ValidationEvent createError(
+    private void createErrors(
             Differences differences,
             ChangedShape<MemberShape> change,
-            boolean wasNullable
+            boolean wasNullable,
+            List<ValidationEvent> events
     ) {
         MemberShape oldMember = change.getOldShape();
         MemberShape newMember = change.getNewShape();
         String message = String.format("Member `%s` changed from %s to %s: ",
                                        oldMember.getMemberName(),
                                        wasNullable ? "nullable" : "non-nullable",
                                        wasNullable ? "non-nullable" : "nullable");
-        StringJoiner joiner = new StringJoiner("; ", message, "");
         boolean oldHasInput = hasInputTrait(differences.getOldModel(), oldMember);
         boolean newHasInput = hasInputTrait(differences.getNewModel(), newMember);
+        ShapeId shape = change.getShapeId();
         Shape newTarget = differences.getNewModel().expectShape(newMember.getTarget());
-        Severity severity = null;
+        int currentEventSize = events.size();
 
         if (oldHasInput && !newHasInput) {
             // If there was an input trait before, but not now, then the nullability must have
             // changed from nullable to non-nullable.
-            joiner.add("The @input trait was removed from " + newMember.getContainer());
-            severity = Severity.ERROR;
+            events.add(emit(Severity.ERROR, "RemovedInputTrait", shape, message,
+                            "The @input trait was removed from " + newMember.getContainer()));
         } else if (!oldHasInput && newHasInput) {
             // If there was no input trait before, but there is now, then the nullability must have
             // changed from non-nullable to nullable.
-            joiner.add("The @input trait was added to " + newMember.getContainer());
-            severity = Severity.DANGER;
+            events.add(emit(Severity.DANGER, "AddedInputTrait", shape, message,
+                            "The @input trait was added to " + newMember.getContainer()));
         } else if (!newHasInput) {
             // Can't add nullable to a preexisting required member.
             if (change.isTraitAdded(ClientOptionalTrait.ID) && change.isTraitInBoth(RequiredTrait.ID)) {
-                joiner.add("The @nullable trait was added to a @required member.");
-                severity = Severity.ERROR;
+                events.add(emit(Severity.ERROR, "AddedNullableTrait", shape, message,
+                                "The @nullable trait was added to a @required member."));
             }
             // Can't add required to a member unless the member is marked as nullable.
             if (change.isTraitAdded(RequiredTrait.ID) && !newMember.hasTrait(ClientOptionalTrait.ID)) {
-                joiner.add("The @required trait was added to a member that is not marked as @nullable.");
-                severity = Severity.ERROR;
+                events.add(emit(Severity.ERROR, "AddedRequiredTrait", shape, message,
+                                "The @required trait was added to a member that is not marked as @nullable."));
             }
             // Can't add the default trait to a member unless the member was previously required.
             if (change.isTraitAdded(DefaultTrait.ID) && !change.isTraitRemoved(RequiredTrait.ID)) {
-                joiner.add("The @default trait was added to a member that was not previously @required.");
-                severity = Severity.ERROR;
+                events.add(emit(Severity.ERROR, "AddedDefaultTrait", shape, message,
+                                "The @default trait was added to a member that was not previously @required."));
             }
             // Can only remove the required trait if the member was nullable or replaced by the default trait.
             if (change.isTraitRemoved(RequiredTrait.ID)
                     && !newMember.hasTrait(DefaultTrait.ID)
                     && !oldMember.hasTrait(ClientOptionalTrait.ID)) {
                 if (newTarget.isStructureShape() || newTarget.isUnionShape()) {
-                    severity = severity == null ? Severity.WARNING : severity;
-                    joiner.add("The @required trait was removed from a member that targets a " + newTarget.getType()
-                               + ". This is backward compatible in generators that always treat structures and "
-                               + "unions as optional (e.g., AWS generators)");
+                    events.add(emit(Severity.WARNING, "RemovedRequiredTrait.StructureOrUnion", shape, message,
+                                    "The @required trait was removed from a member that targets a "
+                                    + newTarget.getType() + ". This is backward compatible in generators that "
+                                    + "always treat structures and unions as optional (e.g., AWS generators)"));
                 } else {
-                    joiner.add("The @required trait was removed and not replaced with the @default trait and "
-                               + "@addedDefault trait.");
-                    severity = Severity.ERROR;
+                    events.add(emit(Severity.ERROR, "RemovedRequiredTrait", shape, message,
+                                    "The @required trait was removed and not replaced with the @default trait and "
+                                    + "@addedDefault trait."));
                 }
             }
         }
 
-        // If no explicit severity was detected, then assume an error.
-        severity = severity == null ? Severity.ERROR : severity;
-
-        return ValidationEvent.builder()
-                .id(getEventId())
-                .shapeId(change.getNewShape())
-                .severity(severity)
-                .message(joiner.toString())
-                .build();
+        // If not specific event was emitted, emit a generic event.
+        if (events.size() == currentEventSize) {
+            events.add(emit(Severity.ERROR, null, shape, null, message));
+        }
     }
 
     private boolean hasInputTrait(Model model, MemberShape member) {
         return model.getShape(member.getContainer()).filter(shape -> shape.hasTrait(InputTrait.ID)).isPresent();
     }
+
+    private ValidationEvent emit(
+            Severity severity,
+            String eventIdSuffix,
+            ShapeId shape,
+            String prefixMessage,
+            String message
+    ) {
+        String actualId = eventIdSuffix == null ? getEventId() : (getEventId() + '.' + eventIdSuffix);
+        String actualMessage = prefixMessage == null ? message : (prefixMessage + "; " + message);
+        return ValidationEvent.builder()
+                .id(actualId)
+                .shapeId(shape)
+                .message(actualMessage)
+                .severity(severity)
+                .message(message)
+                .build();
+    }
 }
@@ -90,7 +90,7 @@ public void detectsInvalidAdditionOfDefaultTrait() {
         assertThat(result.isDiffBreaking(), is(true));
         assertThat(result.getDiffEvents().stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.AddedDefaultTrait"))
                            .filter(event -> event.getShapeId().get().equals(a.getAllMembers().get("foo").getId()))
                            .filter(event -> event.getMessage().contains("The @default trait was added to a member that "
                                                                         + "was not previously @required"))
@@ -115,7 +115,7 @@ public void removingTheRequiredTraitOnInputStructureIsOk() {
         ModelDiff.Result result = ModelDiff.builder().oldModel(model1).newModel(model2).compare();
 
         assertThat(result.getDiffEvents().stream()
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.RemovedRequiredTrait.StructureOrUnion"))
                            .count(), equalTo(0L));
     }
 
@@ -137,7 +137,7 @@ public void detectsInvalidRemovalOfRequired() {
         assertThat(result.isDiffBreaking(), is(true));
         assertThat(result.getDiffEvents().stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.RemovedRequiredTrait"))
                            .filter(event -> event.getShapeId().get().equals(a.getAllMembers().get("foo").getId()))
                            .filter(event -> event.getMessage().contains("The @required trait was removed and not "
                                                                         + "replaced with the @default trait"))
@@ -157,7 +157,7 @@ public void detectAdditionOfRequiredTrait() {
 
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.AddedRequiredTrait"))
                            .filter(event -> event.getMessage().contains("The @required trait was added to a member "
                                                                         + "that is not marked as @nullable"))
                            .count(), equalTo(1L));
@@ -180,7 +180,7 @@ public void detectAdditionOfNullableTrait() {
 
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.AddedNullableTrait"))
                            .filter(event -> event.getMessage().contains("The @nullable trait was added to a "
                                                                         + "@required member"))
                            .count(), equalTo(1L));
@@ -202,7 +202,7 @@ public void detectsAdditionOfInputTrait() {
 
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.DANGER)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.AddedInputTrait"))
                            .filter(event -> event.getMessage().contains("The @input trait was added to"))
                            .count(), equalTo(1L));
     }
@@ -226,7 +226,7 @@ public void detectsRemovalOfInputTrait() {
 
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability"))
+                           .filter(event -> event.getId().equals("ChangedNullability.RemovedInputTrait"))
                            .filter(event -> event.getMessage().contains("The @input trait was removed from"))
                            .count(), equalTo(1L));
     }

@@ -24,7 +24,7 @@
 public final class TestHelper {
     public static List<ValidationEvent> findEvents(List<ValidationEvent> events, String eventId) {
         return events.stream()
-                .filter(event -> event.getId().equals(eventId))
+                .filter(event -> event.containsId(eventId))
                 .collect(Collectors.toList());
     }
 

@@ -241,6 +241,31 @@ public String getEventId() {
         return getId();
     }
 
+    /**
+     * Tests if the event ID hierarchically contains the given ID.
+     *
+     * <p>Event IDs that contain dots (.) are hierarchical. An event ID of
+     * {@code "Foo.Bar"} contains the ID {@code "Foo"} and {@code "Foo.Bar"}.
+     * However, an event ID of {@code "Foo"} does not contain the ID
+     * {@code "Foo.Bar"} as {@code "Foo.Bar"} is more specific than {@code "Foo"}.
+     * If an event ID exactly matches the given {@code id}, then it also contains
+     * the ID (for example, {@code "Foo.Bar."} contains {@code "Foo.Bar."}.
+     *
+     * @param id ID to test.
+     * @return Returns true if the event's event ID contains the given {@code id}.
+     */
+    public boolean containsId(String id) {
+        int eventLength = eventId.length();
+        int suppressionLength = id.length();
+        if (suppressionLength == eventLength) {
+            return id.equals(eventId);
+        } else if (suppressionLength > eventLength) {
+            return false;
+        } else {
+            return eventId.startsWith(id) && eventId.charAt(id.length()) == '.';
+        }
+    }
+
     /**
      * Returns the identifier of the validation event.
      *

@@ -55,7 +55,7 @@ static MetadataSuppression fromNode(Node node) {
 
     @Override
     public boolean test(ValidationEvent event) {
-        return event.getId().equals(id) && matchesNamespace(event);
+        return event.containsId(id) && matchesNamespace(event);
     }
 
     @Override

@@ -34,6 +34,16 @@ final class TraitSuppression implements Suppression {
 
     @Override
     public boolean test(ValidationEvent event) {
-        return event.getShapeId().filter(shape::equals).isPresent() && trait.getValues().contains(event.getId());
+        if (!event.getShapeId().filter(shape::equals).isPresent()) {
+            return false;
+        }
+
+        for (String value : trait.getValues()) {
+            if (event.containsId(value)) {
+                return true;
+            }
+        }
+
+        return false;
     }
 }