Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CLI output improvements #801

Merged
merged 1 commit into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"));
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -128,6 +134,7 @@ public ModelAssembler copy() {
assembler.disablePrelude = disablePrelude;
assembler.properties.putAll(properties);
assembler.disableValidation = disableValidation;
assembler.validationEventListener = validationEventListener;
return assembler;
}

Expand All @@ -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
Expand All @@ -163,6 +171,7 @@ public ModelAssembler reset() {
documentNodes.clear();
disablePrelude = false;
disableValidation = false;
validationEventListener = null;
return this;
}

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand Down
Loading