From ff69f7f744ffa33f287a8e20a1c1ffc7ca19f2bd Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Tue, 31 Jul 2018 12:20:35 -0700 Subject: [PATCH 1/4] Port generated lib files --- src/lib/dom.generated.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dom.generated.d.ts b/src/lib/dom.generated.d.ts index 96bc4f73b57..a74a128f7dd 100644 --- a/src/lib/dom.generated.d.ts +++ b/src/lib/dom.generated.d.ts @@ -14332,7 +14332,8 @@ interface Storage { /** * storage[key] = value */ - setItem(key: string, value: string): void; + setItem(key: string, value: string): void; + [name: string]: any; } declare var Storage: { From 4821f81ce765ed804cc9d471c27cd6256c4f0b79 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 31 Jul 2018 13:07:16 -0700 Subject: [PATCH 2/4] Refactor and improve caching in getTypeOfSymbol callees (#25842) * Refactor+improve caching in getTypeOfSymbol 1. Always cache calls to getTypeOfSymbol, even in the error case. 2. JS expando types are now cached on the original symbol as well as the cloned symbol. Previously they were only cached on the cloned symbol. 3. Large callees of getTypeOfSymbol (variable/param/property, func/class/enum/module, and accessors) now handle only caching and delegate to -Worker functions whose return values are cached unconditionally (unlike previously). * Fix circularity detection in getTypeOfFuncClassEnumModule Previously, successfully obtaining a type from a js special property declaration would forget to pop the circularity detection stack and check its value. --- src/compiler/checker.ts | 320 ++++++++++++++++++++-------------------- 1 file changed, 161 insertions(+), 159 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 42f553bd9d7..5c46f3e7dbd 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4993,86 +4993,87 @@ namespace ts { function getTypeOfVariableOrParameterOrProperty(symbol: Symbol): Type { const links = getSymbolLinks(symbol); - if (!links.type) { - // Handle prototype property - if (symbol.flags & SymbolFlags.Prototype) { - return links.type = getTypeOfPrototypeProperty(symbol); - } - // CommonsJS require and module both have type any. - if (symbol === requireSymbol) { - return links.type = anyType; - } - if (symbol.flags & SymbolFlags.ModuleExports) { - const fileSymbol = getSymbolOfNode(getSourceFileOfNode(symbol.valueDeclaration)); - const members = createSymbolTable(); - members.set("exports" as __String, fileSymbol); - return links.type = createAnonymousType(symbol, members, emptyArray, emptyArray, undefined, undefined); - } - // Handle catch clause variables - const declaration = symbol.valueDeclaration; - if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) { - return links.type = anyType; - } - // Handle export default expressions - if (isSourceFile(declaration)) { - const jsonSourceFile = cast(declaration, isJsonSourceFile); - return links.type = jsonSourceFile.statements.length ? checkExpression(jsonSourceFile.statements[0].expression) : emptyObjectType; - } - if (declaration.kind === SyntaxKind.ExportAssignment) { - return links.type = checkExpression((declaration).expression); - } - // Handle variable, parameter or property - 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)); - } - } - - if (!popTypeResolution()) { - type = reportCircularityError(symbol); - } - links.type = type; - } - return links.type; + return links.type || (links.type = getTypeOfVariableOrParameterOrPropertyWorker(symbol)); } + function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol) { + // Handle prototype property + if (symbol.flags & SymbolFlags.Prototype) { + return getTypeOfPrototypeProperty(symbol); + } + // CommonsJS require and module both have type any. + if (symbol === requireSymbol) { + return anyType; + } + if (symbol.flags & SymbolFlags.ModuleExports) { + const fileSymbol = getSymbolOfNode(getSourceFileOfNode(symbol.valueDeclaration)); + const members = createSymbolTable(); + members.set("exports" as __String, fileSymbol); + return createAnonymousType(symbol, members, emptyArray, emptyArray, undefined, undefined); + } + // Handle catch clause variables + const declaration = symbol.valueDeclaration; + if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) { + return anyType; + } + // Handle export default expressions + if (isSourceFile(declaration)) { + const jsonSourceFile = cast(declaration, isJsonSourceFile); + return jsonSourceFile.statements.length ? checkExpression(jsonSourceFile.statements[0].expression) : emptyObjectType; + } + if (declaration.kind === SyntaxKind.ExportAssignment) { + return checkExpression((declaration).expression); + } + + // Handle variable, parameter or property + 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)); + } + } + + if (!popTypeResolution()) { + type = reportCircularityError(symbol); + } + return type; + } + function getJSSpecialType(symbol: Symbol, decl: Declaration): Type | undefined { if (!isInJavaScriptFile(decl)) { return undefined; @@ -5148,64 +5149,65 @@ namespace ts { function getTypeOfAccessors(symbol: Symbol): Type { const links = getSymbolLinks(symbol); - if (!links.type) { - const getter = getDeclarationOfKind(symbol, SyntaxKind.GetAccessor); - const setter = getDeclarationOfKind(symbol, SyntaxKind.SetAccessor); + return links.type || (links.type = getTypeOfAccessorsWorker(symbol)); + } - if (getter && isInJavaScriptFile(getter)) { - const jsDocType = getTypeForDeclarationFromJSDocComment(getter); - if (jsDocType) { - return links.type = jsDocType; - } + function getTypeOfAccessorsWorker(symbol: Symbol): Type { + const getter = getDeclarationOfKind(symbol, SyntaxKind.GetAccessor); + const setter = getDeclarationOfKind(symbol, SyntaxKind.SetAccessor); + + if (getter && isInJavaScriptFile(getter)) { + const jsDocType = getTypeForDeclarationFromJSDocComment(getter); + if (jsDocType) { + return jsDocType; } + } - if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { - return errorType; - } + if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { + return errorType; + } - let type: Type; + let type: Type; - // First try to see if the user specified a return type on the get-accessor. - const getterReturnType = getAnnotatedAccessorType(getter); - if (getterReturnType) { - type = getterReturnType; + // First try to see if the user specified a return type on the get-accessor. + const getterReturnType = getAnnotatedAccessorType(getter); + if (getterReturnType) { + type = getterReturnType; + } + else { + // If the user didn't specify a return type, try to use the set-accessor's parameter type. + const setterParameterType = getAnnotatedAccessorType(setter); + if (setterParameterType) { + type = setterParameterType; } else { - // If the user didn't specify a return type, try to use the set-accessor's parameter type. - const setterParameterType = getAnnotatedAccessorType(setter); - if (setterParameterType) { - type = setterParameterType; + // If there are no specified types, try to infer it from the body of the get accessor if it exists. + if (getter && getter.body) { + type = getReturnTypeFromBody(getter); } + // Otherwise, fall back to 'any'. else { - // If there are no specified types, try to infer it from the body of the get accessor if it exists. - if (getter && getter.body) { - type = getReturnTypeFromBody(getter); - } - // Otherwise, fall back to 'any'. - else { - if (noImplicitAny) { - if (setter) { - error(setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol)); - } - else { - Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function"); - error(getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol)); - } + if (noImplicitAny) { + if (setter) { + error(setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol)); + } + else { + Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function"); + error(getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol)); } - type = anyType; } + type = anyType; } } - if (!popTypeResolution()) { - type = anyType; - if (noImplicitAny) { - const getter = getDeclarationOfKind(symbol, SyntaxKind.GetAccessor); - error(getter, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, symbolToString(symbol)); - } - } - links.type = type; } - return links.type; + if (!popTypeResolution()) { + type = anyType; + if (noImplicitAny) { + const getter = getDeclarationOfKind(symbol, SyntaxKind.GetAccessor); + error(getter, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, symbolToString(symbol)); + } + } + return type; } function getBaseTypeVariableOfClass(symbol: Symbol) { @@ -5215,6 +5217,7 @@ namespace ts { function getTypeOfFuncClassEnumModule(symbol: Symbol): Type { let links = getSymbolLinks(symbol); + const originalLinks = links; if (!links.type) { const jsDeclaration = getDeclarationOfJSInitializer(symbol.valueDeclaration); if (jsDeclaration) { @@ -5233,48 +5236,47 @@ namespace ts { } } } - if (symbol.flags & SymbolFlags.Module && isShorthandAmbientModuleSymbol(symbol)) { - links.type = anyType; - } - else if (symbol.valueDeclaration.kind === SyntaxKind.BinaryExpression || - symbol.valueDeclaration.kind === SyntaxKind.PropertyAccessExpression && symbol.valueDeclaration.parent.kind === SyntaxKind.BinaryExpression) { - links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); - } - else { - if (symbol.flags & SymbolFlags.ValueModule && symbol.valueDeclaration && isSourceFile(symbol.valueDeclaration) && symbol.valueDeclaration.commonJsModuleIndicator) { - if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { - return errorType; - } - const resolvedModule = resolveExternalModuleSymbol(symbol); - if (resolvedModule !== symbol) { - const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!); - links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); - } - if (!popTypeResolution()) { - links.type = reportCircularityError(symbol); - } - } - if (!links.type) { - const type = createObjectType(ObjectFlags.Anonymous, symbol); - if (symbol.flags & SymbolFlags.Class) { - const baseTypeVariable = getBaseTypeVariableOfClass(symbol); - links.type = baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type; - } - else { - links.type = strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type; - } - } - } + originalLinks.type = links.type = getTypeOfFuncClassEnumModuleWorker(symbol); } return links.type; } + function getTypeOfFuncClassEnumModuleWorker(symbol: Symbol): Type { + const declaration = symbol.valueDeclaration; + if (symbol.flags & SymbolFlags.Module && isShorthandAmbientModuleSymbol(symbol)) { + return anyType; + } + else if (declaration.kind === SyntaxKind.BinaryExpression || + declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) { + return getWidenedTypeFromJSSpecialPropertyDeclarations(symbol); + } + else if (symbol.flags & SymbolFlags.ValueModule && declaration && isSourceFile(declaration) && declaration.commonJsModuleIndicator) { + const resolvedModule = resolveExternalModuleSymbol(symbol); + if (resolvedModule !== symbol) { + if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { + return errorType; + } + const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!); + const type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule); + if (!popTypeResolution()) { + return reportCircularityError(symbol); + } + return type; + } + } + const type = createObjectType(ObjectFlags.Anonymous, symbol); + if (symbol.flags & SymbolFlags.Class) { + const baseTypeVariable = getBaseTypeVariableOfClass(symbol); + return baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type; + } + else { + return strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type; + } + } + function getTypeOfEnumMember(symbol: Symbol): Type { const links = getSymbolLinks(symbol); - if (!links.type) { - links.type = getDeclaredTypeOfEnumMember(symbol); - } - return links.type; + return links.type || (links.type = getDeclaredTypeOfEnumMember(symbol)); } function getTypeOfAlias(symbol: Symbol): Type { @@ -5303,7 +5305,7 @@ namespace ts { } else { if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { - return errorType; + return links.type = errorType; } symbolInstantiationDepth++; let type = instantiateType(getTypeOfSymbol(links.target!), links.mapper!); From dfedb24f759f9d0cb13732a82d20dd70ab0e1e22 Mon Sep 17 00:00:00 2001 From: James Keane Date: Tue, 31 Jul 2018 16:52:39 -0400 Subject: [PATCH 3/4] Jsdoc `@constructor` - in constructor properly infer `this` as class instance (#25980) * Properly infer `this` in tagged `@constructor`s. `c.prototype.method = function() { this }` was already supported. This commit add support to two more kinds relying on the JSDoc `@constructor` tag. These are: 1. `/** @constructor */ function Example() { this }` 2. `/** @constructor */ var Example = function() { this }` * Update the baseline for js constructorFunctions. C3 and C4 `this` was set as `any`, now it is properly showing as the class type. * Fix lint errors * Add circular initialisers to constructo fn tests. * Error (`TS2348`) if calling tagged js constructors When calling a JS function explicitly tagged with either `@class` or `@constructor` the checker should throw a TS2348 not callable error. * Don't resolve jsdoc classes with construct sigs. This undoes the last commit that sought to change how js functions tagged with `@class` were inferred. For some reason, currently unknown, giving those functions construct signatures causes issues in property assignment/member resolution (as seen in the `typeFromPropertyAssignment12` test case). Instead of changing the signature resolution, the error is explicitly generated in `resolveCallExpression` for those functions. --- src/compiler/checker.ts | 20 ++++++- .../reference/constructorFunctions.errors.txt | 56 +++++++++++++++++++ .../reference/constructorFunctions.symbols | 22 ++++++++ .../reference/constructorFunctions.types | 34 ++++++++++- .../conformance/salsa/constructorFunctions.ts | 6 ++ 5 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/constructorFunctions.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5c46f3e7dbd..868957d0fcd 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15404,8 +15404,8 @@ namespace ts { (!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(container))) { // Note: a parameter initializer should refer to class-this unless function-this is explicitly annotated. - // If this is a function in a JS file, it might be a class method. Check if it's the RHS - // of a x.prototype.y = function [name]() { .... } + // If this is a function in a JS file, it might be a class method. + // Check if it's the RHS of a x.prototype.y = function [name]() { .... } if (container.kind === SyntaxKind.FunctionExpression && container.parent.kind === SyntaxKind.BinaryExpression && getSpecialPropertyAssignmentKind(container.parent as BinaryExpression) === SpecialPropertyAssignmentKind.PrototypeProperty) { @@ -15419,6 +15419,17 @@ namespace ts { return getFlowTypeOfReference(node, getInferredClassType(classSymbol)); } } + // Check if it's a constructor definition, can be either a variable decl or function decl + // i.e. + // * /** @constructor */ function [name]() { ... } + // * /** @constructor */ var x = function() { ... } + else if ((container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) && + getJSDocClassTag(container)) { + const classType = getJavaScriptClassType(container.symbol); + if (classType) { + return getFlowTypeOfReference(node, classType); + } + } const thisType = getThisTypeOfDeclaration(container) || getContextualThisParameterType(container); if (thisType) { @@ -19497,6 +19508,11 @@ namespace ts { } return resolveErrorCall(node); } + // If the function is explicitly marked with `@class`, then it must be constructed. + if (callSignatures.some(sig => isInJavaScriptFile(sig.declaration) && !!getJSDocClassTag(sig.declaration!))) { + error(node, Diagnostics.Value_of_type_0_is_not_callable_Did_you_mean_to_include_new, typeToString(funcType)); + return resolveErrorCall(node); + } return resolveCall(node, callSignatures, candidatesOutArray, isForSignatureHelp); } diff --git a/tests/baselines/reference/constructorFunctions.errors.txt b/tests/baselines/reference/constructorFunctions.errors.txt new file mode 100644 index 00000000000..8b2fc319c5a --- /dev/null +++ b/tests/baselines/reference/constructorFunctions.errors.txt @@ -0,0 +1,56 @@ +tests/cases/conformance/salsa/index.js(22,15): error TS2348: Value of type 'typeof C3' is not callable. Did you mean to include 'new'? +tests/cases/conformance/salsa/index.js(30,15): error TS2348: Value of type 'typeof C4' is not callable. Did you mean to include 'new'? + + +==== tests/cases/conformance/salsa/index.js (2 errors) ==== + function C1() { + if (!(this instanceof C1)) return new C1(); + this.x = 1; + } + + const c1_v1 = C1(); + const c1_v2 = new C1(); + + var C2 = function () { + if (!(this instanceof C2)) return new C2(); + this.x = 1; + }; + + const c2_v1 = C2(); + const c2_v2 = new C2(); + + /** @class */ + function C3() { + if (!(this instanceof C3)) return new C3(); + }; + + const c3_v1 = C3(); + ~~~~ +!!! error TS2348: Value of type 'typeof C3' is not callable. Did you mean to include 'new'? + const c3_v2 = new C3(); + + /** @class */ + var C4 = function () { + if (!(this instanceof C4)) return new C4(); + }; + + const c4_v1 = C4(); + ~~~~ +!!! error TS2348: Value of type 'typeof C4' is not callable. Did you mean to include 'new'? + const c4_v2 = new C4(); + + var c5_v1; + c5_v1 = function f() { }; + new c5_v1(); + + var c5_v2; + c5_v2 = class { }; + new c5_v2(); + + /** @class */ + function C6() { + this.functions = [x => x, x => x + 1, x => x - 1] + }; + + var c6_v1 = new C6(); + \ No newline at end of file diff --git a/tests/baselines/reference/constructorFunctions.symbols b/tests/baselines/reference/constructorFunctions.symbols index abd77f3ea21..21dd0fdebf2 100644 --- a/tests/baselines/reference/constructorFunctions.symbols +++ b/tests/baselines/reference/constructorFunctions.symbols @@ -43,6 +43,7 @@ function C3() { >C3 : Symbol(C3, Decl(index.js, 14, 23)) if (!(this instanceof C3)) return new C3(); +>this : Symbol(C3, Decl(index.js, 14, 23)) >C3 : Symbol(C3, Decl(index.js, 14, 23)) >C3 : Symbol(C3, Decl(index.js, 14, 23)) @@ -61,6 +62,7 @@ var C4 = function () { >C4 : Symbol(C4, Decl(index.js, 25, 3)) if (!(this instanceof C4)) return new C4(); +>this : Symbol(C4, Decl(index.js, 25, 8)) >C4 : Symbol(C4, Decl(index.js, 25, 3)) >C4 : Symbol(C4, Decl(index.js, 25, 3)) @@ -93,4 +95,24 @@ c5_v2 = class { }; new c5_v2(); >c5_v2 : Symbol(c5_v2, Decl(index.js, 36, 3)) +/** @class */ +function C6() { +>C6 : Symbol(C6, Decl(index.js, 38, 12)) + + this.functions = [x => x, x => x + 1, x => x - 1] +>this.functions : Symbol(C6.functions, Decl(index.js, 41, 15)) +>this : Symbol(C6, Decl(index.js, 38, 12)) +>functions : Symbol(C6.functions, Decl(index.js, 41, 15)) +>x : Symbol(x, Decl(index.js, 42, 20)) +>x : Symbol(x, Decl(index.js, 42, 20)) +>x : Symbol(x, Decl(index.js, 42, 27)) +>x : Symbol(x, Decl(index.js, 42, 27)) +>x : Symbol(x, Decl(index.js, 42, 39)) +>x : Symbol(x, Decl(index.js, 42, 39)) + +}; + +var c6_v1 = new C6(); +>c6_v1 : Symbol(c6_v1, Decl(index.js, 45, 3)) +>C6 : Symbol(C6, Decl(index.js, 38, 12)) diff --git a/tests/baselines/reference/constructorFunctions.types b/tests/baselines/reference/constructorFunctions.types index 7b8d6b0e2e1..582ba35cfe3 100644 --- a/tests/baselines/reference/constructorFunctions.types +++ b/tests/baselines/reference/constructorFunctions.types @@ -69,7 +69,7 @@ function C3() { >!(this instanceof C3) : boolean >(this instanceof C3) : boolean >this instanceof C3 : boolean ->this : any +>this : C3 >C3 : typeof C3 >new C3() : C3 >C3 : typeof C3 @@ -95,7 +95,7 @@ var C4 = function () { >!(this instanceof C4) : boolean >(this instanceof C4) : boolean >this instanceof C4 : boolean ->this : any +>this : C4 >C4 : typeof C4 >new C4() : C4 >C4 : typeof C4 @@ -137,4 +137,34 @@ new c5_v2(); >new c5_v2() : c5_v2 >c5_v2 : typeof c5_v2 +/** @class */ +function C6() { +>C6 : typeof C6 + + this.functions = [x => x, x => x + 1, x => x - 1] +>this.functions = [x => x, x => x + 1, x => x - 1] : ((x: any) => any)[] +>this.functions : ((x: any) => any)[] +>this : C6 +>functions : ((x: any) => any)[] +>[x => x, x => x + 1, x => x - 1] : ((x: any) => any)[] +>x => x : (x: any) => any +>x : any +>x : any +>x => x + 1 : (x: any) => any +>x : any +>x + 1 : any +>x : any +>1 : 1 +>x => x - 1 : (x: any) => number +>x : any +>x - 1 : number +>x : any +>1 : 1 + +}; + +var c6_v1 = new C6(); +>c6_v1 : C6 +>new C6() : C6 +>C6 : typeof C6 diff --git a/tests/cases/conformance/salsa/constructorFunctions.ts b/tests/cases/conformance/salsa/constructorFunctions.ts index 6c166a22f77..155f8d5c271 100644 --- a/tests/cases/conformance/salsa/constructorFunctions.ts +++ b/tests/cases/conformance/salsa/constructorFunctions.ts @@ -43,3 +43,9 @@ var c5_v2; c5_v2 = class { }; new c5_v2(); +/** @class */ +function C6() { + this.functions = [x => x, x => x + 1, x => x - 1] +}; + +var c6_v1 = new C6(); From 2edc47bc67d947bd222684a8f71b873b6d9e28e8 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Tue, 31 Jul 2018 13:53:28 -0700 Subject: [PATCH 4/4] Fix decorated accessor emit (#26016) --- src/compiler/checker.ts | 21 +++++--- src/compiler/transformers/ts.ts | 10 +++- src/compiler/types.ts | 4 +- src/compiler/utilities.ts | 8 +-- .../reference/decoratorOnClassAccessor8.js | 4 +- ...rgetEs6DecoratorMetadataImportNotElided.js | 54 +++++++++++++++++++ ...s6DecoratorMetadataImportNotElided.symbols | 38 +++++++++++++ ...tEs6DecoratorMetadataImportNotElided.types | 40 ++++++++++++++ ...rgetEs6DecoratorMetadataImportNotElided.ts | 18 +++++++ 9 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.js create mode 100644 tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.symbols create mode 100644 tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.types create mode 100644 tests/cases/compiler/targetEs6DecoratorMetadataImportNotElided.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 868957d0fcd..86dfb26a092 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5124,20 +5124,25 @@ namespace ts { } } - function getAnnotatedAccessorType(accessor: AccessorDeclaration | undefined): Type | undefined { + function getAnnotatedAccessorTypeNode(accessor: AccessorDeclaration | undefined): TypeNode | undefined { if (accessor) { if (accessor.kind === SyntaxKind.GetAccessor) { const getterTypeAnnotation = getEffectiveReturnTypeNode(accessor); - return getterTypeAnnotation && getTypeFromTypeNode(getterTypeAnnotation); + return getterTypeAnnotation; } else { const setterTypeAnnotation = getEffectiveSetAccessorTypeAnnotationNode(accessor); - return setterTypeAnnotation && getTypeFromTypeNode(setterTypeAnnotation); + return setterTypeAnnotation; } } return undefined; } + function getAnnotatedAccessorType(accessor: AccessorDeclaration | undefined): Type | undefined { + const node = getAnnotatedAccessorTypeNode(accessor); + return node && getTypeFromTypeNode(node); + } + function getAnnotatedAccessorThisParameter(accessor: AccessorDeclaration): Symbol | undefined { const parameter = getAccessorThisParameter(accessor); return parameter && parameter.symbol; @@ -23519,9 +23524,13 @@ namespace ts { } break; - case SyntaxKind.MethodDeclaration: case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: + const otherKind = node.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor; + const otherAccessor = getDeclarationOfKind(getSymbolOfNode(node as AccessorDeclaration), otherKind); + markDecoratorMedataDataTypeNodeAsReferenced(getAnnotatedAccessorTypeNode(node as AccessorDeclaration) || otherAccessor && getAnnotatedAccessorTypeNode(otherAccessor)); + break; + case SyntaxKind.MethodDeclaration: for (const parameter of (node).parameters) { markDecoratorMedataDataTypeNodeAsReferenced(getParameterTypeNodeForDecoratorCheck(parameter)); } @@ -28074,8 +28083,8 @@ namespace ts { const otherAccessor = getDeclarationOfKind(getSymbolOfNode(accessor), otherKind); const firstAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? otherAccessor : accessor; const secondAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? accessor : otherAccessor; - const setAccessor = accessor.kind === SyntaxKind.SetAccessor ? accessor : otherAccessor; - const getAccessor = accessor.kind === SyntaxKind.GetAccessor ? accessor : otherAccessor; + const setAccessor = accessor.kind === SyntaxKind.SetAccessor ? accessor : otherAccessor as SetAccessorDeclaration; + const getAccessor = accessor.kind === SyntaxKind.GetAccessor ? accessor : otherAccessor as GetAccessorDeclaration; return { firstAccessor, secondAccessor, diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index bf4ae5ce7d3..7af0b7c5ad7 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -1752,6 +1752,12 @@ namespace ts { type SerializedEntityNameAsExpression = Identifier | BinaryExpression | PropertyAccessExpression; type SerializedTypeNode = SerializedEntityNameAsExpression | VoidExpression | ConditionalExpression; + function getAccessorTypeNode(node: AccessorDeclaration) { + const accessors = resolver.getAllAccessorDeclarations(node); + return accessors.setAccessor && getSetAccessorTypeAnnotationNode(accessors.setAccessor) + || accessors.getAccessor && getEffectiveReturnTypeNode(accessors.getAccessor); + } + /** * Serializes the type of a node for use with decorator type metadata. * @@ -1761,10 +1767,10 @@ namespace ts { switch (node.kind) { case SyntaxKind.PropertyDeclaration: case SyntaxKind.Parameter: - case SyntaxKind.GetAccessor: return serializeTypeNode((node).type); case SyntaxKind.SetAccessor: - return serializeTypeNode(getSetAccessorTypeAnnotationNode(node)); + case SyntaxKind.GetAccessor: + return serializeTypeNode(getAccessorTypeNode(node as AccessorDeclaration)); case SyntaxKind.ClassDeclaration: case SyntaxKind.ClassExpression: case SyntaxKind.MethodDeclaration: diff --git a/src/compiler/types.ts b/src/compiler/types.ts index b59041d3f52..0b2477af197 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3315,8 +3315,8 @@ namespace ts { export interface AllAccessorDeclarations { firstAccessor: AccessorDeclaration; secondAccessor: AccessorDeclaration | undefined; - getAccessor: AccessorDeclaration | undefined; - setAccessor: AccessorDeclaration | undefined; + getAccessor: GetAccessorDeclaration | undefined; + setAccessor: SetAccessorDeclaration | undefined; } /** Indicates how to serialize the name for a TypeReferenceNode when emitting decorator metadata */ diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0ac533f7658..c095ff2a732 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3310,8 +3310,8 @@ namespace ts { // TODO: GH#18217 let firstAccessor!: AccessorDeclaration; let secondAccessor!: AccessorDeclaration; - let getAccessor!: AccessorDeclaration; - let setAccessor!: AccessorDeclaration; + let getAccessor!: GetAccessorDeclaration; + let setAccessor!: SetAccessorDeclaration; if (hasDynamicName(accessor)) { firstAccessor = accessor; if (accessor.kind === SyntaxKind.GetAccessor) { @@ -3339,11 +3339,11 @@ namespace ts { } if (member.kind === SyntaxKind.GetAccessor && !getAccessor) { - getAccessor = member; + getAccessor = member; } if (member.kind === SyntaxKind.SetAccessor && !setAccessor) { - setAccessor = member; + setAccessor = member; } } } diff --git a/tests/baselines/reference/decoratorOnClassAccessor8.js b/tests/baselines/reference/decoratorOnClassAccessor8.js index f0917130521..e1fffdaec83 100644 --- a/tests/baselines/reference/decoratorOnClassAccessor8.js +++ b/tests/baselines/reference/decoratorOnClassAccessor8.js @@ -50,7 +50,7 @@ var A = /** @class */ (function () { }); __decorate([ dec, - __metadata("design:type", Object), + __metadata("design:type", Number), __metadata("design:paramtypes", [Number]) ], A.prototype, "x", null); return A; @@ -98,7 +98,7 @@ var D = /** @class */ (function () { }); __decorate([ dec, - __metadata("design:type", Object), + __metadata("design:type", Number), __metadata("design:paramtypes", [Number]) ], D.prototype, "x", null); return D; diff --git a/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.js b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.js new file mode 100644 index 00000000000..b2126326793 --- /dev/null +++ b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.js @@ -0,0 +1,54 @@ +//// [tests/cases/compiler/targetEs6DecoratorMetadataImportNotElided.ts] //// + +//// [deps.ts] +export function Input(): any { } +export class TemplateRef { } + +//// [index.ts] +import { Input, TemplateRef } from './deps'; + +export class MyComponent { + _ref: TemplateRef; + + @Input() + get ref() { return this._ref; } + set ref(value: TemplateRef) { this._ref = value; } +} + + +//// [deps.js] +export function Input() { } +var TemplateRef = /** @class */ (function () { + function TemplateRef() { + } + return TemplateRef; +}()); +export { TemplateRef }; +//// [index.js] +var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) { + var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d; + if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc); + else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r; + return c > 3 && r && Object.defineProperty(target, key, r), r; +}; +var __metadata = (this && this.__metadata) || function (k, v) { + if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v); +}; +import { Input, TemplateRef } from './deps'; +var MyComponent = /** @class */ (function () { + function MyComponent() { + } + Object.defineProperty(MyComponent.prototype, "ref", { + get: function () { return this._ref; }, + set: function (value) { this._ref = value; }, + enumerable: true, + configurable: true + }); + __decorate([ + Input(), + __metadata("design:type", TemplateRef), + __metadata("design:paramtypes", [TemplateRef]) + ], MyComponent.prototype, "ref", null); + return MyComponent; +}()); +export { MyComponent }; diff --git a/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.symbols b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.symbols new file mode 100644 index 00000000000..9fe0ee6e0f3 --- /dev/null +++ b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.symbols @@ -0,0 +1,38 @@ +=== tests/cases/compiler/deps.ts === +export function Input(): any { } +>Input : Symbol(Input, Decl(deps.ts, 0, 0)) + +export class TemplateRef { } +>TemplateRef : Symbol(TemplateRef, Decl(deps.ts, 0, 32)) + +=== tests/cases/compiler/index.ts === +import { Input, TemplateRef } from './deps'; +>Input : Symbol(Input, Decl(index.ts, 0, 8)) +>TemplateRef : Symbol(TemplateRef, Decl(index.ts, 0, 15)) + +export class MyComponent { +>MyComponent : Symbol(MyComponent, Decl(index.ts, 0, 44)) + + _ref: TemplateRef; +>_ref : Symbol(MyComponent._ref, Decl(index.ts, 2, 26)) +>TemplateRef : Symbol(TemplateRef, Decl(index.ts, 0, 15)) + + @Input() +>Input : Symbol(Input, Decl(index.ts, 0, 8)) + + get ref() { return this._ref; } +>ref : Symbol(MyComponent.ref, Decl(index.ts, 3, 22), Decl(index.ts, 6, 35)) +>this._ref : Symbol(MyComponent._ref, Decl(index.ts, 2, 26)) +>this : Symbol(MyComponent, Decl(index.ts, 0, 44)) +>_ref : Symbol(MyComponent._ref, Decl(index.ts, 2, 26)) + + set ref(value: TemplateRef) { this._ref = value; } +>ref : Symbol(MyComponent.ref, Decl(index.ts, 3, 22), Decl(index.ts, 6, 35)) +>value : Symbol(value, Decl(index.ts, 7, 12)) +>TemplateRef : Symbol(TemplateRef, Decl(index.ts, 0, 15)) +>this._ref : Symbol(MyComponent._ref, Decl(index.ts, 2, 26)) +>this : Symbol(MyComponent, Decl(index.ts, 0, 44)) +>_ref : Symbol(MyComponent._ref, Decl(index.ts, 2, 26)) +>value : Symbol(value, Decl(index.ts, 7, 12)) +} + diff --git a/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.types b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.types new file mode 100644 index 00000000000..44fbbd8621d --- /dev/null +++ b/tests/baselines/reference/targetEs6DecoratorMetadataImportNotElided.types @@ -0,0 +1,40 @@ +=== tests/cases/compiler/deps.ts === +export function Input(): any { } +>Input : () => any + +export class TemplateRef { } +>TemplateRef : TemplateRef + +=== tests/cases/compiler/index.ts === +import { Input, TemplateRef } from './deps'; +>Input : () => any +>TemplateRef : typeof TemplateRef + +export class MyComponent { +>MyComponent : MyComponent + + _ref: TemplateRef; +>_ref : TemplateRef +>TemplateRef : TemplateRef + + @Input() +>Input() : any +>Input : () => any + + get ref() { return this._ref; } +>ref : TemplateRef +>this._ref : TemplateRef +>this : this +>_ref : TemplateRef + + set ref(value: TemplateRef) { this._ref = value; } +>ref : TemplateRef +>value : TemplateRef +>TemplateRef : TemplateRef +>this._ref = value : TemplateRef +>this._ref : TemplateRef +>this : this +>_ref : TemplateRef +>value : TemplateRef +} + diff --git a/tests/cases/compiler/targetEs6DecoratorMetadataImportNotElided.ts b/tests/cases/compiler/targetEs6DecoratorMetadataImportNotElided.ts new file mode 100644 index 00000000000..16629f25879 --- /dev/null +++ b/tests/cases/compiler/targetEs6DecoratorMetadataImportNotElided.ts @@ -0,0 +1,18 @@ +// @module: es6 +// @target: es5 +// @emitDecoratorMetadata: true +// @experimentalDecorators: true +// @filename: deps.ts +export function Input(): any { } +export class TemplateRef { } + +// @filename: index.ts +import { Input, TemplateRef } from './deps'; + +export class MyComponent { + _ref: TemplateRef; + + @Input() + get ref() { return this._ref; } + set ref(value: TemplateRef) { this._ref = value; } +}