Fix comment parsing bug -- back up after seeing @ character

Andy Hanson · Andy Hanson · commit d142fcb7654a · 2018-01-10T10:46:56.000-08:00
diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -6268,7 +6268,6 @@ namespace ts {
                 scanner.scanRange(start + 3, length - 5, () => {
                     // Initially we can parse out a tag.  We also have seen a starting asterisk.
                     // This is so that /** * @type */ doesn't parse.
-                    let advanceToken = true;
                     let state = JSDocState.SawAsterisk;
                     let margin: number | undefined = undefined;
                     // + 4 for leading '/** '
@@ -6300,7 +6299,6 @@ namespace ts {
                                     // Real-world comments may break this rule, so "BeginningOfLine" will not be a real line beginning
                                     // for malformed examples like `/** @param {string} x @returns {number} the length */`
                                     state = JSDocState.BeginningOfLine;
-                                    advanceToken = false;
                                     margin = undefined;
                                     indent++;
                                 }
@@ -6352,13 +6350,7 @@ namespace ts {
                                 pushComment(scanner.getTokenText());
                                 break;
                         }
-                        if (advanceToken) {
-                            t = nextJSDocToken();
-                        }
-                        else {
-                            advanceToken = true;
-                            t = currentToken as JsDocSyntaxKind;
-                        }
+                        t = nextJSDocToken();
                     }
                     removeLeadingNewlines(comments);
                     removeTrailingNewlines(comments);
@@ -6458,7 +6450,7 @@ namespace ts {
                     addTag(tag);
                 }
 
-                function parseTagComments(indent: number): string {
+                function parseTagComments(indent: number): string | undefined {
                     const comments: string[] = [];
                     let state = JSDocState.BeginningOfLine;
                     let margin: number | undefined;
@@ -6480,6 +6472,8 @@ namespace ts {
                                 indent = 0;
                                 break;
                             case SyntaxKind.AtToken:
+                                scanner.setTextPos(scanner.getTextPos() - 1);
+                                // falls through
                             case SyntaxKind.EndOfFileToken:
                                 // Done
                                 break loop;
@@ -6515,7 +6509,7 @@ namespace ts {
 
                     removeLeadingNewlines(comments);
                     removeTrailingNewlines(comments);
-                    return comments.join("");
+                    return comments.length === 0 ? undefined : comments.join("");
                 }
 
                 function parseUnknownTag(atToken: AtToken, tagName: Identifier) {
@@ -6607,10 +6601,7 @@ namespace ts {
                         const start = scanner.getStartPos();
                         let children: JSDocParameterTag[];
                         while (child = tryParse(() => parseChildParameterOrPropertyTag(PropertyLikeParse.Parameter, name))) {
-                            if (!children) {
-                                children = [];
-                            }
-                            children.push(child);
+                            children = append(children, child);
                         }
                         if (children) {
                             jsdocTypeLiteral = <JSDocTypeLiteral>createNode(SyntaxKind.JSDocTypeLiteral, start);
@@ -6727,10 +6718,7 @@ namespace ts {
                                 }
                             }
                             else {
-                                if (!jsdocTypeLiteral.jsDocPropertyTags) {
-                                    jsdocTypeLiteral.jsDocPropertyTags = [] as MutableNodeArray<JSDocPropertyTag>;
-                                }
-                                (jsdocTypeLiteral.jsDocPropertyTags as MutableNodeArray<JSDocPropertyTag>).push(child);
+                                jsdocTypeLiteral.jsDocPropertyTags = append(jsdocTypeLiteral.jsDocPropertyTags as MutableNodeArray<JSDocPropertyTag>, child);
                             }
                         }
                         if (jsdocTypeLiteral) {
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1614,7 +1614,7 @@ namespace ts {
             (node as ModuleDeclaration).body;
     }
 
-    export function getJSDocCommentsAndTags(node: Node): (JSDoc | JSDocTag)[] {
+    export function getJSDocCommentsAndTags(node: Node): ReadonlyArray<JSDoc | JSDocTag> {
         let result: (JSDoc | JSDocTag)[] | undefined;
         getJSDocCommentsAndTagsWorker(node);
         return result || emptyArray;
diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts
@@ -54,7 +54,8 @@ namespace ts.JsDoc {
         // from Array<T> - Array<string> and Array<number>
         const documentationComment: SymbolDisplayPart[] = [];
         forEachUnique(declarations, declaration => {
-            for (const comment of getCommentsFromDeclaration(declaration)) {
+            for (const { comment } of getCommentHavingNodes(declaration)) {
+                if (comment === undefined) continue;
                 if (documentationComment.length) {
                     documentationComment.push(lineBreakPart());
                 }
@@ -64,20 +65,15 @@ namespace ts.JsDoc {
         return documentationComment;
     }
 
-    function getCommentsFromDeclaration(declaration: Declaration): string[] {
+    function getCommentHavingNodes(declaration: Declaration): ReadonlyArray<JSDoc | JSDocTag> {
         switch (declaration.kind) {
             case SyntaxKind.JSDocPropertyTag:
-                return [(declaration as JSDocPropertyTag).comment];
+                return [declaration as JSDocPropertyTag];
+            case SyntaxKind.JSDocTypedefTag:
+                return [(declaration as JSDocTypedefTag).parent];
             default:
-                return mapDefined(getAllJSDocs(declaration), doc => doc.comment);
-        }
-    }
-
-    function getAllJSDocs(node: Node): (JSDoc | JSDocTag)[] {
-        if (isJSDocTypedefTag(node)) {
-            return [node.parent];
+                return getJSDocCommentsAndTags(declaration);
         }
-        return getJSDocCommentsAndTags(node);
     }
 
     export function getJsDocTagsFromDeclarations(declarations?: Declaration[]): JSDocTagInfo[] {
@@ -91,7 +87,7 @@ namespace ts.JsDoc {
         return tags;
     }
 
-    function getCommentText(tag: JSDocTag): string {
+    function getCommentText(tag: JSDocTag): string | undefined {
         const { comment } = tag;
         switch (tag.kind) {
             case SyntaxKind.JSDocAugmentsTag:
@@ -106,15 +102,19 @@ namespace ts.JsDoc {
                 const { name } = tag as JSDocTypedefTag | JSDocPropertyTag | JSDocParameterTag;
                 return name ? withNode(name) : comment;
             default:
-                return comment;
+                return comment || "";
         }
 
         function withNode(node: Node) {
-            return `${node.getText()} ${comment}`;
+            return addComment(node.getText());
         }
 
         function withList(list: NodeArray<Node>): string {
-            return `${list.map(x => x.getText())} ${comment}`;
+            return addComment(list.map(x => x.getText()).join(", "));
+        }
+
+        function addComment(s: string) {
+            return comment === undefined ? s : `${s} ${comment}`;
         }
     }
 
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.leadingAsterisk.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.leadingAsterisk.json
@@ -27,8 +27,7 @@
                     "pos": 15,
                     "end": 21
                 }
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noLeadingAsterisk.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noLeadingAsterisk.json
@@ -27,8 +27,7 @@
                     "pos": 15,
                     "end": 21
                 }
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noReturnType.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noReturnType.json
@@ -17,8 +17,7 @@
                 "pos": 9,
                 "end": 15,
                 "escapedText": "return"
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json
@@ -35,8 +35,7 @@
                 "escapedText": "name1"
             },
             "isNameFirst": false,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json
@@ -35,8 +35,7 @@
                 "escapedText": "name1"
             },
             "isNameFirst": true,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json
@@ -25,8 +25,7 @@
                 "escapedText": "foo"
             },
             "isNameFirst": true,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnTag1.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnTag1.json
@@ -27,8 +27,7 @@
                     "pos": 17,
                     "end": 23
                 }
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnsTag1.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnsTag1.json
@@ -27,8 +27,7 @@
                     "pos": 18,
                     "end": 24
                 }
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag.json
@@ -33,8 +33,7 @@
                 "length": 1,
                 "pos": 18,
                 "end": 20
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag2.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag2.json
@@ -44,8 +44,7 @@
                 "length": 2,
                 "pos": 18,
                 "end": 22
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag3.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag3.json
@@ -44,8 +44,7 @@
                 "length": 2,
                 "pos": 18,
                 "end": 23
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag4.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag4.json
@@ -44,8 +44,7 @@
                 "length": 2,
                 "pos": 18,
                 "end": 23
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag5.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag5.json
@@ -44,8 +44,7 @@
                 "length": 2,
                 "pos": 18,
                 "end": 24
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json
@@ -35,8 +35,7 @@
                 "escapedText": "name1"
             },
             "isNameFirst": false,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "1": {
             "kind": "JSDocParameterTag",
@@ -70,8 +69,7 @@
                 "escapedText": "name2"
             },
             "isNameFirst": false,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "length": 2,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json
@@ -35,8 +35,7 @@
                 "escapedText": "name1"
             },
             "isNameFirst": false,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "1": {
             "kind": "JSDocParameterTag",
@@ -70,8 +69,7 @@
                 "escapedText": "name2"
             },
             "isNameFirst": false,
-            "isBracketed": false,
-            "comment": ""
+            "isBracketed": false
         },
         "length": 2,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typeTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typeTag.json
@@ -27,8 +27,7 @@
                     "pos": 15,
                     "end": 21
                 }
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json
@@ -104,8 +104,7 @@
                         "isBracketed": false
                     }
                 ]
-            },
-            "comment": ""
+            }
         },
         "length": 1,
         "pos": 8,
diff --git a/tests/baselines/reference/jsDocTypeTag1.js b/tests/baselines/reference/jsDocTypeTag1.js
diff --git a/tests/baselines/reference/jsDocTypeTag2.js b/tests/baselines/reference/jsDocTypeTag2.js
diff --git a/tests/baselines/reference/jsDocTypedef1.js b/tests/baselines/reference/jsDocTypedef1.js
diff --git a/tests/baselines/reference/quickInfoJsDocTags.baseline b/tests/baselines/reference/quickInfoJsDocTags.baseline
diff --git a/tests/cases/compiler/propertyTagFoo.ts b/tests/cases/compiler/propertyTagFoo.ts

Original file line number	Diff line number	Diff line change
`@@ -1614,7 +1614,7 @@ namespace ts {`
`1614`	`1614`	`(node as ModuleDeclaration).body;`
`1615`	`1615`	`}`
`1616`	`1616`
`1617`		`- export function getJSDocCommentsAndTags(node: Node): (JSDoc \| JSDocTag)[] {`
	`1617`	`+ export function getJSDocCommentsAndTags(node: Node): ReadonlyArray<JSDoc \| JSDocTag> {`
`1618`	`1618`	`let result: (JSDoc \| JSDocTag)[] \| undefined;`
`1619`	`1619`	`getJSDocCommentsAndTagsWorker(node);`
`1620`	`1620`	`return result \|\| emptyArray;`
Original file line number	Diff line number	Diff line change
`@@ -104,8 +104,7 @@`
`104`	`104`	`"isBracketed": false`
`105`	`105`	`}`
`106`	`106`	`]`
`107`		`- },`
`108`		`- "comment": ""`
	`107`	`+ }`
`109`	`108`	`},`
`110`	`109`	`"length": 1,`
`111`	`110`	`"pos": 8,`