-
Notifications
You must be signed in to change notification settings - Fork 242
Add new shapeExamples trait that communicates and enforces allowed and disallowed values #2851
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
base: main
Are you sure you want to change the base?
Changes from all commits
902d6fe
69c5090
d83ccca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "feature", | ||
| "description": "Add new shapeExamples to express allowed and disallowed values on an individual shape level", | ||
| "pull_requests": [ | ||
| "[#2851](https://github.com/smithy-lang/smithy/pull/2851)" | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package software.amazon.smithy.model.traits; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import software.amazon.smithy.model.SourceException; | ||
| import software.amazon.smithy.model.node.ArrayNode; | ||
| import software.amazon.smithy.model.node.Node; | ||
| import software.amazon.smithy.model.node.ObjectNode; | ||
| import software.amazon.smithy.model.shapes.ShapeId; | ||
| import software.amazon.smithy.utils.MapUtils; | ||
| import software.amazon.smithy.utils.ToSmithyBuilder; | ||
|
|
||
| /** | ||
| * Defines values which are specifically allowed and/or disallowed for a shape. | ||
| */ | ||
| public final class ShapeExamplesTrait extends AbstractTrait implements ToSmithyBuilder<ShapeExamplesTrait> { | ||
| public static final ShapeId ID = ShapeId.from("smithy.api#shapeExamples"); | ||
|
|
||
| private final List<Node> allowed; | ||
| private final List<Node> disallowed; | ||
|
|
||
| private ShapeExamplesTrait(ShapeExamplesTrait.Builder builder) { | ||
| super(ID, builder.sourceLocation); | ||
| this.allowed = builder.allowed; | ||
| this.disallowed = builder.disallowed; | ||
| if (allowed == null && disallowed == null) { | ||
| throw new SourceException("One of 'allowed' or 'disallowed' must be provided.", getSourceLocation()); | ||
| } | ||
| if (allowed != null && allowed.isEmpty()) { | ||
| throw new SourceException("'allowed' must be non-empty when provided.", getSourceLocation()); | ||
| } | ||
| if (disallowed != null && disallowed.isEmpty()) { | ||
| throw new SourceException("'disallowed' must be non-empty when provided.", getSourceLocation()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Gets the allowed values. | ||
| * | ||
| * @return returns the optional allowed values. | ||
| */ | ||
| public Optional<List<Node>> getAllowed() { | ||
| return Optional.ofNullable(allowed); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the disallowed values. | ||
| * | ||
| * @return returns the optional disallowed values. | ||
| */ | ||
| public Optional<List<Node>> getDisallowed() { | ||
| return Optional.ofNullable(disallowed); | ||
| } | ||
|
|
||
| @Override | ||
| protected Node createNode() { | ||
| return new ObjectNode(MapUtils.of(), getSourceLocation()) | ||
| .withOptionalMember("allowed", getAllowed().map(ArrayNode::fromNodes)) | ||
| .withOptionalMember("disallowed", getDisallowed().map(ArrayNode::fromNodes)); | ||
| } | ||
|
|
||
| @Override | ||
| public ShapeExamplesTrait.Builder toBuilder() { | ||
| return builder().allowed(allowed).disallowed(disallowed).sourceLocation(getSourceLocation()); | ||
| } | ||
|
|
||
| /** | ||
| * @return Returns a new ShapeExamplesTrait builder. | ||
| */ | ||
| public static ShapeExamplesTrait.Builder builder() { | ||
| return new ShapeExamplesTrait.Builder(); | ||
| } | ||
|
|
||
| /** | ||
| * Builder used to create a ShapeExamplesTrait. | ||
| */ | ||
| public static final class Builder extends AbstractTraitBuilder<ShapeExamplesTrait, ShapeExamplesTrait.Builder> { | ||
| private List<Node> allowed; | ||
| private List<Node> disallowed; | ||
|
|
||
| public ShapeExamplesTrait.Builder allowed(List<Node> allowed) { | ||
| this.allowed = allowed; | ||
| return this; | ||
| } | ||
|
|
||
| public ShapeExamplesTrait.Builder disallowed(List<Node> disallowed) { | ||
| this.disallowed = disallowed; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public ShapeExamplesTrait build() { | ||
| return new ShapeExamplesTrait(this); | ||
| } | ||
| } | ||
|
|
||
| public static final class Provider implements TraitService { | ||
| @Override | ||
| public ShapeId getShapeId() { | ||
| return ID; | ||
| } | ||
|
|
||
| @Override | ||
| public ShapeExamplesTrait createTrait(ShapeId target, Node value) { | ||
| ShapeExamplesTrait.Builder builder = builder().sourceLocation(value.getSourceLocation()); | ||
| value.expectObjectNode() | ||
| .getMember("allowed", ShapeExamplesTrait.Provider::convertToShapeExampleList, builder::allowed) | ||
| .getMember("disallowed", | ||
| ShapeExamplesTrait.Provider::convertToShapeExampleList, | ||
| builder::disallowed); | ||
| ShapeExamplesTrait result = builder.build(); | ||
| result.setNodeCache(value); | ||
| return result; | ||
| } | ||
|
|
||
| private static List<Node> convertToShapeExampleList(Node node) { | ||
| return node.expectArrayNode().getElements(); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,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.ValidationEventFormatter; | ||
| import software.amazon.smithy.model.validation.Validator; | ||
| import software.amazon.smithy.utils.IoUtils; | ||
|
|
||
|
|
@@ -33,6 +34,8 @@ public final class SmithyTestCase { | |
| private static final Pattern EVENT_PATTERN = Pattern.compile( | ||
| "^\\[(?<severity>SUPPRESSED|NOTE|WARNING|DANGER|ERROR)] (?<shape>[^ ]+): ?(?<message>.*) \\| (?<id>[^)]+)"); | ||
|
|
||
| private static final ValidationEventFormatter VALIDATION_EVENT_FORMATTER = new ErrorsFileValidationEventFormatter(); | ||
|
|
||
| private final List<ValidationEvent> expectedEvents; | ||
| private final String modelLocation; | ||
|
|
||
|
|
@@ -222,7 +225,7 @@ public String toString() { | |
| builder.append("\nDid not match the following events\n" | ||
| + "----------------------------------\n"); | ||
| for (ValidationEvent event : getUnmatchedEvents()) { | ||
| builder.append(event.toString().replace("\n", "\\n")).append('\n'); | ||
| builder.append(VALIDATION_EVENT_FORMATTER.format(event)).append('\n'); | ||
| } | ||
| builder.append('\n'); | ||
| } | ||
|
|
@@ -231,7 +234,7 @@ public String toString() { | |
| builder.append("\nEncountered unexpected events\n" | ||
| + "-----------------------------\n"); | ||
| for (ValidationEvent event : getExtraEvents()) { | ||
| builder.append(event.toString().replace("\n", "\\n")).append("\n"); | ||
| builder.append(VALIDATION_EVENT_FORMATTER.format(event)).append("\n"); | ||
| } | ||
| builder.append('\n'); | ||
| } | ||
|
|
@@ -295,4 +298,25 @@ public static final class Error extends RuntimeException { | |
| this.result = result; | ||
| } | ||
| } | ||
|
|
||
| private static final class ErrorsFileValidationEventFormatter implements ValidationEventFormatter { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated implementation here to make it easier to iterate on error files -- the previous implementation would output the extra file location information and hints if they exist, which are explicitly omitted from the errors file normally. You can now copy+paste directly from the failure message in the test result into the errors file. |
||
| @Override | ||
| public String format(ValidationEvent event) { | ||
| String message = event.getMessage(); | ||
|
|
||
| String reason = event.getSuppressionReason().orElse(null); | ||
| if (reason != null) { | ||
| message += " (" + reason + ")"; | ||
| } | ||
|
|
||
| String formattedEventString = String.format( | ||
| "[%s] %s: %s | %s", | ||
| event.getSeverity(), | ||
| event.getShapeId().map(ShapeId::toString).orElse("-"), | ||
| message, | ||
| event.getId()); | ||
|
|
||
| return formattedEventString.replace("\n", "\\n"); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally traversing into the shape when the value was null pretty much guaranteed that an error would be reported -- generally of the form "<string, number, object> was expected but value was null" (not exact wording).
Since is the
memberShapemethod, this only affects aggregate types which have an explicitnullvalue as opposed to the member being omitted.