Skip to content

Fix binding of jsdoc typedefs with no in-comment name attached to an expression statement #32610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,11 +490,11 @@ namespace ts {
// and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
if (isJSDocTypeAlias(node)) Debug.assert(isInJSFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypeAlias(node)) {
if (hasModifier(node, ModifierFlags.Default) && !getDeclarationName(node)) {
if (!container.locals || (hasModifier(node, ModifierFlags.Default) && !getDeclarationName(node))) {
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
}
const exportKind = symbolFlags & SymbolFlags.Value ? SymbolFlags.ExportValue : 0;
const local = declareSymbol(container.locals!, /*parent*/ undefined, node, exportKind, symbolExcludes);
const local = declareSymbol(container.locals, /*parent*/ undefined, node, exportKind, symbolExcludes);
local.exportSymbol = declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes);
node.localSymbol = local;
return local;
Expand Down Expand Up @@ -1806,7 +1806,19 @@ namespace ts {
currentFlow = { flags: FlowFlags.Start };
parent = typeAlias;
bind(typeAlias.typeExpression);
if (isJSDocEnumTag(typeAlias) || !typeAlias.fullName || typeAlias.fullName.kind === SyntaxKind.Identifier) {
const declName = getNameOfDeclaration(typeAlias);
if ((isJSDocEnumTag(typeAlias) || !typeAlias.fullName) && declName && isPropertyAccessEntityNameExpression(declName.parent)) {
// typedef anchored to an A.B.C assignment - we need to bind into B's namespace under name C
const isTopLevel = isTopLevelNamespaceAssignment(declName.parent);
if (isTopLevel) {
bindPotentiallyMissingNamespaces(file.symbol, declName.parent, isTopLevel, !!findAncestor(declName, d => isPropertyAccessExpression(d) && d.name.escapedText === "prototype"));
const oldContainer = container;
container = isPropertyAccessExpression(declName.parent.expression) ? declName.parent.expression.name : declName.parent.expression;
declareModuleMember(typeAlias, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
container = oldContainer;
}
}
else if (isJSDocEnumTag(typeAlias) || !typeAlias.fullName || typeAlias.fullName.kind === SyntaxKind.Identifier) {
parent = typeAlias.parent;
bindBlockScopedDeclaration(typeAlias, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
}
Expand Down Expand Up @@ -2607,7 +2619,7 @@ namespace ts {
}

function bindPotentiallyMissingNamespaces(namespaceSymbol: Symbol | undefined, entityName: EntityNameExpression, isToplevel: boolean, isPrototypeProperty: boolean) {
if (isToplevel && !isPrototypeProperty && (!namespaceSymbol || !(namespaceSymbol.flags & SymbolFlags.Namespace))) {
if (isToplevel && !isPrototypeProperty) {
// make symbols or add declarations for intermediate containers
const flags = SymbolFlags.Module | SymbolFlags.Assignment;
const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.Assignment;
Expand Down Expand Up @@ -2642,11 +2654,15 @@ namespace ts {
declareSymbol(symbolTable, namespaceSymbol, declaration, includes | SymbolFlags.Assignment, excludes & ~SymbolFlags.Assignment);
}

function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) {
let namespaceSymbol = lookupSymbolForPropertyAccess(name);
const isToplevel = isBinaryExpression(propertyAccess.parent)
function isTopLevelNamespaceAssignment(propertyAccess: PropertyAccessEntityNameExpression) {
return isBinaryExpression(propertyAccess.parent)
? getParentOfBinaryExpression(propertyAccess.parent).parent.kind === SyntaxKind.SourceFile
: propertyAccess.parent.parent.kind === SyntaxKind.SourceFile;
}

function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) {
let namespaceSymbol = lookupSymbolForPropertyAccess(name);
const isToplevel = isTopLevelNamespaceAssignment(propertyAccess);
namespaceSymbol = bindPotentiallyMissingNamespaces(namespaceSymbol, propertyAccess.expression, isToplevel, isPrototypeProperty);
bindPotentiallyNewExpandoMemberToNamespace(propertyAccess, namespaceSymbol, isPrototypeProperty);
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3661,8 +3661,8 @@ namespace ts {

Enum = RegularEnum | ConstEnum,
Variable = FunctionScopedVariable | BlockScopedVariable,
Value = Variable | Property | EnumMember | ObjectLiteral | Function | Class | Enum | ValueModule | Method | GetAccessor | SetAccessor | Assignment,
Type = Class | Interface | Enum | EnumMember | TypeLiteral | TypeParameter | TypeAlias | Assignment,
Value = Variable | Property | EnumMember | ObjectLiteral | Function | Class | Enum | ValueModule | Method | GetAccessor | SetAccessor,
Type = Class | Interface | Enum | EnumMember | TypeLiteral | TypeParameter | TypeAlias,
Namespace = ValueModule | NamespaceModule | Enum,
Module = ValueModule | NamespaceModule,
Accessor = GetAccessor | SetAccessor,
Expand All @@ -3683,7 +3683,7 @@ namespace ts {
InterfaceExcludes = Type & ~(Interface | Class),
RegularEnumExcludes = (Value | Type) & ~(RegularEnum | ValueModule), // regular enums merge only with regular enums and modules
ConstEnumExcludes = (Value | Type) & ~ConstEnum, // const enums merge only with const enums
ValueModuleExcludes = Value & ~(Function | Class | RegularEnum | ValueModule | Assignment),
ValueModuleExcludes = Value & ~(Function | Class | RegularEnum | ValueModule),
NamespaceModuleExcludes = 0,
MethodExcludes = Value & ~Method,
GetAccessorExcludes = Value & ~SetAccessor,
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5108,7 +5108,10 @@ namespace ts {
}
break;
case SyntaxKind.ExpressionStatement:
const expr = hostNode.expression;
let expr = hostNode.expression;
if (expr.kind === SyntaxKind.BinaryExpression && (expr as BinaryExpression).operatorToken.kind === SyntaxKind.EqualsToken) {
expr = (expr as BinaryExpression).left;
}
switch (expr.kind) {
case SyntaxKind.PropertyAccessExpression:
return (expr as PropertyAccessExpression).name;
Expand Down
32 changes: 16 additions & 16 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2127,28 +2127,28 @@ declare namespace ts {
ModuleExports = 134217728,
Enum = 384,
Variable = 3,
Value = 67220415,
Type = 67897832,
Value = 111551,
Type = 788968,
Namespace = 1920,
Module = 1536,
Accessor = 98304,
FunctionScopedVariableExcludes = 67220414,
BlockScopedVariableExcludes = 67220415,
ParameterExcludes = 67220415,
FunctionScopedVariableExcludes = 111550,
BlockScopedVariableExcludes = 111551,
ParameterExcludes = 111551,
PropertyExcludes = 0,
EnumMemberExcludes = 68008959,
FunctionExcludes = 67219887,
ClassExcludes = 68008383,
InterfaceExcludes = 67897736,
RegularEnumExcludes = 68008191,
ConstEnumExcludes = 68008831,
EnumMemberExcludes = 900095,
FunctionExcludes = 111023,
ClassExcludes = 899519,
InterfaceExcludes = 788872,
RegularEnumExcludes = 899327,
ConstEnumExcludes = 899967,
ValueModuleExcludes = 110735,
NamespaceModuleExcludes = 0,
MethodExcludes = 67212223,
GetAccessorExcludes = 67154879,
SetAccessorExcludes = 67187647,
TypeParameterExcludes = 67635688,
TypeAliasExcludes = 67897832,
MethodExcludes = 103359,
GetAccessorExcludes = 46015,
SetAccessorExcludes = 78783,
TypeParameterExcludes = 526824,
TypeAliasExcludes = 788968,
AliasExcludes = 2097152,
ModuleMember = 2623475,
ExportHasLocal = 944,
Expand Down
32 changes: 16 additions & 16 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2127,28 +2127,28 @@ declare namespace ts {
ModuleExports = 134217728,
Enum = 384,
Variable = 3,
Value = 67220415,
Type = 67897832,
Value = 111551,
Type = 788968,
Namespace = 1920,
Module = 1536,
Accessor = 98304,
FunctionScopedVariableExcludes = 67220414,
BlockScopedVariableExcludes = 67220415,
ParameterExcludes = 67220415,
FunctionScopedVariableExcludes = 111550,
BlockScopedVariableExcludes = 111551,
ParameterExcludes = 111551,
PropertyExcludes = 0,
EnumMemberExcludes = 68008959,
FunctionExcludes = 67219887,
ClassExcludes = 68008383,
InterfaceExcludes = 67897736,
RegularEnumExcludes = 68008191,
ConstEnumExcludes = 68008831,
EnumMemberExcludes = 900095,
FunctionExcludes = 111023,
ClassExcludes = 899519,
InterfaceExcludes = 788872,
RegularEnumExcludes = 899327,
ConstEnumExcludes = 899967,
ValueModuleExcludes = 110735,
NamespaceModuleExcludes = 0,
MethodExcludes = 67212223,
GetAccessorExcludes = 67154879,
SetAccessorExcludes = 67187647,
TypeParameterExcludes = 67635688,
TypeAliasExcludes = 67897832,
MethodExcludes = 103359,
GetAccessorExcludes = 46015,
SetAccessorExcludes = 78783,
TypeParameterExcludes = 526824,
TypeAliasExcludes = 788968,
AliasExcludes = 2097152,
ModuleMember = 2623475,
ExportHasLocal = 944,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var lol = "hello Lol"
>lol : Symbol(lol, Decl(0.js, 1, 3))

const obj = {
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))

/** @type {string|undefined} */
foo: undefined,
Expand Down Expand Up @@ -40,31 +40,31 @@ const obj = {
}
obj.foo = 'string'
>obj.foo : Symbol(foo, Decl(0.js, 2, 13))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))
>foo : Symbol(foo, Decl(0.js, 2, 13))

obj.lol
>obj.lol : Symbol(lol, Decl(0.js, 10, 4))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))
>lol : Symbol(lol, Decl(0.js, 10, 4))

obj.bar = undefined;
>obj.bar : Symbol(bar, Decl(0.js, 4, 17))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))
>bar : Symbol(bar, Decl(0.js, 4, 17))
>undefined : Symbol(undefined)

var k = obj.method1(0);
>k : Symbol(k, Decl(0.js, 21, 3))
>obj.method1 : Symbol(method1, Decl(0.js, 6, 12))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))
>method1 : Symbol(method1, Decl(0.js, 6, 12))

obj.bar1 = "42";
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))

obj.arrowFunc(0);
>obj.arrowFunc : Symbol(arrowFunc, Decl(0.js, 14, 20))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1))
>obj : Symbol(obj, Decl(0.js, 2, 5), Decl(0.js, 17, 1), Decl(0.js, 19, 7), Decl(0.js, 21, 23))
>arrowFunc : Symbol(arrowFunc, Decl(0.js, 14, 20))

26 changes: 13 additions & 13 deletions tests/baselines/reference/checkObjectDefineProperty.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ x.middleInit = "R"; // should also fail

=== tests/cases/conformance/jsdoc/index.js ===
const x = {};
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)

Object.defineProperty(x, "name", { value: "Charles", writable: true });
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"name" : Symbol(x.name, Decl(index.js, 0, 13))
>value : Symbol(value, Decl(index.js, 1, 34))
>writable : Symbol(writable, Decl(index.js, 1, 52))
Expand All @@ -85,15 +85,15 @@ Object.defineProperty(x, "middleInit", { value: "H" });
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"middleInit" : Symbol(x.middleInit, Decl(index.js, 1, 71))
>value : Symbol(value, Decl(index.js, 2, 40))

Object.defineProperty(x, "lastName", { value: "Smith", writable: false });
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"lastName" : Symbol(x.lastName, Decl(index.js, 2, 55))
>value : Symbol(value, Decl(index.js, 3, 38))
>writable : Symbol(writable, Decl(index.js, 3, 54))
Expand All @@ -102,7 +102,7 @@ Object.defineProperty(x, "zip", { get() { return 98122 }, set(_) { /*ignore*/ }
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"zip" : Symbol(x.zip, Decl(index.js, 3, 74))
>get : Symbol(get, Decl(index.js, 4, 33))
>set : Symbol(set, Decl(index.js, 4, 57))
Expand All @@ -112,15 +112,15 @@ Object.defineProperty(x, "houseNumber", { get() { return 21.75 } });
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"houseNumber" : Symbol(x.houseNumber, Decl(index.js, 4, 83))
>get : Symbol(get, Decl(index.js, 5, 41))

Object.defineProperty(x, "zipStr", {
>Object.defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>defineProperty : Symbol(ObjectConstructor.defineProperty, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>"zipStr" : Symbol(x.zipStr, Decl(index.js, 5, 68))

/** @param {string} str */
Expand All @@ -147,15 +147,15 @@ function takeName(named) { return named.name; }

takeName(x);
>takeName : Symbol(takeName, Decl(index.js, 11, 3))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)

/**
* @type {number}
*/
var a = x.zip;
>a : Symbol(a, Decl(index.js, 22, 3))
>x.zip : Symbol(x.zip, Decl(index.js, 3, 74))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>zip : Symbol(x.zip, Decl(index.js, 3, 74))

/**
Expand All @@ -164,17 +164,17 @@ var a = x.zip;
var b = x.houseNumber;
>b : Symbol(b, Decl(index.js, 27, 3))
>x.houseNumber : Symbol(x.houseNumber, Decl(index.js, 4, 83))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)
>houseNumber : Symbol(x.houseNumber, Decl(index.js, 4, 83))

const returnExemplar = () => x;
>returnExemplar : Symbol(returnExemplar, Decl(index.js, 29, 5))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)

const needsExemplar = (_ = x) => void 0;
>needsExemplar : Symbol(needsExemplar, Decl(index.js, 30, 5))
>_ : Symbol(_, Decl(index.js, 30, 23))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)

const expected = /** @type {{name: string, readonly middleInit: string, readonly lastName: string, zip: number, readonly houseNumber: number, zipStr: string}} */(/** @type {*} */(null));
>expected : Symbol(expected, Decl(index.js, 32, 5))
Expand All @@ -199,5 +199,5 @@ module.exports = x;
>module.exports : Symbol("tests/cases/conformance/jsdoc/index", Decl(index.js, 0, 0))
>module : Symbol(export=, Decl(index.js, 41, 48))
>exports : Symbol(export=, Decl(index.js, 41, 48))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22))
>x : Symbol(x, Decl(index.js, 0, 5), Decl(index.js, 1, 22), Decl(index.js, 2, 22), Decl(index.js, 3, 22), Decl(index.js, 4, 22) ... and 2 more)

Loading