From 4a8526b54c2cd2e413a8db11f01176c7af4fa12e Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 12 May 2020 12:25:30 -0700 Subject: [PATCH 1/2] Fix crash in JS decl emit --- src/compiler/checker.ts | 27 ++++++++++++++++--- ...sDeclarationsClassLikeHeuristic.errors.txt | 10 +++++++ .../jsDeclarationsClassLikeHeuristic.js | 15 +++++++++++ .../jsDeclarationsClassLikeHeuristic.symbols | 13 +++++++++ .../jsDeclarationsClassLikeHeuristic.types | 19 +++++++++++++ .../jsDeclarationsClassLikeHeuristic.ts | 10 +++++++ 6 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 tests/baselines/reference/jsDeclarationsClassLikeHeuristic.errors.txt create mode 100644 tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js create mode 100644 tests/baselines/reference/jsDeclarationsClassLikeHeuristic.symbols create mode 100644 tests/baselines/reference/jsDeclarationsClassLikeHeuristic.types create mode 100644 tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassLikeHeuristic.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3ac6cc094fe7f..033a779190498 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5949,6 +5949,25 @@ namespace ts { } } + function isEffectiveClassSymbol(symbol: Symbol) { + if (!(symbol.flags & SymbolFlags.Class)) { + return false; + } + if (isInJSFile(symbol.valueDeclaration) && !isClassLike(symbol.valueDeclaration)) { + // For a symbol that isn't syntactically a `class` in a JS file we have heuristics + // that detect prototype assignments that indicate the symbol is *probably* a class. + // Filter out any prototype assignments for non-class symbols, i.e. + // + // let A; + // A = {}; + // A.prototype.b = {}; + const type = getTypeOfSymbol(symbol); + return some(getSignaturesOfType(type, SignatureKind.Construct)) + || some(getSignaturesOfType(type, SignatureKind.Call)); + } + return true; + } + // Synthesize declarations for a symbol - might be an Interface, a Class, a Namespace, a Type, a Variable (const, let, or var), an Alias // or a merge of some number of those. // An interesting challenge is ensuring that when classes merge with namespaces and interfaces, is keeping @@ -5990,14 +6009,14 @@ namespace ts { if (symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.FunctionScopedVariable | SymbolFlags.Property) && symbol.escapedName !== InternalSymbolName.ExportEquals && !(symbol.flags & SymbolFlags.Prototype) - && !(symbol.flags & SymbolFlags.Class) + && !isEffectiveClassSymbol(symbol) && !isConstMergedWithNSPrintableAsSignatureMerge) { serializeVariableOrProperty(symbol, symbolName, isPrivate, needsPostExportDefault, propertyAsAlias, modifierFlags); } if (symbol.flags & SymbolFlags.Enum) { serializeEnum(symbol, symbolName, modifierFlags); } - if (symbol.flags & SymbolFlags.Class) { + if (isEffectiveClassSymbol(symbol)) { if (symbol.flags & SymbolFlags.Property && isBinaryExpression(symbol.valueDeclaration.parent) && isClassExpression(symbol.valueDeclaration.parent.right)) { // Looks like a `module.exports.Sub = class {}` - if we serialize `symbol` as a class, the result will have no members, // since the classiness is actually from the target of the effective alias the symbol is. yes. A BlockScopedVariable|Class|Property @@ -6317,7 +6336,9 @@ namespace ts { const baseTypes = getBaseTypes(classType); const implementsTypes = getImplementsTypes(classType); const staticType = getTypeOfSymbol(symbol); - const staticBaseType = getBaseConstructorTypeOfClass(staticType as InterfaceType); + const staticBaseType = staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration) + ? getBaseConstructorTypeOfClass(staticType as InterfaceType) + : anyType; const heritageClauses = [ ...!length(baseTypes) ? [] : [createHeritageClause(SyntaxKind.ExtendsKeyword, map(baseTypes, b => serializeBaseType(b, staticBaseType, localName)))], ...!length(implementsTypes) ? [] : [createHeritageClause(SyntaxKind.ImplementsKeyword, map(implementsTypes, b => serializeBaseType(b, staticBaseType, localName)))] diff --git a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.errors.txt b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.errors.txt new file mode 100644 index 0000000000000..d57fb401814e6 --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.errors.txt @@ -0,0 +1,10 @@ +tests/cases/conformance/jsdoc/declarations/index.js(4,3): error TS2339: Property 'prototype' does not exist on type '{}'. + + +==== tests/cases/conformance/jsdoc/declarations/index.js (1 errors) ==== + // https://github.com/microsoft/TypeScript/issues/35801 + let A; + A = {}; + A.prototype.b = {}; + ~~~~~~~~~ +!!! error TS2339: Property 'prototype' does not exist on type '{}'. \ No newline at end of file diff --git a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js new file mode 100644 index 0000000000000..b67c60a3cf150 --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js @@ -0,0 +1,15 @@ +//// [index.js] +// https://github.com/microsoft/TypeScript/issues/35801 +let A; +A = {}; +A.prototype.b = {}; + +//// [index.js] +// https://github.com/microsoft/TypeScript/issues/35801 +var A; +A = {}; +A.prototype.b = {}; + + +//// [index.d.ts] +declare let A: any; diff --git a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.symbols b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.symbols new file mode 100644 index 0000000000000..6b1aa5b1fdab6 --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.symbols @@ -0,0 +1,13 @@ +=== tests/cases/conformance/jsdoc/declarations/index.js === +// https://github.com/microsoft/TypeScript/issues/35801 +let A; +>A : Symbol(A, Decl(index.js, 1, 3)) + +A = {}; +>A : Symbol(A, Decl(index.js, 1, 3)) + +A.prototype.b = {}; +>A.prototype : Symbol(A.b, Decl(index.js, 2, 7)) +>A : Symbol(A, Decl(index.js, 1, 3)) +>b : Symbol(A.b, Decl(index.js, 2, 7)) + diff --git a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.types b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.types new file mode 100644 index 0000000000000..ecd9ac99954a6 --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.types @@ -0,0 +1,19 @@ +=== tests/cases/conformance/jsdoc/declarations/index.js === +// https://github.com/microsoft/TypeScript/issues/35801 +let A; +>A : any + +A = {}; +>A = {} : {} +>A : any +>{} : {} + +A.prototype.b = {}; +>A.prototype.b = {} : {} +>A.prototype.b : any +>A.prototype : any +>A : {} +>prototype : any +>b : any +>{} : {} + diff --git a/tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassLikeHeuristic.ts b/tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassLikeHeuristic.ts new file mode 100644 index 0000000000000..176712b1f854a --- /dev/null +++ b/tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassLikeHeuristic.ts @@ -0,0 +1,10 @@ +// @allowJs: true +// @checkJs: true +// @target: es5 +// @outDir: ./out +// @declaration: true +// @filename: index.js +// https://github.com/microsoft/TypeScript/issues/35801 +let A; +A = {}; +A.prototype.b = {}; \ No newline at end of file From 25328069d8eac329c6e67857726367c9e503974e Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 13 May 2020 14:16:46 -0700 Subject: [PATCH 2/2] Emit as class with private ctor --- src/compiler/checker.ts | 37 ++++++++----------- .../jsDeclarationsClassLikeHeuristic.js | 5 ++- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 033a779190498..b5c0978d8c0a6 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5949,24 +5949,6 @@ namespace ts { } } - function isEffectiveClassSymbol(symbol: Symbol) { - if (!(symbol.flags & SymbolFlags.Class)) { - return false; - } - if (isInJSFile(symbol.valueDeclaration) && !isClassLike(symbol.valueDeclaration)) { - // For a symbol that isn't syntactically a `class` in a JS file we have heuristics - // that detect prototype assignments that indicate the symbol is *probably* a class. - // Filter out any prototype assignments for non-class symbols, i.e. - // - // let A; - // A = {}; - // A.prototype.b = {}; - const type = getTypeOfSymbol(symbol); - return some(getSignaturesOfType(type, SignatureKind.Construct)) - || some(getSignaturesOfType(type, SignatureKind.Call)); - } - return true; - } // Synthesize declarations for a symbol - might be an Interface, a Class, a Namespace, a Type, a Variable (const, let, or var), an Alias // or a merge of some number of those. @@ -6009,14 +5991,14 @@ namespace ts { if (symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.FunctionScopedVariable | SymbolFlags.Property) && symbol.escapedName !== InternalSymbolName.ExportEquals && !(symbol.flags & SymbolFlags.Prototype) - && !isEffectiveClassSymbol(symbol) + && !(symbol.flags & SymbolFlags.Class) && !isConstMergedWithNSPrintableAsSignatureMerge) { serializeVariableOrProperty(symbol, symbolName, isPrivate, needsPostExportDefault, propertyAsAlias, modifierFlags); } if (symbol.flags & SymbolFlags.Enum) { serializeEnum(symbol, symbolName, modifierFlags); } - if (isEffectiveClassSymbol(symbol)) { + if (symbol.flags & SymbolFlags.Class) { if (symbol.flags & SymbolFlags.Property && isBinaryExpression(symbol.valueDeclaration.parent) && isClassExpression(symbol.valueDeclaration.parent.right)) { // Looks like a `module.exports.Sub = class {}` - if we serialize `symbol` as a class, the result will have no members, // since the classiness is actually from the target of the effective alias the symbol is. yes. A BlockScopedVariable|Class|Property @@ -6336,7 +6318,8 @@ namespace ts { const baseTypes = getBaseTypes(classType); const implementsTypes = getImplementsTypes(classType); const staticType = getTypeOfSymbol(symbol); - const staticBaseType = staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration) + const isClass = !!staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration); + const staticBaseType = isClass ? getBaseConstructorTypeOfClass(staticType as InterfaceType) : anyType; const heritageClauses = [ @@ -6374,7 +6357,17 @@ namespace ts { const staticMembers = flatMap( filter(getPropertiesOfType(staticType), p => !(p.flags & SymbolFlags.Prototype) && p.escapedName !== "prototype" && !isNamespaceMember(p)), p => serializePropertySymbolForClass(p, /*isStatic*/ true, staticBaseType)); - const constructors = serializeSignatures(SignatureKind.Construct, staticType, baseTypes[0], SyntaxKind.Constructor) as ConstructorDeclaration[]; + // When we encounter an `X.prototype.y` assignment in a JS file, we bind `X` as a class regardless as to whether + // the value is ever initialized with a class or function-like value. For cases where `X` could never be + // created via `new`, we will inject a `private constructor()` declaration to indicate it is not createable. + const isNonConstructableClassLikeInJsFile = + !isClass && + !!symbol.valueDeclaration && + isInJSFile(symbol.valueDeclaration) && + !some(getSignaturesOfType(staticType, SignatureKind.Construct)); + const constructors = isNonConstructableClassLikeInJsFile ? + [createConstructor(/*decorators*/ undefined, createModifiersFromModifierFlags(ModifierFlags.Private), [], /*body*/ undefined)] : + serializeSignatures(SignatureKind.Construct, staticType, baseTypes[0], SyntaxKind.Constructor) as ConstructorDeclaration[]; for (const c of constructors) { // A constructor's return type and type parameters are supposed to be controlled by the enclosing class declaration // `signatureToSignatureDeclarationHelper` appends them regardless, so for now we delete them here diff --git a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js index b67c60a3cf150..084e6f18b91c1 100644 --- a/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js +++ b/tests/baselines/reference/jsDeclarationsClassLikeHeuristic.js @@ -12,4 +12,7 @@ A.prototype.b = {}; //// [index.d.ts] -declare let A: any; +declare class A { + private constructor(); + b: {}; +}