From 8779d4ca888426b58c2c2d76626137c2d6976721 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 19 Oct 2018 09:23:05 -0700 Subject: [PATCH] Fix JS merge crashes from lovefield (#27989) 1. Merge enum with expando. 2. Merge enum member with property assignment. 3. Merge interface-declared method declaration with prototype-property-assignment method declaration. The reason that the enum merges crash is that getTypeOfSymbol assumes that symbol flags are (basically) mutually exclusive. This assumption is shredded, badly, for JS merges. One fix is to drop the assumption of exclusivity and instead order cases by least to most likely. This has the highest chance of working, but is also slow, since you would prefer to order cases by most likely *first*, not *last*. The other fix, which is what I did here, is to add a last-chance re-dispatch at the bottom of `getTypeOfVariableOrParameterOrPropertyWorker`. This dispatch uses the valueDeclaration instead of the symbol flags. --- src/compiler/checker.ts | 11 ++++ .../reference/enumMergeWithExpando.errors.txt | 19 ++++++ .../reference/enumMergeWithExpando.symbols | 31 ++++++++++ .../reference/enumMergeWithExpando.types | 37 +++++++++++ ...ignmentMergeWithInterfaceMethod.errors.txt | 40 ++++++++++++ ...AssignmentMergeWithInterfaceMethod.symbols | 61 +++++++++++++++++++ ...tyAssignmentMergeWithInterfaceMethod.types | 53 ++++++++++++++++ .../conformance/salsa/enumMergeWithExpando.ts | 13 ++++ ...pertyAssignmentMergeWithInterfaceMethod.ts | 22 +++++++ 9 files changed, 287 insertions(+) create mode 100644 tests/baselines/reference/enumMergeWithExpando.errors.txt create mode 100644 tests/baselines/reference/enumMergeWithExpando.symbols create mode 100644 tests/baselines/reference/enumMergeWithExpando.types create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.errors.txt create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.symbols create mode 100644 tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.types create mode 100644 tests/cases/conformance/salsa/enumMergeWithExpando.ts create mode 100644 tests/cases/conformance/salsa/prototypePropertyAssignmentMergeWithInterfaceMethod.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d85a46bd74f20..df817c3a79536 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2214,6 +2214,9 @@ namespace ts { function getDeclarationOfJSPrototypeContainer(symbol: Symbol) { const decl = symbol.parent!.valueDeclaration; + if (!decl) { + return undefined; + } const initializer = isAssignmentDeclaration(decl) ? getAssignedExpandoInitializer(decl) : hasOnlyExpressionInitializer(decl) ? getDeclaredExpandoInitializer(decl) : undefined; @@ -5241,6 +5244,14 @@ namespace ts { || isBindingElement(declaration)) { type = getWidenedTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true); } + // getTypeOfSymbol dispatches some JS merges incorrectly because their symbol flags are not mutually exclusive. + // Re-dispatch based on valueDeclaration.kind instead. + else if (isEnumDeclaration(declaration)) { + type = getTypeOfFuncClassEnumModule(symbol); + } + else if (isEnumMember(declaration)) { + type = getTypeOfEnumMember(symbol); + } else { return Debug.fail("Unhandled declaration kind! " + Debug.showSyntaxKind(declaration) + " for " + Debug.showSymbol(symbol)); } diff --git a/tests/baselines/reference/enumMergeWithExpando.errors.txt b/tests/baselines/reference/enumMergeWithExpando.errors.txt new file mode 100644 index 0000000000000..7911206650c34 --- /dev/null +++ b/tests/baselines/reference/enumMergeWithExpando.errors.txt @@ -0,0 +1,19 @@ +tests/cases/conformance/salsa/enums.js(2,10): error TS2540: Cannot assign to 'DESC' because it is a constant or a read-only property. +tests/cases/conformance/salsa/enums.js(3,10): error TS2540: Cannot assign to 'ASC' because it is a constant or a read-only property. + + +==== tests/cases/conformance/salsa/lovefield-ts.d.ts (0 errors) ==== + // bug #27352, crashes from github.com/google/lovefield + declare namespace lf { + export enum Order { ASC, DESC } + } + +==== tests/cases/conformance/salsa/enums.js (2 errors) ==== + lf.Order = {} + lf.Order.DESC = 0; + ~~~~ +!!! error TS2540: Cannot assign to 'DESC' because it is a constant or a read-only property. + lf.Order.ASC = 1; + ~~~ +!!! error TS2540: Cannot assign to 'ASC' because it is a constant or a read-only property. + \ No newline at end of file diff --git a/tests/baselines/reference/enumMergeWithExpando.symbols b/tests/baselines/reference/enumMergeWithExpando.symbols new file mode 100644 index 0000000000000..854bf6b5169ce --- /dev/null +++ b/tests/baselines/reference/enumMergeWithExpando.symbols @@ -0,0 +1,31 @@ +=== tests/cases/conformance/salsa/lovefield-ts.d.ts === +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(enums.js, 0, 0), Decl(enums.js, 0, 13)) + + export enum Order { ASC, DESC } +>Order : Symbol(Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>ASC : Symbol(Order.ASC, Decl(lovefield-ts.d.ts, 2, 23), Decl(enums.js, 1, 18)) +>DESC : Symbol(Order.DESC, Decl(lovefield-ts.d.ts, 2, 28), Decl(enums.js, 0, 13)) +} + +=== tests/cases/conformance/salsa/enums.js === +lf.Order = {} +>lf.Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(enums.js, 0, 0), Decl(enums.js, 0, 13)) +>Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) + +lf.Order.DESC = 0; +>lf.Order.DESC : Symbol(lf.Order.DESC, Decl(lovefield-ts.d.ts, 2, 28), Decl(enums.js, 0, 13)) +>lf.Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(enums.js, 0, 0), Decl(enums.js, 0, 13)) +>Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>DESC : Symbol(lf.Order.DESC, Decl(lovefield-ts.d.ts, 2, 28), Decl(enums.js, 0, 13)) + +lf.Order.ASC = 1; +>lf.Order.ASC : Symbol(lf.Order.ASC, Decl(lovefield-ts.d.ts, 2, 23), Decl(enums.js, 1, 18)) +>lf.Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(enums.js, 0, 0), Decl(enums.js, 0, 13)) +>Order : Symbol(lf.Order, Decl(lovefield-ts.d.ts, 1, 22), Decl(enums.js, 0, 0), Decl(enums.js, 1, 3)) +>ASC : Symbol(lf.Order.ASC, Decl(lovefield-ts.d.ts, 2, 23), Decl(enums.js, 1, 18)) + diff --git a/tests/baselines/reference/enumMergeWithExpando.types b/tests/baselines/reference/enumMergeWithExpando.types new file mode 100644 index 0000000000000..0e5925782ac17 --- /dev/null +++ b/tests/baselines/reference/enumMergeWithExpando.types @@ -0,0 +1,37 @@ +=== tests/cases/conformance/salsa/lovefield-ts.d.ts === +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { +>lf : typeof lf + + export enum Order { ASC, DESC } +>Order : Order +>ASC : Order +>DESC : Order +} + +=== tests/cases/conformance/salsa/enums.js === +lf.Order = {} +>lf.Order = {} : typeof lf.Order +>lf.Order : typeof lf.Order +>lf : typeof lf +>Order : typeof lf.Order +>{} : {} + +lf.Order.DESC = 0; +>lf.Order.DESC = 0 : 0 +>lf.Order.DESC : any +>lf.Order : typeof lf.Order +>lf : typeof lf +>Order : typeof lf.Order +>DESC : any +>0 : 0 + +lf.Order.ASC = 1; +>lf.Order.ASC = 1 : 1 +>lf.Order.ASC : any +>lf.Order : typeof lf.Order +>lf : typeof lf +>Order : typeof lf.Order +>ASC : any +>1 : 1 + diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.errors.txt b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.errors.txt new file mode 100644 index 0000000000000..eeb83673ab39a --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.errors.txt @@ -0,0 +1,40 @@ +tests/cases/conformance/salsa/lovefield-ts.d.ts(4,19): error TS2503: Cannot find namespace 'query'. +tests/cases/conformance/salsa/lovefield-ts.d.ts(5,24): error TS2503: Cannot find namespace 'schema'. +tests/cases/conformance/salsa/lovefield-ts.d.ts(7,25): error TS2503: Cannot find namespace 'query'. +tests/cases/conformance/salsa/lovefield-ts.d.ts(9,14): error TS2304: Cannot find name 'TransactionStats'. +tests/cases/conformance/salsa/lovefield.js(3,23): error TS2694: Namespace 'lf' has no exported member 'schema'. +tests/cases/conformance/salsa/lovefield.js(4,14): error TS2304: Cannot find name 'IThenable'. + + +==== tests/cases/conformance/salsa/lovefield-ts.d.ts (4 errors) ==== + // bug #27352, crashes from github.com/google/lovefield + declare namespace lf { + export interface Transaction { + attach(query: query.Builder): Promise> + ~~~~~ +!!! error TS2503: Cannot find namespace 'query'. + begin(scope: Array): Promise + ~~~~~~ +!!! error TS2503: Cannot find namespace 'schema'. + commit(): Promise + exec(queries: Array): Promise>> + ~~~~~ +!!! error TS2503: Cannot find namespace 'query'. + rollback(): Promise + stats(): TransactionStats + ~~~~~~~~~~~~~~~~ +!!! error TS2304: Cannot find name 'TransactionStats'. + } + } +==== tests/cases/conformance/salsa/lovefield.js (2 errors) ==== + lf.Transaction = function() {}; + /** + * @param {!Array} scope + ~~~~~~ +!!! error TS2694: Namespace 'lf' has no exported member 'schema'. + * @return {!IThenable} + ~~~~~~~~~ +!!! error TS2304: Cannot find name 'IThenable'. + */ + lf.Transaction.prototype.begin = function(scope) {}; + \ No newline at end of file diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.symbols b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.symbols new file mode 100644 index 0000000000000..4b27813082e69 --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.symbols @@ -0,0 +1,61 @@ +=== tests/cases/conformance/salsa/lovefield-ts.d.ts === +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(lovefield.js, 0, 0)) + + export interface Transaction { +>Transaction : Symbol(Transaction, Decl(lovefield-ts.d.ts, 1, 22), Decl(lovefield.js, 0, 0)) + + attach(query: query.Builder): Promise> +>attach : Symbol(Transaction.attach, Decl(lovefield-ts.d.ts, 2, 32)) +>query : Symbol(query, Decl(lovefield-ts.d.ts, 3, 11)) +>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + + begin(scope: Array): Promise +>begin : Symbol(Transaction.begin, Decl(lovefield-ts.d.ts, 3, 56), Decl(lovefield.js, 0, 31)) +>scope : Symbol(scope, Decl(lovefield-ts.d.ts, 4, 10)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --)) + + commit(): Promise +>commit : Symbol(Transaction.commit, Decl(lovefield-ts.d.ts, 4, 52)) +>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --)) + + exec(queries: Array): Promise>> +>exec : Symbol(Transaction.exec, Decl(lovefield-ts.d.ts, 5, 27)) +>queries : Symbol(queries, Decl(lovefield-ts.d.ts, 6, 9)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + + rollback(): Promise +>rollback : Symbol(Transaction.rollback, Decl(lovefield-ts.d.ts, 6, 70)) +>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --)) + + stats(): TransactionStats +>stats : Symbol(Transaction.stats, Decl(lovefield-ts.d.ts, 7, 29)) + } +} +=== tests/cases/conformance/salsa/lovefield.js === +lf.Transaction = function() {}; +>lf.Transaction : Symbol(lf.Transaction, Decl(lovefield-ts.d.ts, 1, 22), Decl(lovefield.js, 0, 0)) +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(lovefield.js, 0, 0)) +>Transaction : Symbol(lf.Transaction, Decl(lovefield-ts.d.ts, 1, 22), Decl(lovefield.js, 0, 0)) + +/** + * @param {!Array} scope + * @return {!IThenable} + */ +lf.Transaction.prototype.begin = function(scope) {}; +>lf.Transaction.prototype : Symbol(lf.Transaction.begin, Decl(lovefield-ts.d.ts, 3, 56), Decl(lovefield.js, 0, 31)) +>lf.Transaction : Symbol(lf.Transaction, Decl(lovefield-ts.d.ts, 1, 22), Decl(lovefield.js, 0, 0)) +>lf : Symbol(lf, Decl(lovefield-ts.d.ts, 0, 0), Decl(lovefield.js, 0, 0)) +>Transaction : Symbol(lf.Transaction, Decl(lovefield-ts.d.ts, 1, 22), Decl(lovefield.js, 0, 0)) +>prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --)) +>begin : Symbol(lf.Transaction.begin, Decl(lovefield-ts.d.ts, 3, 56), Decl(lovefield.js, 0, 31)) +>scope : Symbol(scope, Decl(lovefield.js, 5, 42)) + diff --git a/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.types b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.types new file mode 100644 index 0000000000000..9776e40ac249a --- /dev/null +++ b/tests/baselines/reference/prototypePropertyAssignmentMergeWithInterfaceMethod.types @@ -0,0 +1,53 @@ +=== tests/cases/conformance/salsa/lovefield-ts.d.ts === +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { + export interface Transaction { + attach(query: query.Builder): Promise> +>attach : (query: any) => Promise +>query : any +>query : any + + begin(scope: Array): Promise +>begin : (scope: any[]) => Promise +>scope : any[] +>schema : any + + commit(): Promise +>commit : () => Promise + + exec(queries: Array): Promise>> +>exec : (queries: any[]) => Promise +>queries : any[] +>query : any + + rollback(): Promise +>rollback : () => Promise + + stats(): TransactionStats +>stats : () => any + } +} +=== tests/cases/conformance/salsa/lovefield.js === +lf.Transaction = function() {}; +>lf.Transaction = function() {} : typeof Transaction +>lf.Transaction : typeof Transaction +>lf : typeof lf +>Transaction : typeof Transaction +>function() {} : typeof Transaction + +/** + * @param {!Array} scope + * @return {!IThenable} + */ +lf.Transaction.prototype.begin = function(scope) {}; +>lf.Transaction.prototype.begin = function(scope) {} : (scope: any[]) => any +>lf.Transaction.prototype.begin : any +>lf.Transaction.prototype : any +>lf.Transaction : typeof Transaction +>lf : typeof lf +>Transaction : typeof Transaction +>prototype : any +>begin : any +>function(scope) {} : (scope: any[]) => any +>scope : any[] + diff --git a/tests/cases/conformance/salsa/enumMergeWithExpando.ts b/tests/cases/conformance/salsa/enumMergeWithExpando.ts new file mode 100644 index 0000000000000..a0367aff318c5 --- /dev/null +++ b/tests/cases/conformance/salsa/enumMergeWithExpando.ts @@ -0,0 +1,13 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true +// @Filename: lovefield-ts.d.ts +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { + export enum Order { ASC, DESC } +} + +// @Filename: enums.js +lf.Order = {} +lf.Order.DESC = 0; +lf.Order.ASC = 1; diff --git a/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeWithInterfaceMethod.ts b/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeWithInterfaceMethod.ts new file mode 100644 index 0000000000000..4f9181e5daa4b --- /dev/null +++ b/tests/cases/conformance/salsa/prototypePropertyAssignmentMergeWithInterfaceMethod.ts @@ -0,0 +1,22 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true +// @Filename: lovefield-ts.d.ts +// bug #27352, crashes from github.com/google/lovefield +declare namespace lf { + export interface Transaction { + attach(query: query.Builder): Promise> + begin(scope: Array): Promise + commit(): Promise + exec(queries: Array): Promise>> + rollback(): Promise + stats(): TransactionStats + } +} +// @Filename: lovefield.js +lf.Transaction = function() {}; +/** + * @param {!Array} scope + * @return {!IThenable} + */ +lf.Transaction.prototype.begin = function(scope) {};