Skip to content

Commit

Permalink
Divergent get/set types and visibility
Browse files Browse the repository at this point in the history
  • Loading branch information
RyanCavanaugh committed Jan 20, 2021
1 parent d33143c commit 6f09705
Show file tree
Hide file tree
Showing 41 changed files with 3,164 additions and 157 deletions.
52 changes: 37 additions & 15 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25967,10 +25967,18 @@ namespace ts {
*/
function checkPropertyAccessibility(
node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
isSuper: boolean, type: Type, prop: Symbol): boolean {
const flags = getDeclarationModifierFlagsFromSymbol(prop);
isSuper: boolean, type: Type, prop: Symbol, isWrite = false): boolean {
let flags = getDeclarationModifierFlagsFromSymbol(prop);
const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right : node.kind === SyntaxKind.ImportType ? node : node.name;

// Writes use the visibility modifier of the set accessor, if one is declared
if (isWrite) {
const setter = forEach(prop.declarations, p => p.kind === SyntaxKind.SetAccessor && p);
if (setter) {
flags = (flags & ~ModifierFlags.AccessibilityModifier) | getEffectiveDeclarationFlags(setter, ModifierFlags.AccessibilityModifier);
}
}

if (isSuper) {
// TS 1.0 spec (April 2014): 4.8.2
// - In a constructor, instance member function, instance member accessor, or
Expand Down Expand Up @@ -26285,7 +26293,7 @@ namespace ts {
markAliasReferenced(parentSymbol, node);
}

let propType: Type;
let propType: Type | undefined;
if (!prop) {
const indexInfo = !isPrivateIdentifier(right) && (assignmentKind === AssignmentKind.None || !isGenericObjectType(leftType) || isThisTypeParameter(leftType)) ? getIndexInfoOfType(apparentType, IndexKind.String) : undefined;
if (!(indexInfo && indexInfo.type)) {
Expand Down Expand Up @@ -26316,19 +26324,30 @@ namespace ts {
}
}
else {
const isWrite = isWriteAccess(node);
if (getDeclarationNodeFlagsFromSymbol(prop) & NodeFlags.Deprecated && isUncalledFunctionReference(node, prop)) {
errorOrSuggestion(/* isError */ false, right, Diagnostics._0_is_deprecated, right.escapedText as string);
}
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
getNodeLinks(node).resolvedSymbol = prop;
checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop);
checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop, isWrite);
if (isAssignmentToReadonlyEntity(node as Expression, prop, assignmentKind)) {
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right));
return errorType;
}
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : getConstraintForLocation(getTypeOfSymbol(prop), node);
if (isWrite) {
const setter = forEach(prop.declarations, p => (p.kind === SyntaxKind.SetAccessor && p));
if (setter) {
propType = getAnnotatedAccessorType(setter as SetAccessorDeclaration);
}
}

if (propType === undefined) {
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : getConstraintForLocation(getTypeOfSymbol(prop), node);
}
}
// For writes to properties declared as accessors, use the 'set' type
return getFlowTypeOfAccessExpression(node, prop, propType, right);
}

Expand Down Expand Up @@ -32321,15 +32340,19 @@ namespace ts {
const otherKind = node.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor;
const otherAccessor = getDeclarationOfKind<AccessorDeclaration>(getSymbolOfNode(node), otherKind);
if (otherAccessor) {
const nodeFlags = getEffectiveModifierFlags(node);
const otherFlags = getEffectiveModifierFlags(otherAccessor);
if ((nodeFlags & ModifierFlags.Abstract) !== (otherFlags & ModifierFlags.Abstract)) {
const getter = node.kind === SyntaxKind.GetAccessor ? node : otherAccessor;
const setter = node.kind === SyntaxKind.SetAccessor ? node : otherAccessor;
const getterFlags = getEffectiveModifierFlags(getter);
const setterFlags = getEffectiveModifierFlags(setter);
if ((getterFlags & ModifierFlags.Abstract) !== (setterFlags & ModifierFlags.Abstract)) {
error(node.name, Diagnostics.Accessors_must_both_be_abstract_or_non_abstract);
}
if (((getterFlags & ModifierFlags.Protected) && !(setterFlags & (ModifierFlags.Protected | ModifierFlags.Private))) ||
((getterFlags & ModifierFlags.Private) && !(setterFlags & ModifierFlags.Private))) {
error(node.name, Diagnostics.A_get_accessor_must_be_at_least_as_accessible_as_the_setter);
}

// TypeScript 1.0 spec (April 2014): 4.5
// If both accessors include type annotations, the specified types must be identical.
checkAccessorDeclarationTypesAssignable(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_comparable_types);
checkAccessorDeclarationTypesAssignable(getter, setter, getAnnotatedAccessorType, Diagnostics.The_return_type_of_a_get_accessor_must_be_assignable_to_its_set_accessor_type);
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getThisTypeOfDeclaration, Diagnostics.get_and_set_accessor_must_have_the_same_this_type);
}
}
Expand All @@ -32345,8 +32368,8 @@ namespace ts {
return checkAccessorDeclarationTypesMatch(first, second, getAnnotatedType, isTypeIdenticalTo, message);
}

function checkAccessorDeclarationTypesAssignable(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, message: DiagnosticMessage) {
return checkAccessorDeclarationTypesMatch(first, second, getAnnotatedType, areTypesComparable, message);
function checkAccessorDeclarationTypesAssignable(getter: AccessorDeclaration, setter: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, message: DiagnosticMessage) {
return checkAccessorDeclarationTypesMatch(getter, setter, getAnnotatedType, isTypeAssignableTo, message);
}

function checkAccessorDeclarationTypesMatch(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, match: typeof areTypesComparable, message: DiagnosticMessage) {
Expand All @@ -32357,7 +32380,6 @@ namespace ts {
}
}


function checkMissingDeclaration(node: Node) {
checkDecorators(node);
}
Expand Down Expand Up @@ -40171,7 +40193,7 @@ namespace ts {
}

function checkGrammarAccessor(accessor: AccessorDeclaration): boolean {
if (!(accessor.flags & NodeFlags.Ambient)) {
if (!(accessor.flags & NodeFlags.Ambient) && (accessor.parent.kind !== SyntaxKind.TypeLiteral) && (accessor.parent.kind !== SyntaxKind.InterfaceDeclaration)) {
if (languageVersion < ScriptTarget.ES5) {
return grammarErrorOnNode(accessor.name, Diagnostics.Accessors_are_only_available_when_targeting_ECMAScript_5_and_higher);
}
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@
"category": "Error",
"code": 2378
},
"'get' and 'set' accessor must have comparable types.": {
"The return type of a 'get' accessor must be assignable to its 'set' accessor type": {
"category": "Error",
"code": 2380
},
Expand Down Expand Up @@ -3231,6 +3231,10 @@
"category": "Error",
"code": 2797
},
"A get accessor must be at least as accessible as the setter": {
"category": "Error",
"code": 2798
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
15 changes: 14 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,8 @@ namespace ts {
}

function parseErrorAtPosition(start: number, length: number, message: DiagnosticMessage, arg0?: any): void {
debugger;

// Don't report another error if it would just be at the same position as the last error.
const lastError = lastOrUndefined(parseDiagnostics);
if (!lastError || start !== lastError.start) {
Expand Down Expand Up @@ -3172,7 +3174,10 @@ namespace ts {

function isTypeMemberStart(): boolean {
// Return true if we have the start of a signature member
if (token() === SyntaxKind.OpenParenToken || token() === SyntaxKind.LessThanToken) {
if (token() === SyntaxKind.OpenParenToken ||
token() === SyntaxKind.LessThanToken ||
token() === SyntaxKind.GetKeyword ||
token() === SyntaxKind.SetKeyword) {
return true;
}
let idToken = false;
Expand Down Expand Up @@ -3213,6 +3218,14 @@ namespace ts {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
const modifiers = parseModifiers();
if (parseContextualModifier(SyntaxKind.GetKeyword)) {
return parseAccessorDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers, SyntaxKind.GetAccessor);
}

if (parseContextualModifier(SyntaxKind.SetKeyword)) {
return parseAccessorDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers, SyntaxKind.SetAccessor);
}

if (isIndexSignature()) {
return parseIndexSignatureDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers);
}
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1453,19 +1453,19 @@ namespace ts {

// See the comment on MethodDeclaration for the intuition behind GetAccessorDeclaration being a
// ClassElement and an ObjectLiteralElement.
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.GetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
/* @internal */ typeParameters?: NodeArray<TypeParameterDeclaration>; // Present for use with reporting a grammar error
}

// See the comment on MethodDeclaration for the intuition behind SetAccessorDeclaration being a
// ClassElement and an ObjectLiteralElement.
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.SetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
/* @internal */ typeParameters?: NodeArray<TypeParameterDeclaration>; // Present for use with reporting a grammar error
Expand Down
9 changes: 3 additions & 6 deletions tests/baselines/reference/abstractPropertyNegative.errors.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
tests/cases/compiler/abstractPropertyNegative.ts(10,18): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/abstractPropertyNegative.ts(11,18): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/abstractPropertyNegative.ts(10,18): error TS2380: The return type of a 'get' accessor must be assignable to its 'set' accessor type
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'm' from class 'B'.
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'mismatch' from class 'B'.
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'prop' from class 'B'.
Expand All @@ -19,7 +18,7 @@ tests/cases/compiler/abstractPropertyNegative.ts(40,9): error TS2676: Accessors
tests/cases/compiler/abstractPropertyNegative.ts(41,18): error TS2676: Accessors must both be abstract or non-abstract.


==== tests/cases/compiler/abstractPropertyNegative.ts (16 errors) ====
==== tests/cases/compiler/abstractPropertyNegative.ts (15 errors) ====
interface A {
prop: string;
m(): string;
Expand All @@ -31,10 +30,8 @@ tests/cases/compiler/abstractPropertyNegative.ts(41,18): error TS2676: Accessors
abstract m(): string;
abstract get mismatch(): string;
~~~~~~~~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: The return type of a 'get' accessor must be assignable to its 'set' accessor type
abstract set mismatch(val: number); // error, not same type
~~~~~~~~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
}
class C extends B {
~
Expand Down

This file was deleted.

8 changes: 4 additions & 4 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -820,15 +820,15 @@ declare namespace ts {
readonly kind: SyntaxKind.SemicolonClassElement;
readonly parent: ClassLikeDeclaration;
}
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.GetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
}
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.SetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -820,15 +820,15 @@ declare namespace ts {
readonly kind: SyntaxKind.SemicolonClassElement;
readonly parent: ClassLikeDeclaration;
}
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.GetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
}
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
readonly kind: SyntaxKind.SetAccessor;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
readonly name: PropertyName;
readonly body?: FunctionBody;
}
Expand Down
Loading

0 comments on commit 6f09705

Please sign in to comment.