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

Add csv output format to validate command #2133

Merged
merged 1 commit into from
Feb 12, 2024
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 @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -326,6 +327,7 @@ protected final Model createNewModel(ModelBuilder builder, List<String> models,
.models(models)
.titleLabel("NEW", ColorTheme.DIFF_EVENT_TITLE)
.config(config)
.disableOutputFormatFraming(true) // don't repeat things like CSV headers.
.build();
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,31 @@ final class ModelBuilder {
private ValidatedResult<Model> 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;
}

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
mtdowling marked this conversation as resolved.
Show resolved Hide resolved
}

@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<String> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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\""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
$version: "2.0"

namespace smithy.example

@deprecated
string Hello

structure Foo {
hello: Hello
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
$version: "2.0"

namespace smithy.example

@deprecated
integer Hello

structure Foo {
hello: Hello
}
Loading