From 7fd2e182150c9b6be9ba21e6450b6e4668ad9f82 Mon Sep 17 00:00:00 2001 From: recih Date: Sat, 27 Jun 2020 06:48:30 +0800 Subject: [PATCH] feat: better comment parse (#1419) * fix trailing comment in last line recognized as leading comment for current token. * add preferTrailingComment to IParseOptions Co-authored-by: Alexander Fenster --- index.d.ts | 3 ++ src/parse.js | 6 ++-- src/tokenize.js | 24 ++++++++----- tests/api_tokenize.js | 7 ++++ tests/data/comments-alternate-parse.proto | 4 +++ tests/docs_comments.js | 25 +++++++++++++ tests/docs_comments_alternate_parse.js | 43 ++++++++++++++++++++++- 7 files changed, 100 insertions(+), 12 deletions(-) diff --git a/index.d.ts b/index.d.ts index b6d4b56f1..40d9d2852 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1046,6 +1046,9 @@ export interface IParseOptions { /** Recognize double-slash comments in addition to doc-block comments. */ alternateCommentMode?: boolean; + + /** Use trailing comment when both leading comment and trailing comment exist. */ + preferTrailingComment?: boolean; } /** Options modifying the behavior of JSON serialization. */ diff --git a/src/parse.js b/src/parse.js index 0b7c24b05..f1ab07b93 100644 --- a/src/parse.js +++ b/src/parse.js @@ -42,6 +42,7 @@ var base10Re = /^[1-9][0-9]*$/, * @interface IParseOptions * @property {boolean} [keepCase=false] Keeps field casing instead of converting to camel case * @property {boolean} [alternateCommentMode=false] Recognize double-slash comments in addition to doc-block comments. + * @property {boolean} [preferTrailingComment=false] Use trailing comment when both leading comment and trailing comment exist. */ /** @@ -68,6 +69,7 @@ function parse(source, root, options) { if (!options) options = parse.defaults; + var preferTrailingComment = options.preferTrailingComment || false; var tn = tokenize(source, options.alternateCommentMode || false), next = tn.next, push = tn.push, @@ -291,8 +293,8 @@ function parse(source, root, options) { if (fnElse) fnElse(); skip(";"); - if (obj && typeof obj.comment !== "string") - obj.comment = cmnt(trailingLine); // try line-type comment if no block + if (obj && (typeof obj.comment !== "string" || preferTrailingComment)) + obj.comment = cmnt(trailingLine) || obj.comment; // try line-type comment } } diff --git a/src/tokenize.js b/src/tokenize.js index b939ef289..b55b29273 100644 --- a/src/tokenize.js +++ b/src/tokenize.js @@ -106,7 +106,8 @@ function tokenize(source, alternateCommentMode) { commentType = null, commentText = null, commentLine = 0, - commentLineEmpty = false; + commentLineEmpty = false, + commentIsLeading = false; var stack = []; @@ -154,13 +155,15 @@ function tokenize(source, alternateCommentMode) { * Sets the current comment text. * @param {number} start Start offset * @param {number} end End offset + * @param {boolean} isLeading set if a leading comment * @returns {undefined} * @inner */ - function setComment(start, end) { + function setComment(start, end, isLeading) { commentType = source.charAt(start++); commentLine = line; commentLineEmpty = false; + commentIsLeading = isLeading; var lookback; if (alternateCommentMode) { lookback = 2; // alternate comment parsing: "//" or "/*" @@ -222,14 +225,17 @@ function tokenize(source, alternateCommentMode) { prev, curr, start, - isDoc; + isDoc, + isLeadingComment = offset === 0; do { if (offset === length) return null; repeat = false; while (whitespaceRe.test(curr = charAt(offset))) { - if (curr === "\n") + if (curr === "\n") { + isLeadingComment = true; ++line; + } if (++offset === length) return null; } @@ -250,7 +256,7 @@ function tokenize(source, alternateCommentMode) { } ++offset; if (isDoc) { - setComment(start, offset - 1); + setComment(start, offset - 1, isLeadingComment); } ++line; repeat = true; @@ -271,7 +277,7 @@ function tokenize(source, alternateCommentMode) { offset = Math.min(length, findEndOfLine(offset) + 1); } if (isDoc) { - setComment(start, offset); + setComment(start, offset, isLeadingComment); } line++; repeat = true; @@ -292,7 +298,7 @@ function tokenize(source, alternateCommentMode) { } while (prev !== "*" || curr !== "/"); ++offset; if (isDoc) { - setComment(start, offset - 2); + setComment(start, offset - 2, isLeadingComment); } repeat = true; } else { @@ -370,7 +376,7 @@ function tokenize(source, alternateCommentMode) { var ret = null; if (trailingLine === undefined) { if (commentLine === line - 1 && (alternateCommentMode || commentType === "*" || commentLineEmpty)) { - ret = commentText; + ret = commentIsLeading ? commentText : null; } } else { /* istanbul ignore else */ @@ -378,7 +384,7 @@ function tokenize(source, alternateCommentMode) { peek(); } if (commentLine === trailingLine && !commentLineEmpty && (alternateCommentMode || commentType === "/")) { - ret = commentText; + ret = commentIsLeading ? null : commentText; } } return ret; diff --git a/tests/api_tokenize.js b/tests/api_tokenize.js index 5db54e544..4ee556ae8 100644 --- a/tests/api_tokenize.js +++ b/tests/api_tokenize.js @@ -35,6 +35,13 @@ tape.test("tokenize", function(test) { tn = tokenize("/// line comment\na\n"); tn.next(); test.equal(tn.cmnt(), "line comment", "should keep leading comments around while on the next line"); + tn = tokenize("/// leading comment A\na /// trailing comment A\nb /// trailing comment B\n"); + tn.next(); + test.equal(tn.cmnt(), "leading comment A", "should parse leading comment"); + test.equal(tn.cmnt(tn.line), "trailing comment A", "should parse trailing comment"); + tn.next(); + test.equal(tn.cmnt(), null, "trailing comment should not be recognized as leading comment for next line"); + test.equal(tn.cmnt(tn.line), "trailing comment B", "should parse trailing comment"); test.ok(expectError("something; /"), "should throw for unterminated line comments"); test.ok(expectError("something; /* comment"), "should throw for unterminated block comments"); diff --git a/tests/data/comments-alternate-parse.proto b/tests/data/comments-alternate-parse.proto index 928e73426..409851aa4 100644 --- a/tests/data/comments-alternate-parse.proto +++ b/tests/data/comments-alternate-parse.proto @@ -42,6 +42,10 @@ message Test1 { * multi-line doc-block comment. */ string field10 = 10; + + // Field with both block comment + string field11 = 11; // and trailing comment. + string field12 = 12; // Trailing comment in last line should not be recognized as leading comment for this field. } /* Message diff --git a/tests/docs_comments.js b/tests/docs_comments.js index bb9375c37..c4c33c387 100644 --- a/tests/docs_comments.js +++ b/tests/docs_comments.js @@ -24,3 +24,28 @@ tape.test("proto comments", function(test) { test.end(); }); }); + +tape.test("proto comments with trailing comment preferred", function(test) { + test.plan(10); + var options = {preferTrailingComment: true}; + var root = new protobuf.Root(); + root.load("tests/data/comments.proto", options, function(err, root) { + if (err) + throw test.fail(err.message); + + test.equal(root.lookup("Test1").comment, "Message\nwith\na\ncomment.", "should parse /**-blocks"); + test.equal(root.lookup("Test2").comment, null, "should not parse //-blocks"); + test.equal(root.lookup("Test3").comment, null, "should not parse /*-blocks"); + + test.equal(root.lookup("Test1.field1").comment, "Field with a comment.", "should parse blocks for message fields"); + test.equal(root.lookup("Test1.field2").comment, null, "should not parse lines for message fields"); + test.equal(root.lookup("Test1.field3").comment, "Field with a comment and a link", "should parse triple-slash lines for message fields"); + + test.equal(root.lookup("Test3").comments.ONE, "Value with a comment.", "should parse blocks for enum values"); + test.equal(root.lookup("Test3").comments.TWO, null, "should not parse lines for enum values"); + test.equal(root.lookup("Test3").comments.THREE, "Value with a comment.", "should prefer trailing comment when preferTrailingComment option enabled"); + test.equal(root.lookup("Test3").comments.FOUR, "Other value with a comment.", "should not confuse previous trailing comments with comments for the next field"); + + test.end(); + }); +}); diff --git a/tests/docs_comments_alternate_parse.js b/tests/docs_comments_alternate_parse.js index 4cacc475c..8b1254e47 100644 --- a/tests/docs_comments_alternate_parse.js +++ b/tests/docs_comments_alternate_parse.js @@ -3,7 +3,7 @@ var tape = require("tape"); var protobuf = require(".."); tape.test("proto comments in alternate-parse mode", function(test) { - test.plan(21); + test.plan(23); var options = {alternateCommentMode: true}; var root = new protobuf.Root(); root.load("tests/data/comments-alternate-parse.proto", options, function(err, root) { @@ -24,6 +24,8 @@ tape.test("proto comments in alternate-parse mode", function(test) { test.equal(root.lookup("Test1.field8").comment, null, "should parse no comment"); test.equal(root.lookup("Test1.field9").comment, "Field with a\nmulti-line comment.", "should parse multiline double-slash field comment"); test.equal(root.lookup("Test1.field10").comment, "Field with a\nmulti-line doc-block comment.", "should parse multiline doc-block field comment"); + test.equal(root.lookup("Test1.field11").comment, "Field with both block comment", "should parse both trailing comment and trailing comment"); + test.equal(root.lookup("Test1.field12").comment, "Trailing comment in last line should not be recognized as leading comment for this field.", "trailing comment in last line should not be recognized as leading comment for this field"); test.equal(root.lookup("Test3").comments.ONE, "Value with a comment.", "should parse blocks for enum values"); test.equal(root.lookup("Test3").comments.TWO, "Value with a single-line comment.", "should parse double-slash comments for enum values"); @@ -38,3 +40,42 @@ tape.test("proto comments in alternate-parse mode", function(test) { test.end(); }); }); + +tape.test("proto comments in alternate-parse mode with trailing comment preferred", function(test) { + test.plan(23); + var options = {alternateCommentMode: true, preferTrailingComment: true}; + var root = new protobuf.Root(); + root.load("tests/data/comments-alternate-parse.proto", options, function(err, root) { + if (err) + throw test.fail(err.message); + + test.equal(root.lookup("Test1").comment, "Message with\na\nmulti-line comment.", "should parse double-slash multiline comment"); + test.equal(root.lookup("Test2").comment, "Message\nwith\na multiline plain slash-star\ncomment.", "should parse slash-star multiline comment"); + test.equal(root.lookup("Test3").comment, "Message\nwith\na\ncomment and stars.", "should parse doc-block multiline comment"); + + test.equal(root.lookup("Test1.field1").comment, "Field with a doc-block comment.", "should parse doc-block field comment"); + test.equal(root.lookup("Test1.field2").comment, "Field with a single-line comment starting with two slashes.", "should parse double-slash field comment"); + test.equal(root.lookup("Test1.field3").comment, "Field with a single-line comment starting with three slashes.", "should parse triple-slash field comment"); + test.equal(root.lookup("Test1.field4").comment, "Field with a single-line slash-star comment.", "should parse single-line slash-star field comment"); + test.equal(root.lookup("Test1.field5").comment, "Field with a trailing single-line two-slash comment.", "should parse trailing double-slash comment"); + test.equal(root.lookup("Test1.field6").comment, "Field with a trailing single-line three-slash comment.", "should parse trailing triple-slash comment"); + test.equal(root.lookup("Test1.field7").comment, "Field with a trailing single-line slash-star comment.", "should parse trailing slash-star comment"); + test.equal(root.lookup("Test1.field8").comment, null, "should parse no comment"); + test.equal(root.lookup("Test1.field9").comment, "Field with a\nmulti-line comment.", "should parse multiline double-slash field comment"); + test.equal(root.lookup("Test1.field10").comment, "Field with a\nmulti-line doc-block comment.", "should parse multiline doc-block field comment"); + test.equal(root.lookup("Test1.field11").comment, "and trailing comment.", "should parse both trailing comment and trailing comment"); + test.equal(root.lookup("Test1.field12").comment, "Trailing comment in last line should not be recognized as leading comment for this field.", "trailing comment in last line should not be recognized as leading comment for this field"); + + test.equal(root.lookup("Test3").comments.ONE, "Value with a comment.", "should parse blocks for enum values"); + test.equal(root.lookup("Test3").comments.TWO, "Value with a single-line comment.", "should parse double-slash comments for enum values"); + test.equal(root.lookup("Test3").comments.THREE, "ignored", "should prefer trailing comment when preferTrailingComment option enabled"); + test.equal(root.lookup("Test3").comments.FOUR, "Other value with a comment.", "should not confuse previous trailing comments with comments for the next field"); + + test.equal(root.lookup("ServiceTest.SingleLineMethod").comment, 'My method does things'); + test.equal(root.lookup("ServiceTest.TwoLineMethodWithComment").comment, 'TwoLineMethodWithComment documentation'); + test.equal(root.lookup("ServiceTest.ThreeLine012345678901234567890123456712345671234567123456783927483923473892837489238749832432874983274983274983274").comment, 'Very very long method'); + test.equal(root.lookup("ServiceTest.TwoLineMethodNoComment").comment, null); + + test.end(); + }); +});