From e7ba980c07b438eeb16091bb4f0fa7660b11360d Mon Sep 17 00:00:00 2001 From: Kevin Stich Date: Thu, 29 Aug 2024 16:53:52 -0700 Subject: [PATCH] Fix multiple doc comment parse issues This commit fixes several issues where documentation comments were not emitting events individually, an issue where a valid documentation comment was being declared invalid, and an invalid documentation comment was being prepended to a valid documtation comment on the next member. --- .../smithy/model/loader/IdlInternalTokenizer.java | 2 +- .../amazon/smithy/model/loader/IdlModelLoader.java | 13 ++++++++----- .../amazon/smithy/model/loader/IdlTraitParser.java | 8 +++++++- .../detects-invalid-documentation-comments.errors | 8 +++++--- .../loader/valid/doc-comments/doc-comments.json | 6 ++++++ .../loader/valid/doc-comments/doc-comments.smithy | 3 +++ 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlInternalTokenizer.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlInternalTokenizer.java index d30fe637b46..786f5dba68d 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlInternalTokenizer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlInternalTokenizer.java @@ -149,7 +149,7 @@ void expectAndSkipBr() { } } - private void clearDocCommentLinesForBr() { + void clearDocCommentLinesForBr() { if (!docCommentLines.isEmpty()) { validationEventListener.accept(LoaderUtils.emitBadDocComment(getCurrentTokenLocation(), removePendingDocCommentLines())); 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 562c07e8ab3..81fbda08e46 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 @@ -448,7 +448,7 @@ private void parseFirstShapeStatement(SourceLocation possibleDocCommentLocation) try { tokenizer.skipWsAndDocs(); String docLines = tokenizer.removePendingDocCommentLines(); - List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this); + List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this, false); if (docLines != null) { traits.add(new IdlTraitParser.Result(DocumentationTrait.ID.toString(), new StringNode(docLines, possibleDocCommentLocation), @@ -467,7 +467,7 @@ private void parseSubsequentShapeStatements() { while (tokenizer.hasNext()) { try { boolean hasDocComment = tokenizer.getCurrentToken() == IdlToken.DOC_COMMENT; - List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this); + List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this, false); if (parseShapeDefinition(traits, hasDocComment)) { parseShapeOrApply(traits); } @@ -672,7 +672,7 @@ private void parseEnumShape(ShapeId id, SourceLocation location, AbstractShapeBu while (tokenizer.getCurrentToken() != IdlToken.EOF && tokenizer.getCurrentToken() != IdlToken.RBRACE) { List memberTraits = IdlTraitParser - .parseDocsAndTraitsBeforeShape(this); + .parseDocsAndTraitsBeforeShape(this, true); SourceLocation memberLocation = tokenizer.getCurrentTokenLocation(); tokenizer.expect(IdlToken.IDENTIFIER); String memberName = internString(tokenizer.getCurrentTokenLexeme()); @@ -723,6 +723,9 @@ private void parseMembers(LoadOperation.DefineShape op) { Set definedMembers = new HashSet<>(); tokenizer.skipWsAndDocs(); + // Clear out any doc comments between the shape type or + // identifier and the opening LBRACE token. + tokenizer.clearDocCommentLinesForBr(); tokenizer.expect(IdlToken.LBRACE); tokenizer.next(); tokenizer.skipWs(); @@ -740,7 +743,7 @@ private void parseMember(LoadOperation.DefineShape operation, Set define ShapeId parent = operation.toShapeId(); // Parse optional member traits. - List memberTraits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this); + List memberTraits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this, true); SourceLocation memberLocation = tokenizer.getCurrentTokenLocation(); // Handle the case of a dangling documentation comment followed by "}". @@ -988,7 +991,7 @@ private void parseInlineableOperationMember( } private ShapeId parseInlineStructure(String name, IdlTraitParser.Result defaultTrait) { - List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this); + List traits = IdlTraitParser.parseDocsAndTraitsBeforeShape(this, true); if (defaultTrait != null) { traits.add(defaultTrait); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java index 82b52de577b..90aabfa3a07 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java @@ -69,7 +69,7 @@ private IdlTraitParser() { } * @param loader IDL parser. * @return Return the parsed traits. */ - static List parseDocsAndTraitsBeforeShape(IdlModelLoader loader) { + static List parseDocsAndTraitsBeforeShape(IdlModelLoader loader, boolean isParsingMember) { IdlInternalTokenizer tokenizer = loader.getTokenizer(); tokenizer.skipWs(); @@ -91,6 +91,12 @@ static List parseDocsAndTraitsBeforeShape(IdlModelLoader loader) { traits.add(docComment); } tokenizer.skipWsAndDocs(); + // If there's a doc comment between traits and their target member, + // clear them so they're not prepended to a doc comment on the next + // member shape. + if (isParsingMember) { + tokenizer.clearDocCommentLinesForBr(); + } return traits; } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-invalid-documentation-comments.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-invalid-documentation-comments.errors index 56aced838bb..978716049f2 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-invalid-documentation-comments.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-invalid-documentation-comments.errors @@ -1,7 +1,9 @@ [WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 1 (before control) | Model.BadDocumentationComment [WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 2 (before namespace) | Model.BadDocumentationComment [WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 3 (before use) | Model.BadDocumentationComment -[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 4 (inside trait)\nInvalid 5 (not before traits)\nValid docs for Foo | Model.BadDocumentationComment -[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 6 (not before traits)\nInvalid 7 (inside shape line)\nInvalid 8 (dangling) | Model.BadDocumentationComment -[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 9 (not before traits)\nInvalid 10 (dangling) | Model.BadDocumentationComment +[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 4 (inside trait)\nInvalid 5 (not before traits) | Model.BadDocumentationComment +[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 6 (not before traits)\nInvalid 7 (inside shape line) | Model.BadDocumentationComment +[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 8 (dangling) | Model.BadDocumentationComment +[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 9 (not before traits) | Model.BadDocumentationComment +[WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 10 (dangling) | Model.BadDocumentationComment [WARNING] -: Found documentation comments ('///') attached to nothing. Documentation comments must appear on their own lines, directly before shapes and members, and before any traits. The invalid comments were: Invalid 11 (dangling) | Model.BadDocumentationComment diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.json index ea06639a006..fa4fc669779 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.json @@ -30,6 +30,12 @@ "smithy.api#documentation": "Docs on another member!" } }, + "boot": { + "target": "smithy.api#String", + "traits": { + "smithy.api#documentation": "I'll only have this documentation." + } + }, "bam": { "target": "smithy.api#String" } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.smithy index 2ded3aa192e..3f96dc95c42 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.smithy +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments/doc-comments.smithy @@ -29,6 +29,9 @@ structure MyStructure { /// IMPORTANT: These docs are ignored since they come after traits! baz: String, + /// I'll only have this documentation. + boot: String + // no docs. bam: String, }