Add validation events decorators (#1774)

* Add validation events decorators

Adds a new interface `ValidationEventDecorator` that libraries can
implement and plug into Smithy by making those discoverable using the
java builtin
[`ServiceLoader`](https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html). All
of the `ValidationEventDecorator` instances are loaded and each
`ValidationEvent` is passed to each of the decorators. The decorators
can make use of the `ValidationEvent#eventId()` method to quickly
filter out the events that it knows how to decorate form the events it
does not care about, and the `ValidationEvent#toBuilder()` to create a
builder that can be used to mutate the instance.

The current use case is to add hints to a know set of events to point
the user into the right solution for it (e.g., you need to pull X or Y
dependency for this unresolved shape). Since often these solutions
might require specific details about the environment in which Smithy
is being used, those decorators cannot be part of Smithy itself but
now can be implemented by libraries and loaded automatically by Smithy
in a similar way in which validators are currently loaded.

For instance, a decorator that suggest the user to use or remove
unused structures might look like this

```java
    @OverRide
    public ValidationEvent decorate(ValidationEvent ev) {
        if (ev.containsId("UnreferencedShape")) {
            if (ev.getMessage().contains("The structure ")) {
                return ev.toBuilder()
                    .hint("Consider using this structure in your service or remove it from the model")
                    .build();
            }
        }
        return ev;
    }
```

* Load the decorators using its own factory

In order to be able to decorate events that are returned before
running the validation logic we need to pull the decoration logic up
to the ModelAssambler itself. For this, a new factory was created to
load the decorators instead of coupling this logic with the
ValidatorFactory one and the assembler will take care of loading them
and passing them down to the validation logic.

* Fix an event duplication bug

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

Co-authored-by: Michael Dowling <michael@mtdowling.com>

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

Co-authored-by: Michael Dowling <michael@mtdowling.com>

* Address pull request comments

* Move back the responsibility of loading the decorators to the
  ValidatorFactory and pass it down to the ModelValidator from the
  ModelAssembler

* Overwrite the events in place in the list instead of crating a new
  list and copying + decorating.

* Create a new method to decorate a single event and use it as part of
  the stream that returns core error events.

* Remove the posibility of manually adding decorators in the validator
  and assembler and just use the ones loaded by the
  `ValidatorFacotry`. We don't need this now and we can add it back if
  and when needed.

* Address pull request comments

* Do not catch exceptions thrown by loaded decorators. We will ship
  like this for now and add later if the need arises.

* Add a new method to create the validation factory instead of using
  the existing one to avoid braking backwards compatibility.

* Address pull request comments

Remove the 'builtin' qualifier that doesn't make sense in this context

---------

Co-authored-by: Michael Dowling <michael@mtdowling.com>

smithy-cli/src/main/java/software/amazon/smithy/cli/ColorTheme.java

@@ -47,5 +47,7 @@ public final class ColorTheme {
     public static final Style DIFF_TITLE = Style.of(Style.BG_BRIGHT_BLACK, Style.WHITE);
     public static final Style DIFF_EVENT_TITLE = Style.of(Style.BG_BRIGHT_BLUE, Style.BLACK);
 
+    public static final Style HINT_TITLE = Style.of(Style.BRIGHT_GREEN);
+
     private ColorTheme() {}
 }

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java

@@ -104,6 +104,13 @@ public String format(ValidationEvent event) {
 
             writeMessage(writer, event.getMessage());
             writer.append(ls);
+            event.getHint().ifPresent(hint -> {
+                String hintLabel = "HINT: ";
+                writer.append(ls);
+                colors.style(writer, hintLabel, ColorTheme.HINT_TITLE);
+                writer.append(StyleHelper.formatMessage(hint, LINE_LENGTH - hintLabel.length(), colors));
+                writer.append(ls);
+            });
 
             return writer.toString();
         }

smithy-cli/src/test/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatterTest.java

@@ -51,23 +51,60 @@ public void formatsEventsWithNoColors() {
     }
 
     @Test
-    public void formatsEventsWithColors() {
-        PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR);
-        String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR);
+    public void formatsEventsWithHintWithNoColors() {
+        PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.NO_COLOR);
+        String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR, "Some hint");
 
         assertThat(formatted, equalTo(
                 "\n"
-                + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n"
-                + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n"
-                + "\u001B[90mFile:  \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n"
+                + "──  ERROR  ───────────────────────────────────────────────────────────────── Foo\n"
+                + "Shape: smithy.example#Foo\n"
+                + "File:  build/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\n"
                 + "\n"
-                + "\u001B[90m4| \u001B[0m\n"
-                + "\u001B[90m5| \u001B[0mresource Foo {\n"
-                + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n"
+                + "4| \n"
+                + "5| resource Foo {\n"
+                + " | ^\n"
                 + "\n"
-                + "Hello, \u001B[36mthere\u001B[0m\n"));
+                + "Hello, `there`\n\n"
+                + "HINT: Some hint\n")); // keeps ticks because formatting is disabled.
+    }
+
+    @Test
+    public void formatsEventsWithColors() {
+        PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR);
+        String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR);
+
+        assertThat(formatted, equalTo(
+            "\n"
+            + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n"
+            + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n"
+            + "\u001B[90mFile:  \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n"
+            + "\n"
+            + "\u001B[90m4| \u001B[0m\n"
+            + "\u001B[90m5| \u001B[0mresource Foo {\n"
+            + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n"
+            + "\n"
+            + "Hello, \u001B[36mthere\u001B[0m\n"));
     }
 
+    @Test
+    public void formatsEventsWithHintWithColors() {
+        PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR);
+        String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR, "Some hint");
+
+        assertThat(formatted, equalTo(
+            "\n"
+            + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n"
+            + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n"
+            + "\u001B[90mFile:  \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n"
+            + "\n"
+            + "\u001B[90m4| \u001B[0m\n"
+            + "\u001B[90m5| \u001B[0mresource Foo {\n"
+            + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n"
+            + "\n"
+            + "Hello, \u001B[36mthere\u001B[0m\n\n"
+            + "\u001B[92mHINT: \u001B[0mSome hint\n"));
+    }
     @Test
     public void doesNotIncludeSourceLocationNoneInOutput() {
         PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.NO_COLOR);
@@ -116,12 +153,17 @@ private PrettyAnsiValidationFormatter createFormatter(ColorFormatter colors) {
     }
 
     private String formatTestEventWithSeverity(PrettyAnsiValidationFormatter pretty, Severity severity) {
+        return formatTestEventWithSeverity(pretty, severity, null);
+    }
+
+    private String formatTestEventWithSeverity(PrettyAnsiValidationFormatter pretty, Severity severity, String hint) {
         Model model = Model.assembler().addImport(getClass().getResource("valid-model.smithy")).assemble().unwrap();
         ValidationEvent event = ValidationEvent.builder()
                 .id("Foo")
                 .severity(severity)
                 .shape(model.expectShape(ShapeId.from("smithy.example#Foo")))
                 .message("Hello, `there`")
+                .hint(hint)
                 .build();
         return normalizeLinesAndFiles(pretty.format(event));
     }

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

@@ -38,6 +38,7 @@
 final class LoaderTraitMap {
 
     private static final Logger LOGGER = Logger.getLogger(LoaderTraitMap.class.getName());
+    private static final String UNRESOLVED_TRAIT_SUFFIX = ".UnresolvedTrait";
 
     private final TraitFactory traitFactory;
     private final Map<ShapeId, Map<ShapeId, Node>> traits = new HashMap<>();
@@ -114,7 +115,7 @@ private void validateTraitIsKnown(ShapeId target, ShapeId traitId, Trait trait,
         if (!shapeMap.isRootShapeDefined(traitId) && (trait == null || !trait.isSynthetic())) {
             Severity severity = allowUnknownTraits ? Severity.WARNING : Severity.ERROR;
             events.add(ValidationEvent.builder()
-                    .id(Validator.MODEL_ERROR)
+                    .id(Validator.MODEL_ERROR + UNRESOLVED_TRAIT_SUFFIX)
                     .severity(severity)
                     .sourceLocation(sourceLocation)
                     .shapeId(target)

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

@@ -50,6 +50,7 @@
 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.utils.Pair;
@@ -509,6 +510,9 @@ public ValidatedResult<Model> assemble() {
         if (traitFactory == null) {
             traitFactory = LazyTraitFactoryHolder.INSTANCE;
         }
+        if (validatorFactory == null) {
+            validatorFactory = ModelValidator.defaultValidationFactory();
+        }
 
         Model prelude = disablePrelude ? null : Prelude.getPreludeModel();
         LoadOperationProcessor processor = new LoadOperationProcessor(
@@ -576,7 +580,8 @@ public ValidatedResult<Model> assemble() {
         }
 
         if (disableValidation) {
-            return new ValidatedResult<>(transformed, events);
+            List<ValidationEventDecorator> decorators = validatorFactory.loadDecorators();
+            return new ValidatedResult<>(transformed, ModelValidator.decorateEvents(decorators, events));
         }
 
         try {
@@ -594,9 +599,11 @@ private void addMetadataToProcessor(Map<String, Node> metadataMap, LoadOperation
     }
 
     private ValidatedResult<Model> returnOnlyErrors(Model model, List<ValidationEvent> events) {
+        List<ValidationEventDecorator> decorators = validatorFactory.loadDecorators();
         return new ValidatedResult<>(model, events.stream()
-                .filter(event -> event.getSeverity() == Severity.ERROR)
-                .collect(Collectors.toList()));
+                                                  .filter(event -> event.getSeverity() == Severity.ERROR)
+                                                  .map(event -> ModelValidator.decorateEvent(decorators, event))
+                                                  .collect(Collectors.toList()));
     }
 
     private ValidatedResult<Model> validate(Model model, List<ValidationEvent> events) {

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

@@ -32,6 +32,7 @@
 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.Suppression;
@@ -49,7 +50,6 @@
  * loaded from metadata.
  */
 final class ModelValidator {
-
     private static final String SUPPRESSIONS = "suppressions";
 
     // Lazy initialization holder class idiom to hold a default validator factory.
@@ -169,6 +169,7 @@ public Validator createValidator() {
         }
 
         List<Validator> staticValidators = resolveStaticValidators();
+        List<ValidationEventDecorator> staticDecorators = validatorFactory.loadDecorators();
 
         return model -> {
             List<ValidationEvent> coreEvents = new ArrayList<>();
@@ -186,6 +187,8 @@ public Validator createValidator() {
             // which will only obscure the root cause.
             coreEvents.addAll(suppressEvents(model, new TargetValidator().validate(model), modelSuppressions));
             coreEvents.addAll(suppressEvents(model, new ResourceCycleValidator().validate(model), modelSuppressions));
+            // Decorate all the events
+            coreEvents = decorateEvents(staticDecorators, coreEvents);
             // Emit any events that have already occurred.
             coreEvents.forEach(eventListener);
 
@@ -194,16 +197,18 @@ public Validator createValidator() {
             }
 
             List<ValidationEvent> result = modelValidators.parallelStream()
-                    .flatMap(validator -> validator.validate(model).stream())
+                    .map(validator -> validator.validate(model))
+                    .flatMap(Collection::stream)
                     .filter(ModelValidator::filterPrelude)
                     .map(event -> suppressEvent(model, event, modelSuppressions))
+                    .map(event -> decorateEvent(staticDecorators, event))
                     // Emit events as they occur during validation.
                     .peek(eventListener)
                     .collect(Collectors.toList());
 
             for (ValidationEvent event : includeEvents) {
                 if (ModelValidator.filterPrelude(event)) {
-                    result.add(suppressEvent(model, event, modelSuppressions));
+                    result.add(decorateEvent(staticDecorators, suppressEvent(model, event, modelSuppressions)));
                 }
             }
 
@@ -214,6 +219,32 @@ public Validator createValidator() {
         };
     }
 
+    static List<ValidationEvent> decorateEvents(
+        List<ValidationEventDecorator> decorators,
+        List<ValidationEvent> events
+    ) {
+        if (!decorators.isEmpty()) {
+            for (int idx = 0; idx < events.size(); idx++) {
+                events.set(idx, decorateEvent(decorators, events.get(idx)));
+            }
+        }
+        return events;
+    }
+
+    static ValidationEvent decorateEvent(List<ValidationEventDecorator> decorators, ValidationEvent event) {
+        ValidationEvent decoratedEvent = event;
+        for (ValidationEventDecorator decorator : decorators) {
+            if (decorator.canDecorate(event)) {
+                decoratedEvent = decorator.decorate(decoratedEvent);
+            }
+        }
+        return decoratedEvent;
+    }
+
+    static ValidatorFactory defaultValidationFactory() {
+        return LazyValidatorFactoryHolder.INSTANCE;
+    }
+
     private List<Validator> resolveStaticValidators() {
         List<Validator> resolvedValidators = new ArrayList<>(validatorFactory.loadBuiltinValidators());
         resolvedValidators.addAll(validators);

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

@@ -29,6 +29,10 @@ public String format(ValidationEvent event) {
         if (reason != null) {
             message += " (" + reason + ")";
         }
+        String hint = event.getHint().orElse(null);
+        if (hint != null) {
+            message += " [" + hint + "]";
+        }
 
         return String.format(
                 "[%s] %s: %s | %s %s:%s:%s",

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

@@ -48,6 +48,7 @@ public final class ValidationEvent
     private final Severity severity;
     private final ShapeId shapeId;
     private final String suppressionReason;
+    private final String hint;
     private int hash;
 
     private ValidationEvent(Builder builder) {
@@ -61,6 +62,7 @@ private ValidationEvent(Builder builder) {
         this.eventId = SmithyBuilder.requiredState("id", builder.eventId);
         this.shapeId = builder.shapeId;
         this.suppressionReason = builder.suppressionReason;
+        this.hint = builder.hint;
     }
 
     public static Builder builder() {
@@ -141,6 +143,7 @@ public Builder toBuilder() {
         builder.eventId = eventId;
         builder.shapeId = shapeId;
         builder.suppressionReason = suppressionReason;
+        builder.hint = hint;
         return builder;
     }
 
@@ -158,14 +161,15 @@ public boolean equals(Object o) {
                 && severity.equals(other.severity)
                 && eventId.equals(other.eventId)
                 && getShapeId().equals(other.getShapeId())
-                && getSuppressionReason().equals(other.getSuppressionReason());
+                && getSuppressionReason().equals(other.getSuppressionReason())
+                && getHint().equals(other.getHint());
     }
 
     @Override
     public int hashCode() {
         int result = hash;
         if (result == 0) {
-            result = Objects.hash(eventId, shapeId, severity, sourceLocation, message, suppressionReason);
+            result = Objects.hash(eventId, shapeId, severity, sourceLocation, message, suppressionReason, hint);
             hash = result;
         }
         return result;
@@ -184,6 +188,7 @@ public Node toNode() {
                 .withOptionalMember("shapeId", getShapeId().map(Object::toString).map(Node::from))
                 .withMember("message", Node.from(getMessage()))
                 .withOptionalMember("suppressionReason", getSuppressionReason().map(Node::from))
+                .withOptionalMember("hint", getHint().map(Node::from))
                 .withMember("filename", Node.from(getSourceLocation().getFilename()))
                 .withMember("line", Node.from(getSourceLocation().getLine()))
                 .withMember("column", Node.from(getSourceLocation().getColumn()))
@@ -205,6 +210,7 @@ public static ValidationEvent fromNode(Node node) {
                 .expectMember("severity", Severity::fromNode, builder::severity)
                 .expectStringMember("message", builder::message)
                 .getStringMember("suppressionReason", builder::suppressionReason)
+                .getStringMember("hint", builder::hint)
                 .getMember("shapeId", ShapeId::fromNode, builder::shapeId);
         return builder.build();
     }
@@ -294,6 +300,15 @@ public Optional<String> getSuppressionReason() {
         return Optional.ofNullable(suppressionReason);
     }
 
+    /**
+     * Get an optional hint that adds more detail about how to fix a specific issue.
+     *
+     * @return Returns the hint if available.
+     */
+    public Optional<String> getHint() {
+        return Optional.ofNullable(hint);
+    }
+
     /**
      * Builds ValidationEvent values.
      */
@@ -305,6 +320,7 @@ public static final class Builder implements SmithyBuilder<ValidationEvent> {
         private String eventId;
         private ShapeId shapeId;
         private String suppressionReason;
+        private String hint;
 
         private Builder() {}
 
@@ -402,6 +418,17 @@ public Builder suppressionReason(String eventSuppressionReason) {
             return this;
         }
 
+        /**
+         * Sets an optional hint adding more detail about how to fix a specific issue.
+         *
+         * @param hint Hint to set
+         * @return Returns the builder.
+         */
+        public Builder hint(String hint) {
+            this.hint = hint;
+            return this;
+        }
+
         @Override
         public ValidationEvent build() {
             return new ValidationEvent(this);