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 validation for bad syntactic shape ID #542

Merged
merged 1 commit into from
Aug 25, 2020
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
11 changes: 11 additions & 0 deletions docs/source/1.0/spec/core/idl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,17 @@ shape IDs inside of strings, and this difference is inconsequential in the
resolved to a string that contains a fully-qualified shape ID when parsing
the model.

.. rubric:: Validation

When a syntactic shape ID is found that does not target an actual shape in
the fully loaded semantic model, an implementation SHOULD emit a DANGER
:ref:`validation event <validation>` with an ID of `SyntacticShapeIdTarget`.
This validation brings attention to the broken reference and helps to ensure
that modelers do not unintentionally use a syntactic shape ID when they should
have used a string. A DANGER severity is used so that the validation can be
:ref:`suppressed <suppression-definition>` in the rare cases that the broken
reference can be ignored.


Defining shapes
===============
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@
import software.amazon.smithy.model.node.NullNode;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.Pair;

/**
* Parses IDL nodes.
*/
final class IdlNodeParser {

private static final String SYNTACTIC_SHAPE_ID_TARGET = "SyntacticShapeIdTarget";

private IdlNodeParser() {}

static Node parseNode(IdlModelParser parser) {
Expand Down Expand Up @@ -84,7 +88,19 @@ static Node parseNodeTextWithKeywords(IdlModelParser parser, SourceLocation loca
// not be able to be resolved until after the entire model is loaded.
Pair<StringNode, Consumer<String>> pair = StringNode.createLazyString(text, location);
Consumer<String> consumer = pair.right;
parser.modelFile.addForwardReference(text, id -> consumer.accept(id.toString()));
parser.modelFile.addForwardReference(text, (id, typeFunction) -> {
if (typeFunction.apply(id) == null) {
parser.modelFile.events().add(ValidationEvent.builder()
.id(SYNTACTIC_SHAPE_ID_TARGET)
.severity(Severity.DANGER)
.message(String.format("Syntactic shape ID `%s` does not resolve to a valid shape ID: "
+ "`%s`. Did you mean to quote this string? Are you missing a "
+ "model file?", text, id))
.sourceLocation(location)
.build());
}
consumer.accept(id.toString());
});
return pair.left;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
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.util.List;
import java.util.Optional;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
Expand All @@ -31,7 +33,10 @@
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.StringTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidatedResult;
import software.amazon.smithy.model.validation.ValidatedResultException;
import software.amazon.smithy.model.validation.ValidationEvent;

public class IdlModelLoaderTest {
@Test
Expand Down Expand Up @@ -131,4 +136,22 @@ public void handlesMultilineDocComments() {

assertThat(docs, equalTo("This is the first line.\nThis is the second line."));
}

@Test
public void warnsWhenInvalidSyntacticShapeIdIsFound() {
ValidatedResult<Model> result = Model.assembler()
.addUnparsedModel("foo.smithy", "namespace smithy.example\n"
+ "@tags([nonono])\n"
+ "string Foo\n")
.assemble();

assertThat(result.isBroken(), is(true));
List<ValidationEvent> events = result.getValidationEvents(Severity.DANGER);
assertThat(events.stream().filter(e -> e.getId().equals("SyntacticShapeIdTarget")).count(), equalTo(1L));
assertThat(events.stream()
.filter(e -> e.getId().equals("SyntacticShapeIdTarget"))
.filter(e -> e.getMessage().contains("`nonono`"))
.count(),
equalTo(1L));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[DANGER] -: Syntactic shape ID `GET` does not resolve to a valid shape ID: `smithy.example#GET`. Did you mean to quote this string? Are you missing a model file? | SyntacticShapeIdTarget
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
namespace smithy.example

@http(method: GET, uri: "/") // <- Use "GET" not GET!
operation Foo {}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ operation Resource1Operation {}
resource Resource1_2 {}

resource Resource1_1 {
type: resource,
operations: [Resource1_1_Operation],
resources: [Resource1_1_1, Resource1_1_2]
}
Expand Down