Merge pull request microsoft#8463 from Microsoft/this-types-for-accessors

This types for accessors

Author: sandersn

src/compiler/checker.ts

@@ -2924,7 +2924,12 @@ namespace ts {
  if (func.kind === SyntaxKind.SetAccessor && !hasDynamicName(func)) {
  const getter = <AccessorDeclaration>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
@@ -3140,6 +3145,16 @@ namespace ts {
  return undefined;
  }
 
+ function getAnnotatedAccessorThisType(accessor: AccessorDeclaration): Type {
+ if (accessor) {
+ const parameter = getAccessorThisParameter(accessor);
+ if (parameter && parameter.type) {
+ return getTypeFromTypeNode(accessor.parameters[0].type);
+ }
+ }
+ return undefined;
+ }
+
  function getTypeOfAccessors(symbol: Symbol): Type {
  const links = getSymbolLinks(symbol);
  if (!links.type) {
@@ -4361,20 +4376,12 @@ namespace ts {
  function getSignatureFromDeclaration(declaration: SignatureDeclaration): Signature {
  const links = getNodeLinks(declaration);
  if (!links.resolvedSignature) {
- const classType = declaration.kind === SyntaxKind.Constructor ?
- getDeclaredTypeOfClassOrInterface(getMergedSymbol((<ClassDeclaration>declaration.parent).symbol))
- : undefined;
- const typeParameters = classType ? classType.localTypeParameters :
- declaration.typeParameters ? getTypeParametersFromDeclaration(declaration.typeParameters) :
- getTypeParametersFromJSDocTemplate(declaration);
  const parameters: Symbol[] = [];
  let hasStringLiterals = false;
  let minArgumentCount = -1;
  let thisType: Type = undefined;
  let hasThisParameter: boolean;
  const isJSConstructSignature = isJSDocConstructSignature(declaration);
- let returnType: Type = undefined;
- let typePredicate: TypePredicate = undefined;
 
  // If this is a JSDoc construct signature, then skip the first parameter in the
  // parameter list. The first parameter represents the return type of the construct
@@ -4411,48 +4418,68 @@ 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 || thisType === unknownType)) {
+ const otherKind = declaration.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor;
+ const setter = <AccessorDeclaration>getDeclarationOfKind(declaration.symbol, otherKind);
+ thisType = getAnnotatedAccessorThisType(setter);
+ }
+
  if (minArgumentCount < 0) {
  minArgumentCount = declaration.parameters.length - (hasThisParameter ? 1 : 0);
  }
-
  if (isJSConstructSignature) {
  minArgumentCount--;
- returnType = getTypeFromTypeNode(declaration.parameters[0].type);
  }
- else if (classType) {
- returnType = classType;
- }
- else if (declaration.type) {
- returnType = getTypeFromTypeNode(declaration.type);
- if (declaration.type.kind === SyntaxKind.TypePredicate) {
- typePredicate = createTypePredicateFromTypePredicateNode(declaration.type as TypePredicateNode);
- }
- }
- else {
- if (declaration.flags & NodeFlags.JavaScriptFile) {
- const type = getReturnTypeFromJSDocComment(declaration);
- if (type && type !== unknownType) {
- returnType = type;
- }
- }
-
- // TypeScript 1.0 spec (April 2014):
- // If only one accessor includes a type annotation, the other behaves as if it had the same type annotation.
- if (declaration.kind === SyntaxKind.GetAccessor && !hasDynamicName(declaration)) {
- const setter = <AccessorDeclaration>getDeclarationOfKind(declaration.symbol, SyntaxKind.SetAccessor);
- returnType = getAnnotatedAccessorType(setter);
- }
 
- if (!returnType && nodeIsMissing((<FunctionLikeDeclaration>declaration).body)) {
- returnType = anyType;
- }
- }
+ const classType = declaration.kind === SyntaxKind.Constructor ?
+ getDeclaredTypeOfClassOrInterface(getMergedSymbol((<ClassDeclaration>declaration.parent).symbol))
+ : undefined;
+ const typeParameters = classType ? classType.localTypeParameters :
+ declaration.typeParameters ? getTypeParametersFromDeclaration(declaration.typeParameters) :
+ getTypeParametersFromJSDocTemplate(declaration);
+ const returnType = getSignatureReturnTypeFromDeclaration(declaration, minArgumentCount, isJSConstructSignature, classType);
+ const typePredicate = declaration.type && declaration.type.kind === SyntaxKind.TypePredicate ?
+ createTypePredicateFromTypePredicateNode(declaration.type as TypePredicateNode) :
+ undefined;
 
  links.resolvedSignature = createSignature(declaration, typeParameters, thisType, parameters, returnType, typePredicate, minArgumentCount, hasRestParameter(declaration), hasStringLiterals);
  }
  return links.resolvedSignature;
  }
 
+ function getSignatureReturnTypeFromDeclaration(declaration: SignatureDeclaration, minArgumentCount: number, isJSConstructSignature: boolean, classType: Type) {
+ if (isJSConstructSignature) {
+ return getTypeFromTypeNode(declaration.parameters[0].type);
+ }
+ else if (classType) {
+ return classType;
+ }
+ else if (declaration.type) {
+ return getTypeFromTypeNode(declaration.type);
+ }
+
+ if (declaration.flags & NodeFlags.JavaScriptFile) {
+ const type = getReturnTypeFromJSDocComment(declaration);
+ if (type && type !== unknownType) {
+ return type;
+ }
+ }
+
+ // TypeScript 1.0 spec (April 2014):
+ // If only one accessor includes a type annotation, the other behaves as if it had the same type annotation.
+ if (declaration.kind === SyntaxKind.GetAccessor && !hasDynamicName(declaration)) {
+ const setter = <AccessorDeclaration>getDeclarationOfKind(declaration.symbol, SyntaxKind.SetAccessor);
+ return getAnnotatedAccessorType(setter);
+ }
+
+ if (nodeIsMissing((<FunctionLikeDeclaration>declaration).body)) {
+ return anyType;
+ }
+ }
+
  function getSignaturesOfSymbol(symbol: Symbol): Signature[] {
  if (!symbol) return emptyArray;
  const result: Signature[] = [];
@@ -12644,9 +12671,6 @@ namespace ts {
  if (func.kind === SyntaxKind.Constructor || func.kind === SyntaxKind.ConstructSignature || func.kind === SyntaxKind.ConstructorType) {
  error(node, Diagnostics.A_constructor_cannot_have_a_this_parameter);
  }
- if (func.kind === SyntaxKind.SetAccessor) {
- error(node, Diagnostics.A_setter_cannot_have_a_this_parameter);
- }
  }
 
  // Only check rest parameter type if it's not a binding pattern. Since binding patterns are
@@ -13033,15 +13057,10 @@ namespace ts {
  error(node.name, Diagnostics.Accessors_must_both_be_abstract_or_non_abstract);
  }
 
- const currentAccessorType = getAnnotatedAccessorType(node);
- const otherAccessorType = getAnnotatedAccessorType(otherAccessor);
  // TypeScript 1.0 spec (April 2014): 4.5
  // If both accessors include type annotations, the specified types must be identical.
- if (currentAccessorType && otherAccessorType) {
- if (!isTypeIdenticalTo(currentAccessorType, otherAccessorType)) {
- error(node, Diagnostics.get_and_set_accessor_must_have_the_same_type);
- }
- }
+ checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_same_type);
+ checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorThisType, Diagnostics.get_and_set_accessor_must_have_the_same_this_type);
  }
  }
  getTypeOfAccessors(getSymbolOfNode(node));
@@ -13054,6 +13073,14 @@ namespace ts {
  }
  }
 
+ function checkAccessorDeclarationTypesIdentical(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type, message: DiagnosticMessage) {
+ const firstType = getAnnotatedType(first);
+ const secondType = getAnnotatedType(second);
+ if (firstType && secondType && !isTypeIdenticalTo(firstType, secondType)) {
+ error(first, message);
+ }
+ }
+
  function checkAccessorDeferred(node: AccessorDeclaration) {
  checkSourceElement(node.body);
  }
@@ -18127,7 +18154,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);
@@ -18141,16 +18168,16 @@ namespace ts {
  else if (accessor.typeParameters) {
  return grammarErrorOnNode(accessor.name, Diagnostics.An_accessor_cannot_have_type_parameters);
  }
- else if (kind === SyntaxKind.GetAccessor && accessor.parameters.length) {
- return grammarErrorOnNode(accessor.name, Diagnostics.A_get_accessor_cannot_have_parameters);
+ else if (!doesAccessorHaveCorrectParameterCount(accessor)) {
+ return grammarErrorOnNode(accessor.name,
+ kind === SyntaxKind.GetAccessor ?
+ Diagnostics.A_get_accessor_cannot_have_parameters :
+ Diagnostics.A_set_accessor_must_have_exactly_one_parameter);
  }
  else if (kind === SyntaxKind.SetAccessor) {
  if (accessor.type) {
  return grammarErrorOnNode(accessor.name, Diagnostics.A_set_accessor_cannot_have_a_return_type_annotation);
  }
- else if (accessor.parameters.length !== 1) {
- return grammarErrorOnNode(accessor.name, Diagnostics.A_set_accessor_must_have_exactly_one_parameter);
- }
  else {
  const parameter = accessor.parameters[0];
  if (parameter.dotDotDotToken) {
@@ -18169,6 +18196,22 @@ namespace ts {
  }
  }
 
+ /** Does the accessor have the right number of parameters?
+
+ 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: 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 &&
+ (<Identifier>accessor.parameters[0].name).originalKeywordKind === SyntaxKind.ThisKeyword) {
+ return accessor.parameters[0];
+ }
+ }
+
  function checkGrammarForNonSymbolComputedProperty(node: DeclarationName, message: DiagnosticMessage) {
  if (isDynamicName(node)) {
  return grammarErrorOnNode(node, message);

src/compiler/diagnosticMessages.json

@@ -1907,7 +1907,7 @@
  "category": "Error",
  "code": 2681
  },
- "A setter cannot have a 'this' parameter.": {
+ "'get' and 'set' accessor must have the same 'this' type.": {
  "category": "Error",
  "code": 2682
  },

src/compiler/utilities.ts

@@ -2269,7 +2269,12 @@ namespace ts {
  }
 
  export function getSetAccessorTypeAnnotationNode(accessor: AccessorDeclaration): TypeNode {
- return accessor && accessor.parameters.length > 0 && accessor.parameters[0].type;
+ if (accessor && accessor.parameters.length > 0) {
+ const hasThis = accessor.parameters.length === 2 &&
+ accessor.parameters[0].name.kind === SyntaxKind.Identifier &&
+ (accessor.parameters[0].name as Identifier).originalKeywordKind === SyntaxKind.ThisKeyword;
+ return accessor.parameters[hasThis ? 1 : 0].type;
+ }
  }
 
  export function getAllAccessorDeclarations(declarations: NodeArray<Declaration>, accessor: AccessorDeclaration) {

tests/baselines/reference/thisTypeInAccessors.js

@@ -0,0 +1,83 @@
+//// [thisTypeInAccessors.ts]
+interface Foo {
+ n: number;
+ x: number;
+}
+
+const explicit = {
+ n: 12,
+ get x(this: Foo): number { return this.n; },
+ set x(this: Foo, n: number) { this.n = n; }
+}
+const copiedFromGetter = {
+ n: 14,
+ get x(this: Foo): number { return this.n; },
+ set x(n) { this.n = n; }
+}
+const copiedFromSetter = {
+ n: 15,
+ 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;
+ get x(this: Foo): number { return this.n; }
+ set x(this: Foo, n: number) { this.n = n; }
+}
+class Contextual {
+ n = 21;
+ get x() { return this.n } // inside a class, so already correct
+}
+
+
+//// [thisTypeInAccessors.js]
+var explicit = {
+ n: 12,
+ get x() { return this.n; },
+ set x(n) { this.n = n; }
+};
+var copiedFromGetter = {
+ n: 14,
+ get x() { return this.n; },
+ set x(n) { this.n = n; }
+};
+var copiedFromSetter = {
+ n: 15,
+ 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;
+ }
+ Object.defineProperty(Explicit.prototype, "x", {
+ get: function () { return this.n; },
+ set: function (n) { this.n = n; },
+ enumerable: true,
+ configurable: true
+ });
+ return Explicit;
+}());
+var Contextual = (function () {
+ function Contextual() {
+ this.n = 21;
+ }
+ Object.defineProperty(Contextual.prototype, "x", {
+ get: function () { return this.n; } // inside a class, so already correct
+ ,
+ enumerable: true,
+ configurable: true
+ });
+ return Contextual;
+}());