diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java index 56f847b3c2f..e6c53157969 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java @@ -232,44 +232,48 @@ private void parseControlSection() { Set definedKeys = new HashSet<>(); while (tokenizer.getCurrentToken() == IdlToken.DOLLAR) { - tokenizer.next(); - tokenizer.expect(IdlToken.IDENTIFIER, IdlToken.STRING); - String key = tokenizer.internString(tokenizer.getCurrentTokenStringSlice()); + try { + tokenizer.next(); + tokenizer.expect(IdlToken.IDENTIFIER, IdlToken.STRING); + String key = tokenizer.internString(tokenizer.getCurrentTokenStringSlice()); - tokenizer.next(); - tokenizer.skipSpaces(); - tokenizer.expect(IdlToken.COLON); - tokenizer.next(); - tokenizer.skipSpaces(); + tokenizer.next(); + tokenizer.skipSpaces(); + tokenizer.expect(IdlToken.COLON); + tokenizer.next(); + tokenizer.skipSpaces(); - if (!definedKeys.add(key)) { - throw syntax(format("Duplicate control statement `%s`", key)); - } + if (!definedKeys.add(key)) { + throw syntax(format("Duplicate control statement `%s`", key)); + } - Node value = IdlNodeParser.expectAndSkipNode(tokenizer, resolver); + Node value = IdlNodeParser.expectAndSkipNode(tokenizer, resolver); - switch (key) { - case "version": - onVersion(value); - break; - case "operationInputSuffix": - operationInputSuffix = value.expectStringNode().getValue(); - break; - case "operationOutputSuffix": - operationOutputSuffix = value.expectStringNode().getValue(); - break; - default: - emit(ValidationEvent.builder() - .id(Validator.MODEL_ERROR) - .sourceLocation(value) - .severity(Severity.WARNING) - .message(format("Unknown control statement `%s` with value `%s", - key, Node.printJson(value))) - .build()); - break; - } + switch (key) { + case "version": + onVersion(value); + break; + case "operationInputSuffix": + operationInputSuffix = value.expectStringNode().getValue(); + break; + case "operationOutputSuffix": + operationOutputSuffix = value.expectStringNode().getValue(); + break; + default: + emit(ValidationEvent.builder() + .id(Validator.MODEL_ERROR) + .sourceLocation(value) + .severity(Severity.WARNING) + .message(format("Unknown control statement `%s` with value `%s", + key, Node.printJson(value))) + .build()); + break; + } - tokenizer.expectAndSkipBr(); + tokenizer.expectAndSkipBr(); + } catch (ModelSyntaxException e) { + errorRecovery(e); + } } } @@ -293,19 +297,23 @@ private void onVersion(Node value) { private void parseMetadataSection() { while (tokenizer.doesCurrentIdentifierStartWith('m')) { - tokenizer.expectCurrentLexeme("metadata"); - tokenizer.next(); // skip "metadata" - tokenizer.expectAndSkipSpaces(); - tokenizer.expect(IdlToken.IDENTIFIER, IdlToken.STRING); - String key = tokenizer.internString(tokenizer.getCurrentTokenStringSlice()); - tokenizer.next(); - tokenizer.skipSpaces(); - tokenizer.expect(IdlToken.EQUAL); - tokenizer.next(); - tokenizer.skipSpaces(); - Node value = IdlNodeParser.expectAndSkipNode(tokenizer, resolver); - operations.accept(new LoadOperation.PutMetadata(modelVersion, key, value)); - tokenizer.expectAndSkipBr(); + try { + tokenizer.expectCurrentLexeme("metadata"); + tokenizer.next(); // skip "metadata" + tokenizer.expectAndSkipSpaces(); + tokenizer.expect(IdlToken.IDENTIFIER, IdlToken.STRING); + String key = tokenizer.internString(tokenizer.getCurrentTokenStringSlice()); + tokenizer.next(); + tokenizer.skipSpaces(); + tokenizer.expect(IdlToken.EQUAL); + tokenizer.next(); + tokenizer.skipSpaces(); + Node value = IdlNodeParser.expectAndSkipNode(tokenizer, resolver); + operations.accept(new LoadOperation.PutMetadata(modelVersion, key, value)); + tokenizer.expectAndSkipBr(); + } catch (ModelSyntaxException e) { + errorRecovery(e); + } } } @@ -474,7 +482,7 @@ private void errorRecovery(ModelSyntaxException e) { } // Here we do rudimentary error recovery to attempt to make sense of the remaining model. - // We do this by scanning tokens until we find the next identifier, docs, or trait at the start of a line. + // We do this by scanning tokens until we find the next "$", identifier, docs, or trait at the start of a line. // The model is still invalid and will fail to validate, but things like IDEs should still be able to do // things like goto definition. emit(ValidationEvent.fromSourceException(e)); @@ -490,7 +498,15 @@ private void errorRecovery(ModelSyntaxException e) { // These tokens are good signals that the next shape is starting. private boolean isErrorRecoveryToken(IdlToken token) { - return token == IdlToken.IDENTIFIER || token == IdlToken.DOC_COMMENT || token == IdlToken.AT; + switch (token) { + case IDENTIFIER: + case DOC_COMMENT: + case AT: + case DOLLAR: + return true; + default: + return false; + } } private boolean parseShapeDefinition(List traits, boolean hasDocComment) { diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTokenizer.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTokenizer.java index 4b9b828397c..15ab1741c6e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTokenizer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTokenizer.java @@ -750,9 +750,15 @@ private IdlToken parseNumber() { } private IdlToken parseIdentifier() { - ParserUtils.consumeIdentifier(parser); + try { + ParserUtils.consumeIdentifier(parser); + currentTokenType = IdlToken.IDENTIFIER; + } catch (RuntimeException e) { + currentTokenType = IdlToken.ERROR; + currentTokenError = e.getMessage(); + } currentTokenEnd = parser.position(); - return currentTokenType = IdlToken.IDENTIFIER; + return currentTokenType; } private IdlToken parseString() { diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/IdlModelLoaderTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/IdlModelLoaderTest.java index 0002a59e71f..e457b638936 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/IdlModelLoaderTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/IdlModelLoaderTest.java @@ -16,6 +16,7 @@ package software.amazon.smithy.model.loader; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -34,6 +35,7 @@ import org.opentest4j.AssertionFailedError; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.EnumShape; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; @@ -45,6 +47,7 @@ import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.traits.DocumentationTrait; import software.amazon.smithy.model.traits.DynamicTrait; +import software.amazon.smithy.model.traits.ExternalDocumentationTrait; import software.amazon.smithy.model.traits.StringTrait; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.traits.TraitFactory; @@ -341,9 +344,9 @@ public void setsCorrectLocationForEnum() { } @Test - public void doesBasicErrorRecovery() { + public void doesBasicErrorRecoveryToTrait() { ValidatedResult result = Model.assembler() - .addImport(getClass().getResource("error-recovery.smithy")) + .addImport(getClass().getResource("error-recovery/to-trait.smithy")) .assemble(); assertThat(result.isBroken(), is(true)); @@ -356,8 +359,6 @@ public void doesBasicErrorRecovery() { assertThat(model.getShape(ShapeId.from("smithy.example#MyInteger")).isPresent(), is(false)); assertThat(model.getShape(ShapeId.from("smithy.example#MyInteger2")).isPresent(), is(true)); - System.out.println(result.getValidationEvents()); - boolean foundSyntax = false; boolean foundTrait = false; for (ValidationEvent e : result.getValidationEvents()) { @@ -373,4 +374,103 @@ public void doesBasicErrorRecovery() { assertThat(foundSyntax, is(true)); assertThat(foundTrait, is(true)); } + + @Test + public void doesBasicErrorRecoveryToDocs() { + ValidatedResult result = Model.assembler() + .addImport(getClass().getResource("error-recovery/to-docs.smithy")) + .assemble(); + + assertThat(result.isBroken(), is(true)); + assertThat(result.getResult().isPresent(), is(true)); + + Model model = result.getResult().get(); + + ShapeId myInteger = ShapeId.from("smithy.example#MyInteger"); + assertThat(model.getShape(myInteger).isPresent(), is(true)); + // Ensure recovery happened on the docs trait, capturing it on the shape. + assertThat(model.expectShape(myInteger).hasTrait(DocumentationTrait.class), is(true)); + + boolean foundSyntax = false; + boolean foundTrait = false; + for (ValidationEvent e : result.getValidationEvents()) { + if (e.getSeverity() == Severity.ERROR && e.getMessage().contains("Syntax error at line 6, column 28")) { + foundSyntax = true; + } + if (e.getSeverity() == Severity.ERROR && e.getMessage().contains("Unable to resolve trait")) { + foundTrait = true; + } + } + + assertThat(foundSyntax, is(true)); + assertThat(foundTrait, is(true)); + } + + @Test + public void doesBasicErrorRecoveryToIdentifier() { + ValidatedResult result = Model.assembler() + .addImport(getClass().getResource("error-recovery/to-identifier.smithy")) + .assemble(); + + assertThat(result.isBroken(), is(true)); + assertThat(result.getResult().isPresent(), is(true)); + + Model model = result.getResult().get(); + + ShapeId myString = ShapeId.from("smithy.example#MyString"); + assertThat(model.getShape(myString).isPresent(), is(true)); + assertThat(model.expectShape(myString).hasTrait(ExternalDocumentationTrait.class), is(false)); + + boolean foundSyntax = result.getValidationEvents().stream() + .anyMatch(e -> e.getSeverity() == Severity.ERROR + && e.getMessage().contains("Syntax error at line 6, column 28")); + + assertThat(foundSyntax, is(true)); + } + + @Test + public void doesBasicErrorRecoveryToControl() { + ValidatedResult result = Model.assembler() + .addImport(getClass().getResource("error-recovery/to-dollar.smithy")) + .assemble(); + + assertThat(result.isBroken(), is(true)); + assertThat(result.getResult().isPresent(), is(true)); + + Model model = result.getResult().get(); + + assertThat(model.getShape(ShapeId.from("smithy.example#MyInteger")).isPresent(), is(true)); + + boolean foundSyntax = result.getValidationEvents().stream() + .anyMatch(e -> e.getSeverity() == Severity.ERROR + && e.getMessage().contains("Syntax error at line 1, column 5")); + + assertThat(foundSyntax, is(true)); + } + + @Test + public void doesBasicErrorRecoveryInMetadata() { + ValidatedResult result = Model.assembler() + .addImport(getClass().getResource("error-recovery/in-metadata.smithy")) + .assemble(); + + assertThat(result.isBroken(), is(true)); + assertThat(result.getResult().isPresent(), is(true)); + + Model model = result.getResult().get(); + + // Ensure the value keys were parsed and invalid keys were not. + assertThat(model.getMetadata().keySet(), containsInAnyOrder("valid1", "valid2", "valid3", "valid4")); + assertThat(model.getMetadata().get("valid1"), equalTo(Node.from("ok"))); + assertThat(model.getMetadata().get("valid2"), equalTo(Node.from("ok"))); + assertThat(model.getMetadata().get("valid3"), equalTo(Node.from("ok"))); + assertThat(model.getMetadata().get("valid4"), equalTo(Node.from("ok"))); + + // Find all four invalid metadata keys. + long foundSyntax = result.getValidationEvents().stream() + .filter(e -> e.getSeverity() == Severity.ERROR && e.getMessage().contains("Syntax error")) + .count(); + + assertThat(foundSyntax, equalTo(4L)); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/in-metadata.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/in-metadata.smithy new file mode 100644 index 00000000000..c6eccd88141 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/in-metadata.smithy @@ -0,0 +1,10 @@ +$version: "2.0" + +metadata invalid1 == +metadata valid1 = "ok" +metadata invalid2 = {"bad"} +metadata valid2 = "ok" +metadata invalid3 = ] +metadata valid3 = "ok" +metadata invalid4 = +metadata valid4 = "ok" diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-docs.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-docs.smithy new file mode 100644 index 00000000000..d9dd5af6773 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-docs.smithy @@ -0,0 +1,11 @@ +$version: "2.0" + +namespace smithy.example + +// Parse error here. +@externalDocumentation(foo:) + +// Then recover on the line below. +/// Hello +@unknown +integer MyInteger diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-dollar.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-dollar.smithy new file mode 100644 index 00000000000..db0a334bb6f --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-dollar.smithy @@ -0,0 +1,6 @@ +$foo +$version: "2.0" +// ^ Recover here +namespace smithy.example + +integer MyInteger diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-identifier.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-identifier.smithy new file mode 100644 index 00000000000..86b545ecbab --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-identifier.smithy @@ -0,0 +1,8 @@ +$version: "2.0" + +namespace smithy.example + +// Parse error here. +@externalDocumentation(foo:) +string MyString +// ^ Recover here diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-trait.smithy similarity index 100% rename from smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery.smithy rename to smithy-model/src/test/resources/software/amazon/smithy/model/loader/error-recovery/to-trait.smithy