diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/ColorTheme.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/ColorTheme.java index 0e9e6e33f32..5e012d005fb 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/ColorTheme.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/ColorTheme.java @@ -47,5 +47,7 @@ public final class ColorTheme { public static final Style DIFF_TITLE = Style.of(Style.BG_BRIGHT_BLACK, Style.WHITE); public static final Style DIFF_EVENT_TITLE = Style.of(Style.BG_BRIGHT_BLUE, Style.BLACK); + public static final Style HINT_TITLE = Style.of(Style.BRIGHT_GREEN); + private ColorTheme() {} } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java index 728642db73b..550fdabde85 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java @@ -104,6 +104,13 @@ public String format(ValidationEvent event) { writeMessage(writer, event.getMessage()); writer.append(ls); + event.getHint().ifPresent(hint -> { + String hintLabel = "HINT: "; + writer.append(ls); + colors.style(writer, hintLabel, ColorTheme.HINT_TITLE); + writer.append(StyleHelper.formatMessage(hint, LINE_LENGTH - hintLabel.length(), colors)); + writer.append(ls); + }); return writer.toString(); } diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatterTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatterTest.java index 4d3c9a314e9..35593ce2f55 100644 --- a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatterTest.java +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatterTest.java @@ -51,23 +51,60 @@ public void formatsEventsWithNoColors() { } @Test - public void formatsEventsWithColors() { - PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR); - String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR); + public void formatsEventsWithHintWithNoColors() { + PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.NO_COLOR); + String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR, "Some hint"); assertThat(formatted, equalTo( "\n" - + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n" - + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n" - + "\u001B[90mFile: \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n" + + "── ERROR ───────────────────────────────────────────────────────────────── Foo\n" + + "Shape: smithy.example#Foo\n" + + "File: build/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\n" + "\n" - + "\u001B[90m4| \u001B[0m\n" - + "\u001B[90m5| \u001B[0mresource Foo {\n" - + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n" + + "4| \n" + + "5| resource Foo {\n" + + " | ^\n" + "\n" - + "Hello, \u001B[36mthere\u001B[0m\n")); + + "Hello, `there`\n\n" + + "HINT: Some hint\n")); // keeps ticks because formatting is disabled. + } + + @Test + public void formatsEventsWithColors() { + PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR); + String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR); + + assertThat(formatted, equalTo( + "\n" + + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n" + + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n" + + "\u001B[90mFile: \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n" + + "\n" + + "\u001B[90m4| \u001B[0m\n" + + "\u001B[90m5| \u001B[0mresource Foo {\n" + + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n" + + "\n" + + "Hello, \u001B[36mthere\u001B[0m\n")); } + @Test + public void formatsEventsWithHintWithColors() { + PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.FORCE_COLOR); + String formatted = formatTestEventWithSeverity(pretty, Severity.ERROR, "Some hint"); + + assertThat(formatted, equalTo( + "\n" + + "\u001B[31m── \u001B[0m\u001B[41;30m ERROR \u001B[0m\u001B[31m ───────────────────────────────────────────────────────────────── \u001B[0mFoo\n" + + "\u001B[90mShape: \u001B[0m\u001B[95msmithy.example#Foo\u001B[0m\n" + + "\u001B[90mFile: \u001B[0m\u001B[90mbuild/resources/test/software/amazon/smithy/cli/commands/valid-model.smithy:5:1\u001B[0m\n" + + "\n" + + "\u001B[90m4| \u001B[0m\n" + + "\u001B[90m5| \u001B[0mresource Foo {\n" + + "\u001B[90m |\u001B[0m \u001B[31m^\u001B[0m\n" + + "\n" + + "Hello, \u001B[36mthere\u001B[0m\n\n" + + "\u001B[92mHINT: \u001B[0mSome hint\n")); + } @Test public void doesNotIncludeSourceLocationNoneInOutput() { PrettyAnsiValidationFormatter pretty = createFormatter(AnsiColorFormatter.NO_COLOR); @@ -116,12 +153,17 @@ private PrettyAnsiValidationFormatter createFormatter(ColorFormatter colors) { } private String formatTestEventWithSeverity(PrettyAnsiValidationFormatter pretty, Severity severity) { + return formatTestEventWithSeverity(pretty, severity, null); + } + + private String formatTestEventWithSeverity(PrettyAnsiValidationFormatter pretty, Severity severity, String hint) { Model model = Model.assembler().addImport(getClass().getResource("valid-model.smithy")).assemble().unwrap(); ValidationEvent event = ValidationEvent.builder() .id("Foo") .severity(severity) .shape(model.expectShape(ShapeId.from("smithy.example#Foo"))) .message("Hello, `there`") + .hint(hint) .build(); return normalizeLinesAndFiles(pretty.format(event)); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java index 25e184beabd..132d1661b89 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java @@ -38,6 +38,7 @@ final class LoaderTraitMap { private static final Logger LOGGER = Logger.getLogger(LoaderTraitMap.class.getName()); + private static final String UNRESOLVED_TRAIT_SUFFIX = ".UnresolvedTrait"; private final TraitFactory traitFactory; private final Map> traits = new HashMap<>(); @@ -114,7 +115,7 @@ private void validateTraitIsKnown(ShapeId target, ShapeId traitId, Trait trait, if (!shapeMap.isRootShapeDefined(traitId) && (trait == null || !trait.isSynthetic())) { Severity severity = allowUnknownTraits ? Severity.WARNING : Severity.ERROR; events.add(ValidationEvent.builder() - .id(Validator.MODEL_ERROR) + .id(Validator.MODEL_ERROR + UNRESOLVED_TRAIT_SUFFIX) .severity(severity) .sourceLocation(sourceLocation) .shapeId(target) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java index 53c2abab1bc..365ffe1f94f 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java @@ -50,6 +50,7 @@ import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidationEventDecorator; import software.amazon.smithy.model.validation.Validator; import software.amazon.smithy.model.validation.ValidatorFactory; import software.amazon.smithy.utils.Pair; @@ -509,6 +510,9 @@ public ValidatedResult assemble() { if (traitFactory == null) { traitFactory = LazyTraitFactoryHolder.INSTANCE; } + if (validatorFactory == null) { + validatorFactory = ModelValidator.defaultValidationFactory(); + } Model prelude = disablePrelude ? null : Prelude.getPreludeModel(); LoadOperationProcessor processor = new LoadOperationProcessor( @@ -576,7 +580,8 @@ public ValidatedResult assemble() { } if (disableValidation) { - return new ValidatedResult<>(transformed, events); + List decorators = validatorFactory.loadDecorators(); + return new ValidatedResult<>(transformed, ModelValidator.decorateEvents(decorators, events)); } try { @@ -594,9 +599,11 @@ private void addMetadataToProcessor(Map metadataMap, LoadOperation } private ValidatedResult returnOnlyErrors(Model model, List events) { + List decorators = validatorFactory.loadDecorators(); return new ValidatedResult<>(model, events.stream() - .filter(event -> event.getSeverity() == Severity.ERROR) - .collect(Collectors.toList())); + .filter(event -> event.getSeverity() == Severity.ERROR) + .map(event -> ModelValidator.decorateEvent(decorators, event)) + .collect(Collectors.toList())); } private ValidatedResult validate(Model model, List events) { diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java index cdc8bbd30ea..27f540cb77e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java @@ -32,6 +32,7 @@ import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidationEventDecorator; import software.amazon.smithy.model.validation.Validator; import software.amazon.smithy.model.validation.ValidatorFactory; import software.amazon.smithy.model.validation.suppressions.Suppression; @@ -49,7 +50,6 @@ * loaded from metadata. */ final class ModelValidator { - private static final String SUPPRESSIONS = "suppressions"; // Lazy initialization holder class idiom to hold a default validator factory. @@ -169,6 +169,7 @@ public Validator createValidator() { } List staticValidators = resolveStaticValidators(); + List staticDecorators = validatorFactory.loadDecorators(); return model -> { List coreEvents = new ArrayList<>(); @@ -186,6 +187,8 @@ public Validator createValidator() { // which will only obscure the root cause. coreEvents.addAll(suppressEvents(model, new TargetValidator().validate(model), modelSuppressions)); coreEvents.addAll(suppressEvents(model, new ResourceCycleValidator().validate(model), modelSuppressions)); + // Decorate all the events + coreEvents = decorateEvents(staticDecorators, coreEvents); // Emit any events that have already occurred. coreEvents.forEach(eventListener); @@ -194,16 +197,18 @@ public Validator createValidator() { } List result = modelValidators.parallelStream() - .flatMap(validator -> validator.validate(model).stream()) + .map(validator -> validator.validate(model)) + .flatMap(Collection::stream) .filter(ModelValidator::filterPrelude) .map(event -> suppressEvent(model, event, modelSuppressions)) + .map(event -> decorateEvent(staticDecorators, event)) // Emit events as they occur during validation. .peek(eventListener) .collect(Collectors.toList()); for (ValidationEvent event : includeEvents) { if (ModelValidator.filterPrelude(event)) { - result.add(suppressEvent(model, event, modelSuppressions)); + result.add(decorateEvent(staticDecorators, suppressEvent(model, event, modelSuppressions))); } } @@ -214,6 +219,32 @@ public Validator createValidator() { }; } + static List decorateEvents( + List decorators, + List events + ) { + if (!decorators.isEmpty()) { + for (int idx = 0; idx < events.size(); idx++) { + events.set(idx, decorateEvent(decorators, events.get(idx))); + } + } + return events; + } + + static ValidationEvent decorateEvent(List decorators, ValidationEvent event) { + ValidationEvent decoratedEvent = event; + for (ValidationEventDecorator decorator : decorators) { + if (decorator.canDecorate(event)) { + decoratedEvent = decorator.decorate(decoratedEvent); + } + } + return decoratedEvent; + } + + static ValidatorFactory defaultValidationFactory() { + return LazyValidatorFactoryHolder.INSTANCE; + } + private List resolveStaticValidators() { List resolvedValidators = new ArrayList<>(validatorFactory.loadBuiltinValidators()); resolvedValidators.addAll(validators); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java index 03f7abbea62..e3b39670c19 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java @@ -29,6 +29,10 @@ public String format(ValidationEvent event) { if (reason != null) { message += " (" + reason + ")"; } + String hint = event.getHint().orElse(null); + if (hint != null) { + message += " [" + hint + "]"; + } return String.format( "[%s] %s: %s | %s %s:%s:%s", diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEvent.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEvent.java index d295d0162d5..f286c7d145e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEvent.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEvent.java @@ -48,6 +48,7 @@ public final class ValidationEvent private final Severity severity; private final ShapeId shapeId; private final String suppressionReason; + private final String hint; private int hash; private ValidationEvent(Builder builder) { @@ -61,6 +62,7 @@ private ValidationEvent(Builder builder) { this.eventId = SmithyBuilder.requiredState("id", builder.eventId); this.shapeId = builder.shapeId; this.suppressionReason = builder.suppressionReason; + this.hint = builder.hint; } public static Builder builder() { @@ -141,6 +143,7 @@ public Builder toBuilder() { builder.eventId = eventId; builder.shapeId = shapeId; builder.suppressionReason = suppressionReason; + builder.hint = hint; return builder; } @@ -158,14 +161,15 @@ public boolean equals(Object o) { && severity.equals(other.severity) && eventId.equals(other.eventId) && getShapeId().equals(other.getShapeId()) - && getSuppressionReason().equals(other.getSuppressionReason()); + && getSuppressionReason().equals(other.getSuppressionReason()) + && getHint().equals(other.getHint()); } @Override public int hashCode() { int result = hash; if (result == 0) { - result = Objects.hash(eventId, shapeId, severity, sourceLocation, message, suppressionReason); + result = Objects.hash(eventId, shapeId, severity, sourceLocation, message, suppressionReason, hint); hash = result; } return result; @@ -184,6 +188,7 @@ public Node toNode() { .withOptionalMember("shapeId", getShapeId().map(Object::toString).map(Node::from)) .withMember("message", Node.from(getMessage())) .withOptionalMember("suppressionReason", getSuppressionReason().map(Node::from)) + .withOptionalMember("hint", getHint().map(Node::from)) .withMember("filename", Node.from(getSourceLocation().getFilename())) .withMember("line", Node.from(getSourceLocation().getLine())) .withMember("column", Node.from(getSourceLocation().getColumn())) @@ -205,6 +210,7 @@ public static ValidationEvent fromNode(Node node) { .expectMember("severity", Severity::fromNode, builder::severity) .expectStringMember("message", builder::message) .getStringMember("suppressionReason", builder::suppressionReason) + .getStringMember("hint", builder::hint) .getMember("shapeId", ShapeId::fromNode, builder::shapeId); return builder.build(); } @@ -294,6 +300,15 @@ public Optional getSuppressionReason() { return Optional.ofNullable(suppressionReason); } + /** + * Get an optional hint that adds more detail about how to fix a specific issue. + * + * @return Returns the hint if available. + */ + public Optional getHint() { + return Optional.ofNullable(hint); + } + /** * Builds ValidationEvent values. */ @@ -305,6 +320,7 @@ public static final class Builder implements SmithyBuilder { private String eventId; private ShapeId shapeId; private String suppressionReason; + private String hint; private Builder() {} @@ -402,6 +418,17 @@ public Builder suppressionReason(String eventSuppressionReason) { return this; } + /** + * Sets an optional hint adding more detail about how to fix a specific issue. + * + * @param hint Hint to set + * @return Returns the builder. + */ + public Builder hint(String hint) { + this.hint = hint; + return this; + } + @Override public ValidationEvent build() { return new ValidationEvent(this); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEventDecorator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEventDecorator.java new file mode 100644 index 00000000000..a18560cbd7f --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationEventDecorator.java @@ -0,0 +1,40 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.validation; + +/** + * Validation event decorators take validation events and transform them by adding more contextual information, + * usually adding a hint to let the user know what can it be done to solve the issue. This might add context specific + * information that is not relevant for all cases such as links to internal knowledge sites or explicit instructions + * relevant only to the context where Smithy is being used. + */ +public interface ValidationEventDecorator { + /** + * Returns true if this decorator knows how to decorate this event, usually by looking at the event id. + * + * @param ev The event to test against + * @return true if this decorator knows how to decorate this event + */ + boolean canDecorate(ValidationEvent ev); + + /** + * Takes an event and potentially updates it to decorate it. Returns the same event if this decorators does not know + * how to handle the event. + * + * @return The decorated event or the original one if no decoration took place. + */ + ValidationEvent decorate(ValidationEvent ev); +} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidatorFactory.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidatorFactory.java index 844d934a20e..a338a37e4a8 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidatorFactory.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidatorFactory.java @@ -39,6 +39,15 @@ public interface ValidatorFactory { */ List loadBuiltinValidators(); + /** + * Returns a list of decorators. + * + * @return Returns the loaded decorators. + */ + default List loadDecorators() { + return Collections.emptyList(); + } + /** * Creates and configures a Validator by name. * @@ -85,6 +94,53 @@ public Optional createValidator(String name, ObjectNode configuration }; } + /** + * Creates a ValidatorFactory that uses a collection of built-in validators + * and a collection of ValidatorService instances for configurable validators. + * + *

The validators and services passed to this method are copied into a + * List before creating the actual factory to avoid holding on to + * {@code ServiceLoader} VM-wide instances that potentially utilize a + * Thread's context ClassLoader. + * + * @param validators Built-in validators to provide from the factory. + * @param services ValidatorService instances to use to create validators. + * @param decorators ValidationEventDecorator instances to use to create decorators. + * @return Returns the created ValidatorFactory. + */ + static ValidatorFactory createServiceFactory( + Iterable validators, + Iterable services, + Iterable decorators + ) { + List serviceList = new ArrayList<>(); + services.forEach(serviceList::add); + List validatorsList = new ArrayList<>(); + validators.forEach(validatorsList::add); + List decoratorsList = new ArrayList<>(); + decorators.forEach(decoratorsList::add); + + return new ValidatorFactory() { + @Override + public List loadBuiltinValidators() { + return Collections.unmodifiableList(validatorsList); + } + + @Override + public List loadDecorators() { + return Collections.unmodifiableList(decoratorsList); + } + + @Override + public Optional createValidator(String name, ObjectNode configuration) { + return serviceList.stream() + .filter(service -> service.getName().equals(name)) + .map(service -> service.createValidator(configuration)) + .findFirst(); + } + }; + } + /** * Creates a ValidatorFactory that discovers service providers using * the given ClassLoader. @@ -95,6 +151,7 @@ public Optional createValidator(String name, ObjectNode configuration static ValidatorFactory createServiceFactory(ClassLoader classLoader) { return createServiceFactory( ServiceLoader.load(Validator.class, classLoader), - ServiceLoader.load(ValidatorService.class, classLoader)); + ServiceLoader.load(ValidatorService.class, classLoader), + ServiceLoader.load(ValidationEventDecorator.class, classLoader)); } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java index bbee958916e..2070e54fb5a 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java @@ -53,6 +53,8 @@ public final class TargetValidator extends AbstractValidator { private static final Set INVALID_MEMBER_TARGETS = SetUtils.of( ShapeType.SERVICE, ShapeType.RESOURCE, ShapeType.OPERATION, ShapeType.MEMBER); + private static final String UNRESOLVED_SHAPE_PART = "UnresolvedShape"; + // Relationship types listed here are checked to see if a shape refers to a deprecated shape. private static final Map RELATIONSHIP_TYPE_DEPRECATION_MAPPINGS = MapUtils.of( RelationshipType.MEMBER_TARGET, "Member targets a deprecated shape", @@ -236,7 +238,8 @@ private ValidationEvent unresolvedTarget(Model model, Shape shape, Relationship if (rel.getRelationshipType() == RelationshipType.MEMBER_TARGET) { // Don't show the relationship type for invalid member targets. return error(shape, String.format( - "member shape targets an unresolved shape `%s`%s", rel.getNeighborShapeId(), suggestionText)); + "member shape targets an unresolved shape `%s`%s", rel.getNeighborShapeId(), suggestionText), + UNRESOLVED_SHAPE_PART); } else { // Use "a" or "an" depending on if the relationship starts with a vowel. String indefiniteArticle = isUppercaseVowel(rel.getRelationshipType().toString().charAt(0)) @@ -248,7 +251,7 @@ private ValidationEvent unresolvedTarget(Model model, Shape shape, Relationship indefiniteArticle, rel.getRelationshipType().toString().toLowerCase(Locale.US), rel.getNeighborShapeId(), - suggestionText)); + suggestionText), UNRESOLVED_SHAPE_PART); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ValidationEventDecoratorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ValidationEventDecoratorTest.java new file mode 100644 index 00000000000..ce744045187 --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ValidationEventDecoratorTest.java @@ -0,0 +1,123 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.loader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.validation.NodeValidationVisitorTest; +import software.amazon.smithy.model.validation.ValidatedResult; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidationEventDecorator; +import software.amazon.smithy.model.validation.Validator; +import software.amazon.smithy.model.validation.ValidatorFactory; +import software.amazon.smithy.utils.SetUtils; + +public class ValidationEventDecoratorTest { + + static final String HINT = "Consider connecting this structure to a service"; + static final String UNREFERENCED_SHAPE_EVENT_ID = "UnreferencedShape"; + static final Set STRUCT_SHAPE_IDS = SetUtils.of(ShapeId.from("ns.foo#Structure"), + ShapeId.from("ns.foo#Structure2"), + ShapeId.from("ns.foo#Structure3")); + + @Test + public void canDecorateValidationEvents() { + ValidatedResult result = Model.assembler() + .addImport(NodeValidationVisitorTest.class.getResource("node-validator" + + ".json")) + .validatorFactory(testFactory(new DummyHintValidationEventDecorator())) + .assemble(); + for (ValidationEvent event : result.getValidationEvents()) { + ShapeId eventShapeId = event.getShapeId().orElse(null); + if (STRUCT_SHAPE_IDS.contains(eventShapeId)) { + assertThat(event.getHint().isPresent(), equalTo(true)); + assertThat(event.getHint().get(), equalTo(HINT)); + } else { + assertThat(event.getHint().isPresent(), equalTo(false)); + } + } + } + + @Test + public void exceptionsAreNotCaughtWhenDecoratorsThrow() { + assertThrows(RuntimeException.class, () -> { + Model.assembler() + .addImport(NodeValidationVisitorTest.class.getResource("node-validator" + + ".json")) + .validatorFactory(testFactory(new ThrowingValidationEventDecorator())) + .assemble(); + }); + } + + static ValidatorFactory testFactory(ValidationEventDecorator decorator) { + ValidatorFactory defaultValidatorFactory = ModelValidator.defaultValidationFactory(); + return new ValidatorFactory() { + @Override + public List loadBuiltinValidators() { + return defaultValidatorFactory.loadBuiltinValidators(); + } + + @Override + public List loadDecorators() { + return Arrays.asList(decorator); + } + + @Override + public Optional createValidator(String name, ObjectNode configuration) { + return defaultValidatorFactory.createValidator(name, configuration); + } + }; + } + + static class DummyHintValidationEventDecorator implements ValidationEventDecorator { + + @Override + public boolean canDecorate(ValidationEvent ev) { + return ev.containsId(UNREFERENCED_SHAPE_EVENT_ID) && ev.getMessage().contains("The structure "); + } + + @Override + public ValidationEvent decorate(ValidationEvent ev) { + return ev.toBuilder().hint(HINT).build(); + } + } + + static class ThrowingValidationEventDecorator implements ValidationEventDecorator { + + @Override + public boolean canDecorate(ValidationEvent ev) { + return ev.containsId(UNREFERENCED_SHAPE_EVENT_ID) && ev.getMessage().contains("The structure "); + } + + @Override + public ValidationEvent decorate(ValidationEvent ev) { + throw new RuntimeException("ups"); + } + } +} diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java index 564a0827a0c..f4ab51bfd1d 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java @@ -92,6 +92,23 @@ public void loadsWithFromNode() { assertThat(event.getShapeId().get(), is(id)); } + @Test + public void loadsWithFromNodeWithHint() { + ShapeId id = ShapeId.from("ns.foo#baz"); + ValidationEvent event = ValidationEvent.fromNode(Node.parse( + "{\"id\": \"abc.foo\", \"severity\": \"SUPPRESSED\", \"suppressionReason\": \"my reason\", " + + "\"shapeId\": \"ns.foo#baz\", \"message\": \"The message\", " + + "\"hint\": \"The hint\", \"filename\": \"/path/to/file.smithy\", \"line\": 7, \"column\": 2}")); + + assertThat(event.getSeverity(), equalTo(Severity.SUPPRESSED)); + assertThat(event.getMessage(), equalTo("The message")); + assertThat(event.getId(), equalTo("abc.foo")); + assertThat(event.getSuppressionReason().get(), equalTo("my reason")); + assertThat(event.getShapeId().get(), is(id)); + assertThat(event.getHint().get(),equalTo("The hint")); + } + + @Test public void hasGetters() { ShapeId id = ShapeId.from("ns.foo#baz"); @@ -101,6 +118,7 @@ public void hasGetters() { .shapeId(id) .id("abc.foo") .suppressionReason("my reason") + .hint("The hint") .build(); assertThat(event.getSeverity(), equalTo(Severity.SUPPRESSED)); @@ -108,6 +126,7 @@ public void hasGetters() { assertThat(event.getId(), equalTo("abc.foo")); assertThat(event.getSuppressionReason().get(), equalTo("my reason")); assertThat(event.getShapeId().get(), is(id)); + assertThat(event.getHint().get(), equalTo("The hint")); assertThat(event.getSeverity(), is(Severity.SUPPRESSED)); } @@ -145,6 +164,7 @@ public void createsEventBuilderFromEvent() { .severity(Severity.SUPPRESSED) .shapeId(ShapeId.from("ns.foo#baz")) .id("abc.foo") + .hint("The hint") .suppressionReason("my reason") .build(); ValidationEvent other = event.toBuilder().build(); @@ -268,6 +288,23 @@ public void differentSuppressionReasonAreNotEqual() { assertNotEquals(a.hashCode(), b.hashCode()); } + @Test + public void differentHintAreNotEqual() { + ValidationEvent a = ValidationEvent.builder() + .message("The message") + .severity(Severity.SUPPRESSED) + .shapeId(ShapeId.from("ns.foo#bar")) + .id("abc.foo") + .suppressionReason("my reason") + .hint("The hint") + .sourceLocation(SourceLocation.none()) + .build(); + ValidationEvent b = a.toBuilder().hint("other hint").build(); + + assertNotEquals(a, b); + assertNotEquals(a.hashCode(), b.hashCode()); + } + @Test public void toStringContainsSeverityAndEventId() { ValidationEvent a = ValidationEvent.builder() @@ -318,6 +355,21 @@ public void toStringContainsSuppressionReason() { assertEquals(a.toString(), "[SUPPRESSED] ns.foo#baz: The message (Foo baz bar) | abc.foo file:1:2"); } + @Test + public void toStringDoesContainsHint() { + ValidationEvent a = ValidationEvent.builder() + .message("The message") + .severity(Severity.SUPPRESSED) + .id("abc.foo") + .shapeId(ShapeId.from("ns.foo#baz")) + .suppressionReason("Foo baz bar") + .hint("The hint") + .sourceLocation(new SourceLocation("file", 1, 2)) + .build(); + + assertEquals(a.toString(), "[SUPPRESSED] ns.foo#baz: The message (Foo baz bar) [The hint] | abc.foo file:1:2"); + } + @Test public void convertsToNode() { ValidationEvent a = ValidationEvent.builder()