From 08eb99d8ec72294003a2567f5a6453c7bfd10372 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 15 Aug 2018 14:53:30 -0700 Subject: [PATCH] For a this-property assignment with an empty object initializer, use type annotation if present (#26428) * This-property w/empty object init: use type annotation * JS initializer type doesn't apply to this-property assignments * Move getJSExpandoType into getWidenedType* functions Plus some cleanup. * Improved style from PR comments * Parameters are not expando --- src/compiler/checker.ts | 159 ++++++++---------- ...hisPropertyInitializerDoesntNarrow.symbols | 28 +++ ...dThisPropertyInitializerDoesntNarrow.types | 31 ++++ ...checkDestructuringShorthandAssigment.types | 4 +- .../jsContainerMergeTsDeclaration3.errors.txt | 13 ++ ...atedThisPropertyInitializerDoesntNarrow.ts | 17 ++ 6 files changed, 161 insertions(+), 91 deletions(-) create mode 100644 tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.symbols create mode 100644 tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.types create mode 100644 tests/baselines/reference/jsContainerMergeTsDeclaration3.errors.txt create mode 100644 tests/cases/conformance/salsa/annotatedThisPropertyInitializerDoesntNarrow.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8f6b5aceb61..bf91c73eaf8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4696,6 +4696,12 @@ namespace ts { return addOptionality(type, isOptional); } } + else if (isInJavaScriptFile(declaration)) { + const expandoType = getJSExpandoObjectType(declaration, getSymbolOfNode(declaration), getDeclaredJavascriptInitializer(declaration)); + if (expandoType) { + return expandoType; + } + } // Use the type of the initializer expression if one is present if (declaration.initializer) { @@ -4718,7 +4724,7 @@ namespace ts { return undefined; } - function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol, resolvedSymbol?: Symbol) { + function getWidenedTypeFromJSPropertyAssignments(symbol: Symbol, resolvedSymbol?: Symbol) { // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { @@ -4726,7 +4732,8 @@ namespace ts { if (tag && tag.typeExpression) { return getTypeFromTypeNode(tag.typeExpression); } - return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); + const expando = getJSExpandoObjectType(symbol.valueDeclaration, symbol, specialDeclaration); + return expando || getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } let definedInConstructor = false; let definedInMethod = false; @@ -4778,6 +4785,27 @@ namespace ts { return widened; } + function getJSExpandoObjectType(decl: Node, symbol: Symbol, init: Expression | undefined): Type | undefined { + if (!init || !isObjectLiteralExpression(init) || init.properties.length) { + return undefined; + } + const exports = createSymbolTable(); + while (isBinaryExpression(decl) || isPropertyAccessExpression(decl)) { + const s = getSymbolOfNode(decl); + if (s && hasEntries(s.exports)) { + mergeSymbolTable(exports, s.exports); + } + decl = isBinaryExpression(decl) ? decl.parent : decl.parent.parent; + } + const s = getSymbolOfNode(decl); + if (s && hasEntries(s.exports)) { + mergeSymbolTable(exports, s.exports); + } + const type = createAnonymousType(symbol, exports, emptyArray, emptyArray, undefined, undefined); + type.objectFlags |= ObjectFlags.JSLiteral; + return type; + } + function getJSDocTypeFromSpecialDeclarations(declaredType: Type | undefined, expression: Expression, _symbol: Symbol, declaration: Declaration) { const typeNode = getJSDocType(expression.parent); if (typeNode) { @@ -5041,98 +5069,51 @@ namespace ts { if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { return errorType; } - let type = getJSSpecialType(symbol, declaration); - if (!type) { - if (isJSDocPropertyLikeTag(declaration) - || isPropertyAccessExpression(declaration) - || isIdentifier(declaration) - || isClassDeclaration(declaration) - || isFunctionDeclaration(declaration) - || (isMethodDeclaration(declaration) && !isObjectLiteralMethod(declaration)) - || isMethodSignature(declaration)) { - // Symbol is property of some kind that is merged with something - should use `getTypeOfFuncClassEnumModule` and not `getTypeOfVariableOrParameterOrProperty` - if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.Enum | SymbolFlags.ValueModule)) { - return getTypeOfFuncClassEnumModule(symbol); - } - type = tryGetTypeFromEffectiveTypeNode(declaration) || anyType; - } - else if (isPropertyAssignment(declaration)) { - type = tryGetTypeFromEffectiveTypeNode(declaration) || checkPropertyAssignment(declaration); - } - else if (isJsxAttribute(declaration)) { - type = tryGetTypeFromEffectiveTypeNode(declaration) || checkJsxAttribute(declaration); - } - else if (isShorthandPropertyAssignment(declaration)) { - type = tryGetTypeFromEffectiveTypeNode(declaration) || checkExpressionForMutableLocation(declaration.name, CheckMode.Normal); - } - else if (isObjectLiteralMethod(declaration)) { - type = tryGetTypeFromEffectiveTypeNode(declaration) || checkObjectLiteralMethod(declaration, CheckMode.Normal); - } - else if (isParameter(declaration) - || isPropertyDeclaration(declaration) - || isPropertySignature(declaration) - || isVariableDeclaration(declaration) - || isBindingElement(declaration)) { - type = getWidenedTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true); - } - else { - return Debug.fail("Unhandled declaration kind! " + Debug.showSyntaxKind(declaration) + " for " + Debug.showSymbol(symbol)); + let type: Type | undefined; + if (isInJavaScriptFile(declaration) && + (isBinaryExpression(declaration) || isPropertyAccessExpression(declaration) && isBinaryExpression(declaration.parent))) { + type = getWidenedTypeFromJSPropertyAssignments(symbol); + } + else if (isJSDocPropertyLikeTag(declaration) + || isPropertyAccessExpression(declaration) + || isIdentifier(declaration) + || isClassDeclaration(declaration) + || isFunctionDeclaration(declaration) + || (isMethodDeclaration(declaration) && !isObjectLiteralMethod(declaration)) + || isMethodSignature(declaration)) { + // Symbol is property of some kind that is merged with something - should use `getTypeOfFuncClassEnumModule` and not `getTypeOfVariableOrParameterOrProperty` + if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.Enum | SymbolFlags.ValueModule)) { + return getTypeOfFuncClassEnumModule(symbol); } + type = tryGetTypeFromEffectiveTypeNode(declaration) || anyType; + } + else if (isPropertyAssignment(declaration)) { + type = tryGetTypeFromEffectiveTypeNode(declaration) || checkPropertyAssignment(declaration); + } + else if (isJsxAttribute(declaration)) { + type = tryGetTypeFromEffectiveTypeNode(declaration) || checkJsxAttribute(declaration); + } + else if (isShorthandPropertyAssignment(declaration)) { + type = tryGetTypeFromEffectiveTypeNode(declaration) || checkExpressionForMutableLocation(declaration.name, CheckMode.Normal); + } + else if (isObjectLiteralMethod(declaration)) { + type = tryGetTypeFromEffectiveTypeNode(declaration) || checkObjectLiteralMethod(declaration, CheckMode.Normal); + } + else if (isParameter(declaration) + || isPropertyDeclaration(declaration) + || isPropertySignature(declaration) + || isVariableDeclaration(declaration) + || isBindingElement(declaration)) { + type = getWidenedTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true); + } + else { + return Debug.fail("Unhandled declaration kind! " + Debug.showSyntaxKind(declaration) + " for " + Debug.showSymbol(symbol)); } if (!popTypeResolution()) { type = reportCircularityError(symbol); } return type; - } - - function getJSSpecialType(symbol: Symbol, decl: Declaration): Type | undefined { - if (!isInJavaScriptFile(decl)) { - return undefined; - } - // Handle certain special assignment kinds, which happen to union across multiple declarations: - // * module.exports = expr - // * exports.p = expr - // * this.p = expr - // * className.prototype.method = expr - else if (isBinaryExpression(decl) || - isPropertyAccessExpression(decl) && isBinaryExpression(decl.parent)) { - return getJSInitializerType(decl, symbol, getAssignedJavascriptInitializer(isBinaryExpression(decl) ? decl.left : decl)) || - getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); - } - else if (isParameter(decl) - || isPropertyDeclaration(decl) - || isPropertySignature(decl) - || isVariableDeclaration(decl) - || isBindingElement(decl)) { - // Use type from type annotation if one is present - const isOptional = isParameter(decl) && isJSDocOptionalParameter(decl) || - !isBindingElement(decl) && !isVariableDeclaration(decl) && !!decl.questionToken; - const declaredType = tryGetTypeFromEffectiveTypeNode(decl); - return declaredType && addOptionality(declaredType, isOptional) || - getJSInitializerType(decl, symbol, getDeclaredJavascriptInitializer(decl)) || - getWidenedTypeForVariableLikeDeclaration(decl, /*includeOptionality*/ true); - } - } - - function getJSInitializerType(decl: Node, symbol: Symbol, init: Expression | undefined): Type | undefined { - if (init && isInJavaScriptFile(init) && isObjectLiteralExpression(init) && init.properties.length === 0) { - const exports = createSymbolTable(); - while (isBinaryExpression(decl) || isPropertyAccessExpression(decl)) { - const s = getSymbolOfNode(decl); - if (s && hasEntries(s.exports)) { - mergeSymbolTable(exports, s.exports); - } - decl = isBinaryExpression(decl) ? decl.parent : decl.parent.parent; - } - const s = getSymbolOfNode(decl); - if (s && hasEntries(s.exports)) { - mergeSymbolTable(exports, s.exports); - } - const type = createAnonymousType(symbol, exports, emptyArray, emptyArray, undefined, undefined); - type.objectFlags |= ObjectFlags.JSLiteral; - return type; - } } function getAnnotatedAccessorTypeNode(accessor: AccessorDeclaration | undefined): TypeNode | undefined { @@ -5264,7 +5245,7 @@ namespace ts { } else if (declaration.kind === SyntaxKind.BinaryExpression || declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) { - return getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); + return getWidenedTypeFromJSPropertyAssignments(symbol); } else if (symbol.flags & SymbolFlags.ValueModule && declaration && isSourceFile(declaration) && declaration.commonJsModuleIndicator) { const resolvedModule = resolveExternalModuleSymbol(symbol); @@ -5273,7 +5254,7 @@ namespace ts { return errorType; } const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!); - const type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); + const type = getWidenedTypeFromJSPropertyAssignments(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); if (!popTypeResolution()) { return reportCircularityError(symbol); } diff --git a/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.symbols b/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.symbols new file mode 100644 index 00000000000..44e6e8e5634 --- /dev/null +++ b/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.symbols @@ -0,0 +1,28 @@ +=== tests/cases/conformance/salsa/Compilation.js === +// from webpack/lib/Compilation.js and filed at #26427 +/** @param {{ [s: string]: number }} map */ +function mappy(map) {} +>mappy : Symbol(mappy, Decl(Compilation.js, 0, 0)) +>map : Symbol(map, Decl(Compilation.js, 2, 15)) + +export class C { +>C : Symbol(C, Decl(Compilation.js, 2, 22)) + + constructor() { + /** @type {{ [assetName: string]: number}} */ + this.assets = {}; +>this.assets : Symbol(C.assets, Decl(Compilation.js, 5, 19)) +>this : Symbol(C, Decl(Compilation.js, 2, 22)) +>assets : Symbol(C.assets, Decl(Compilation.js, 5, 19)) + } + m() { +>m : Symbol(C.m, Decl(Compilation.js, 8, 5)) + + mappy(this.assets) +>mappy : Symbol(mappy, Decl(Compilation.js, 0, 0)) +>this.assets : Symbol(C.assets, Decl(Compilation.js, 5, 19)) +>this : Symbol(C, Decl(Compilation.js, 2, 22)) +>assets : Symbol(C.assets, Decl(Compilation.js, 5, 19)) + } +} + diff --git a/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.types b/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.types new file mode 100644 index 00000000000..2772c19c75c --- /dev/null +++ b/tests/baselines/reference/annotatedThisPropertyInitializerDoesntNarrow.types @@ -0,0 +1,31 @@ +=== tests/cases/conformance/salsa/Compilation.js === +// from webpack/lib/Compilation.js and filed at #26427 +/** @param {{ [s: string]: number }} map */ +function mappy(map) {} +>mappy : (map: { [s: string]: number; }) => void +>map : { [s: string]: number; } + +export class C { +>C : C + + constructor() { + /** @type {{ [assetName: string]: number}} */ + this.assets = {}; +>this.assets = {} : {} +>this.assets : { [assetName: string]: number; } +>this : this +>assets : { [assetName: string]: number; } +>{} : {} + } + m() { +>m : () => void + + mappy(this.assets) +>mappy(this.assets) : void +>mappy : (map: { [s: string]: number; }) => void +>this.assets : { [assetName: string]: number; } +>this : this +>assets : { [assetName: string]: number; } + } +} + diff --git a/tests/baselines/reference/checkDestructuringShorthandAssigment.types b/tests/baselines/reference/checkDestructuringShorthandAssigment.types index 5e22b7f7e07..336a679607d 100644 --- a/tests/baselines/reference/checkDestructuringShorthandAssigment.types +++ b/tests/baselines/reference/checkDestructuringShorthandAssigment.types @@ -1,14 +1,14 @@ === tests/cases/compiler/bug25434.js === // should not crash while checking function Test({ b = '' } = {}) {} ->Test : ({ b }?: {}) => void +>Test : ({ b }?: { b?: string; }) => void >b : string >'' : "" >{} : { b?: string; } Test(({ b = '5' } = {})); >Test(({ b = '5' } = {})) : void ->Test : ({ b }?: {}) => void +>Test : ({ b }?: { b?: string; }) => void >({ b = '5' } = {}) : { b?: any; } >{ b = '5' } = {} : { b?: any; } >{ b = '5' } : { b?: any; } diff --git a/tests/baselines/reference/jsContainerMergeTsDeclaration3.errors.txt b/tests/baselines/reference/jsContainerMergeTsDeclaration3.errors.txt new file mode 100644 index 00000000000..8e675ae1472 --- /dev/null +++ b/tests/baselines/reference/jsContainerMergeTsDeclaration3.errors.txt @@ -0,0 +1,13 @@ +tests/cases/conformance/salsa/b.js(1,7): error TS2322: Type '{}' is not assignable to type 'typeof A'. + Property 'prototype' is missing in type '{}'. + + +==== tests/cases/conformance/salsa/a.d.ts (0 errors) ==== + declare class A {} +==== tests/cases/conformance/salsa/b.js (1 errors) ==== + const A = { }; + ~ +!!! error TS2322: Type '{}' is not assignable to type 'typeof A'. +!!! error TS2322: Property 'prototype' is missing in type '{}'. + A.d = { }; + \ No newline at end of file diff --git a/tests/cases/conformance/salsa/annotatedThisPropertyInitializerDoesntNarrow.ts b/tests/cases/conformance/salsa/annotatedThisPropertyInitializerDoesntNarrow.ts new file mode 100644 index 00000000000..5c0e2c01af9 --- /dev/null +++ b/tests/cases/conformance/salsa/annotatedThisPropertyInitializerDoesntNarrow.ts @@ -0,0 +1,17 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true +// @Filename: Compilation.js +// from webpack/lib/Compilation.js and filed at #26427 +/** @param {{ [s: string]: number }} map */ +function mappy(map) {} + +export class C { + constructor() { + /** @type {{ [assetName: string]: number}} */ + this.assets = {}; + } + m() { + mappy(this.assets) + } +}