Skip to content

Commit

Permalink
Allow accessors to have different access modifiers and types
Browse files Browse the repository at this point in the history 
Fixes #2845, #2521
  • Loading branch information
@RyanCavanaugh
RyanCavanaugh committed Mar 24, 2021
1 parent fb60c9f commit b7f93bb
Show file tree
Hide file tree
Showing 39 changed files with 3,195 additions and 191 deletions.
136 changes: 87 additions & 49 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8844,7 +8844,7 @@ namespace ts {
return links.type;
}

function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol) {
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol): Type {
// Handle prototype property
if (symbol.flags & SymbolFlags.Prototype) {
return getTypeOfPrototypeProperty(symbol);
Expand Down Expand Up @@ -8894,7 +8894,7 @@ namespace ts {
}
return reportCircularityError(symbol);
}
let type: Type | undefined;
let type: Type;
if (declaration.kind === SyntaxKind.ExportAssignment) {
type = widenTypeForVariableLikeDeclaration(checkExpressionCached((<ExportAssignment>declaration).expression), declaration);
}
Expand Down Expand Up @@ -8951,7 +8951,7 @@ namespace ts {
type = getTypeOfEnumMember(symbol);
}
else if (isAccessor(declaration)) {
type = resolveTypeOfAccessors(symbol);
type = resolveTypeOfAccessors(symbol) || Debug.fail("Non-write accessor resolution must always produce a type");
}
else {
return Debug.fail("Unhandled declaration kind! " + Debug.formatSyntaxKind(declaration.kind) + " for " + Debug.formatSymbol(symbol));
Expand Down Expand Up @@ -8997,15 +8997,20 @@ namespace ts {

function getTypeOfAccessors(symbol: Symbol): Type {
const links = getSymbolLinks(symbol);
return links.type || (links.type = getTypeOfAccessorsWorker(symbol));
return links.type || (links.type = getTypeOfAccessorsWorker(symbol) || Debug.fail("Read type of accessor must always produce a type"));
}

function getTypeOfSetAccessor(symbol: Symbol): Type | undefined {
const links = getSymbolLinks(symbol);
return links.writeType || (links.writeType = getTypeOfAccessorsWorker(symbol, /*isWrite*/ true));
}

function getTypeOfAccessorsWorker(symbol: Symbol): Type {
function getTypeOfAccessorsWorker(symbol: Symbol, writing = false): Type | undefined {
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
return errorType;
}

let type = resolveTypeOfAccessors(symbol);
let type = resolveTypeOfAccessors(symbol, writing);

if (!popTypeResolution()) {
type = anyType;
Expand All @@ -9017,49 +9022,58 @@ namespace ts {
return type;
}

function resolveTypeOfAccessors(symbol: Symbol) {
function resolveTypeOfAccessors(symbol: Symbol, writing = false) {
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);

// For write operations, prioritize type annotations on the setter
if (writing) {
const setterParameterType = getAnnotatedAccessorType(setter);
if (setterParameterType) {
return setterParameterType;
}
}
// Else defer to the getter type

if (getter && isInJSFile(getter)) {
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
if (jsDocType) {
return jsDocType;
}
}
// First try to see if the user specified a return type on the get-accessor.

// Try to see if the user specified a return type on the get-accessor.
const getterReturnType = getAnnotatedAccessorType(getter);
if (getterReturnType) {
return 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) {
return setterParameterType;

// If the user didn't specify a return type, try to use the set-accessor's parameter type.
const setterParameterType = getAnnotatedAccessorType(setter);
if (setterParameterType) {
return 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) {
return getReturnTypeFromBody(getter);
}

// Otherwise, fall back to 'any'.
if (setter) {
if (!isPrivateWithinAmbient(setter)) {
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
}
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) {
return getReturnTypeFromBody(getter);
}
// Otherwise, fall back to 'any'.
else {
if (setter) {
if (!isPrivateWithinAmbient(setter)) {
errorOrSuggestion(noImplicitAny, 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 exist a getter as we are current checking either setter or getter in this function");
if (!isPrivateWithinAmbient(getter)) {
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
}
return anyType;
}
return anyType;
}
else if (getter) {
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
if (!isPrivateWithinAmbient(getter)) {
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
return anyType;
}
return undefined;
}

function getBaseTypeVariableOfClass(symbol: Symbol) {
Expand Down Expand Up @@ -9186,6 +9200,16 @@ namespace ts {
return links.type;
}

function getWriteTypeOfSymbol(symbol: Symbol): Type {
if (symbol.flags & SymbolFlags.Accessor) {
const type = getTypeOfSetAccessor(symbol);
if (type) {
return type;
}
}
return getTypeOfSymbol(symbol);
}

function getTypeOfSymbol(symbol: Symbol): Type {
const checkFlags = getCheckFlags(symbol);
if (checkFlags & CheckFlags.DeferredType) {
Expand Down Expand Up @@ -26550,8 +26574,8 @@ 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 {
const flags = getDeclarationModifierFlagsFromSymbol(prop, isWrite);
const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right :
node.kind === SyntaxKind.ImportType ? node :
node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
Expand Down Expand Up @@ -26870,7 +26894,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 @@ -26907,13 +26931,18 @@ namespace ts {
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
markPropertyAsReferenced(prop, node, isSelfTypeAccess(left, parentSymbol));
getNodeLinks(node).resolvedSymbol = prop;
checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop);
const isWrite = isWriteAccess(node);
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 : getTypeOfSymbol(prop);

if (propType === undefined) {
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : isWrite ? getWriteTypeOfSymbol(prop) : getTypeOfSymbol(prop);
}
}

return getFlowTypeOfAccessExpression(node, prop, propType, right, checkMode);
}

Expand Down Expand Up @@ -32926,18 +32955,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.AccessibilityModifier) !== (otherFlags & ModifierFlags.AccessibilityModifier)) {
error(node.name, Diagnostics.Getter_and_setter_accessors_do_not_agree_in_visibility);
}
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.
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_same_type);
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 @@ -32950,9 +32980,17 @@ namespace ts {
}

function checkAccessorDeclarationTypesIdentical(first: AccessorDeclaration, second: AccessorDeclaration, getAnnotatedType: (a: AccessorDeclaration) => Type | undefined, message: DiagnosticMessage) {
return checkAccessorDeclarationTypesMatch(first, second, getAnnotatedType, isTypeIdenticalTo, 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) {
const firstType = getAnnotatedType(first);
const secondType = getAnnotatedType(second);
if (firstType && secondType && !isTypeIdenticalTo(firstType, secondType)) {
if (firstType && secondType && !match(firstType, secondType)) {
error(first, message);
}
}
Expand Down Expand Up @@ -40865,7 +40903,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
10 changes: 5 additions & 5 deletions src/compiler/diagnosticMessages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1706,11 +1706,7 @@
"category": "Error",
"code": 2378
},
"Getter and setter accessors do not agree in visibility.": {
"category": "Error",
"code": 2379
},
"'get' and 'set' accessor must have the same type.": {
"The return type of a 'get' accessor must be assignable to its 'set' accessor type": {
"category": "Error",
"code": 2380
},
Expand Down Expand Up @@ -3288,6 +3284,10 @@
"category": "Error",
"code": 2802
},
"A get accessor must be at least as accessible as the setter": {
"category": "Error",
"code": 2803
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
15 changes: 14 additions & 1 deletion src/compiler/parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,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 @@ -3213,7 +3215,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 @@ -3254,6 +3259,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
9 changes: 5 additions & 4 deletions src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1455,19 +1455,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 Expand Up @@ -4749,6 +4749,7 @@ namespace ts {
immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead.
target?: Symbol; // Resolved (non-alias) target of an alias
type?: Type; // Type of value symbol
writeType?: Type; // Type of value symbol in write contexts
nameType?: Type; // Type associated with a late-bound symbol
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
Expand Down
Loading

0 comments on commit b7f93bb

Please sign in to comment.