From 339a56fbf014dfdf9f9fd7a41affd242ae70137f Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Tue, 15 May 2018 15:12:29 -0700
Subject: [PATCH] Avoid duplicates when finding jsdoc (#24086)

* Avoid duplicates when finding jsdoc

1. Add a cheap assert for detecting duplicates. getJSDocTags must find
either [1] 0 or 1 comments or [2] the first two comments must not be
identical. This does not always find duplicates for nodes with 3 or more
comments, but these nodes are exceptionally rare.

This assert fired for over 20 of the around 250 tests we have that
retrieve JSDoc at all. So I fixed the asserts in [2] and [3].

2. There were overlapping cases from calls to getSourceOfAssignment and
getSpecialPropertyAssignment. getSpecialPropertyAssignment is too
restrictive, but was in the correct location (parent vs parent.parent),
so I removed the getSourceOfAssignment call and replaced the
getSpecialPropertyAssignment calls with a less restrictive check.

3. When a node's parent is a PropertyDeclaration,
getJSDocCOmmentsAndTags would check the parent for jsdoc. But when the
*node* is a PropertyDeclaration, getJSDocCommentsAndTags would use the
jsdoc from the initializer. This second case is useful to make sure that
property declarations get all their jsdoc, but results in duplicates for
their initializers. I couldn't think of a better fix than tracking the
previous node in the recursive lookup of getJSDocCommentsAndTags, which
is a little clunky.

* Fix lint; remove new context parameter

* Update importJsNodeModule3 with fix
---
 src/compiler/utilities.ts                    | 16 +++++++++-------
 tests/cases/fourslash/importJsNodeModule3.ts |  5 +----
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
index 29bd38803d0..d5cd22d7b7c 100644
--- a/src/compiler/utilities.ts
+++ b/src/compiler/utilities.ts
@@ -1842,9 +1842,9 @@ namespace ts {
             (node as ModuleDeclaration).body;
     }
 
-    export function getJSDocCommentsAndTags(node: Node): ReadonlyArray<JSDoc | JSDocTag> {
+    export function getJSDocCommentsAndTags(hostNode: Node): ReadonlyArray<JSDoc | JSDocTag> {
         let result: (JSDoc | JSDocTag)[] | undefined;
-        getJSDocCommentsAndTagsWorker(node);
+        getJSDocCommentsAndTagsWorker(hostNode);
         return result || emptyArray;
 
         function getJSDocCommentsAndTagsWorker(node: Node): void {
@@ -1860,7 +1860,7 @@ namespace ts {
             //   */
             // var x = function(name) { return name.length; }
             if (parent.parent &&
-                (getSingleVariableOfVariableStatement(parent.parent) === node || getSourceOfAssignment(parent.parent))) {
+                (getSingleVariableOfVariableStatement(parent.parent) === node)) {
                 getJSDocCommentsAndTagsWorker(parent.parent);
             }
             if (parent.parent && parent.parent.parent &&
@@ -1869,8 +1869,8 @@ namespace ts {
                     getSourceOfDefaultedAssignment(parent.parent.parent))) {
                 getJSDocCommentsAndTagsWorker(parent.parent.parent);
             }
-            if (isBinaryExpression(node) && getSpecialPropertyAssignmentKind(node) !== SpecialPropertyAssignmentKind.None ||
-                isBinaryExpression(parent) && getSpecialPropertyAssignmentKind(parent) !== SpecialPropertyAssignmentKind.None ||
+            if (isBinaryExpression(node) && node.operatorToken.kind === SyntaxKind.EqualsToken ||
+                isBinaryExpression(parent) && parent.operatorToken.kind === SyntaxKind.EqualsToken ||
                 node.kind === SyntaxKind.PropertyAccessExpression && node.parent && node.parent.kind === SyntaxKind.ExpressionStatement) {
                 getJSDocCommentsAndTagsWorker(parent);
             }
@@ -1880,7 +1880,7 @@ namespace ts {
                 result = addRange(result, getJSDocParameterTags(node as ParameterDeclaration));
             }
 
-            if (isVariableLike(node) && hasInitializer(node) && hasJSDocNodes(node.initializer)) {
+            if (isVariableLike(node) && hasInitializer(node) && node.initializer !== hostNode && hasJSDocNodes(node.initializer)) {
                 result = addRange(result, node.initializer.jsDoc);
             }
 
@@ -4771,7 +4771,9 @@ namespace ts {
         let tags = (node as JSDocContainer).jsDocCache;
         // If cache is 'null', that means we did the work of searching for JSDoc tags and came up with nothing.
         if (tags === undefined) {
-            (node as JSDocContainer).jsDocCache = tags = flatMap(getJSDocCommentsAndTags(node), j => isJSDoc(j) ? j.tags : j);
+            const comments = getJSDocCommentsAndTags(node);
+            Debug.assert(comments.length < 2 || comments[0] !== comments[1]);
+            (node as JSDocContainer).jsDocCache = tags = flatMap(comments, j => isJSDoc(j) ? j.tags : j);
         }
         return tags;
     }
diff --git a/tests/cases/fourslash/importJsNodeModule3.ts b/tests/cases/fourslash/importJsNodeModule3.ts
index 6c44dd65b63..46ff4d009be 100644
--- a/tests/cases/fourslash/importJsNodeModule3.ts
+++ b/tests/cases/fourslash/importJsNodeModule3.ts
@@ -37,12 +37,9 @@ edit.backspace(2);
 edit.insert('z(');
 verify.signatureHelp({
     text: "z(a: number | boolean, b: string[]): string",
-    // TODO: GH#24129
-    parameterDocComment: "The first param\nThe first param",
+    parameterDocComment: "The first param",
     tags: [
         { name: "param", text: "a The first param" },
         { name: "param", text: "b The second param" },
-        { name: "param", text: "a The first param" },
-        { name: "param", text: "b The second param" },
     ],
 });