Emit node validation events with locations

This commit rewrites a lot of NodeValidationVisitors to now emit events
from validation plugins and then allow the visitor to associate the
right context, shape, and severity. This allows the events emitted to
the visitor to contain a more specific source location than N/A.

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

@@ -15,13 +15,13 @@
 
 package software.amazon.smithy.model.validation;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.knowledge.BoxIndex;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.NodeType;
@@ -52,19 +52,10 @@
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.shapes.TimestampShape;
 import software.amazon.smithy.model.shapes.UnionShape;
-import software.amazon.smithy.model.validation.node.BlobLengthPlugin;
-import software.amazon.smithy.model.validation.node.CollectionLengthPlugin;
-import software.amazon.smithy.model.validation.node.IdRefPlugin;
-import software.amazon.smithy.model.validation.node.MapLengthPlugin;
 import software.amazon.smithy.model.validation.node.NodeValidatorPlugin;
-import software.amazon.smithy.model.validation.node.PatternTraitPlugin;
-import software.amazon.smithy.model.validation.node.RangeTraitPlugin;
-import software.amazon.smithy.model.validation.node.StringEnumPlugin;
-import software.amazon.smithy.model.validation.node.StringLengthPlugin;
 import software.amazon.smithy.model.validation.node.TimestampValidationStrategy;
 import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.SmithyBuilder;
-import software.amazon.smithy.utils.SmithyUnstableApi;
 
 /**
  * Validates {@link Node} values provided for {@link Shape} definitions.
@@ -78,12 +69,13 @@
  */
 public final class NodeValidationVisitor implements ShapeVisitor<List<ValidationEvent>> {
 
+    private static final List<NodeValidatorPlugin> BUILTIN = NodeValidatorPlugin.getBuiltins();
+
     private final String eventId;
     private final Node value;
     private final Model model;
     private final String context;
     private final ShapeId eventShapeId;
-    private final List<NodeValidatorPlugin> plugins;
     private final TimestampValidationStrategy timestampValidationStrategy;
     private final boolean allowBoxedNull;
 
@@ -95,18 +87,6 @@ private NodeValidationVisitor(Builder builder) {
         this.eventShapeId = builder.eventShapeId;
         this.timestampValidationStrategy = builder.timestampValidationStrategy;
         this.allowBoxedNull = builder.allowBoxedNull;
-
-        plugins = Arrays.asList(
-                new BlobLengthPlugin(),
-                new CollectionLengthPlugin(),
-                new IdRefPlugin(),
-                new MapLengthPlugin(),
-                new PatternTraitPlugin(),
-                new RangeTraitPlugin(),
-                new StringEnumPlugin(),
-                new StringLengthPlugin(),
-                timestampValidationStrategy
-        );
     }
 
     public static Builder builder() {
@@ -168,9 +148,9 @@ private List<ValidationEvent> validateNaturalNumber(Shape shape, Long min, Long
         return value.asNumberNode()
                 .map(number -> {
                     if (!number.isNaturalNumber()) {
-                        return ListUtils.of(event(
-                                shape.getType() + " shapes must not have floating point values, but found `"
-                                + number.getValue() + "` provided for `" + shape.getId() + "`"));
+                        return ListUtils.of(event(String.format(
+                                "%s shapes must not have floating point values, but found `%s` provided for `%s`",
+                                shape.getType(), number.getValue(), shape.getId())));
                     }
 
                     Long numberValue = number.getValue().longValue();
@@ -273,19 +253,21 @@ public List<ValidationEvent> structureShape(StructureShape shape) {
                     object.getMembers().forEach((keyNode, value) -> {
                         String key = keyNode.getValue();
                         if (!members.containsKey(key)) {
-                            events.add(event("Invalid structure member `" + key + "` found for `"
-                                             + shape.getId() + "`", Severity.WARNING));
+                            String message = String.format(
+                                    "Invalid structure member `%s` found for `%s`", key, shape.getId());
+                            events.add(event(message, Severity.WARNING));
                         } else {
                             events.addAll(withNode(key, value).memberShape(members.get(key)));
                         }
                     });
+
                     members.forEach((memberName, member) -> {
                         if (member.isRequired()
                                 && !object.getMember(memberName).isPresent()
                                 // Ignore missing required primitive members because they have a default value.
                                 && !isMemberPrimitive(member)) {
-                            events.add(event("Missing required structure member `" + memberName + "` for `"
-                                             + shape.getId() + "`"));
+                            events.add(event(String.format(
+                                    "Missing required structure member `%s` for `%s`", memberName, shape.getId())));
                         }
                     });
                     return events;
@@ -309,8 +291,8 @@ public List<ValidationEvent> unionShape(UnionShape shape) {
                         object.getMembers().forEach((keyNode, value) -> {
                             String key = keyNode.getValue();
                             if (!members.containsKey(key)) {
-                                events.add(event(
-                                        "Invalid union member `" + key + "` found for `" + shape.getId() + "`"));
+                                events.add(event(String.format(
+                                        "Invalid union member `%s` found for `%s`", key, shape.getId())));
                             } else {
                                 events.addAll(withNode(key, value).memberShape(members.get(key)));
                             }
@@ -373,20 +355,32 @@ private ValidationEvent event(String message) {
     }
 
     private ValidationEvent event(String message, Severity severity) {
+        return event(message, severity, value.getSourceLocation());
+    }
+
+    private ValidationEvent event(String message, Severity severity, SourceLocation sourceLocation) {
         return ValidationEvent.builder()
                 .eventId(eventId)
                 .severity(severity)
-                .sourceLocation(value.getSourceLocation())
+                .sourceLocation(sourceLocation)
                 .shapeId(eventShapeId)
                 .message(context.isEmpty() ? message : context + ": " + message)
                 .build();
     }
 
     private List<ValidationEvent> applyPlugins(Shape shape) {
-        return plugins.stream()
-                .flatMap(plugin -> plugin.apply(shape, value, model).stream())
-                .map(this::event)
-                .collect(Collectors.toList());
+        List<ValidationEvent> events = new ArrayList<>();
+        timestampValidationStrategy.apply(shape, value, model, (location, message) -> {
+            events.add(event(message, Severity.ERROR, location.getSourceLocation()));
+        });
+
+        for (NodeValidatorPlugin plugin : BUILTIN) {
+            plugin.apply(shape, value, model, (location, message) -> {
+                events.add(event(message, Severity.ERROR, location.getSourceLocation()));
+            });
+        }
+
+        return events;
     }
 
     /**
@@ -470,7 +464,6 @@ public Builder eventShapeId(ShapeId eventShapeId) {
          * @param timestampValidationStrategy Timestamp validation strategy.
          * @return Returns the builder.
          */
-        @SmithyUnstableApi
         public Builder timestampValidationStrategy(TimestampValidationStrategy timestampValidationStrategy) {
             this.timestampValidationStrategy = timestampValidationStrategy;
             return this;

smithy-model/src/main/java/software/amazon/smithy/model/validation/node/BlobLengthPlugin.java

@@ -16,8 +16,8 @@
 package software.amazon.smithy.model.validation.node;
 
 import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.function.BiConsumer;
+import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.StringNode;
 import software.amazon.smithy.model.shapes.BlobShape;
@@ -29,28 +29,35 @@
  * Validates length trait on blob shapes and members that target blob shapes.
  */
 @SmithyInternalApi
-public final class BlobLengthPlugin extends MemberAndShapeTraitPlugin<BlobShape, StringNode, LengthTrait> {
-    public BlobLengthPlugin() {
+final class BlobLengthPlugin extends MemberAndShapeTraitPlugin<BlobShape, StringNode, LengthTrait> {
+
+    BlobLengthPlugin() {
         super(BlobShape.class, StringNode.class, LengthTrait.class);
     }
 
     @Override
-    protected List<String> check(Shape shape, LengthTrait trait, StringNode node, Model model) {
+    protected void check(
+            Shape shape,
+            LengthTrait trait,
+            StringNode node,
+            Model model,
+            BiConsumer<FromSourceLocation, String> emitter
+    ) {
         String value = node.getValue();
-        List<String> messages = new ArrayList<>();
         int size = value.getBytes(Charset.forName("UTF-8")).length;
+
         trait.getMin().ifPresent(min -> {
             if (size < min) {
-                messages.add("Value provided for `" + shape.getId() + "` must have at least "
-                             + min + " bytes, but the provided value only has " + size + " bytes");
+                emitter.accept(node, "Value provided for `" + shape.getId() + "` must have at least "
+                                     + min + " bytes, but the provided value only has " + size + " bytes");
             }
         });
+
         trait.getMax().ifPresent(max -> {
             if (value.getBytes(Charset.forName("UTF-8")).length > max) {
-                messages.add("Value provided for `" + shape.getId() + "` must have no more than "
-                             + max + " bytes, but the provided value has " + size + " bytes");
+                emitter.accept(node, "Value provided for `" + shape.getId() + "` must have no more than "
+                                     + max + " bytes, but the provided value has " + size + " bytes");
             }
         });
-        return messages;
     }
 }

smithy-model/src/main/java/software/amazon/smithy/model/validation/node/CollectionLengthPlugin.java

@@ -15,8 +15,8 @@
 
 package software.amazon.smithy.model.validation.node;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.function.BiConsumer;
+import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.ArrayNode;
 import software.amazon.smithy.model.shapes.CollectionShape;
@@ -29,28 +29,34 @@
  * target them.
  */
 @SmithyInternalApi
-public final class CollectionLengthPlugin extends MemberAndShapeTraitPlugin<CollectionShape, ArrayNode, LengthTrait> {
-    public CollectionLengthPlugin() {
+final class CollectionLengthPlugin extends MemberAndShapeTraitPlugin<CollectionShape, ArrayNode, LengthTrait> {
+
+    CollectionLengthPlugin() {
         super(CollectionShape.class, ArrayNode.class, LengthTrait.class);
     }
 
     @Override
-    protected List<String> check(Shape shape, LengthTrait trait, ArrayNode node, Model model) {
-        List<String> messages = new ArrayList<>();
+    protected void check(
+            Shape shape,
+            LengthTrait trait,
+            ArrayNode node,
+            Model model,
+            BiConsumer<FromSourceLocation, String> emitter
+    ) {
         trait.getMin().ifPresent(min -> {
             if (node.size() < min) {
-                messages.add(String.format(
+                emitter.accept(node, String.format(
                         "Value provided for `%s` must have at least %d elements, but the provided value only "
                         + "has %d elements", shape.getId(), min, node.size()));
             }
         });
+
         trait.getMax().ifPresent(max -> {
             if (node.size() > max) {
-                messages.add(String.format(
+                emitter.accept(node, String.format(
                         "Value provided for `%s` must have no more than %d elements, but the provided value "
                         + "has %d elements", shape.getId(), max, node.size()));
             }
         });
-        return messages;
     }
 }

smithy-model/src/main/java/software/amazon/smithy/model/validation/node/FilteredPlugin.java

@@ -15,11 +15,11 @@
 
 package software.amazon.smithy.model.validation.node;
 
-import java.util.List;
+import java.util.function.BiConsumer;
+import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.shapes.Shape;
-import software.amazon.smithy.utils.ListUtils;
 
 abstract class FilteredPlugin<S extends Shape, N extends Node> implements NodeValidatorPlugin {
     private final Class<S> shapeClass;
@@ -30,14 +30,13 @@ abstract class FilteredPlugin<S extends Shape, N extends Node> implements NodeVa
         this.nodeClass = nodeClass;
     }
 
+    @Override
     @SuppressWarnings("unchecked")
-    public final List<String> apply(Shape shape, Node node, Model model) {
-        if (shapeClass.isInstance(shape) && nodeClass.isInstance(node)) {
-            return check((S) shape, (N) node, model);
-        } else {
-            return ListUtils.of();
+    public final void apply(Shape shape, Node value, Model model, BiConsumer<FromSourceLocation, String> emitter) {
+        if (shapeClass.isInstance(shape) && nodeClass.isInstance(value)) {
+            check((S) shape, (N) value, model, emitter);
         }
     }
 
-    abstract List<String> check(S shape, N node, Model model);
+    abstract void check(S shape, N node, Model model, BiConsumer<FromSourceLocation, String> emitter);
 }

smithy-model/src/main/java/software/amazon/smithy/model/validation/node/IdRefPlugin.java

@@ -15,15 +15,15 @@
 
 package software.amazon.smithy.model.validation.node;
 
-import java.util.List;
+import java.util.function.BiConsumer;
+import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceException;
 import software.amazon.smithy.model.node.StringNode;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.traits.IdRefTrait;
-import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.SmithyInternalApi;
 
 /**
@@ -32,30 +32,36 @@
  * matching the selector.
  */
 @SmithyInternalApi
-public final class IdRefPlugin extends MemberAndShapeTraitPlugin<StringShape, StringNode, IdRefTrait> {
+final class IdRefPlugin extends MemberAndShapeTraitPlugin<StringShape, StringNode, IdRefTrait> {
 
-    public IdRefPlugin() {
+    IdRefPlugin() {
         super(StringShape.class, StringNode.class, IdRefTrait.class);
     }
 
     @Override
-    protected List<String> check(Shape shape, IdRefTrait trait, StringNode node, Model model) {
+    protected void check(
+            Shape shape,
+            IdRefTrait trait,
+            StringNode node,
+            Model model,
+            BiConsumer<FromSourceLocation, String> emitter
+    ) {
         try {
             ShapeId target = node.expectShapeId();
             Shape resolved = model.getShape(target).orElse(null);
 
             if (resolved == null) {
-                return trait.failWhenMissing()
-                       ? failWhenNoMatch(trait, String.format("Shape ID `%s` was not found in the model", target))
-                       : ListUtils.of();
-            } else if (matchesSelector(trait, resolved.getId(), model)) {
-                return ListUtils.of();
+                if (trait.failWhenMissing()) {
+                    failWhenNoMatch(node, trait, emitter, String.format(
+                            "Shape ID `%s` was not found in the model", target));
+                }
+            } else if (!matchesSelector(trait, resolved.getId(), model)) {
+                failWhenNoMatch(node, trait, emitter, String.format(
+                        "Shape ID `%s` does not match selector `%s`",
+                        resolved.getId(), trait.getSelector()));
             }
-
-            return failWhenNoMatch(trait, String.format(
-                    "Shape ID `%s` does not match selector `%s`", resolved.getId(), trait.getSelector()));
         } catch (SourceException e) {
-            return ListUtils.of(e.getMessage());
+            emitter.accept(node, e.getMessageWithoutLocation());
         }
     }
 
@@ -65,7 +71,12 @@ private boolean matchesSelector(IdRefTrait trait, ShapeId needle, Model haystack
                 .anyMatch(shapeId -> shapeId.equals(needle));
     }
 
-    private List<String> failWhenNoMatch(IdRefTrait trait, String message) {
-        return ListUtils.of(trait.getErrorMessage().orElse(message));
+    private void failWhenNoMatch(
+            FromSourceLocation location,
+            IdRefTrait trait,
+            BiConsumer<FromSourceLocation, String> emitter,
+            String message
+    ) {
+        emitter.accept(location, trait.getErrorMessage().orElse(message));
     }
 }