From 3c4276076595446ddefd24e16d549e1323a3ed8a Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 21 Aug 2019 15:36:35 -0700 Subject: [PATCH] Cache JS inferred class type symbol (#33010) * Cache JS inferred class type symbol Note that many sources merge into a single target, so the *source* [links] is the one that caches the merged target. The reason this is a problem is not that many sources merge into a single target, but that both getTypeOfSymbol and getDeclaredTypeOfSymbol end up calling mergeJSSymbols with the same [source,target] pair. The merge should not happen twice. * Remove more verbose debug assertion message * Fix isJSConstructor check + update baselines * inferClassSymbol cache now track multiple targets --- src/compiler/checker.ts | 27 ++++---- src/compiler/types.ts | 47 +++++++------- .../chainedPrototypeAssignment.errors.txt | 4 +- .../chainedPrototypeAssignment.symbols | 10 +-- .../chainedPrototypeAssignment.types | 10 +-- ...ertyAssignmentMergeAcrossFiles2.errors.txt | 31 +++++++++ ...ropertyAssignmentMergeAcrossFiles2.symbols | 52 +++++++++++++++ ...ePropertyAssignmentMergeAcrossFiles2.types | 65 +++++++++++++++++++ .../salsa/chainedPrototypeAssignment.ts | 4 +- ...typePropertyAssignmentMergeAcrossFiles2.ts | 25 +++++++ 10 files changed, 229 insertions(+), 46 deletions(-) create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.errors.txt create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.symbols create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.types create mode 100644 tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2444c5ab1bc..3490ca961c5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -22879,24 +22879,29 @@ namespace ts { // If the symbol of the node has members, treat it like a constructor. const symbol = getSymbolOfNode(func); - return !!symbol && symbol.members !== undefined; + return !!symbol && hasEntries(symbol.members); } return false; } function mergeJSSymbols(target: Symbol, source: Symbol | undefined) { if (source && (hasEntries(source.exports) || hasEntries(source.members))) { - target = cloneSymbol(target); - if (hasEntries(source.exports)) { - target.exports = target.exports || createSymbolTable(); - mergeSymbolTable(target.exports, source.exports); + const links = getSymbolLinks(source); + if (!links.inferredClassSymbol || !links.inferredClassSymbol.has("" + getSymbolId(target))) { + const inferred = isTransientSymbol(target) ? target : cloneSymbol(target) as TransientSymbol; + inferred.exports = inferred.exports || createSymbolTable(); + inferred.members = inferred.members || createSymbolTable(); + inferred.flags |= source.flags & SymbolFlags.Class; + if (hasEntries(source.exports)) { + mergeSymbolTable(inferred.exports, source.exports); + } + if (hasEntries(source.members)) { + mergeSymbolTable(inferred.members, source.members); + } + (links.inferredClassSymbol || (links.inferredClassSymbol = createMap())).set("" + getSymbolId(inferred), inferred); + return inferred; } - if (hasEntries(source.members)) { - target.members = target.members || createSymbolTable(); - mergeSymbolTable(target.members, source.members); - } - target.flags |= source.flags & SymbolFlags.Class; - return target as TransientSymbol; + return links.inferredClassSymbol.get("" + getSymbolId(target)); } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5e6f36346e3..dbb2dec0c51 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3770,30 +3770,31 @@ namespace ts { /* @internal */ export interface SymbolLinks { - immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead. - target?: Symbol; // Resolved (non-alias) target of an alias - type?: Type; // Type of value symbol - uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol - declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter - resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference - typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic) - outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type - instantiations?: Map; // Instantiations of generic type alias (undefined if non-generic) - mapper?: TypeMapper; // Type mapper for instantiation alias - referenced?: boolean; // True if alias symbol has been referenced as a value - containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property - leftSpread?: Symbol; // Left source for synthetic spread property - rightSpread?: Symbol; // Right source for synthetic spread property - syntheticOrigin?: Symbol; // For a property on a mapped or spread type, points back to the original property - isDiscriminantProperty?: boolean; // True if discriminant synthetic property - resolvedExports?: SymbolTable; // Resolved exports of module or combined early- and late-bound static members of a class. - resolvedMembers?: SymbolTable; // Combined early- and late-bound members of a symbol - exportsChecked?: boolean; // True if exports of external module have been checked - typeParametersChecked?: boolean; // True if type parameters of merged class and interface declarations have been checked. + immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead. + target?: Symbol; // Resolved (non-alias) target of an alias + type?: Type; // Type of value symbol + uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol + declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter + resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference + typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic) + outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type + instantiations?: Map; // Instantiations of generic type alias (undefined if non-generic) + inferredClassSymbol?: Map; // Symbol of an inferred ES5 constructor function + mapper?: TypeMapper; // Type mapper for instantiation alias + referenced?: boolean; // True if alias symbol has been referenced as a value + containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property + leftSpread?: Symbol; // Left source for synthetic spread property + rightSpread?: Symbol; // Right source for synthetic spread property + syntheticOrigin?: Symbol; // For a property on a mapped or spread type, points back to the original property + isDiscriminantProperty?: boolean; // True if discriminant synthetic property + resolvedExports?: SymbolTable; // Resolved exports of module or combined early- and late-bound static members of a class. + resolvedMembers?: SymbolTable; // Combined early- and late-bound members of a symbol + exportsChecked?: boolean; // True if exports of external module have been checked + typeParametersChecked?: boolean; // True if type parameters of merged class and interface declarations have been checked. isDeclarationWithCollidingName?: boolean; // True if symbol is block scoped redeclaration - bindingElement?: BindingElement; // Binding element associated with property symbol - exportsSomeValue?: boolean; // True if module exports some value (not just types) - enumKind?: EnumKind; // Enum declaration classification + bindingElement?: BindingElement; // Binding element associated with property symbol + exportsSomeValue?: boolean; // True if module exports some value (not just types) + enumKind?: EnumKind; // Enum declaration classification originatingImport?: ImportDeclaration | ImportCall; // Import declaration which produced the symbol, present if the symbol is marked as uncallable but had call signatures in `resolveESModuleSymbol` lateSymbol?: Symbol; // Late-bound symbol for a computed property specifierCache?: Map; // For symbols corresponding to external modules, a cache of incoming path -> module specifier name mappings diff --git a/tests/baselines/reference/chainedPrototypeAssignment.errors.txt b/tests/baselines/reference/chainedPrototypeAssignment.errors.txt index a79daa92658..154a66c2a58 100644 --- a/tests/baselines/reference/chainedPrototypeAssignment.errors.txt +++ b/tests/baselines/reference/chainedPrototypeAssignment.errors.txt @@ -19,10 +19,10 @@ tests/cases/conformance/salsa/use.js(6,5): error TS2345: Argument of type '"not declare var exports: any; ==== tests/cases/conformance/salsa/mod.js (0 errors) ==== /// - var A = function() { + var A = function A() { this.a = 1 } - var B = function() { + var B = function B() { this.b = 2 } exports.A = A diff --git a/tests/baselines/reference/chainedPrototypeAssignment.symbols b/tests/baselines/reference/chainedPrototypeAssignment.symbols index 800915e5316..a37ce9f01db 100644 --- a/tests/baselines/reference/chainedPrototypeAssignment.symbols +++ b/tests/baselines/reference/chainedPrototypeAssignment.symbols @@ -37,17 +37,19 @@ declare var exports: any; === tests/cases/conformance/salsa/mod.js === /// -var A = function() { +var A = function A() { >A : Symbol(A, Decl(mod.js, 1, 3), Decl(mod.js, 8, 13)) +>A : Symbol(A, Decl(mod.js, 1, 7)) this.a = 1 ->a : Symbol(A.a, Decl(mod.js, 1, 20)) +>a : Symbol(A.a, Decl(mod.js, 1, 22)) } -var B = function() { +var B = function B() { >B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13)) +>B : Symbol(B, Decl(mod.js, 4, 7)) this.b = 2 ->b : Symbol(B.b, Decl(mod.js, 4, 20)) +>b : Symbol(B.b, Decl(mod.js, 4, 22)) } exports.A = A >exports.A : Symbol(A, Decl(mod.js, 6, 1)) diff --git a/tests/baselines/reference/chainedPrototypeAssignment.types b/tests/baselines/reference/chainedPrototypeAssignment.types index 6e877361ae6..0e86f9e4049 100644 --- a/tests/baselines/reference/chainedPrototypeAssignment.types +++ b/tests/baselines/reference/chainedPrototypeAssignment.types @@ -44,9 +44,10 @@ declare var exports: any; === tests/cases/conformance/salsa/mod.js === /// -var A = function() { +var A = function A() { +>A : typeof A +>function A() { this.a = 1} : typeof A >A : typeof A ->function() { this.a = 1} : typeof A this.a = 1 >this.a = 1 : 1 @@ -55,9 +56,10 @@ var A = function() { >a : any >1 : 1 } -var B = function() { +var B = function B() { +>B : typeof B +>function B() { this.b = 2} : typeof B >B : typeof B ->function() { this.b = 2} : typeof B this.b = 2 >this.b = 2 : 2 diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.errors.txt b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.errors.txt new file mode 100644 index 00000000000..602294ea5ff --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.errors.txt @@ -0,0 +1,31 @@ +tests/cases/conformance/salsa/other.js(5,5): error TS2339: Property 'wat' does not exist on type 'One'. +tests/cases/conformance/salsa/other.js(10,5): error TS2339: Property 'wat' does not exist on type 'Two'. + + +==== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js (0 errors) ==== + var Ns = {} + Ns.One = function() {}; + Ns.Two = function() {}; + + Ns.One.prototype = { + ok() {}, + }; + Ns.Two.prototype = { + } + +==== tests/cases/conformance/salsa/other.js (2 errors) ==== + /** + * @type {Ns.One} + */ + var one; + one.wat; + ~~~ +!!! error TS2339: Property 'wat' does not exist on type 'One'. + /** + * @type {Ns.Two} + */ + var two; + two.wat; + ~~~ +!!! error TS2339: Property 'wat' does not exist on type 'Two'. + \ No newline at end of file diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.symbols b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.symbols new file mode 100644 index 00000000000..6646bac5ee9 --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.symbols @@ -0,0 +1,52 @@ +=== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js === +var Ns = {} +>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) + +Ns.One = function() {}; +>Ns.One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3)) +>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +>One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3)) + +Ns.Two = function() {}; +>Ns.Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3)) +>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +>Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3)) + +Ns.One.prototype = { +>Ns.One.prototype : Symbol(Ns.One.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23)) +>Ns.One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3)) +>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +>One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3)) +>prototype : Symbol(Ns.One.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23)) + + ok() {}, +>ok : Symbol(ok, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 20)) + +}; +Ns.Two.prototype = { +>Ns.Two.prototype : Symbol(Ns.Two.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +>Ns.Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3)) +>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +>Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3)) +>prototype : Symbol(Ns.Two.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2)) +} + +=== tests/cases/conformance/salsa/other.js === +/** + * @type {Ns.One} + */ +var one; +>one : Symbol(one, Decl(other.js, 3, 3)) + +one.wat; +>one : Symbol(one, Decl(other.js, 3, 3)) + +/** + * @type {Ns.Two} + */ +var two; +>two : Symbol(two, Decl(other.js, 8, 3)) + +two.wat; +>two : Symbol(two, Decl(other.js, 8, 3)) + diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.types b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.types new file mode 100644 index 00000000000..691eefb2f1a --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.types @@ -0,0 +1,65 @@ +=== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js === +var Ns = {} +>Ns : typeof Ns +>{} : {} + +Ns.One = function() {}; +>Ns.One = function() {} : typeof One +>Ns.One : typeof One +>Ns : typeof Ns +>One : typeof One +>function() {} : typeof One + +Ns.Two = function() {}; +>Ns.Two = function() {} : typeof Two +>Ns.Two : typeof Two +>Ns : typeof Ns +>Two : typeof Two +>function() {} : typeof Two + +Ns.One.prototype = { +>Ns.One.prototype = { ok() {},} : { ok(): void; } +>Ns.One.prototype : { ok(): void; } +>Ns.One : typeof One +>Ns : typeof Ns +>One : typeof One +>prototype : { ok(): void; } +>{ ok() {},} : { ok(): void; } + + ok() {}, +>ok : () => void + +}; +Ns.Two.prototype = { +>Ns.Two.prototype = {} : {} +>Ns.Two.prototype : {} +>Ns.Two : typeof Two +>Ns : typeof Ns +>Two : typeof Two +>prototype : {} +>{} : {} +} + +=== tests/cases/conformance/salsa/other.js === +/** + * @type {Ns.One} + */ +var one; +>one : One + +one.wat; +>one.wat : any +>one : One +>wat : any + +/** + * @type {Ns.Two} + */ +var two; +>two : Two + +two.wat; +>two.wat : any +>two : Two +>wat : any + diff --git a/tests/cases/conformance/salsa/chainedPrototypeAssignment.ts b/tests/cases/conformance/salsa/chainedPrototypeAssignment.ts index ffac0eef1fc..df6686dda9d 100644 --- a/tests/cases/conformance/salsa/chainedPrototypeAssignment.ts +++ b/tests/cases/conformance/salsa/chainedPrototypeAssignment.ts @@ -7,10 +7,10 @@ declare function require(name: string): any; declare var exports: any; // @Filename: mod.js /// -var A = function() { +var A = function A() { this.a = 1 } -var B = function() { +var B = function B() { this.b = 2 } exports.A = A diff --git a/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.ts b/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.ts new file mode 100644 index 00000000000..6d1018d7afc --- /dev/null +++ b/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.ts @@ -0,0 +1,25 @@ +// @Filename: prototypePropertyAssignmentMergeAcrossFiles2.js +// @allowJs: true +// @checkJs: true +// @noEmit: true +var Ns = {} +Ns.One = function() {}; +Ns.Two = function() {}; + +Ns.One.prototype = { + ok() {}, +}; +Ns.Two.prototype = { +} + +// @Filename: other.js +/** + * @type {Ns.One} + */ +var one; +one.wat; +/** + * @type {Ns.Two} + */ +var two; +two.wat;