Add support for suppressions to smithy-diff

Closes #1861

smithy-diff/README.md

@@ -14,6 +14,9 @@ Model modelB = loadModelB();
 List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
 ```
 
+Smithy Diff will load and apply metadata suppressions and severityOverrides
+from the new model to any emitted diff events. 
+
 # Adding a custom DiffEvaluator
 
 This library finds all instances of `DiffEvaluator`

smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java

@@ -27,6 +27,8 @@
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
+import software.amazon.smithy.model.validation.ValidationEventDecorator;
+import software.amazon.smithy.model.validation.suppressions.ModelBasedEventDecorator;
 import software.amazon.smithy.utils.SmithyBuilder;
 
 /**
@@ -280,8 +282,17 @@ public Result compare() {
             List<DiffEvaluator> evaluators = new ArrayList<>();
             ServiceLoader.load(DiffEvaluator.class, classLoader).forEach(evaluators::add);
             Differences differences = Differences.detect(oldModel, newModel);
+
+            // Applies suppressions and elevates event severities.
+            ValidationEventDecorator decoratorResult = new ModelBasedEventDecorator()
+                    .createDecorator(newModel)
+                    .getResult()
+                    .orElse(ValidationEventDecorator.IDENTITY);
+
             List<ValidationEvent> diffEvents = evaluators.parallelStream()
                     .flatMap(evaluator -> evaluator.evaluate(differences).stream())
+                    // No need to call canDecorate first since that method will always return true in any code path.
+                    .map(decoratorResult::decorate)
                     .collect(Collectors.toList());
 
             return new Result(differences, diffEvents, oldModelEvents, newModelEvents);

smithy-diff/src/test/java/software/amazon/smithy/diff/ModelDiffTest.java

@@ -94,4 +94,33 @@ public void detectsWhenNoBreakingChanges() {
 
         assertThat(result.isDiffBreaking(), is(false));
     }
+
+    @Test
+    public void appliesSuppressionsToDiff() {
+        Model oldModel = Model.assembler()
+                .addImport(getClass().getResource("suppressions-a.smithy"))
+                .assemble()
+                .unwrap();
+        Model newModel = Model.assembler()
+                .addImport(getClass().getResource("suppressions-b.smithy"))
+                .assemble()
+                .unwrap();
+
+        ModelDiff.Result result = ModelDiff.builder()
+                .oldModel(oldModel)
+                .newModel(newModel)
+                .compare();
+
+        assertThat(result.isDiffBreaking(), is(false));
+
+        boolean found = false;
+        for (ValidationEvent event : result.getDiffEvents()) {
+            if (event.getId().equals("ChangedMemberOrder")) {
+                assertThat(event.getSeverity(), equalTo(Severity.SUPPRESSED));
+                found = true;
+            }
+        }
+
+        assertThat(found, is(true));
+    }
 }

smithy-diff/src/test/resources/software/amazon/smithy/diff/suppressions-a.smithy

@@ -0,0 +1,16 @@
+$version: "2.0"
+
+metadata suppressions = [
+    {
+        id: "ChangedMemberOrder"
+        namespace: "smithy.example"
+    }
+]
+
+namespace smithy.example
+
+structure Foo {
+    a: String
+    b: String
+    c: String
+}

smithy-diff/src/test/resources/software/amazon/smithy/diff/suppressions-b.smithy

@@ -0,0 +1,16 @@
+$version: "2.0"
+
+metadata suppressions = [
+    {
+        id: "ChangedMemberOrder"
+        namespace: "smithy.example"
+    }
+]
+
+namespace smithy.example
+
+structure Foo {
+    c: String
+    a: String
+    b: String
+}

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java

@@ -24,22 +24,18 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
-import software.amazon.smithy.model.SourceException;
 import software.amazon.smithy.model.SourceLocation;
-import software.amazon.smithy.model.node.ObjectNode;
-import software.amazon.smithy.model.shapes.Shape;
-import software.amazon.smithy.model.shapes.ShapeId;
-import software.amazon.smithy.model.traits.SuppressTrait;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationEventDecorator;
 import software.amazon.smithy.model.validation.Validator;
 import software.amazon.smithy.model.validation.ValidatorFactory;
+import software.amazon.smithy.model.validation.suppressions.ModelBasedEventDecorator;
 import software.amazon.smithy.model.validation.suppressions.SeverityOverride;
-import software.amazon.smithy.model.validation.suppressions.Suppression;
 import software.amazon.smithy.model.validation.validators.ResourceCycleValidator;
 import software.amazon.smithy.model.validation.validators.TargetValidator;
+import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.SetUtils;
 import software.amazon.smithy.utils.SmithyBuilder;
 
@@ -65,9 +61,6 @@
  */
 final class ModelValidator implements Validator {
 
-    private static final String SUPPRESSIONS = "suppressions";
-    private static final String SEVERITY_OVERRIDES = "severityOverrides";
-
     // Lazy initialization holder class idiom to hold a default validator factory.
     private static final class LazyValidatorFactoryHolder {
         static final ValidatorFactory INSTANCE = ValidatorFactory.createServiceFactory(
@@ -194,7 +187,6 @@ public ModelValidator build() {
     private static final class LoadedModelValidator {
 
         private final Model model;
-        private final List<Suppression> suppressions = new ArrayList<>();
         private final List<SeverityOverride> severityOverrides;
         private final List<Validator> validators;
         private final List<ValidationEvent> events = new ArrayList<>();
@@ -203,56 +195,38 @@ private static final class LoadedModelValidator {
 
         private LoadedModelValidator(Model model, ModelValidator validator) {
             this.model = model;
-            this.validationEventDecorator = validator.validationEventDecorator;
             this.eventListener = validator.eventListener;
             this.severityOverrides = new ArrayList<>(validator.severityOverrides);
             this.validators = new ArrayList<>(validator.validators);
 
-            loadMetadataSuppressions();
-            loadMetadataSeverityOverrides();
+            // Suppressing and elevating events is handled by composing a given decorator with a
+            // ModelBasedEventDecorator.
+            ModelBasedEventDecorator modelBasedEventDecorator = new ModelBasedEventDecorator();
+            modelBasedEventDecorator.severityOverrides(validator.severityOverrides);
+            ValidatedResult<ValidationEventDecorator> result = modelBasedEventDecorator.createDecorator(model);
+            this.validationEventDecorator = result.getResult()
+                    .map(decorator -> ValidationEventDecorator.compose(
+                            ListUtils.of(decorator, validator.validationEventDecorator)))
+                    .orElse(validator.validationEventDecorator);
+
+            // Events encountered while loading suppressions and overrides have been modified by everything the
+            // modelBasedEventDecorator knows about, but has not been modified by any custom decorator (if any).
+            for (ValidationEvent event : result.getValidationEvents()) {
+                if (validationEventDecorator.canDecorate(event)) {
+                    event = validationEventDecorator.decorate(event);
+                }
+                events.add(event);
+            }
 
-            // Given events have already been emitted and decorated, but have not been suppressed/elevated.
+            // Now that the decorator is available, emit/decorate/suppress/collect explicitly provided events.
             for (ValidationEvent event : validator.events) {
-                events.add(modifyEventSeverity(event));
+                pushEvent(event);
             }
 
+            // The decorator itself doesn't handle loading and applying validators, just modifying events.
             loadModelValidators(validator.validatorFactory);
         }
 
-        private void loadMetadataSeverityOverrides() {
-            model.getMetadataProperty(SEVERITY_OVERRIDES).ifPresent(value -> {
-                try {
-                    List<ObjectNode> values = value.expectArrayNode().getElementsAs(ObjectNode.class);
-                    for (ObjectNode rule : values) {
-                        try {
-                            severityOverrides.add(SeverityOverride.fromMetadata(rule));
-                        } catch (SourceException e) {
-                            pushEvent(ValidationEvent.fromSourceException(e));
-                        }
-                    }
-                } catch (SourceException e) {
-                    pushEvent(ValidationEvent.fromSourceException(e));
-                }
-            });
-        }
-
-        private void loadMetadataSuppressions() {
-            model.getMetadataProperty(SUPPRESSIONS).ifPresent(value -> {
-                try {
-                    List<ObjectNode> values = value.expectArrayNode().getElementsAs(ObjectNode.class);
-                    for (ObjectNode rule : values) {
-                        try {
-                            suppressions.add(Suppression.fromMetadata(rule));
-                        } catch (SourceException e) {
-                            pushEvent(ValidationEvent.fromSourceException(e));
-                        }
-                    }
-                } catch (SourceException e) {
-                    pushEvent(ValidationEvent.fromSourceException(e));
-                }
-            });
-        }
-
         private void loadModelValidators(ValidatorFactory validatorFactory) {
             // Load validators defined in metadata.
             ValidatedResult<List<ValidatorDefinition>> loaded = ValidationLoader
@@ -291,8 +265,9 @@ private void pushEvents(List<ValidationEvent> source) {
         }
 
         private void pushEvent(ValidationEvent event) {
-            event = modifyEventSeverity(event);
-            event = validationEventDecorator.decorate(event);
+            if (validationEventDecorator.canDecorate(event)) {
+                event = validationEventDecorator.decorate(event);
+            }
             events.add(event);
             eventListener.accept(event);
         }
@@ -310,8 +285,7 @@ private List<ValidationEvent> validate() {
             List<ValidationEvent> result = validators.parallelStream()
                     .flatMap(validator -> validator.validate(model).stream())
                     .filter(this::filterPrelude)
-                    .map(this::modifyEventSeverity)
-                    .map(validationEventDecorator::decorate)
+                    .map(e -> validationEventDecorator.canDecorate(e) ? validationEventDecorator.decorate(e) : e)
                     // Emit events as they occur during validation.
                     .peek(eventListener)
                     .collect(Collectors.toList());
@@ -330,52 +304,5 @@ private boolean filterPrelude(ValidationEvent event) {
                     .filter(Prelude::isPreludeShape)
                     .isPresent();
         }
-
-        private ValidationEvent modifyEventSeverity(ValidationEvent event) {
-            // Use a suppress trait if present.
-            if (event.getShapeId().isPresent()) {
-                ShapeId target = event.getShapeId().get();
-                Shape shape = model.getShape(target).orElse(null);
-                if (shape != null) {
-                    if (shape.hasTrait(SuppressTrait.class)) {
-                        Suppression suppression = Suppression.fromSuppressTrait(shape);
-                        if (suppression.test(event)) {
-                            return changeSeverity(event, Severity.SUPPRESSED, suppression.getReason().orElse(null));
-                        }
-                    }
-                }
-            }
-
-            // Check metadata and manual suppressions.
-            for (Suppression suppression : suppressions) {
-                if (suppression.test(event)) {
-                    return changeSeverity(event, Severity.SUPPRESSED, suppression.getReason().orElse(null));
-                }
-            }
-
-            Severity appliedSeverity = event.getSeverity();
-            for (SeverityOverride override : severityOverrides) {
-                Severity overrideResult = override.apply(event);
-                if (overrideResult.ordinal() > appliedSeverity.ordinal()) {
-                    appliedSeverity = overrideResult;
-                }
-            }
-
-            return changeSeverity(event, appliedSeverity, null);
-        }
-
-        private static ValidationEvent changeSeverity(ValidationEvent event, Severity severity, String reason) {
-            if (event.getSeverity() == severity) {
-                return event;
-            } else {
-                // The event was suppressed so change the severity and reason.
-                ValidationEvent.Builder builder = event.toBuilder();
-                builder.severity(severity);
-                if (reason != null) {
-                    builder.suppressionReason(reason);
-                }
-                return builder.build();
-            }
-        }
     }
 }

smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEventDecorator.java

@@ -24,6 +24,20 @@
  * relevant only to the context where Smithy is being used.
  */
 public interface ValidationEventDecorator {
+
+    /** A decorator that does nothing. */
+    ValidationEventDecorator IDENTITY = new ValidationEventDecorator() {
+        @Override
+        public boolean canDecorate(ValidationEvent ev) {
+            return false;
+        }
+
+        @Override
+        public ValidationEvent decorate(ValidationEvent ev) {
+            return ev;
+        }
+    };
+
     /**
      * Returns true if this decorator knows how to decorate this event, usually by looking at the event id.
      *
@@ -48,17 +62,7 @@ public interface ValidationEventDecorator {
      */
     static ValidationEventDecorator compose(List<ValidationEventDecorator> decorators) {
         if (decorators.isEmpty()) {
-            return new ValidationEventDecorator() {
-                @Override
-                public boolean canDecorate(ValidationEvent ev) {
-                    return false;
-                }
-
-                @Override
-                public ValidationEvent decorate(ValidationEvent ev) {
-                    return ev;
-                }
-            };
+            return IDENTITY;
         } else {
             return new ValidationEventDecorator() {
                 @Override