Make CLI output improvements

This commit closes #797

The CLI will now log validation events as they occur rather than waiting
until all events are encountered before writing them. This makes the CLI
a bit more responsive when validating hundreds of thousands of shapes.

Other improvements were added to the CLI output as well, including the
--severity parameter to set the minimum severity to display in output.
This can significantly cut down on noise when model files emit things <
DANGER.

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

@@ -29,6 +29,7 @@ public final class SmithyCli {
     public static final String DISCOVER = "--discover";
     public static final String DISCOVER_CLASSPATH = "--discover-classpath";
     public static final String ALLOW_UNKNOWN_TRAITS = "--allow-unknown-traits";
+    public static final String SEVERITY = "--severity";
 
     private ClassLoader classLoader = getClass().getClassLoader();
 

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

@@ -68,6 +68,9 @@ public Parser getParser() {
                 .option(SmithyCli.DISCOVER, "-d", "Enables model discovery, merging in models found inside of jars")
                 .parameter(SmithyCli.DISCOVER_CLASSPATH, "Enables model discovery using a custom classpath for models")
                 .option(SmithyCli.ALLOW_UNKNOWN_TRAITS, "Ignores unknown traits when building models")
+                .parameter(SmithyCli.SEVERITY, "Sets a minimum validation event severity to display. "
+                                               + "Defaults to NOTE. Can be set to SUPPRESSED, NOTE, WARNING, "
+                                               + "DANGER, ERROR.")
                 .positional("<MODELS>", "Path to Smithy models or directories")
                 .build();
     }

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

@@ -21,14 +21,20 @@
 import java.nio.file.Paths;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.logging.Logger;
 import software.amazon.smithy.cli.Arguments;
+import software.amazon.smithy.cli.Cli;
 import software.amazon.smithy.cli.CliError;
+import software.amazon.smithy.cli.Colors;
 import software.amazon.smithy.cli.SmithyCli;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
+import software.amazon.smithy.model.validation.ContextualValidationEventFormatter;
+import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
 
 final class CommandUtils {
@@ -40,6 +46,32 @@ private CommandUtils() {}
     static Model buildModel(Arguments arguments, ClassLoader classLoader, Set<Validator.Feature> features) {
         List<String> models = arguments.positionalArguments();
         ModelAssembler assembler = CommandUtils.createModelAssembler(classLoader);
+
+        ContextualValidationEventFormatter formatter = new ContextualValidationEventFormatter();
+        boolean stdout = features.contains(Validator.Feature.STDOUT);
+        boolean quiet = features.contains(Validator.Feature.QUIET);
+        Consumer<String> writer = stdout ? Cli.getStdout() : Cli.getStderr();
+
+        // --severity defaults to NOTE.
+        Severity minSeverity = arguments.has(SmithyCli.SEVERITY)
+                ? parseSeverity(arguments.parameter(SmithyCli.SEVERITY))
+                : Severity.NOTE;
+
+        assembler.validationEventListener(event -> {
+            // Only log events that are >= --severity.
+            if (event.getSeverity().ordinal() >= minSeverity.ordinal()) {
+                if (event.getSeverity() == Severity.WARNING && !quiet) {
+                    // Only log warnings when not quiet
+                    Colors.YELLOW.write(writer, formatter.format(event) + System.lineSeparator());
+                } else if (event.getSeverity() == Severity.DANGER || event.getSeverity() == Severity.ERROR) {
+                    // Always output error and danger events, even when quiet.
+                    Colors.RED.write(writer, formatter.format(event) + System.lineSeparator());
+                } else if (!quiet) {
+                    writer.accept(formatter.format(event) + System.lineSeparator());
+                }
+            }
+        });
+
         CommandUtils.handleModelDiscovery(arguments, assembler, classLoader);
         CommandUtils.handleUnknownTraitsOption(arguments, assembler);
         models.forEach(assembler::addImport);
@@ -48,6 +80,11 @@ static Model buildModel(Arguments arguments, ClassLoader classLoader, Set<Valida
         return result.getResult().orElseThrow(() -> new RuntimeException("Expected Validator to throw"));
     }
 
+    static Severity parseSeverity(String str) {
+        return Severity.fromString(str).orElseThrow(() -> new IllegalArgumentException(
+                "Invalid severity: " + str + ". Expected one of: " + Arrays.toString(Severity.values())));
+    }
+
     static ModelAssembler createModelAssembler(ClassLoader classLoader) {
         return Model.assembler(classLoader).putProperty(ModelAssembler.DISABLE_JAR_CACHE, true);
     }

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

@@ -42,6 +42,9 @@ public Parser getParser() {
                 .option(SmithyCli.ALLOW_UNKNOWN_TRAITS, "Ignores unknown traits when validating models")
                 .option(SmithyCli.DISCOVER, "-d", "Enables model discovery, merging in models found inside of jars")
                 .parameter(SmithyCli.DISCOVER_CLASSPATH, "Enables model discovery using a custom classpath for models")
+                .parameter(SmithyCli.SEVERITY, "Sets a minimum validation event severity to display. "
+                                               + "Defaults to NOTE. Can be set to SUPPRESSED, NOTE, WARNING, "
+                                               + "DANGER, ERROR.")
                 .positional("<MODELS>", "Path to Smithy models or directories")
                 .build();
     }

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

@@ -21,9 +21,7 @@
 import java.util.function.Consumer;
 import software.amazon.smithy.cli.Cli;
 import software.amazon.smithy.cli.CliError;
-import software.amazon.smithy.cli.Colors;
 import software.amazon.smithy.model.Model;
-import software.amazon.smithy.model.validation.ContextualValidationEventFormatter;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
 
@@ -46,26 +44,10 @@ enum Feature {
     }
 
     static void validate(ValidatedResult<Model> result, Set<Feature> features) {
-        ContextualValidationEventFormatter formatter = new ContextualValidationEventFormatter();
-
-        boolean stdout = features.contains(Feature.STDOUT);
         boolean quiet = features.contains(Feature.QUIET);
+        boolean stdout = features.contains(Feature.STDOUT);
         Consumer<String> writer = stdout ? Cli.getStdout() : Cli.getStderr();
 
-        result.getValidationEvents().stream()
-                .filter(event -> event.getSeverity() != Severity.SUPPRESSED)
-                .sorted()
-                .forEach(event -> {
-                    if (event.getSeverity() == Severity.WARNING) {
-                        Colors.YELLOW.write(writer, formatter.format(event));
-                    } else if (event.getSeverity() == Severity.DANGER || event.getSeverity() == Severity.ERROR) {
-                        Colors.RED.write(writer, formatter.format(event));
-                    } else {
-                        writer.accept(event.toString());
-                    }
-                    writer.accept("");
-                });
-
         long errors = result.getValidationEvents(Severity.ERROR).size();
         long dangers = result.getValidationEvents(Severity.DANGER).size();
 

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

@@ -17,15 +17,18 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.net.URISyntaxException;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.cli.CliError;
 import software.amazon.smithy.cli.SmithyCli;
+import software.amazon.smithy.model.validation.Severity;
 
 public class ValidateCommandTest {
     @Test
@@ -72,4 +75,83 @@ public void allowsUnknownTrait() throws URISyntaxException {
         String model = Paths.get(getClass().getResource("unknown-trait.smithy").toURI()).toString();
         SmithyCli.create().run("validate", "--allow-unknown-traits", model);
     }
+
+    @Test
+    public void canSetSeverityToSuppressed() throws Exception {
+        String result = runValidationEventsTest(Severity.SUPPRESSED);
+
+        assertThat(result, containsString("EmitSuppressed"));
+        assertThat(result, containsString("EmitNotes"));
+        assertThat(result, containsString("EmitWarnings"));
+        assertThat(result, containsString("EmitDangers"));
+        assertThat(result, containsString("TraitTarget"));
+    }
+
+    @Test
+    public void canSetSeverityToNote() throws Exception {
+        String result = runValidationEventsTest(Severity.NOTE);
+
+        assertThat(result, not(containsString("EmitSuppressed")));
+        assertThat(result, containsString("EmitNotes"));
+        assertThat(result, containsString("EmitWarnings"));
+        assertThat(result, containsString("EmitDangers"));
+        assertThat(result, containsString("TraitTarget"));
+    }
+
+    @Test
+    public void canSetSeverityToWarning() throws Exception {
+        String result = runValidationEventsTest(Severity.WARNING);
+
+        assertThat(result, not(containsString("EmitSuppressed")));
+        assertThat(result, not(containsString("EmitNotes")));
+        assertThat(result, containsString("EmitWarnings"));
+        assertThat(result, containsString("EmitDangers"));
+        assertThat(result, containsString("TraitTarget"));
+    }
+
+    @Test
+    public void canSetSeverityToDanger() throws Exception {
+        String result = runValidationEventsTest(Severity.DANGER);
+
+        assertThat(result, not(containsString("EmitSuppressed")));
+        assertThat(result, not(containsString("EmitNotes")));
+        assertThat(result, not(containsString("EmitWarnings")));
+        assertThat(result, containsString("EmitDangers"));
+        assertThat(result, containsString("TraitTarget"));
+    }
+
+    @Test
+    public void canSetSeverityToError() throws Exception {
+        String result = runValidationEventsTest(Severity.ERROR);
+
+        assertThat(result, not(containsString("EmitSuppressed")));
+        assertThat(result, not(containsString("EmitNotes")));
+        assertThat(result, not(containsString("EmitWarnings")));
+        assertThat(result, not(containsString("EmitDangers")));
+        assertThat(result, containsString("TraitTarget"));
+    }
+
+    private String runValidationEventsTest(Severity severity) throws Exception {
+        PrintStream err = System.err;
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        PrintStream printStream = new PrintStream(outputStream);
+        System.setErr(printStream);
+
+        Path validationEventsModel = Paths.get(getClass().getResource("validation-events.smithy").toURI());
+        try {
+            SmithyCli.create().run("validate", "--severity", severity.toString(), validationEventsModel.toString());
+        } catch (RuntimeException e) {
+            // ignore the error since everything we need was captured via stderr.
+        }
+
+        System.setErr(err);
+        return outputStream.toString("UTF-8");
+    }
+
+    @Test
+    public void validatesSeverity() {
+        Assertions.assertThrows(
+                IllegalArgumentException.class,
+                () -> SmithyCli.create().run("validate", "--severity", "FOO"));
+    }
 }

smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy

@@ -0,0 +1,50 @@
+$version: "1.0"
+
+metadata validators = [
+    {
+        name: "EmitEachSelector",
+        id: "EmitSuppressed",
+        severity: "NOTE",
+        configuration: {
+            selector: "[id = smithy.example#Suppressed]"
+        }
+    },
+    {
+        name: "EmitEachSelector",
+        id: "EmitNotes",
+        severity: "NOTE",
+        configuration: {
+            selector: "[id = smithy.example#Note]"
+        }
+    },
+    {
+        name: "EmitEachSelector",
+        id: "EmitWarnings",
+        severity: "WARNING",
+        configuration: {
+            selector: "[id = smithy.example#Warning]"
+        }
+    },
+    {
+        name: "EmitEachSelector",
+        id: "EmitDangers",
+        severity: "DANGER",
+        configuration: {
+            selector: "[id = smithy.example#Danger]"
+        }
+    }
+]
+
+namespace smithy.example
+
+@suppress(["EmitSuppressed"])
+string Suppressed
+
+string Note
+
+string Warning
+
+string Danger
+
+@required // this trait is invalid and causes an error.
+string Error

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

@@ -33,6 +33,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -85,6 +86,10 @@ public final class ModelAssembler {
 
     private static final Logger LOGGER = Logger.getLogger(ModelAssembler.class.getName());
 
+    private static final Consumer<ValidationEvent> DEFAULT_EVENT_LISTENER = ValidationEvent -> {
+        // Ignore events by default.
+    };
+
     private TraitFactory traitFactory;
     private ValidatorFactory validatorFactory;
     private boolean disableValidation;
@@ -97,6 +102,7 @@ public final class ModelAssembler {
     private final Map<String, Node> metadata = new HashMap<>();
     private final Map<String, Object> properties = new HashMap<>();
     private boolean disablePrelude;
+    private Consumer<ValidationEvent> validationEventListener = DEFAULT_EVENT_LISTENER;
 
     // Lazy initialization holder class idiom to hold a default validator factory.
     private static final class LazyValidatorFactoryHolder {
@@ -128,6 +134,7 @@ public ModelAssembler copy() {
         assembler.disablePrelude = disablePrelude;
         assembler.properties.putAll(properties);
         assembler.disableValidation = disableValidation;
+        assembler.validationEventListener = validationEventListener;
         return assembler;
     }
 
@@ -146,6 +153,7 @@ public ModelAssembler copy() {
      *     <li>Shape registered via {@link #addModel}</li>
      *     <li>Metadata registered via {@link #putMetadata}</li>
      *     <li>Validation is re-enabled if it was disabled.</li>
+     *     <li>Validation event listener via {@link #validationEventListener(Consumer)}</li>
      * </ul>
      *
      * <p>The state of {@link #disablePrelude} is reset such that the prelude
@@ -163,6 +171,7 @@ public ModelAssembler reset() {
         documentNodes.clear();
         disablePrelude = false;
         disableValidation = false;
+        validationEventListener = null;
         return this;
     }
 
@@ -475,6 +484,24 @@ public ModelAssembler disableValidation() {
         return this;
     }
 
+    /**
+     * Sets a listener that is invoked each time a ValidationEvent is encountered
+     * while loading and validating the model.
+     *
+     * <p>The consumer could be invoked simultaneously by multiple threads. It's
+     * up to the consumer to perform any necessary synchronization. Providing
+     * an event listener is useful for things like CLIs so that events can
+     * be streamed to stdout as soon as they are encountered, rather than
+     * waiting until the entire model is parsed and validated.
+     *
+     * @param eventListener Listener invoked for each ValidationEvent.
+     * @return Returns the assembler.
+     */
+    public ModelAssembler validationEventListener(Consumer<ValidationEvent> eventListener) {
+        validationEventListener = eventListener == null ? DEFAULT_EVENT_LISTENER : eventListener;
+        return this;
+    }
+
     /**
      * Assembles the model and returns the validated result.
      *
@@ -599,6 +626,7 @@ private List<ModelFile> createModelFiles() {
 
     private ValidatedResult<Model> validate(Model model, TraitContainer traits, List<ValidationEvent> events) {
         validateTraits(model.getShapeIds(), traits, events);
+        events.forEach(validationEventListener);
 
         // If ERROR validation events occur while loading, then performing more
         // granular semantic validation will only obscure the root cause of errors.
@@ -611,7 +639,10 @@ private ValidatedResult<Model> validate(Model model, TraitContainer traits, List
         }
 
         // Validate the model based on the explicit validators and model metadata.
-        List<ValidationEvent> mergedEvents = ModelValidator.validate(model, validatorFactory, assembleValidators());
+        // Note the ModelValidator handles emitting events to the validationEventListener.
+        List<ValidationEvent> mergedEvents = ModelValidator
+                .validate(model, validatorFactory, assembleValidators(), validationEventListener);
+
         mergedEvents.addAll(events);
         return new ValidatedResult<>(model, mergedEvents);
     }