From 3d3bcb4a5b8256617655fcfb2e675312a8647580 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 4 May 2016 13:35:35 -0700 Subject: [PATCH] Correctly copy annotated this getter -> setter Previously it only went the other direction. --- src/compiler/checker.ts | 40 +++++++++------ .../reference/thisTypeInAccessors.js | 10 ++++ .../reference/thisTypeInAccessors.symbols | 51 ++++++++++++++----- .../reference/thisTypeInAccessors.types | 26 ++++++++++ .../types/thisType/thisTypeInAccessors.ts | 5 ++ 5 files changed, 102 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 89e8a2b93d4..8a09c2dcfde 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2920,7 +2920,12 @@ namespace ts { if (func.kind === SyntaxKind.SetAccessor && !hasDynamicName(func)) { const getter = getDeclarationOfKind(declaration.parent.symbol, SyntaxKind.GetAccessor); if (getter) { - return getReturnTypeOfSignature(getSignatureFromDeclaration(getter)); + const signature = getSignatureFromDeclaration(getter); + const thisParameter = getAccessorThisParameter(func as AccessorDeclaration); + if (thisParameter && declaration === thisParameter) { + return signature.thisType; + } + return getReturnTypeOfSignature(signature); } } // Use contextual parameter type if one is available @@ -3137,12 +3142,11 @@ namespace ts { } function getAnnotatedAccessorThisType(accessor: AccessorDeclaration): Type { - if (accessor && - accessor.parameters.length === (accessor.kind === SyntaxKind.GetAccessor ? 1 : 2) && - accessor.parameters[0].name.kind === SyntaxKind.Identifier && - (accessor.parameters[0].name as Identifier).originalKeywordKind === SyntaxKind.ThisKeyword && - accessor.parameters[0].type ) { - return getTypeFromTypeNode(accessor.parameters[0].type); + if (accessor) { + const parameter = getAccessorThisParameter(accessor); + if (parameter && parameter.type) { + return getTypeFromTypeNode(accessor.parameters[0].type); + } } return undefined; } @@ -4413,7 +4417,7 @@ namespace ts { // If only one accessor includes a this-type annotation, the other behaves as if it had the same type annotation if ((declaration.kind === SyntaxKind.GetAccessor || declaration.kind === SyntaxKind.SetAccessor) && !hasDynamicName(declaration) && - !hasThisParameter) { + (!hasThisParameter || thisType === unknownType)) { const otherKind = declaration.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor; const setter = getDeclarationOfKind(declaration.symbol, otherKind); thisType = getAnnotatedAccessorThisType(setter); @@ -18084,7 +18088,7 @@ namespace ts { return false; } - function checkGrammarAccessor(accessor: MethodDeclaration): boolean { + function checkGrammarAccessor(accessor: AccessorDeclaration): boolean { const kind = accessor.kind; if (languageVersion < ScriptTarget.ES5) { return grammarErrorOnNode(accessor.name, Diagnostics.Accessors_are_only_available_when_targeting_ECMAScript_5_and_higher); @@ -18130,13 +18134,17 @@ namespace ts { A get accessor has no parameters or a single `this` parameter. A set accessor has one parameter or a `this` parameter and one more parameter */ - function doesAccessorHaveCorrectParameterCount(accessor: MethodDeclaration) { - const isGet = accessor.kind === SyntaxKind.GetAccessor; - return (accessor.parameters.length === (isGet ? 1 : 2) && - accessor.parameters[0].name.kind === SyntaxKind.Identifier && - (accessor.parameters[0].name).originalKeywordKind === SyntaxKind.ThisKeyword) || - accessor.parameters.length === (isGet ? 0 : 1); - } + function doesAccessorHaveCorrectParameterCount(accessor: AccessorDeclaration) { + return getAccessorThisParameter(accessor) || accessor.parameters.length === (accessor.kind === SyntaxKind.GetAccessor ? 0 : 1); + } + + function getAccessorThisParameter(accessor: AccessorDeclaration) { + if (accessor.parameters.length === (accessor.kind === SyntaxKind.GetAccessor ? 1 : 2) && + accessor.parameters[0].name.kind === SyntaxKind.Identifier && + (accessor.parameters[0].name).originalKeywordKind === SyntaxKind.ThisKeyword) { + return accessor.parameters[0]; + } + } function checkGrammarForNonSymbolComputedProperty(node: DeclarationName, message: DiagnosticMessage) { if (isDynamicName(node)) { diff --git a/tests/baselines/reference/thisTypeInAccessors.js b/tests/baselines/reference/thisTypeInAccessors.js index f3108398445..ed339d0366a 100644 --- a/tests/baselines/reference/thisTypeInAccessors.js +++ b/tests/baselines/reference/thisTypeInAccessors.js @@ -19,6 +19,11 @@ const copiedFromSetter = { get x() { return this.n }, set x(this: Foo, n: number) { this.n = n; } } +const copiedFromGetterUnannotated = { + n: 16, + get x(this: Foo) { return this.n }, + set x(this, n) { this.n = n; } +} class Explicit { n = 17; @@ -47,6 +52,11 @@ var copiedFromSetter = { get x() { return this.n; }, set x(n) { this.n = n; } }; +var copiedFromGetterUnannotated = { + n: 16, + get x() { return this.n; }, + set x(n) { this.n = n; } +}; var Explicit = (function () { function Explicit() { this.n = 17; diff --git a/tests/baselines/reference/thisTypeInAccessors.symbols b/tests/baselines/reference/thisTypeInAccessors.symbols index 9344d4656f4..ef2201d3808 100644 --- a/tests/baselines/reference/thisTypeInAccessors.symbols +++ b/tests/baselines/reference/thisTypeInAccessors.symbols @@ -77,41 +77,64 @@ const copiedFromSetter = { >n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) >n : Symbol(n, Decl(thisTypeInAccessors.ts, 18, 20)) } +const copiedFromGetterUnannotated = { +>copiedFromGetterUnannotated : Symbol(copiedFromGetterUnannotated, Decl(thisTypeInAccessors.ts, 20, 5)) + + n: 16, +>n : Symbol(n, Decl(thisTypeInAccessors.ts, 20, 37)) + + get x(this: Foo) { return this.n }, +>x : Symbol(x, Decl(thisTypeInAccessors.ts, 21, 10), Decl(thisTypeInAccessors.ts, 22, 39)) +>this : Symbol(this, Decl(thisTypeInAccessors.ts, 22, 10)) +>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) +>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) +>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) +>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) + + set x(this, n) { this.n = n; } +>x : Symbol(x, Decl(thisTypeInAccessors.ts, 21, 10), Decl(thisTypeInAccessors.ts, 22, 39)) +>this : Symbol(this, Decl(thisTypeInAccessors.ts, 23, 10)) +>n : Symbol(n, Decl(thisTypeInAccessors.ts, 23, 15)) +>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) +>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) +>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) +>n : Symbol(n, Decl(thisTypeInAccessors.ts, 23, 15)) +} class Explicit { ->Explicit : Symbol(Explicit, Decl(thisTypeInAccessors.ts, 19, 1)) +>Explicit : Symbol(Explicit, Decl(thisTypeInAccessors.ts, 24, 1)) n = 17; ->n : Symbol(Explicit.n, Decl(thisTypeInAccessors.ts, 21, 16)) +>n : Symbol(Explicit.n, Decl(thisTypeInAccessors.ts, 26, 16)) get x(this: Foo): number { return this.n; } ->x : Symbol(Explicit.x, Decl(thisTypeInAccessors.ts, 22, 11), Decl(thisTypeInAccessors.ts, 23, 47)) ->this : Symbol(this, Decl(thisTypeInAccessors.ts, 23, 10)) +>x : Symbol(Explicit.x, Decl(thisTypeInAccessors.ts, 27, 11), Decl(thisTypeInAccessors.ts, 28, 47)) +>this : Symbol(this, Decl(thisTypeInAccessors.ts, 28, 10)) >Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) >this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) >this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) >n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) set x(this: Foo, n: number) { this.n = n; } ->x : Symbol(Explicit.x, Decl(thisTypeInAccessors.ts, 22, 11), Decl(thisTypeInAccessors.ts, 23, 47)) ->this : Symbol(this, Decl(thisTypeInAccessors.ts, 24, 10)) +>x : Symbol(Explicit.x, Decl(thisTypeInAccessors.ts, 27, 11), Decl(thisTypeInAccessors.ts, 28, 47)) +>this : Symbol(this, Decl(thisTypeInAccessors.ts, 29, 10)) >Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) ->n : Symbol(n, Decl(thisTypeInAccessors.ts, 24, 20)) +>n : Symbol(n, Decl(thisTypeInAccessors.ts, 29, 20)) >this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) >this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0)) >n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15)) ->n : Symbol(n, Decl(thisTypeInAccessors.ts, 24, 20)) +>n : Symbol(n, Decl(thisTypeInAccessors.ts, 29, 20)) } class Contextual { ->Contextual : Symbol(Contextual, Decl(thisTypeInAccessors.ts, 25, 1)) +>Contextual : Symbol(Contextual, Decl(thisTypeInAccessors.ts, 30, 1)) n = 21; ->n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 26, 18)) +>n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 31, 18)) get x() { return this.n } // inside a class, so already correct ->x : Symbol(Contextual.x, Decl(thisTypeInAccessors.ts, 27, 11)) ->this.n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 26, 18)) ->this : Symbol(Contextual, Decl(thisTypeInAccessors.ts, 25, 1)) ->n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 26, 18)) +>x : Symbol(Contextual.x, Decl(thisTypeInAccessors.ts, 32, 11)) +>this.n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 31, 18)) +>this : Symbol(Contextual, Decl(thisTypeInAccessors.ts, 30, 1)) +>n : Symbol(Contextual.n, Decl(thisTypeInAccessors.ts, 31, 18)) } diff --git a/tests/baselines/reference/thisTypeInAccessors.types b/tests/baselines/reference/thisTypeInAccessors.types index f280b397fee..998658c5d58 100644 --- a/tests/baselines/reference/thisTypeInAccessors.types +++ b/tests/baselines/reference/thisTypeInAccessors.types @@ -86,6 +86,32 @@ const copiedFromSetter = { >n : number >n : number } +const copiedFromGetterUnannotated = { +>copiedFromGetterUnannotated : { n: number; x: number; } +>{ n: 16, get x(this: Foo) { return this.n }, set x(this, n) { this.n = n; }} : { n: number; x: number; } + + n: 16, +>n : number +>16 : number + + get x(this: Foo) { return this.n }, +>x : number +>this : Foo +>Foo : Foo +>this.n : number +>this : Foo +>n : number + + set x(this, n) { this.n = n; } +>x : number +>this : Foo +>n : number +>this.n = n : number +>this.n : number +>this : Foo +>n : number +>n : number +} class Explicit { >Explicit : Explicit diff --git a/tests/cases/conformance/types/thisType/thisTypeInAccessors.ts b/tests/cases/conformance/types/thisType/thisTypeInAccessors.ts index a59fa1ead56..b0903759767 100644 --- a/tests/cases/conformance/types/thisType/thisTypeInAccessors.ts +++ b/tests/cases/conformance/types/thisType/thisTypeInAccessors.ts @@ -21,6 +21,11 @@ const copiedFromSetter = { get x() { return this.n }, set x(this: Foo, n: number) { this.n = n; } } +const copiedFromGetterUnannotated = { + n: 16, + get x(this: Foo) { return this.n }, + set x(this, n) { this.n = n; } +} class Explicit { n = 17;