Fixed: parser should not confuse previous trailing line comments with comments for the next declaration, see #762

Author: dcodeIO

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;
 }
 
 /**

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;

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 */
 }

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.
 }

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();
     });