Fix jsdoc typedef symbol scope + avoid bind twice if the tag has a jsdoc namespace

zhengbli · zhengbli · commit 20cc428785b2 · 2017-02-10T19:24:56.000-08:00
diff --git a/.gitignore b/.gitignore
@@ -57,3 +57,4 @@ internal/
 !tests/cases/projects/NodeModulesSearch/**/*
 !tests/baselines/reference/project/nodeModules*/**/*
 .idea
+yarn.lock
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -669,6 +669,12 @@ namespace ts {
                 case SyntaxKind.CallExpression:
                     bindCallExpressionFlow(<CallExpression>node);
                     break;
+                case SyntaxKind.JSDocComment:
+                    bindJSDocComment(<JSDoc>node);
+                    break;
+                case SyntaxKind.JSDocTypedefTag:
+                    bindJSDocTypedefTag(<JSDocTypedefTag>node);
+                    break;
                 default:
                     bindEachChild(node);
                     break;
@@ -1298,6 +1304,26 @@ namespace ts {
             }
         }
 
+        function bindJSDocComment(node: JSDoc) {
+            forEachChild(node, n => {
+                if (n.kind !== SyntaxKind.JSDocTypedefTag) {
+                    bind(n);
+                }
+            });
+        }
+
+        function bindJSDocTypedefTag(node: JSDocTypedefTag) {
+            forEachChild(node, n => {
+                // if the node has a fullName "A.B.C", that means symbol "C" was already bond
+                // when we visit "fullName"; so when we visit the name "C" as the next child of
+                // the jsDocTypedefTag, we should skip binding it.
+                if (n === node.name && node.fullName.kind !== SyntaxKind.Identifier) {
+                    return;
+                }
+                bind(n);
+            });
+        }
+
         function bindCallExpressionFlow(node: CallExpression) {
             // If the target of the call expression is a function expression or arrow function we have
             // an immediately invoked function expression (IIFE). Initialize the flowNode property to
@@ -1827,6 +1853,18 @@ namespace ts {
             }
             node.parent = parent;
             const saveInStrictMode = inStrictMode;
+
+            // Even though in the AST the jsdoc @typedef node belongs to the current node,
+            // its symbol might be in the same scope with the current node's symbol. Consider:
+            // 
+            //     /** @typedef {string | number} MyType */
+            //     function foo();
+            //
+            // Here the current node is "foo", which is a container, but the scope of "MyType" should
+            // not be inside "foo". Therefore we always bind @typedef before bind the parent node,
+            // and skip binding this tag later when binding all the other jsdoc tags.
+            bindJSDocTypedefTagIfAny(node);
+
             // First we bind declaration nodes to a symbol if possible. We'll both create a symbol
             // and then potentially add the symbol to an appropriate symbol table. Possible
             // destination symbol tables are:
@@ -1861,6 +1899,27 @@ namespace ts {
             inStrictMode = saveInStrictMode;
         }
 
+        function bindJSDocTypedefTagIfAny(node: Node) {
+            if (!node.jsDoc) {
+                return;
+            }
+
+            for (const jsDoc of node.jsDoc) {
+                if (!jsDoc.tags) {
+                    continue;
+                }
+
+                for (const tag of jsDoc.tags) {
+                    if (tag.kind === SyntaxKind.JSDocTypedefTag) {
+                        const savedParent = parent;
+                        parent = jsDoc;
+                        bind(tag);
+                        parent = savedParent;
+                    }
+                }
+            }
+        }
+
         function updateStrictModeStatementList(statements: NodeArray<Statement>) {
             if (!inStrictMode) {
                 for (const statement of statements) {
diff --git a/tests/cases/fourslash/server/jsdocTypedefTag1.ts b/tests/cases/fourslash/server/jsdocTypedefTag1.ts
@@ -0,0 +1,20 @@
+/// <reference path="../fourslash.ts"/>
+
+// @allowNonTsExtensions: true
+// @Filename: jsdocCompletion_typedef.js
+
+//// /**
+////  * @typedef {Object} MyType
+////  * @property {string} yes
+////  */
+//// function foo() { }
+
+//// /** 
+////  * @param {MyType} my
+////  */
+//// function a(my) {
+////     my.yes./*1*/
+//// }
+
+goTo.marker('1');
+verify.completionListContains('charAt');
diff --git a/tests/cases/fourslash/server/jsdocTypedefTag2.ts b/tests/cases/fourslash/server/jsdocTypedefTag2.ts
@@ -0,0 +1,30 @@
+/// <reference path="../fourslash.ts"/>
+
+// @allowNonTsExtensions: true
+// @Filename: jsdocCompletion_typedef.js
+
+//// /**
+////  * @typedef {Object} A.B.MyType
+////  * @property {string} yes
+////  */
+//// function foo() {}
+
+//// /** 
+////  * @param {A.B.MyType} my2
+////  */
+//// function a(my2) {
+////     my2.yes./*1*/
+//// }
+
+//// /** 
+////  * @param {MyType} my2
+////  */
+//// function b(my2) {
+////     my2.yes./*2*/
+//// }
+
+
+goTo.marker('1');
+verify.completionListContains('charAt');
+goTo.marker('2');
+verify.not.completionListContains('charAt');
