From ec6a133ff541c638517e00f47b772990207c8640 Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Tue, 18 Apr 2017 21:15:39 +0200 Subject: [PATCH] Fixed: parser should not confuse previous trailing line comments with comments for the next declaration, see #762 --- index.d.ts | 30 ++------------------ src/parse.js | 4 +-- src/tokenize.js | 58 ++++++++++++++++++++------------------- tests/data/comments.proto | 5 +++- tests/docs_comments.js | 5 ++-- 5 files changed, 42 insertions(+), 60 deletions(-) diff --git a/index.d.ts b/index.d.ts index 7234fda3b..9717dcb2c 100644 --- a/index.d.ts +++ b/index.d.ts @@ -10,24 +10,6 @@ export function common(name: string, json: { [k: string]: any }): void; export namespace common { - /** Any */ - // let google/protobuf/any.proto: INamespace; - - /** Duration */ - // let google/protobuf/duration.proto: INamespace; - - /** Empty */ - // let google/protobuf/empty.proto: INamespace; - - /** Struct, Value, NullValue and ListValue */ - // let google/protobuf/struct.proto: INamespace; - - /** Timestamp */ - // let google/protobuf/timestamp.proto: INamespace; - - /** Wrappers */ - // let google/protobuf/wrappers.proto: INamespace; - /** * Gets the root definition of the specified common proto file. * @@ -1349,12 +1331,6 @@ export interface IService extends INamespace { methods: { [k: string]: IMethod }; } -/** - * Gets the current line number. - * @returns Line number - */ -type TokenizerHandleLine = () => number; - /** * Gets the next token and advances. * @returns Next token or `null` on eof @@ -1392,9 +1368,6 @@ type TokenizerHandleCmnt = (line?: number) => string; /** Handle object returned from {@link tokenize}. */ export interface ITokenizerHandle { - /** Gets the current line number */ - line: TokenizerHandleLine; - /** Gets the next token and advances (`null` on eof) */ next: TokenizerHandleNext; @@ -1409,6 +1382,9 @@ export interface ITokenizerHandle { /** Gets the comment on the previous line or the line comment on the specified line, if any */ cmnt: TokenizerHandleCmnt; + + /** Current line number */ + line: number; } /** diff --git a/src/parse.js b/src/parse.js index cec19449b..75dd90274 100644 --- a/src/parse.js +++ b/src/parse.js @@ -95,7 +95,7 @@ function parse(source, root, options) { var filename = parse.filename; if (!insideTryCatch) parse.filename = null; - return Error("illegal " + (name || "token") + " '" + token + "' (" + (filename ? filename + ", " : "") + "line " + tn.line() + ")"); + return Error("illegal " + (name || "token") + " '" + token + "' (" + (filename ? filename + ", " : "") + "line " + tn.line + ")"); } function readString() { @@ -279,7 +279,7 @@ function parse(source, root, options) { } function ifBlock(obj, fnIf, fnElse) { - var trailingLine = tn.line(); + var trailingLine = tn.line; if (obj) { obj.comment = cmnt(); // try block-type comment obj.filename = parse.filename; diff --git a/src/tokenize.js b/src/tokenize.js index 1b9c9e182..3be27e41f 100644 --- a/src/tokenize.js +++ b/src/tokenize.js @@ -38,13 +38,6 @@ function unescape(str) { tokenize.unescape = unescape; -/** - * Gets the current line number. - * @typedef TokenizerHandleLine - * @type {function} - * @returns {number} Line number - */ - /** * Gets the next token and advances. * @typedef TokenizerHandleNext @@ -88,12 +81,12 @@ tokenize.unescape = unescape; /** * Handle object returned from {@link tokenize}. * @interface ITokenizerHandle - * @property {TokenizerHandleLine} line Gets the current line number * @property {TokenizerHandleNext} next Gets the next token and advances (`null` on eof) * @property {TokenizerHandlePeek} peek Peeks for the next token (`null` on eof) * @property {TokenizerHandlePush} push Pushes a token back to the stack * @property {TokenizerHandleSkip} skip Skips a token, returns its presence and advances or, if non-optional and not present, throws * @property {TokenizerHandleCmnt} cmnt Gets the comment on the previous line or the line comment on the specified line, if any + * @property {number} line Current line number */ /** @@ -110,7 +103,8 @@ function tokenize(source) { line = 1, commentType = null, commentText = null, - commentLine = 0; + commentLine = 0, + commentLineEmpty = false; var stack = []; @@ -164,6 +158,15 @@ function tokenize(source) { function setComment(start, end) { commentType = source.charAt(start++); commentLine = line; + commentLineEmpty = false; + var offset = start - 3, // "///" or "/**" + c; + do { + if (--offset < 0 || (c = source.charAt(offset)) === "\n") { + commentLineEmpty = true; + break; + } + } while (c === " " || c === "\t"); var lines = source .substring(start, end) .split(setCommentSplitRe); @@ -188,7 +191,7 @@ function tokenize(source) { prev, curr, start, - isComment; + isDoc; do { if (offset === length) return null; @@ -203,17 +206,17 @@ function tokenize(source) { if (++offset === length) throw illegal("comment"); if (charAt(offset) === "/") { // Line - isComment = charAt(start = offset + 1) === "/"; + isDoc = charAt(start = offset + 1) === "/"; while (charAt(++offset) !== "\n") if (offset === length) return null; ++offset; - if (isComment) + if (isDoc) /// Comment setComment(start, offset - 1); ++line; repeat = true; } else if ((curr = charAt(offset)) === "*") { /* Block */ - isComment = charAt(start = offset + 1) === "*"; + isDoc = charAt(start = offset + 1) === "*"; do { if (curr === "\n") ++line; @@ -223,7 +226,7 @@ function tokenize(source) { curr = charAt(offset); } while (prev !== "*" || curr !== "/"); ++offset; - if (isComment) + if (isDoc) /** Comment */ setComment(start, offset - 2); repeat = true; } else @@ -292,33 +295,32 @@ function tokenize(source) { /** * Gets a comment. - * @param {number} [trailingLine] Trailing line number if applicable + * @param {number} [trailingLine] Line number if looking for a trailing comment * @returns {?string} Comment text * @inner */ function cmnt(trailingLine) { - var ret; - if (trailingLine === undefined) - ret = commentLine === line - 1 && commentText || null; - else { - if (!commentText) + var ret = null; + if (trailingLine === undefined) { + if (commentLine === line - 1 && (commentType === "*" || commentLineEmpty)) + ret = commentText; + } else { + if (commentLine !== trailingLine || commentLineEmpty || commentType !== "/") peek(); - ret = commentLine === trailingLine && commentType === "/" && commentText || null; + if (commentLine === trailingLine && !commentLineEmpty && commentType === "/") + ret = commentText; } - commentType = commentText = null; - commentLine = 0; return ret; } - return { + return Object.defineProperty({ next: next, peek: peek, push: push, skip: skip, - line: function() { - return line; - }, cmnt: cmnt - }; + }, "line", { + get: function() { return line; } + }); /* eslint-enable callback-return */ } diff --git a/tests/data/comments.proto b/tests/data/comments.proto index f1639db90..7f1032502 100644 --- a/tests/data/comments.proto +++ b/tests/data/comments.proto @@ -10,7 +10,7 @@ syntax = "proto3"; a ** comment. */ -message Test1 { +message Test1 { /// not a valid comment /** * Field with a comment. @@ -44,5 +44,8 @@ enum Test3 { // Value with no comment. TWO = 2; + /// Preferred value with a comment. THREE = 3; /// Value with a comment. + + FOUR = 4; /// Other value with a comment. } diff --git a/tests/docs_comments.js b/tests/docs_comments.js index 04a5c3f54..bb9375c37 100644 --- a/tests/docs_comments.js +++ b/tests/docs_comments.js @@ -3,7 +3,7 @@ var tape = require("tape"); var protobuf = require(".."); tape.test("proto comments", function(test) { - test.plan(9); + test.plan(10); protobuf.load("tests/data/comments.proto", function(err, root) { if (err) throw test.fail(err.message); @@ -18,7 +18,8 @@ tape.test("proto comments", function(test) { 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 parse triple-slash lines for enum values"); + test.equal(root.lookup("Test3").comments.THREE, "Preferred value with a comment.", "should parse lines for enum values and prefer on top over trailing"); + 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(); });