Skip to content

Commit

Permalink
In TypeScript code, never bind JSDoc normally, just set parent pointe…
Browse files Browse the repository at this point in the history
…rs (#16555) (#16561)
  • Loading branch information
Andy committed Jun 17, 2017
1 parent 0968ed9 commit 2721fd4
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 18 deletions.
44 changes: 26 additions & 18 deletions src/compiler/binder.ts
Expand Up @@ -595,7 +595,19 @@ namespace ts {
// Binding of JsDocComment should be done before the current block scope container changes.
// because the scope of JsDocComment should not be affected by whether the current node is a
// container or not.
forEach(node.jsDoc, bind);
if (node.jsDoc) {
if (isInJavaScriptFile(node)) {
for (const j of node.jsDoc) {
bind(j);
}
}
else {
for (const j of node.jsDoc) {
setParentPointers(node, j);
}
}
}

if (checkUnreachable(node)) {
bindEachChild(node);
return;
Expand Down Expand Up @@ -1903,7 +1915,7 @@ namespace ts {
// 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);
if (isInJavaScriptFile(node)) 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
Expand Down Expand Up @@ -1991,7 +2003,7 @@ namespace ts {
// for typedef type names with namespaces, bind the new jsdoc type symbol here
// because it requires all containing namespaces to be in effect, namely the
// current "blockScopeContainer" needs to be set to its immediate namespace parent.
if (isInJavaScriptFile(node) && (<Identifier>node).isInJSDocNamespace) {
if ((<Identifier>node).isInJSDocNamespace) {
let parentNode = node.parent;
while (parentNode && parentNode.kind !== SyntaxKind.JSDocTypedefTag) {
parentNode = parentNode.parent;
Expand Down Expand Up @@ -2053,10 +2065,7 @@ namespace ts {
case SyntaxKind.TypePredicate:
return checkTypePredicate(node as TypePredicateNode);
case SyntaxKind.TypeParameter:
if (node.parent.kind !== ts.SyntaxKind.JSDocTemplateTag || isInJavaScriptFile(node)) {
return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes);
}
return;
return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes);
case SyntaxKind.Parameter:
return bindParameter(<ParameterDeclaration>node);
case SyntaxKind.VariableDeclaration:
Expand Down Expand Up @@ -2138,10 +2147,7 @@ namespace ts {
case SyntaxKind.EnumDeclaration:
return bindEnumDeclaration(<EnumDeclaration>node);
case SyntaxKind.ModuleDeclaration:
if (node.parent.kind !== ts.SyntaxKind.JSDocTypedefTag || isInJavaScriptFile(node)) {
return bindModuleDeclaration(<ModuleDeclaration>node);
}
return undefined;
return bindModuleDeclaration(<ModuleDeclaration>node);
// Jsx-attributes
case SyntaxKind.JsxAttributes:
return bindJsxAttributes(<JsxAttributes>node);
Expand Down Expand Up @@ -2173,13 +2179,6 @@ namespace ts {
case SyntaxKind.ModuleBlock:
return updateStrictModeStatementList((<Block | ModuleBlock>node).statements);

default:
if (isInJavaScriptFile(node)) return bindJSDocWorker(node);
}
}

function bindJSDocWorker(node: Node) {
switch (node.kind) {
case SyntaxKind.JSDocRecordMember:
return bindPropertyWorker(node as JSDocRecordMember);
case SyntaxKind.JSDocPropertyTag:
Expand Down Expand Up @@ -3605,4 +3604,13 @@ namespace ts {
return TransformFlags.NodeExcludes;
}
}

/**
* "Binds" JSDoc nodes in TypeScript code.
* Since we will never create symbols for JSDoc, we just set parent pointers instead.
*/
function setParentPointers(parent: Node, child: Node): void {
child.parent = parent;
forEachChild(child, (childsChild) => setParentPointers(child, childsChild));
}
}
6 changes: 6 additions & 0 deletions tests/baselines/reference/jsdocInTypeScript.errors.txt
Expand Up @@ -61,4 +61,10 @@ tests/cases/compiler/jsdocInTypeScript.ts(42,12): error TS2503: Cannot find name
import M = N; // Error: @typedef does not create namespaces in TypeScript code.
~
!!! error TS2503: Cannot find namespace 'N'.

// Not legal JSDoc, but that shouldn't matter in TypeScript.
/**
* @type {{foo: (function(string, string): string)}}
*/
const obj = { foo: (a, b) => a + b };

11 changes: 11 additions & 0 deletions tests/baselines/reference/jsdocInTypeScript.js
Expand Up @@ -41,6 +41,12 @@ let i: I; // Should succeed thanks to type parameter default

/** @typedef {string} N.Str */
import M = N; // Error: @typedef does not create namespaces in TypeScript code.

// Not legal JSDoc, but that shouldn't matter in TypeScript.
/**
* @type {{foo: (function(string, string): string)}}
*/
const obj = { foo: (a, b) => a + b };


//// [jsdocInTypeScript.js]
Expand Down Expand Up @@ -68,3 +74,8 @@ function tem(t) { return {}; }
var i; // Should succeed thanks to type parameter default
/** @typedef {string} N.Str */
var M = N; // Error: @typedef does not create namespaces in TypeScript code.
// Not legal JSDoc, but that shouldn't matter in TypeScript.
/**
* @type {{foo: (function(string, string): string)}}
*/
var obj = { foo: function (a, b) { return a + b; } };
6 changes: 6 additions & 0 deletions tests/cases/compiler/jsdocInTypeScript.ts
Expand Up @@ -40,3 +40,9 @@ let i: I; // Should succeed thanks to type parameter default

/** @typedef {string} N.Str */
import M = N; // Error: @typedef does not create namespaces in TypeScript code.

// Not legal JSDoc, but that shouldn't matter in TypeScript.
/**
* @type {{foo: (function(string, string): string)}}
*/
const obj = { foo: (a, b) => a + b };

0 comments on commit 2721fd4

Please sign in to comment.