Revise accessor resolution logic and error reporting (microsoft#48459)

* Revise accessor resolution logic and error reporting

* Accept new baselines

* Update isTypeElement

* Add tests

Author: ahejlsberg

src/compiler/checker.ts

@@ -172,6 +172,7 @@ namespace ts {
         EnumTagType,
         ResolvedTypeArguments,
         ResolvedBaseTypes,
+        WriteType,
     }
 
     const enum CheckMode {
@@ -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);
         }
@@ -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)) {
@@ -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));
             }
@@ -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) {
@@ -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) {
@@ -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);
         }
@@ -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

src/compiler/utilitiesPublic.ts

@@ -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 {

tests/baselines/reference/circularAccessorAnnotations.errors.txt

@@ -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"]);
+    }
+    

tests/baselines/reference/circularAccessorAnnotations.js

@@ -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"]);
+};