From e2a6efd80d7da360ce89b38ddaab6c2ebf5f03e3 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Sun, 22 Jul 2018 14:46:41 -0700 Subject: [PATCH 1/5] Split getWidenedTypeFromJSSpecialPropertyDeclaration 1. Handle this-property assignments 2. Handle other special property assignment I have only started simplifying [2]. --- src/compiler/checker.ts | 116 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 108 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cdcf1448d2de0..3939c0f9e0ec9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4703,7 +4703,109 @@ namespace ts { return undefined; } - function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol, resolvedSymbol?: Symbol) { + function getWidenedTypeFromJSPropertyDeclarations(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) { + const tag = getJSDocTypeTag(specialDeclaration); + if (tag && tag.typeExpression) { + return getTypeFromTypeNode(tag.typeExpression); + } + return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); + } + const types: Type[] = []; + let declaredType: Type | undefined; + for (const declaration of symbol.declarations) { + const expression = isBinaryExpression(declaration) ? declaration : + isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : + undefined; + + if (!expression) { + return errorType; + } + + // If there is a JSDoc type, use it (TODO: Should just do this in a loop before checking the initialisers) + const typeNode = getJSDocType(expression.parent); + if (typeNode) { + const type = getWidenedType(getTypeFromTypeNode(typeNode)); + if (!declaredType) { + declaredType = type; + } + else if (declaredType !== errorType && type !== errorType && + !isTypeIdenticalTo(declaredType, type) && + !(symbol.flags & SymbolFlags.JSContainer)) { + errorNextVariableOrPropertyDeclarationMustHaveSameType(declaredType, declaration, type); + } + } + // TODO: Maybe we should try harder in the case that expression is not a binary expression. + else if (!declaredType && isBinaryExpression(expression)) { + // If we don't have an explicit JSDoc type, get the type from the expression. + let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); + + if (type.flags & TypeFlags.Object && + special === SpecialPropertyAssignmentKind.ModuleExports && + symbol.escapedName === InternalSymbolName.ExportEquals) { + const exportedType = resolveStructuredTypeMembers(type as ObjectType); + const members = createSymbolTable(); + copyEntries(exportedType.members, members); + if (resolvedSymbol && !resolvedSymbol.exports) { + resolvedSymbol.exports = createSymbolTable(); + } + (resolvedSymbol || symbol).exports!.forEach((s, name) => { + if (members.has(name)) { + const exportedMember = exportedType.members.get(name)!; + const union = createSymbol(s.flags | exportedMember.flags, name); + union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); + members.set(name, union); + } + else { + members.set(name, s); + } + }); + type = createAnonymousType( + exportedType.symbol, + members, + exportedType.callSignatures, + exportedType.constructSignatures, + exportedType.stringIndexInfo, + exportedType.numberIndexInfo); + } + let anyedType = type; + if (isEmptyArrayLiteralType(type)) { + anyedType = anyArrayType; + if (noImplicitAny) { + reportImplicitAnyError(expression, anyArrayType); + } + } + types.push(anyedType); + } + } + + const type = getWidenedType(declaredType || getUnionType(types, UnionReduction.Subtype)); + if (filterType(type, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { + if (noImplicitAny) { + reportImplicitAnyError(symbol.valueDeclaration, anyType); + } + return anyType; + } + return type; + } + + function getSpecial(declaration: Declaration) { + const expression = isBinaryExpression(declaration) ? declaration : + isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : + undefined; + if (expression === undefined) { + return SpecialPropertyAssignmentKind.None; + } + return isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression as BinaryExpression); + } + + function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol) { + if (!symbol.declarations.some(d => getSpecial(d) === SpecialPropertyAssignmentKind.ThisProperty)) { + return getWidenedTypeFromJSPropertyDeclarations(symbol, undefined); + } // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { @@ -4759,7 +4861,7 @@ namespace ts { } else if (!jsDocType && isBinaryExpression(expression)) { // If we don't have an explicit JSDoc type, get the type from the expression. - let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + let type = getWidenedLiteralType(checkExpressionCached(expression.right)); if (type.flags & TypeFlags.Object && special === SpecialPropertyAssignmentKind.ModuleExports && @@ -4767,10 +4869,7 @@ namespace ts { const exportedType = resolveStructuredTypeMembers(type as ObjectType); const members = createSymbolTable(); copyEntries(exportedType.members, members); - if (resolvedSymbol && !resolvedSymbol.exports) { - resolvedSymbol.exports = createSymbolTable(); - } - (resolvedSymbol || symbol).exports!.forEach((s, name) => { + symbol.exports!.forEach((s, name) => { if (members.has(name)) { const exportedMember = exportedType.members.get(name)!; const union = createSymbol(s.flags | exportedMember.flags, name); @@ -4802,6 +4901,7 @@ namespace ts { } } } + let type = jsDocType; if (!type) { // use only the constructor types unless they were only assigned null | undefined (including widening variants) @@ -5219,7 +5319,7 @@ namespace ts { } else if (symbol.valueDeclaration.kind === SyntaxKind.BinaryExpression || symbol.valueDeclaration.kind === SyntaxKind.PropertyAccessExpression && symbol.valueDeclaration.parent.kind === SyntaxKind.BinaryExpression) { - links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); + links.type = getWidenedTypeFromJSPropertyDeclarations(symbol); } else { if (symbol.flags & SymbolFlags.ValueModule && symbol.valueDeclaration && isSourceFile(symbol.valueDeclaration) && symbol.valueDeclaration.commonJsModuleIndicator) { @@ -5229,7 +5329,7 @@ namespace ts { const resolvedModule = resolveExternalModuleSymbol(symbol); if (resolvedModule !== symbol) { const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!); - links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); + links.type = getWidenedTypeFromJSPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); } if (!popTypeResolution()) { links.type = reportCircularityError(symbol); From 2cb73ed5d6d07e3626d848e239aa1e0739286c72 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Sun, 22 Jul 2018 15:21:47 -0700 Subject: [PATCH 2/5] Split into two passes 1. Look for jsdoc 2. Look for type of initializers Both can be broken into their own functions. --- src/compiler/checker.ts | 95 ++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 45 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3939c0f9e0ec9..2058b89dca3c3 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4713,18 +4713,14 @@ namespace ts { } return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } - const types: Type[] = []; let declaredType: Type | undefined; for (const declaration of symbol.declarations) { const expression = isBinaryExpression(declaration) ? declaration : isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : - undefined; - - if (!expression) { + undefined; + if (expression === undefined) { return errorType; } - - // If there is a JSDoc type, use it (TODO: Should just do this in a loop before checking the initialisers) const typeNode = getJSDocType(expression.parent); if (typeNode) { const type = getWidenedType(getTypeFromTypeNode(typeNode)); @@ -4737,52 +4733,60 @@ namespace ts { errorNextVariableOrPropertyDeclarationMustHaveSameType(declaredType, declaration, type); } } - // TODO: Maybe we should try harder in the case that expression is not a binary expression. - else if (!declaredType && isBinaryExpression(expression)) { - // If we don't have an explicit JSDoc type, get the type from the expression. - let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); - const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); - - if (type.flags & TypeFlags.Object && - special === SpecialPropertyAssignmentKind.ModuleExports && - symbol.escapedName === InternalSymbolName.ExportEquals) { - const exportedType = resolveStructuredTypeMembers(type as ObjectType); - const members = createSymbolTable(); - copyEntries(exportedType.members, members); - if (resolvedSymbol && !resolvedSymbol.exports) { - resolvedSymbol.exports = createSymbolTable(); - } - (resolvedSymbol || symbol).exports!.forEach((s, name) => { - if (members.has(name)) { - const exportedMember = exportedType.members.get(name)!; - const union = createSymbol(s.flags | exportedMember.flags, name); - union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); - members.set(name, union); + } + if (!declaredType) { + const types: Type[] = []; + for (const declaration of symbol.declarations) { + const expression = isBinaryExpression(declaration) ? declaration : + isBinaryExpression(declaration.parent) ? declaration.parent : declaration; + // TODO: Maybe we should try harder in the case that expression is not a binary expression. + if (isBinaryExpression(expression)) { + // If we don't have an explicit JSDoc type, get the type from the expression. + let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); + + if (type.flags & TypeFlags.Object && + special === SpecialPropertyAssignmentKind.ModuleExports && + symbol.escapedName === InternalSymbolName.ExportEquals) { + const exportedType = resolveStructuredTypeMembers(type as ObjectType); + const members = createSymbolTable(); + copyEntries(exportedType.members, members); + if (resolvedSymbol && !resolvedSymbol.exports) { + resolvedSymbol.exports = createSymbolTable(); } - else { - members.set(name, s); + (resolvedSymbol || symbol).exports!.forEach((s, name) => { + if (members.has(name)) { + const exportedMember = exportedType.members.get(name)!; + const union = createSymbol(s.flags | exportedMember.flags, name); + union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); + members.set(name, union); + } + else { + members.set(name, s); + } + }); + type = createAnonymousType( + exportedType.symbol, + members, + exportedType.callSignatures, + exportedType.constructSignatures, + exportedType.stringIndexInfo, + exportedType.numberIndexInfo); + } + let anyedType = type; + if (isEmptyArrayLiteralType(type)) { + anyedType = anyArrayType; + if (noImplicitAny) { + reportImplicitAnyError(expression, anyArrayType); } - }); - type = createAnonymousType( - exportedType.symbol, - members, - exportedType.callSignatures, - exportedType.constructSignatures, - exportedType.stringIndexInfo, - exportedType.numberIndexInfo); - } - let anyedType = type; - if (isEmptyArrayLiteralType(type)) { - anyedType = anyArrayType; - if (noImplicitAny) { - reportImplicitAnyError(expression, anyArrayType); } + types.push(anyedType); } - types.push(anyedType); } + declaredType = getUnionType(types, UnionReduction.Subtype); } - const type = getWidenedType(declaredType || getUnionType(types, UnionReduction.Subtype)); + const type = getWidenedType(declaredType); if (filterType(type, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { if (noImplicitAny) { reportImplicitAnyError(symbol.valueDeclaration, anyType); @@ -4806,6 +4810,7 @@ namespace ts { if (!symbol.declarations.some(d => getSpecial(d) === SpecialPropertyAssignmentKind.ThisProperty)) { return getWidenedTypeFromJSPropertyDeclarations(symbol, undefined); } + // TODO: This section shouldn't be needed for this-property assignments // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { From c996a88293f0469e13f86a8222efee860e9a81f5 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Sun, 22 Jul 2018 21:28:59 -0700 Subject: [PATCH 3/5] Break into two functions --- src/compiler/checker.ts | 114 +++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 55 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2058b89dca3c3..4c46021120b52 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4713,6 +4713,18 @@ namespace ts { } return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } + const declaredType = getJSDocTypeFromSpecialDeclarations(symbol) || getInitializerTypeFromSpecialDeclarations(symbol, resolvedSymbol); + const type = getWidenedType(declaredType); + if (filterType(type, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { + if (noImplicitAny) { + reportImplicitAnyError(symbol.valueDeclaration, anyType); + } + return anyType; + } + return type; + } + + function getJSDocTypeFromSpecialDeclarations(symbol: Symbol) { let declaredType: Type | undefined; for (const declaration of symbol.declarations) { const expression = isBinaryExpression(declaration) ? declaration : @@ -4734,68 +4746,60 @@ namespace ts { } } } - if (!declaredType) { - const types: Type[] = []; - for (const declaration of symbol.declarations) { - const expression = isBinaryExpression(declaration) ? declaration : - isBinaryExpression(declaration.parent) ? declaration.parent : declaration; - // TODO: Maybe we should try harder in the case that expression is not a binary expression. - if (isBinaryExpression(expression)) { - // If we don't have an explicit JSDoc type, get the type from the expression. - let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); - const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); - - if (type.flags & TypeFlags.Object && - special === SpecialPropertyAssignmentKind.ModuleExports && - symbol.escapedName === InternalSymbolName.ExportEquals) { - const exportedType = resolveStructuredTypeMembers(type as ObjectType); - const members = createSymbolTable(); - copyEntries(exportedType.members, members); - if (resolvedSymbol && !resolvedSymbol.exports) { - resolvedSymbol.exports = createSymbolTable(); + return declaredType; + } + + /** If we don't have an explicit JSDoc type, get the type from the expression. */ + function getInitializerTypeFromSpecialDeclarations(symbol: Symbol, resolvedSymbol: Symbol | undefined) { + const types: Type[] = []; + for (const declaration of symbol.declarations) { + const expression = isBinaryExpression(declaration) ? declaration : + isBinaryExpression(declaration.parent) ? declaration.parent : declaration; + // TODO: Maybe we should try harder in the case that expression is not a binary expression. + if (isBinaryExpression(expression)) { + let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); + + if (type.flags & TypeFlags.Object && + special === SpecialPropertyAssignmentKind.ModuleExports && + symbol.escapedName === InternalSymbolName.ExportEquals) { + const exportedType = resolveStructuredTypeMembers(type as ObjectType); + const members = createSymbolTable(); + copyEntries(exportedType.members, members); + if (resolvedSymbol && !resolvedSymbol.exports) { + resolvedSymbol.exports = createSymbolTable(); + } + (resolvedSymbol || symbol).exports!.forEach((s, name) => { + if (members.has(name)) { + const exportedMember = exportedType.members.get(name)!; + const union = createSymbol(s.flags | exportedMember.flags, name); + union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); + members.set(name, union); } - (resolvedSymbol || symbol).exports!.forEach((s, name) => { - if (members.has(name)) { - const exportedMember = exportedType.members.get(name)!; - const union = createSymbol(s.flags | exportedMember.flags, name); - union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); - members.set(name, union); - } - else { - members.set(name, s); - } - }); - type = createAnonymousType( - exportedType.symbol, - members, - exportedType.callSignatures, - exportedType.constructSignatures, - exportedType.stringIndexInfo, - exportedType.numberIndexInfo); - } - let anyedType = type; - if (isEmptyArrayLiteralType(type)) { - anyedType = anyArrayType; - if (noImplicitAny) { - reportImplicitAnyError(expression, anyArrayType); + else { + members.set(name, s); } + }); + type = createAnonymousType( + exportedType.symbol, + members, + exportedType.callSignatures, + exportedType.constructSignatures, + exportedType.stringIndexInfo, + exportedType.numberIndexInfo); + } + let anyedType = type; + if (isEmptyArrayLiteralType(type)) { + anyedType = anyArrayType; + if (noImplicitAny) { + reportImplicitAnyError(expression, anyArrayType); } - types.push(anyedType); } + types.push(anyedType); } - declaredType = getUnionType(types, UnionReduction.Subtype); - } - - const type = getWidenedType(declaredType); - if (filterType(type, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { - if (noImplicitAny) { - reportImplicitAnyError(symbol.valueDeclaration, anyType); - } - return anyType; } - return type; + return getUnionType(types, UnionReduction.Subtype); } - function getSpecial(declaration: Declaration) { const expression = isBinaryExpression(declaration) ? declaration : isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : From d768a4a2b0ec1553fc818759af7130f391e23ff4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 23 Jul 2018 10:30:53 -0700 Subject: [PATCH 4/5] Move back to original function and single loop Then, convert the 2 extracted functions to reduce-style functions that take state and return the updated state. --- src/compiler/checker.ts | 295 ++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 192 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4c46021120b52..e6e31a84cd593 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4703,118 +4703,13 @@ namespace ts { return undefined; } - function getWidenedTypeFromJSPropertyDeclarations(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) { - const tag = getJSDocTypeTag(specialDeclaration); - if (tag && tag.typeExpression) { - return getTypeFromTypeNode(tag.typeExpression); - } - return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); - } - const declaredType = getJSDocTypeFromSpecialDeclarations(symbol) || getInitializerTypeFromSpecialDeclarations(symbol, resolvedSymbol); - const type = getWidenedType(declaredType); - if (filterType(type, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { - if (noImplicitAny) { - reportImplicitAnyError(symbol.valueDeclaration, anyType); - } - return anyType; - } - return type; - } - - function getJSDocTypeFromSpecialDeclarations(symbol: Symbol) { - let declaredType: Type | undefined; - for (const declaration of symbol.declarations) { - const expression = isBinaryExpression(declaration) ? declaration : - isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : - undefined; - if (expression === undefined) { - return errorType; - } - const typeNode = getJSDocType(expression.parent); - if (typeNode) { - const type = getWidenedType(getTypeFromTypeNode(typeNode)); - if (!declaredType) { - declaredType = type; - } - else if (declaredType !== errorType && type !== errorType && - !isTypeIdenticalTo(declaredType, type) && - !(symbol.flags & SymbolFlags.JSContainer)) { - errorNextVariableOrPropertyDeclarationMustHaveSameType(declaredType, declaration, type); - } - } - } - return declaredType; - } - - /** If we don't have an explicit JSDoc type, get the type from the expression. */ - function getInitializerTypeFromSpecialDeclarations(symbol: Symbol, resolvedSymbol: Symbol | undefined) { - const types: Type[] = []; - for (const declaration of symbol.declarations) { - const expression = isBinaryExpression(declaration) ? declaration : - isBinaryExpression(declaration.parent) ? declaration.parent : declaration; - // TODO: Maybe we should try harder in the case that expression is not a binary expression. - if (isBinaryExpression(expression)) { - let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); - const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); - - if (type.flags & TypeFlags.Object && - special === SpecialPropertyAssignmentKind.ModuleExports && - symbol.escapedName === InternalSymbolName.ExportEquals) { - const exportedType = resolveStructuredTypeMembers(type as ObjectType); - const members = createSymbolTable(); - copyEntries(exportedType.members, members); - if (resolvedSymbol && !resolvedSymbol.exports) { - resolvedSymbol.exports = createSymbolTable(); - } - (resolvedSymbol || symbol).exports!.forEach((s, name) => { - if (members.has(name)) { - const exportedMember = exportedType.members.get(name)!; - const union = createSymbol(s.flags | exportedMember.flags, name); - union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); - members.set(name, union); - } - else { - members.set(name, s); - } - }); - type = createAnonymousType( - exportedType.symbol, - members, - exportedType.callSignatures, - exportedType.constructSignatures, - exportedType.stringIndexInfo, - exportedType.numberIndexInfo); - } - let anyedType = type; - if (isEmptyArrayLiteralType(type)) { - anyedType = anyArrayType; - if (noImplicitAny) { - reportImplicitAnyError(expression, anyArrayType); - } - } - types.push(anyedType); - } - } - return getUnionType(types, UnionReduction.Subtype); - } - function getSpecial(declaration: Declaration) { - const expression = isBinaryExpression(declaration) ? declaration : - isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : - undefined; - if (expression === undefined) { - return SpecialPropertyAssignmentKind.None; - } - return isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression as BinaryExpression); + const enum JSThisAssignmentLocation { + None = 0, + Constructor = 1 << 0, + Method = 1 << 1, } - function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol) { - if (!symbol.declarations.some(d => getSpecial(d) === SpecialPropertyAssignmentKind.ThisProperty)) { - return getWidenedTypeFromJSPropertyDeclarations(symbol, undefined); - } - // TODO: This section shouldn't be needed for this-property assignments + function getWidenedTypeFromJSSpecialPropertyDeclarations(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) { @@ -4824,107 +4719,41 @@ namespace ts { } return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } - const types: Type[] = []; - let constructorTypes: Type[] | undefined; - let definedInConstructor = false; - let definedInMethod = false; - let jsDocType: Type | undefined; + let definedIn = JSThisAssignmentLocation.None; + let jsdocType: Type | undefined; + let types: Type[] | undefined; for (const declaration of symbol.declarations) { - let declarationInConstructor = false; const expression = isBinaryExpression(declaration) ? declaration : isPropertyAccessExpression(declaration) ? isBinaryExpression(declaration.parent) ? declaration.parent : declaration : undefined; - if (!expression) { return errorType; } const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); if (special === SpecialPropertyAssignmentKind.ThisProperty) { - const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false); - // Properties defined in a constructor (or base constructor, or javascript constructor function) don't get undefined added. - // Function expressions that are assigned to the prototype count as methods. - declarationInConstructor = thisContainer.kind === SyntaxKind.Constructor || - thisContainer.kind === SyntaxKind.FunctionDeclaration || - (thisContainer.kind === SyntaxKind.FunctionExpression && !isPrototypePropertyAssignment(thisContainer.parent)); - if (declarationInConstructor) { - definedInConstructor = true; - } - else { - definedInMethod = true; - } + definedIn |= isDeclarationInConstructor(expression) ? JSThisAssignmentLocation.Constructor : JSThisAssignmentLocation.Method; } - - // If there is a JSDoc type, use it - const type = getTypeForDeclarationFromJSDocComment(expression.parent); - if (type) { - const declarationType = getWidenedType(type); - if (!jsDocType) { - jsDocType = declarationType; - } - else if (jsDocType !== errorType && declarationType !== errorType && - !isTypeIdenticalTo(jsDocType, declarationType) && - !(symbol.flags & SymbolFlags.JSContainer)) { - errorNextVariableOrPropertyDeclarationMustHaveSameType(jsDocType, declaration, declarationType); - } - } - else if (!jsDocType && isBinaryExpression(expression)) { - // If we don't have an explicit JSDoc type, get the type from the expression. - let type = getWidenedLiteralType(checkExpressionCached(expression.right)); - - if (type.flags & TypeFlags.Object && - special === SpecialPropertyAssignmentKind.ModuleExports && - symbol.escapedName === InternalSymbolName.ExportEquals) { - const exportedType = resolveStructuredTypeMembers(type as ObjectType); - const members = createSymbolTable(); - copyEntries(exportedType.members, members); - symbol.exports!.forEach((s, name) => { - if (members.has(name)) { - const exportedMember = exportedType.members.get(name)!; - const union = createSymbol(s.flags | exportedMember.flags, name); - union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); - members.set(name, union); - } - else { - members.set(name, s); - } - }); - type = createAnonymousType( - exportedType.symbol, - members, - exportedType.callSignatures, - exportedType.constructSignatures, - exportedType.stringIndexInfo, - exportedType.numberIndexInfo); - } - let anyedType = type; - if (isEmptyArrayLiteralType(type)) { - anyedType = anyArrayType; - if (noImplicitAny) { - reportImplicitAnyError(expression, anyArrayType); - } - } - types.push(anyedType); - if (declarationInConstructor) { - (constructorTypes || (constructorTypes = [])).push(anyedType); - } + jsdocType = getJSDocTypeFromSpecialDeclarations(jsdocType, expression, symbol, declaration); + if (!jsdocType) { + (types || (types = [])).push(getInitializerTypeFromSpecialDeclarations(symbol, resolvedSymbol, expression, special)); } } - - let type = jsDocType; + let type = jsdocType; if (!type) { + let constructorTypes = definedIn & JSThisAssignmentLocation.Constructor ? getConstructuredDefinedThisAssignmentTypes(types!, symbol.declarations) : undefined; // use only the constructor types unless they were only assigned null | undefined (including widening variants) - if (definedInMethod) { + if (definedIn & JSThisAssignmentLocation.Method) { const propType = getTypeOfSpecialPropertyOfBaseType(symbol); if (propType) { (constructorTypes || (constructorTypes = [])).push(propType); - definedInConstructor = true; + definedIn |= JSThisAssignmentLocation.Constructor; } } - const sourceTypes = some(constructorTypes, t => !!(t.flags & ~(TypeFlags.Nullable | TypeFlags.ContainsWideningType))) ? constructorTypes! : types; // TODO: GH#18217 - type = getUnionType(sourceTypes, UnionReduction.Subtype); + const sourceTypes = some(constructorTypes, t => !!(t.flags & ~(TypeFlags.Nullable | TypeFlags.ContainsWideningType))) ? constructorTypes : types; // TODO: GH#18217 + type = getUnionType(sourceTypes!, UnionReduction.Subtype); } - const widened = getWidenedType(addOptionality(type, definedInMethod && !definedInConstructor)); + const widened = getWidenedType(addOptionality(type, definedIn === JSThisAssignmentLocation.Method)); if (filterType(widened, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { if (noImplicitAny) { reportImplicitAnyError(symbol.valueDeclaration, anyType); @@ -4934,6 +4763,88 @@ namespace ts { return widened; } + function getJSDocTypeFromSpecialDeclarations(declaredType: Type | undefined, expression: Expression, symbol: Symbol, declaration: Declaration) { + const typeNode = getJSDocType(expression.parent); + if (typeNode) { + const type = getWidenedType(getTypeFromTypeNode(typeNode)); + if (!declaredType) { + return type; + } + else if (declaredType !== errorType && type !== errorType && + !isTypeIdenticalTo(declaredType, type) && + !(symbol.flags & SymbolFlags.JSContainer)) { + errorNextVariableOrPropertyDeclarationMustHaveSameType(declaredType, declaration, type); + } + } + return declaredType; + } + + /** If we don't have an explicit JSDoc type, get the type from the initializer. */ + function getInitializerTypeFromSpecialDeclarations(symbol: Symbol, resolvedSymbol: Symbol | undefined, expression: Expression, special: SpecialPropertyAssignmentKind) { + if (isBinaryExpression(expression)) { + let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + if (type.flags & TypeFlags.Object && + special === SpecialPropertyAssignmentKind.ModuleExports && + symbol.escapedName === InternalSymbolName.ExportEquals) { + const exportedType = resolveStructuredTypeMembers(type as ObjectType); + const members = createSymbolTable(); + copyEntries(exportedType.members, members); + if (resolvedSymbol && !resolvedSymbol.exports) { + resolvedSymbol.exports = createSymbolTable(); + } + (resolvedSymbol || symbol).exports!.forEach((s, name) => { + if (members.has(name)) { + const exportedMember = exportedType.members.get(name)!; + const union = createSymbol(s.flags | exportedMember.flags, name); + union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]); + members.set(name, union); + } + else { + members.set(name, s); + } + }); + type = createAnonymousType( + exportedType.symbol, + members, + exportedType.callSignatures, + exportedType.constructSignatures, + exportedType.stringIndexInfo, + exportedType.numberIndexInfo); + } + if (isEmptyArrayLiteralType(type)) { + type = anyArrayType; + if (noImplicitAny) { + reportImplicitAnyError(expression, anyArrayType); + } + } + return type; + } + return neverType; + } + + function isDeclarationInConstructor(expression: Expression) { + const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false); + // Properties defined in a constructor (or base constructor, or javascript constructor function) don't get undefined added. + // Function expressions that are assigned to the prototype count as methods. + return thisContainer.kind === SyntaxKind.Constructor || + thisContainer.kind === SyntaxKind.FunctionDeclaration || + (thisContainer.kind === SyntaxKind.FunctionExpression && !isPrototypePropertyAssignment(thisContainer.parent)); + } + + function getConstructuredDefinedThisAssignmentTypes(types: Type[], declarations: Declaration[]): Type[] | undefined { + Debug.assert(types.length === declarations.length); + const constructorTypes = [] + for (let i = 0; i < types.length; i++) { + const declaration = declarations[i]; + const expression = isBinaryExpression(declaration) ? declaration : + isBinaryExpression(declaration.parent) ? declaration.parent : declaration; + if (isBinaryExpression(expression) && isDeclarationInConstructor(expression)) { + constructorTypes.push(types[i]); + } + } + return constructorTypes; + } + /** check for definition in base class if any declaration is in a class */ function getTypeOfSpecialPropertyOfBaseType(specialProperty: Symbol) { const parentDeclaration = forEach(specialProperty.declarations, d => { @@ -5328,7 +5239,7 @@ namespace ts { } else if (symbol.valueDeclaration.kind === SyntaxKind.BinaryExpression || symbol.valueDeclaration.kind === SyntaxKind.PropertyAccessExpression && symbol.valueDeclaration.parent.kind === SyntaxKind.BinaryExpression) { - links.type = getWidenedTypeFromJSPropertyDeclarations(symbol); + links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); } else { if (symbol.flags & SymbolFlags.ValueModule && symbol.valueDeclaration && isSourceFile(symbol.valueDeclaration) && symbol.valueDeclaration.commonJsModuleIndicator) { @@ -5338,7 +5249,7 @@ namespace ts { const resolvedModule = resolveExternalModuleSymbol(symbol); if (resolvedModule !== symbol) { const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!); - links.type = getWidenedTypeFromJSPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); + links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); } if (!popTypeResolution()) { links.type = reportCircularityError(symbol); From 5b8d29aef3fe9d38c27245916b27eaff7567f0db Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 23 Jul 2018 13:59:29 -0700 Subject: [PATCH 5/5] Cleanup suggestions from review --- src/compiler/checker.ts | 52 ++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e6e31a84cd593..6d3ac8d2e789b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4703,12 +4703,6 @@ namespace ts { return undefined; } - const enum JSThisAssignmentLocation { - None = 0, - Constructor = 1 << 0, - Method = 1 << 1, - } - function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol, resolvedSymbol?: Symbol) { // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); @@ -4719,7 +4713,8 @@ namespace ts { } return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } - let definedIn = JSThisAssignmentLocation.None; + let definedInConstructor = false; + let definedInMethod = false; let jsdocType: Type | undefined; let types: Type[] | undefined; for (const declaration of symbol.declarations) { @@ -4732,28 +4727,33 @@ namespace ts { const special = isPropertyAccessExpression(expression) ? getSpecialPropertyAccessKind(expression) : getSpecialPropertyAssignmentKind(expression); if (special === SpecialPropertyAssignmentKind.ThisProperty) { - definedIn |= isDeclarationInConstructor(expression) ? JSThisAssignmentLocation.Constructor : JSThisAssignmentLocation.Method; + if (isDeclarationInConstructor(expression)) { + definedInConstructor = true; + } + else { + definedInMethod = true; + } } jsdocType = getJSDocTypeFromSpecialDeclarations(jsdocType, expression, symbol, declaration); if (!jsdocType) { - (types || (types = [])).push(getInitializerTypeFromSpecialDeclarations(symbol, resolvedSymbol, expression, special)); + (types || (types = [])).push(isBinaryExpression(expression) ? getInitializerTypeFromSpecialDeclarations(symbol, resolvedSymbol, expression, special) : neverType); } } let type = jsdocType; if (!type) { - let constructorTypes = definedIn & JSThisAssignmentLocation.Constructor ? getConstructuredDefinedThisAssignmentTypes(types!, symbol.declarations) : undefined; + let constructorTypes = definedInConstructor ? getConstructorDefinedThisAssignmentTypes(types!, symbol.declarations) : undefined; // use only the constructor types unless they were only assigned null | undefined (including widening variants) - if (definedIn & JSThisAssignmentLocation.Method) { + if (definedInMethod) { const propType = getTypeOfSpecialPropertyOfBaseType(symbol); if (propType) { (constructorTypes || (constructorTypes = [])).push(propType); - definedIn |= JSThisAssignmentLocation.Constructor; + definedInConstructor = true; } } const sourceTypes = some(constructorTypes, t => !!(t.flags & ~(TypeFlags.Nullable | TypeFlags.ContainsWideningType))) ? constructorTypes : types; // TODO: GH#18217 type = getUnionType(sourceTypes!, UnionReduction.Subtype); } - const widened = getWidenedType(addOptionality(type, definedIn === JSThisAssignmentLocation.Method)); + const widened = getWidenedType(addOptionality(type, definedInMethod && !definedInConstructor)); if (filterType(widened, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) { if (noImplicitAny) { reportImplicitAnyError(symbol.valueDeclaration, anyType); @@ -4763,16 +4763,14 @@ namespace ts { return widened; } - function getJSDocTypeFromSpecialDeclarations(declaredType: Type | undefined, expression: Expression, symbol: Symbol, declaration: Declaration) { + function getJSDocTypeFromSpecialDeclarations(declaredType: Type | undefined, expression: Expression, _symbol: Symbol, declaration: Declaration) { const typeNode = getJSDocType(expression.parent); if (typeNode) { const type = getWidenedType(getTypeFromTypeNode(typeNode)); if (!declaredType) { return type; } - else if (declaredType !== errorType && type !== errorType && - !isTypeIdenticalTo(declaredType, type) && - !(symbol.flags & SymbolFlags.JSContainer)) { + else if (declaredType !== errorType && type !== errorType && !isTypeIdenticalTo(declaredType, type)) { errorNextVariableOrPropertyDeclarationMustHaveSameType(declaredType, declaration, type); } } @@ -4782,7 +4780,7 @@ namespace ts { /** If we don't have an explicit JSDoc type, get the type from the initializer. */ function getInitializerTypeFromSpecialDeclarations(symbol: Symbol, resolvedSymbol: Symbol | undefined, expression: Expression, special: SpecialPropertyAssignmentKind) { if (isBinaryExpression(expression)) { - let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); + const type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right)); if (type.flags & TypeFlags.Object && special === SpecialPropertyAssignmentKind.ModuleExports && symbol.escapedName === InternalSymbolName.ExportEquals) { @@ -4803,7 +4801,7 @@ namespace ts { members.set(name, s); } }); - type = createAnonymousType( + return createAnonymousType( exportedType.symbol, members, exportedType.callSignatures, @@ -4812,10 +4810,10 @@ namespace ts { exportedType.numberIndexInfo); } if (isEmptyArrayLiteralType(type)) { - type = anyArrayType; if (noImplicitAny) { reportImplicitAnyError(expression, anyArrayType); } + return anyArrayType; } return type; } @@ -4831,18 +4829,14 @@ namespace ts { (thisContainer.kind === SyntaxKind.FunctionExpression && !isPrototypePropertyAssignment(thisContainer.parent)); } - function getConstructuredDefinedThisAssignmentTypes(types: Type[], declarations: Declaration[]): Type[] | undefined { + function getConstructorDefinedThisAssignmentTypes(types: Type[], declarations: Declaration[]): Type[] | undefined { Debug.assert(types.length === declarations.length); - const constructorTypes = [] - for (let i = 0; i < types.length; i++) { + return types.filter((_, i) => { const declaration = declarations[i]; const expression = isBinaryExpression(declaration) ? declaration : - isBinaryExpression(declaration.parent) ? declaration.parent : declaration; - if (isBinaryExpression(expression) && isDeclarationInConstructor(expression)) { - constructorTypes.push(types[i]); - } - } - return constructorTypes; + isBinaryExpression(declaration.parent) ? declaration.parent : undefined; + return expression && isDeclarationInConstructor(expression); + }); } /** check for definition in base class if any declaration is in a class */