Skip to content

Commit b7f93bb

Browse files
RyanCavanaughRyanCavanaugh
committed
Allow accessors to have different access modifiers and types
Fixes microsoft#2845, microsoft#2521
1 parent fb60c9f commit b7f93bb

39 files changed

+3195
-191
lines changed

‎src/compiler/checker.ts

+87-49
Original file line numberDiff line numberDiff line change
@@ -8844,7 +8844,7 @@ namespace ts {
88448844
return links.type;
88458845
}
88468846

8847-
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol) {
8847+
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol): Type {
88488848
// Handle prototype property
88498849
if (symbol.flags & SymbolFlags.Prototype) {
88508850
return getTypeOfPrototypeProperty(symbol);
@@ -8894,7 +8894,7 @@ namespace ts {
88948894
}
88958895
return reportCircularityError(symbol);
88968896
}
8897-
let type: Type | undefined;
8897+
let type: Type;
88988898
if (declaration.kind === SyntaxKind.ExportAssignment) {
88998899
type = widenTypeForVariableLikeDeclaration(checkExpressionCached((<ExportAssignment>declaration).expression), declaration);
89008900
}
@@ -8951,7 +8951,7 @@ namespace ts {
89518951
type = getTypeOfEnumMember(symbol);
89528952
}
89538953
else if (isAccessor(declaration)) {
8954-
type = resolveTypeOfAccessors(symbol);
8954+
type = resolveTypeOfAccessors(symbol) || Debug.fail("Non-write accessor resolution must always produce a type");
89558955
}
89568956
else {
89578957
return Debug.fail("Unhandled declaration kind! " + Debug.formatSyntaxKind(declaration.kind) + " for " + Debug.formatSymbol(symbol));
@@ -8997,15 +8997,20 @@ namespace ts {
89978997

89988998
function getTypeOfAccessors(symbol: Symbol): Type {
89998999
const links = getSymbolLinks(symbol);
9000-
return links.type || (links.type = getTypeOfAccessorsWorker(symbol));
9000+
return links.type || (links.type = getTypeOfAccessorsWorker(symbol) || Debug.fail("Read type of accessor must always produce a type"));
9001+
}
9002+
9003+
function getTypeOfSetAccessor(symbol: Symbol): Type | undefined {
9004+
const links = getSymbolLinks(symbol);
9005+
return links.writeType || (links.writeType = getTypeOfAccessorsWorker(symbol, /*isWrite*/ true));
90019006
}
90029007

9003-
function getTypeOfAccessorsWorker(symbol: Symbol): Type {
9008+
function getTypeOfAccessorsWorker(symbol: Symbol, writing = false): Type | undefined {
90049009
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
90059010
return errorType;
90069011
}
90079012

9008-
let type = resolveTypeOfAccessors(symbol);
9013+
let type = resolveTypeOfAccessors(symbol, writing);
90099014

90109015
if (!popTypeResolution()) {
90119016
type = anyType;
@@ -9017,49 +9022,58 @@ namespace ts {
90179022
return type;
90189023
}
90199024

9020-
function resolveTypeOfAccessors(symbol: Symbol) {
9025+
function resolveTypeOfAccessors(symbol: Symbol, writing = false) {
90219026
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
90229027
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
90239028

9029+
// For write operations, prioritize type annotations on the setter
9030+
if (writing) {
9031+
const setterParameterType = getAnnotatedAccessorType(setter);
9032+
if (setterParameterType) {
9033+
return setterParameterType;
9034+
}
9035+
}
9036+
// Else defer to the getter type
9037+
90249038
if (getter && isInJSFile(getter)) {
90259039
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
90269040
if (jsDocType) {
90279041
return jsDocType;
90289042
}
90299043
}
9030-
// First try to see if the user specified a return type on the get-accessor.
9044+
9045+
// Try to see if the user specified a return type on the get-accessor.
90319046
const getterReturnType = getAnnotatedAccessorType(getter);
90329047
if (getterReturnType) {
90339048
return getterReturnType;
90349049
}
9035-
else {
9036-
// If the user didn't specify a return type, try to use the set-accessor's parameter type.
9037-
const setterParameterType = getAnnotatedAccessorType(setter);
9038-
if (setterParameterType) {
9039-
return setterParameterType;
9050+
9051+
// If the user didn't specify a return type, try to use the set-accessor's parameter type.
9052+
const setterParameterType = getAnnotatedAccessorType(setter);
9053+
if (setterParameterType) {
9054+
return setterParameterType;
9055+
}
9056+
9057+
// If there are no specified types, try to infer it from the body of the get accessor if it exists.
9058+
if (getter && getter.body) {
9059+
return getReturnTypeFromBody(getter);
9060+
}
9061+
9062+
// Otherwise, fall back to 'any'.
9063+
if (setter) {
9064+
if (!isPrivateWithinAmbient(setter)) {
9065+
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
90409066
}
9041-
else {
9042-
// If there are no specified types, try to infer it from the body of the get accessor if it exists.
9043-
if (getter && getter.body) {
9044-
return getReturnTypeFromBody(getter);
9045-
}
9046-
// Otherwise, fall back to 'any'.
9047-
else {
9048-
if (setter) {
9049-
if (!isPrivateWithinAmbient(setter)) {
9050-
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
9051-
}
9052-
}
9053-
else {
9054-
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
9055-
if (!isPrivateWithinAmbient(getter)) {
9056-
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
9057-
}
9058-
}
9059-
return anyType;
9060-
}
9067+
return anyType;
9068+
}
9069+
else if (getter) {
9070+
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
9071+
if (!isPrivateWithinAmbient(getter)) {
9072+
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
90619073
}
9074+
return anyType;
90629075
}
9076+
return undefined;
90639077
}
90649078

90659079
function getBaseTypeVariableOfClass(symbol: Symbol) {
@@ -9186,6 +9200,16 @@ namespace ts {
91869200
return links.type;
91879201
}
91889202

9203+
function getWriteTypeOfSymbol(symbol: Symbol): Type {
9204+
if (symbol.flags & SymbolFlags.Accessor) {
9205+
const type = getTypeOfSetAccessor(symbol);
9206+
if (type) {
9207+
return type;
9208+
}
9209+
}
9210+
return getTypeOfSymbol(symbol);
9211+
}
9212+
91899213
function getTypeOfSymbol(symbol: Symbol): Type {
91909214
const checkFlags = getCheckFlags(symbol);
91919215
if (checkFlags & CheckFlags.DeferredType) {
@@ -26550,8 +26574,8 @@ namespace ts {
2655026574
*/
2655126575
function checkPropertyAccessibility(
2655226576
node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
26553-
isSuper: boolean, type: Type, prop: Symbol): boolean {
26554-
const flags = getDeclarationModifierFlagsFromSymbol(prop);
26577+
isSuper: boolean, type: Type, prop: Symbol, isWrite = false): boolean {
26578+
const flags = getDeclarationModifierFlagsFromSymbol(prop, isWrite);
2655526579
const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right :
2655626580
node.kind === SyntaxKind.ImportType ? node :
2655726581
node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
@@ -26870,7 +26894,7 @@ namespace ts {
2687026894
markAliasReferenced(parentSymbol, node);
2687126895
}
2687226896

26873-
let propType: Type;
26897+
let propType: Type | undefined;
2687426898
if (!prop) {
2687526899
const indexInfo = !isPrivateIdentifier(right) && (assignmentKind === AssignmentKind.None || !isGenericObjectType(leftType) || isThisTypeParameter(leftType)) ? getIndexInfoOfType(apparentType, IndexKind.String) : undefined;
2687626900
if (!(indexInfo && indexInfo.type)) {
@@ -26907,13 +26931,18 @@ namespace ts {
2690726931
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
2690826932
markPropertyAsReferenced(prop, node, isSelfTypeAccess(left, parentSymbol));
2690926933
getNodeLinks(node).resolvedSymbol = prop;
26910-
checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop);
26934+
const isWrite = isWriteAccess(node);
26935+
checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop, isWrite);
2691126936
if (isAssignmentToReadonlyEntity(node as Expression, prop, assignmentKind)) {
2691226937
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right));
2691326938
return errorType;
2691426939
}
26915-
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : getTypeOfSymbol(prop);
26940+
26941+
if (propType === undefined) {
26942+
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : isWrite ? getWriteTypeOfSymbol(prop) : getTypeOfSymbol(prop);
26943+
}
2691626944
}
26945+
2691726946
return getFlowTypeOfAccessExpression(node, prop, propType, right, checkMode);
2691826947
}
2691926948

@@ -32926,18 +32955,19 @@ namespace ts {
3292632955
const otherKind = node.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor;
3292732956
const otherAccessor = getDeclarationOfKind<AccessorDeclaration>(getSymbolOfNode(node), otherKind);
3292832957
if (otherAccessor) {
32929-
const nodeFlags = getEffectiveModifierFlags(node);
32930-
const otherFlags = getEffectiveModifierFlags(otherAccessor);
32931-
if ((nodeFlags & ModifierFlags.AccessibilityModifier) !== (otherFlags & ModifierFlags.AccessibilityModifier)) {
32932-
error(node.name, Diagnostics.Getter_and_setter_accessors_do_not_agree_in_visibility);
32933-
}
32934-
if ((nodeFlags & ModifierFlags.Abstract) !== (otherFlags & ModifierFlags.Abstract)) {
32958+
const getter = node.kind === SyntaxKind.GetAccessor ? node : otherAccessor;
32959+
const setter = node.kind === SyntaxKind.SetAccessor ? node : otherAccessor;
32960+
const getterFlags = getEffectiveModifierFlags(getter);
32961+
const setterFlags = getEffectiveModifierFlags(setter);
32962+
if ((getterFlags & ModifierFlags.Abstract) !== (setterFlags & ModifierFlags.Abstract)) {
3293532963
error(node.name, Diagnostics.Accessors_must_both_be_abstract_or_non_abstract);
3293632964
}
32965+
if (((getterFlags & ModifierFlags.Protected) && !(setterFlags & (ModifierFlags.Protected | ModifierFlags.Private))) ||
32966+
((getterFlags & ModifierFlags.Private) && !(setterFlags & ModifierFlags.Private))) {
32967+
error(node.name, Diagnostics.A_get_accessor_must_be_at_least_as_accessible_as_the_setter);
32968+
}
3293732969

32938-
// TypeScript 1.0 spec (April 2014): 4.5
32939-
// If both accessors include type annotations, the specified types must be identical.
32940-
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_same_type);
32970+
checkAccessorDeclarationTypesAssignable(getter, setter, getAnnotatedAccessorType, Diagnostics.The_return_type_of_a_get_accessor_must_be_assignable_to_its_set_accessor_type);
3294132971
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getThisTypeOfDeclaration, Diagnostics.get_and_set_accessor_must_have_the_same_this_type);
3294232972
}
3294332973
}
@@ -32950,9 +32980,17 @@ namespace ts {
3295032980
}
3295132981

3295232982
function checkAccessorDeclarationTypesIdentical(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, message: DiagnosticMessage) {
32983+
return checkAccessorDeclarationTypesMatch(first, second, getAnnotatedType, isTypeIdenticalTo, message);
32984+
}
32985+
32986+
function checkAccessorDeclarationTypesAssignable(getter: AccessorDeclaration, setter: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, message: DiagnosticMessage) {
32987+
return checkAccessorDeclarationTypesMatch(getter, setter, getAnnotatedType, isTypeAssignableTo, message);
32988+
}
32989+
32990+
function checkAccessorDeclarationTypesMatch(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, match: typeof areTypesComparable, message: DiagnosticMessage) {
3295332991
const firstType = getAnnotatedType(first);
3295432992
const secondType = getAnnotatedType(second);
32955-
if (firstType && secondType && !isTypeIdenticalTo(firstType, secondType)) {
32993+
if (firstType && secondType && !match(firstType, secondType)) {
3295632994
error(first, message);
3295732995
}
3295832996
}
@@ -40865,7 +40903,7 @@ namespace ts {
4086540903
}
4086640904

4086740905
function checkGrammarAccessor(accessor: AccessorDeclaration): boolean {
40868-
if (!(accessor.flags & NodeFlags.Ambient)) {
40906+
if (!(accessor.flags & NodeFlags.Ambient) && (accessor.parent.kind !== SyntaxKind.TypeLiteral) && (accessor.parent.kind !== SyntaxKind.InterfaceDeclaration)) {
4086940907
if (languageVersion < ScriptTarget.ES5) {
4087040908
return grammarErrorOnNode(accessor.name, Diagnostics.Accessors_are_only_available_when_targeting_ECMAScript_5_and_higher);
4087140909
}

‎src/compiler/diagnosticMessages.json

+5-5
Original file line numberDiff line numberDiff line change
@@ -1706,11 +1706,7 @@
17061706
"category": "Error",
17071707
"code": 2378
17081708
},
1709-
"Getter and setter accessors do not agree in visibility.": {
1710-
"category": "Error",
1711-
"code": 2379
1712-
},
1713-
"'get' and 'set' accessor must have the same type.": {
1709+
"The return type of a 'get' accessor must be assignable to its 'set' accessor type": {
17141710
"category": "Error",
17151711
"code": 2380
17161712
},
@@ -3288,6 +3284,10 @@
32883284
"category": "Error",
32893285
"code": 2802
32903286
},
3287+
"A get accessor must be at least as accessible as the setter": {
3288+
"category": "Error",
3289+
"code": 2803
3290+
},
32913291

32923292
"Import declaration '{0}' is using private name '{1}'.": {
32933293
"category": "Error",

‎src/compiler/parser.ts

+14-1
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,8 @@ namespace ts {
13401340
}
13411341

13421342
function parseErrorAtPosition(start: number, length: number, message: DiagnosticMessage, arg0?: any): void {
1343+
debugger;
1344+
13431345
// Don't report another error if it would just be at the same position as the last error.
13441346
const lastError = lastOrUndefined(parseDiagnostics);
13451347
if (!lastError || start !== lastError.start) {
@@ -3213,7 +3215,10 @@ namespace ts {
32133215

32143216
function isTypeMemberStart(): boolean {
32153217
// Return true if we have the start of a signature member
3216-
if (token() === SyntaxKind.OpenParenToken || token() === SyntaxKind.LessThanToken) {
3218+
if (token() === SyntaxKind.OpenParenToken ||
3219+
token() === SyntaxKind.LessThanToken ||
3220+
token() === SyntaxKind.GetKeyword ||
3221+
token() === SyntaxKind.SetKeyword) {
32173222
return true;
32183223
}
32193224
let idToken = false;
@@ -3254,6 +3259,14 @@ namespace ts {
32543259
const pos = getNodePos();
32553260
const hasJSDoc = hasPrecedingJSDocComment();
32563261
const modifiers = parseModifiers();
3262+
if (parseContextualModifier(SyntaxKind.GetKeyword)) {
3263+
return parseAccessorDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers, SyntaxKind.GetAccessor);
3264+
}
3265+
3266+
if (parseContextualModifier(SyntaxKind.SetKeyword)) {
3267+
return parseAccessorDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers, SyntaxKind.SetAccessor);
3268+
}
3269+
32573270
if (isIndexSignature()) {
32583271
return parseIndexSignatureDeclaration(pos, hasJSDoc, /*decorators*/ undefined, modifiers);
32593272
}

‎src/compiler/types.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -1455,19 +1455,19 @@ namespace ts {
14551455

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

14661466
// See the comment on MethodDeclaration for the intuition behind SetAccessorDeclaration being a
14671467
// ClassElement and an ObjectLiteralElement.
1468-
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
1468+
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, TypeElement, ObjectLiteralElement, JSDocContainer {
14691469
readonly kind: SyntaxKind.SetAccessor;
1470-
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression;
1470+
readonly parent: ClassLikeDeclaration | ObjectLiteralExpression | TypeLiteralNode | InterfaceDeclaration;
14711471
readonly name: PropertyName;
14721472
readonly body?: FunctionBody;
14731473
/* @internal */ typeParameters?: NodeArray<TypeParameterDeclaration>; // Present for use with reporting a grammar error
@@ -4749,6 +4749,7 @@ namespace ts {
47494749
immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead.
47504750
target?: Symbol; // Resolved (non-alias) target of an alias
47514751
type?: Type; // Type of value symbol
4752+
writeType?: Type; // Type of value symbol in write contexts
47524753
nameType?: Type; // Type associated with a late-bound symbol
47534754
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
47544755
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter

0 commit comments

Comments
 (0)