Skip to content

Commit

Permalink
Revise accessor resolution logic and error reporting (#48459)
Browse files Browse the repository at this point in the history 
* Revise accessor resolution logic and error reporting

* Accept new baselines

* Update isTypeElement

* Add tests
  • Loading branch information
@ahejlsberg
ahejlsberg committed Mar 28, 2022
1 parent c720ad6 commit df7ed82
Show file tree
Hide file tree
Showing 9 changed files with 330 additions and 127 deletions.
192 changes: 72 additions & 120 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ namespace ts {
EnumTagType,
ResolvedTypeArguments,
ResolvedBaseTypes,
WriteType,
}

const enum CheckMode {
Expand Down Expand Up @@ -8519,6 +8520,8 @@ namespace ts {
return !!(target as TypeReference).resolvedTypeArguments;
case TypeSystemPropertyName.ResolvedBaseTypes:
return !!(target as InterfaceType).baseTypesResolved;
case TypeSystemPropertyName.WriteType:
return !!getSymbolLinks(target as Symbol).writeType;
}
return Debug.assertNever(propertyName);
}
Expand Down Expand Up @@ -9502,6 +9505,11 @@ namespace ts {
}
return getWidenedType(getWidenedLiteralType(checkExpression(declaration.statements[0].expression)));
}
if (isAccessor(declaration)) {
// Binding of certain patterns in JS code will occasionally mark symbols as both properties
// and accessors. Here we dispatch to accessor resolution if needed.
return getTypeOfAccessors(symbol);
}

// Handle variable, parameter or property
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
Expand Down Expand Up @@ -9567,9 +9575,6 @@ namespace ts {
else if (isEnumMember(declaration)) {
type = getTypeOfEnumMember(symbol);
}
else if (isAccessor(declaration)) {
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 @@ -9614,97 +9619,62 @@ namespace ts {

function getTypeOfAccessors(symbol: Symbol): Type {
const links = getSymbolLinks(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, /*writing*/ true));
}

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

let type = resolveTypeOfAccessors(symbol, writing);
if (!popTypeResolution()) {
if (!links.type) {
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
return errorType;
}
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
if (getter) {
if (getEffectiveTypeAnnotationNode(getter)) {
error(getter.name, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
// We try to resolve a getter type annotation, a setter type annotation, or a getter function
// body return type inference, in that order.
let type = getter && isInJSFile(getter) && getTypeForDeclarationFromJSDocComment(getter) ||
getAnnotatedAccessorType(getter) ||
getAnnotatedAccessorType(setter) ||
getter && getter.body && getReturnTypeFromBody(getter);
if (!type) {
if (setter && !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 (getter && !isPrivateWithinAmbient(getter)) {
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
type = anyType;
}
if (!popTypeResolution()) {
if (getAnnotatedAccessorTypeNode(getter)) {
error(getter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
}
else if (noImplicitAny) {
else if (getAnnotatedAccessorTypeNode(setter)) {
error(setter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
}
else if (getter && noImplicitAny) {
error(getter, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, symbolToString(symbol));
}
type = anyType;
}
type = anyType;
links.type = type;
}
return type;
return links.type;
}

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 setterType = getAnnotatedAccessorType(setter);
if (setterType) {
return instantiateTypeIfNeeded(setterType, symbol);
}
}
// Else defer to the getter type

if (getter && isInJSFile(getter)) {
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
if (jsDocType) {
return instantiateTypeIfNeeded(jsDocType, symbol);
}
}

// Try to see if the user specified a return type on the get-accessor.
const getterType = getAnnotatedAccessorType(getter);
if (getterType) {
return instantiateTypeIfNeeded(getterType, symbol);
}

// If the user didn't specify a return type, try to use the set-accessor's parameter type.
const setterType = getAnnotatedAccessorType(setter);
if (setterType) {
return setterType;
}

// If there are no specified types, try to infer it from the body of the get accessor if it exists.
if (getter && getter.body) {
const returnTypeFromBody = getReturnTypeFromBody(getter);
return instantiateTypeIfNeeded(returnTypeFromBody, symbol);
}

// 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));
}
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));
function getWriteTypeOfAccessors(symbol: Symbol): Type {
const links = getSymbolLinks(symbol);
if (!links.writeType) {
if (!pushTypeResolution(symbol, TypeSystemPropertyName.WriteType)) {
return errorType;
}
return anyType;
}
return undefined;

function instantiateTypeIfNeeded(type: Type, symbol: Symbol) {
if (getCheckFlags(symbol) & CheckFlags.Instantiated) {
const links = getSymbolLinks(symbol);
return instantiateType(type, links.mapper);
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
let writeType = getAnnotatedAccessorType(setter);
if (!popTypeResolution()) {
if (getAnnotatedAccessorTypeNode(setter)) {
error(setter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
}
writeType = anyType;
}

return type;
// Absent an explicit setter type annotation we use the read type of the accessor.
links.writeType = writeType || getTypeOfAccessors(symbol);
}
return links.writeType;
}

function getBaseTypeVariableOfClass(symbol: Symbol) {
Expand Down Expand Up @@ -9792,17 +9762,12 @@ namespace ts {

function getTypeOfInstantiatedSymbol(symbol: Symbol): Type {
const links = getSymbolLinks(symbol);
if (!links.type) {
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
return links.type = errorType;
}
let type = instantiateType(getTypeOfSymbol(links.target!), links.mapper);
if (!popTypeResolution()) {
type = reportCircularityError(symbol);
}
links.type = type;
}
return links.type;
return links.type || (links.type = instantiateType(getTypeOfSymbol(links.target!), links.mapper));
}

function getWriteTypeOfInstantiatedSymbol(symbol: Symbol): Type {
const links = getSymbolLinks(symbol);
return links.writeType || (links.writeType = instantiateType(getWriteTypeOfSymbol(links.target!), links.mapper));
}

function reportCircularityError(symbol: Symbol) {
Expand Down Expand Up @@ -9845,36 +9810,23 @@ namespace ts {
}

/**
* Distinct write types come only from set accessors, but union and intersection
* properties deriving from set accessors will either pre-compute or defer the
* union or intersection of the writeTypes of their constituents. To account for
* this, we will assume that any deferred type or transient symbol may have a
* `writeType` (or a deferred write type ready to be computed) that should be
* used before looking for set accessor declarations.
* Distinct write types come only from set accessors, but synthetic union and intersection
* properties deriving from set accessors will either pre-compute or defer the union or
* intersection of the writeTypes of their constituents.
*/
function getWriteTypeOfSymbol(symbol: Symbol): Type {
const checkFlags = getCheckFlags(symbol);
if (checkFlags & CheckFlags.DeferredType) {
const writeType = getWriteTypeOfSymbolWithDeferredType(symbol);
if (writeType) {
return writeType;
}
}
if (symbol.flags & SymbolFlags.Transient) {
const { writeType } = symbol as TransientSymbol;
if (writeType) {
return writeType;
}
if (symbol.flags & SymbolFlags.Property) {
return checkFlags & CheckFlags.SyntheticProperty ?
checkFlags & CheckFlags.DeferredType ?
getWriteTypeOfSymbolWithDeferredType(symbol) || getTypeOfSymbolWithDeferredType(symbol) :
(symbol as TransientSymbol).writeType || (symbol as TransientSymbol).type! :
getTypeOfSymbol(symbol);
}
return getSetAccessorTypeOfSymbol(symbol);
}

function getSetAccessorTypeOfSymbol(symbol: Symbol): Type {
if (symbol.flags & SymbolFlags.Accessor) {
const type = getTypeOfSetAccessor(symbol);
if (type) {
return type;
}
return checkFlags & CheckFlags.Instantiated ?
getWriteTypeOfInstantiatedSymbol(symbol) :
getWriteTypeOfAccessors(symbol);
}
return getTypeOfSymbol(symbol);
}
Expand Down Expand Up @@ -25237,7 +25189,7 @@ namespace ts {
}
}
if (isDeclarationName(location) && isSetAccessor(location.parent) && getAnnotatedAccessorTypeNode(location.parent)) {
return resolveTypeOfAccessors(location.parent.symbol, /*writing*/ true)!;
return getWriteTypeOfAccessors(location.parent.symbol);
}
// The location isn't a reference to the given symbol, meaning we're being asked
// a hypothetical question of what type the symbol would have if there was a reference
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/utilitiesPublic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,9 @@ namespace ts {
|| kind === SyntaxKind.CallSignature
|| kind === SyntaxKind.PropertySignature
|| kind === SyntaxKind.MethodSignature
|| kind === SyntaxKind.IndexSignature;
|| kind === SyntaxKind.IndexSignature
|| kind === SyntaxKind.GetAccessor
|| kind === SyntaxKind.SetAccessor;
}

export function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement {
Expand Down
41 changes: 41 additions & 0 deletions tests/baselines/reference/circularAccessorAnnotations.errors.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
tests/cases/compiler/circularAccessorAnnotations.ts(2,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
tests/cases/compiler/circularAccessorAnnotations.ts(6,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
tests/cases/compiler/circularAccessorAnnotations.ts(15,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
tests/cases/compiler/circularAccessorAnnotations.ts(19,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.


==== tests/cases/compiler/circularAccessorAnnotations.ts (4 errors) ====
declare const c1: {
get foo(): typeof c1.foo;
~~~
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
}

declare const c2: {
set foo(value: typeof c2.foo);
~~~
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
}

declare const c3: {
get foo(): string;
set foo(value: typeof c3.foo);
}

type T1 = {
get foo(): T1["foo"];
~~~
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
}

type T2 = {
set foo(value: T2["foo"]);
~~~
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
}

type T3 = {
get foo(): string;
set foo(value: T3["foo"]);
}

53 changes: 53 additions & 0 deletions tests/baselines/reference/circularAccessorAnnotations.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//// [circularAccessorAnnotations.ts]
declare const c1: {
get foo(): typeof c1.foo;
}

declare const c2: {
set foo(value: typeof c2.foo);
}

declare const c3: {
get foo(): string;
set foo(value: typeof c3.foo);
}

type T1 = {
get foo(): T1["foo"];
}

type T2 = {
set foo(value: T2["foo"]);
}

type T3 = {
get foo(): string;
set foo(value: T3["foo"]);
}


//// [circularAccessorAnnotations.js]
"use strict";


//// [circularAccessorAnnotations.d.ts]
declare const c1: {
get foo(): typeof c1.foo;
};
declare const c2: {
set foo(value: typeof c2.foo);
};
declare const c3: {
get foo(): string;
set foo(value: typeof c3.foo);
};
declare type T1 = {
get foo(): T1["foo"];
};
declare type T2 = {
set foo(value: T2["foo"]);
};
declare type T3 = {
get foo(): string;
set foo(value: T3["foo"]);
};

0 comments on commit df7ed82

Please sign in to comment.