From 4139b3e78b605f59a7921ce99a8717f8bdbae18a Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Thu, 8 Feb 2024 09:57:52 -0600 Subject: [PATCH] Add csv output format to validate and diff command You can now pass `--format text|csv` to the validate and diff commands to get CSV or text output. Text continues to be the default when not specified. This commit also updates the diff command to now correctly output events to stdout rather than stderr. --- .../amazon/smithy/cli/DiffCommandTest.java | 18 ++++ .../smithy/cli/commands/DiffCommand.java | 5 +- .../smithy/cli/commands/ModelBuilder.java | 31 +++++- .../smithy/cli/commands/ValidateCommand.java | 1 + .../ValidationEventFormatOptions.java | 97 +++++++++++++++++++ .../smithy/cli/commands/DiffCommandTest.java | 39 ++++++++ .../cli/commands/ValidateCommandTest.java | 37 ++++++- .../smithy/cli/commands/diff/new.smithy | 10 ++ .../smithy/cli/commands/diff/old.smithy | 10 ++ 9 files changed, 245 insertions(+), 3 deletions(-) create mode 100644 smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidationEventFormatOptions.java create mode 100644 smithy-cli/src/test/java/software/amazon/smithy/cli/commands/DiffCommandTest.java create mode 100644 smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/new.smithy create mode 100644 smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/old.smithy diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java index fb2313380ed..05ea1375310 100644 --- a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java +++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java @@ -71,6 +71,24 @@ public void showsLabelForNewModelEvents() { }); } + @Test + public void canWriteCsvOutput() { + IntegUtils.withTempDir("diff", dir -> { + Path a = dir.resolve("a.smithy"); + writeFile(a, "$version: \"2.0\"\nnamespace example\nstring A\n"); + + Path b = dir.resolve("b.smithy"); + writeFile(b, "$version: \"2.0\"\nnamespace example\n@aaaaaa\nstring A\n"); + + RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), + "--new", b.toString(), + "--format", "csv")); + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); + assertThat(result.getOutput(), containsString("severity,id,")); + assertThat(result.getOutput(), containsString("ERROR")); + }); + } + @Test public void showsLabelForDiffEvents() { IntegUtils.withTempDir("diff", dir -> { diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java index e6567e1ae29..9ff05b87fde 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java @@ -67,6 +67,7 @@ public int execute(Arguments arguments, Env env) { arguments.addReceiver(new ConfigOptions()); arguments.addReceiver(new ValidatorOptions()); arguments.addReceiver(new BuildOptions()); + arguments.addReceiver(new ValidationEventFormatOptions()); arguments.addReceiver(new Options()); arguments.getReceiver(BuildOptions.class).noPositionalArguments(true); @@ -314,7 +315,7 @@ protected final ModelBuilder createModelBuilder(SmithyBuildConfig config, Argume .config(config) .arguments(arguments) .env(env) - .validationPrinter(env.stderr()) + .validationPrinter(env.stdout()) // Only report issues that fail the build. .validationMode(Validator.Mode.QUIET_CORE_ONLY) .defaultSeverity(Severity.DANGER); @@ -326,6 +327,7 @@ protected final Model createNewModel(ModelBuilder builder, List models, .models(models) .titleLabel("NEW", ColorTheme.DIFF_EVENT_TITLE) .config(config) + .disableOutputFormatFraming(true) // don't repeat things like CSV headers. .build(); } @@ -337,6 +339,7 @@ protected final void runDiff(ModelBuilder builder, Env env, Model oldModel, Mode .titleLabel("DIFF", ColorTheme.DIFF_TITLE) .validatedResult(new ValidatedResult<>(newModel, events)) .defaultSeverity(null) // reset so it takes on standard option settings. + .disableOutputFormatFraming(true) // don't repeat things like CSV headers. .build(); } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java index de67ab126b7..fc9ef35e7e5 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java @@ -61,10 +61,31 @@ final class ModelBuilder { private ValidatedResult validatedResult; private String titleLabel; private Style[] titleLabelStyles; + private ValidationEventFormatOptions.Format validationOutputFormat; + private boolean disableOutputFormatFraming = false; private boolean disableConfigModels; public ModelBuilder arguments(Arguments arguments) { this.arguments = arguments; + + // Determine how to format the output, whether it's text (the default) or CSV. + // Only some commands (like validate) actually let you customize the output format, so assume a default. + if (validationOutputFormat == null) { + validationOutputFormat(arguments.hasReceiver(ValidationEventFormatOptions.class) + ? arguments.getReceiver(ValidationEventFormatOptions.class).format() + : ValidationEventFormatOptions.Format.TEXT); + } + + return this; + } + + public ModelBuilder disableOutputFormatFraming(boolean disableOutputFormatFraming) { + this.disableOutputFormatFraming = disableOutputFormatFraming; + return this; + } + + public ModelBuilder validationOutputFormat(ValidationEventFormatOptions.Format validationOutputFormat) { + this.validationOutputFormat = validationOutputFormat; return this; } @@ -180,14 +201,22 @@ public Model build() { .titleLabel(titleLabel, titleLabelStyles) .build(); + if (!disableOutputFormatFraming) { + validationOutputFormat.beginPrinting(validationPrinter); + } + for (ValidationEvent event : sortedEvents) { // Only log events that are >= --severity. Note that setting --quiet inherently // configures events to need to be >= DANGER. Also filter using --show-validators and --hide-validators. if (validatorOptions.isVisible(event)) { - validationPrinter.println(formatter.format(event)); + validationOutputFormat.print(validationPrinter, formatter, event); } } + if (!disableOutputFormatFraming) { + validationOutputFormat.endPrinting(validationPrinter); + } + env.flush(); // Note: disabling validation will still show a summary of failures if the model can't be loaded. Validator.validate(validationMode != Validator.Mode.ENABLE, colors, stderr, validatedResult); diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidateCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidateCommand.java index 34c7ea5ea52..aca6d6a49f0 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidateCommand.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidateCommand.java @@ -48,6 +48,7 @@ public int execute(Arguments arguments, Env env) { arguments.addReceiver(new DiscoveryOptions()); arguments.addReceiver(new ValidatorOptions()); arguments.addReceiver(new BuildOptions()); + arguments.addReceiver(new ValidationEventFormatOptions()); CommandAction action = HelpActionWrapper.fromCommand( this, diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidationEventFormatOptions.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidationEventFormatOptions.java new file mode 100644 index 00000000000..7e1b265bec4 --- /dev/null +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidationEventFormatOptions.java @@ -0,0 +1,97 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.cli.commands; + +import java.util.function.Consumer; +import software.amazon.smithy.cli.ArgumentReceiver; +import software.amazon.smithy.cli.CliError; +import software.amazon.smithy.cli.CliPrinter; +import software.amazon.smithy.cli.HelpPrinter; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidationEventFormatter; + +final class ValidationEventFormatOptions implements ArgumentReceiver { + + private static final String VALIDATION_FORMAT = "--format"; + + enum Format { + TEXT { + @Override + void print(CliPrinter printer, ValidationEventFormatter formatter, ValidationEvent event) { + printer.println(formatter.format(event)); + } + }, + + CSV { + @Override + void beginPrinting(CliPrinter printer) { + printer.println("severity,id,shape,file,message,hint,suppressionReason"); + } + + @Override + void print(CliPrinter printer, ValidationEventFormatter formatter, ValidationEvent event) { + printer.println( + String.format("\"%s\",\"%s\",\"%s\",\"%s\",%d,%d,\"%s\",\"%s\",\"%s\"", + event.getSeverity().toString(), + formatCsv(event.getId()), + event.getShapeId().map(ShapeId::toString).orElse(""), + formatCsv(event.getSourceLocation().getFilename()), + event.getSourceLocation().getLine(), + event.getSourceLocation().getColumn(), + formatCsv(event.getMessage()), + formatCsv(event.getHint().orElse("")), + formatCsv(event.getSuppressionReason().orElse("")))); + } + }; + + void beginPrinting(CliPrinter printer) {} + + abstract void print(CliPrinter printer, ValidationEventFormatter formatter, ValidationEvent event); + + void endPrinting(CliPrinter printer) {} + + private static String formatCsv(String value) { + // Replace DQUOTE with DQUOTEDQUOTE, escape newlines, and escape carriage returns. + return value.replace("\"", "\"\"").replace("\n", "\\n").replace("\r", "\\r"); + } + } + + private Format format = Format.TEXT; + + @Override + public void registerHelp(HelpPrinter printer) { + printer.param(VALIDATION_FORMAT, null, "text|csv", + "Specifies the format to write validation events (text or csv). Defaults to text."); + } + + @Override + public Consumer testParameter(String name) { + if (name.equals(VALIDATION_FORMAT)) { + return s -> { + switch (s) { + case "csv": + format(Format.CSV); + break; + case "text": + format(Format.TEXT); + break; + default: + throw new CliError("Unexpected " + VALIDATION_FORMAT + ": `" + s + "`"); + } + }; + } + return null; + } + + void format(Format format) { + this.format = format; + } + + Format format() { + return format; + } +} diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/DiffCommandTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/DiffCommandTest.java new file mode 100644 index 00000000000..f71620cb39f --- /dev/null +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/DiffCommandTest.java @@ -0,0 +1,39 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.cli.commands; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +import java.nio.file.Path; +import java.nio.file.Paths; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.cli.CliUtils; + +public class DiffCommandTest { + @Test + public void canOutputCsv() throws Exception { + Path oldModel = Paths.get(getClass().getResource("diff/old.smithy").toURI()); + Path newModel = Paths.get(getClass().getResource("diff/new.smithy").toURI()); + CliUtils.Result result = CliUtils.runSmithy("diff", + "--old", oldModel.toString(), + "--new", newModel.toString(), + "--format", "csv"); + + assertThat(result.code(), not(0)); + + // Make sure FAILURE is sent to stderr. + assertThat(result.stderr(), containsString("FAILURE")); + assertThat(result.stdout(), not(containsString("FAILURE"))); + + String[] lines = result.stdout().split("(\\r\\n|\\r|\\n)"); + assertThat(lines.length, is(2)); + assertThat(lines[0], containsString("severity,id,shape,file,message,hint,suppressionReason")); + assertThat(lines[1], containsString("\"ERROR\",\"ChangedShapeType\",\"smithy.example#Hello\"")); + } +} diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java index 942ead46723..91654f57c6b 100644 --- a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java @@ -18,7 +18,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import java.net.URISyntaxException; @@ -216,4 +215,40 @@ public void canHideEventsById() throws Exception { assertThat(result.stdout(), not(containsString("EmitDangers"))); assertThat(result.stdout(), containsString("HttpLabelTrait")); } + + @Test + public void canOutputCsv() throws Exception { + Path validationEventsModel = Paths.get(getClass().getResource("validation-events.smithy").toURI()); + CliUtils.Result result = CliUtils.runSmithy("validate", "--format", "csv", + validationEventsModel.toString()); + + assertThat(result.code(), not(0)); + assertThat(result.stdout(), containsString("suppressionReason")); + assertThat(result.stdout(), containsString("EmitWarnings")); + assertThat(result.stdout(), containsString("EmitDangers")); + assertThat(result.stdout(), containsString("HttpLabelTrait")); + assertThat(result.stdout(), not(containsString("FAILURE"))); // stderr + } + + @Test + public void canOutputText() throws Exception { + Path validationEventsModel = Paths.get(getClass().getResource("validation-events.smithy").toURI()); + CliUtils.Result result = CliUtils.runSmithy("validate", "--format", "text", + validationEventsModel.toString()); + + assertThat(result.code(), not(0)); + assertThat(result.stdout(), not(containsString("suppressionReason"))); + assertThat(result.stdout(), containsString("EmitWarnings")); + assertThat(result.stdout(), containsString("EmitDangers")); + assertThat(result.stdout(), containsString("HttpLabelTrait")); + assertThat(result.stdout(), not(containsString("FAILURE"))); // stderr + } + + @Test + public void outputFormatMustBeValid() { + CliUtils.Result result = CliUtils.runSmithy("validate", "--format", "HELLO"); + + assertThat(result.code(), not(0)); + assertThat(result.stderr(), containsString("Unexpected --format: `HELLO`")); + } } diff --git a/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/new.smithy b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/new.smithy new file mode 100644 index 00000000000..cdae50e0181 --- /dev/null +++ b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/new.smithy @@ -0,0 +1,10 @@ +$version: "2.0" + +namespace smithy.example + +@deprecated +string Hello + +structure Foo { + hello: Hello +} diff --git a/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/old.smithy b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/old.smithy new file mode 100644 index 00000000000..9bfda345bb0 --- /dev/null +++ b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/old.smithy @@ -0,0 +1,10 @@ +$version: "2.0" + +namespace smithy.example + +@deprecated +integer Hello + +structure Foo { + hello: Hello +}