Skip to content

Commit df7ed82

Browse files
ahejlsbergahejlsberg
authored
Revise accessor resolution logic and error reporting (microsoft#48459)
* Revise accessor resolution logic and error reporting * Accept new baselines * Update isTypeElement * Add tests
1 parent c720ad6 commit df7ed82

9 files changed

+330
-127
lines changed

Diff for: ‎src/compiler/checker.ts

+72-120
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ namespace ts {
172172
EnumTagType,
173173
ResolvedTypeArguments,
174174
ResolvedBaseTypes,
175+
WriteType,
175176
}
176177

177178
const enum CheckMode {
@@ -8519,6 +8520,8 @@ namespace ts {
85198520
return !!(target as TypeReference).resolvedTypeArguments;
85208521
case TypeSystemPropertyName.ResolvedBaseTypes:
85218522
return !!(target as InterfaceType).baseTypesResolved;
8523+
case TypeSystemPropertyName.WriteType:
8524+
return !!getSymbolLinks(target as Symbol).writeType;
85228525
}
85238526
return Debug.assertNever(propertyName);
85248527
}
@@ -9502,6 +9505,11 @@ namespace ts {
95029505
}
95039506
return getWidenedType(getWidenedLiteralType(checkExpression(declaration.statements[0].expression)));
95049507
}
9508+
if (isAccessor(declaration)) {
9509+
// Binding of certain patterns in JS code will occasionally mark symbols as both properties
9510+
// and accessors. Here we dispatch to accessor resolution if needed.
9511+
return getTypeOfAccessors(symbol);
9512+
}
95059513

95069514
// Handle variable, parameter or property
95079515
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
@@ -9567,9 +9575,6 @@ namespace ts {
95679575
else if (isEnumMember(declaration)) {
95689576
type = getTypeOfEnumMember(symbol);
95699577
}
9570-
else if (isAccessor(declaration)) {
9571-
type = resolveTypeOfAccessors(symbol) || Debug.fail("Non-write accessor resolution must always produce a type");
9572-
}
95739578
else {
95749579
return Debug.fail("Unhandled declaration kind! " + Debug.formatSyntaxKind(declaration.kind) + " for " + Debug.formatSymbol(symbol));
95759580
}
@@ -9614,97 +9619,62 @@ namespace ts {
96149619

96159620
function getTypeOfAccessors(symbol: Symbol): Type {
96169621
const links = getSymbolLinks(symbol);
9617-
return links.type || (links.type = getTypeOfAccessorsWorker(symbol) || Debug.fail("Read type of accessor must always produce a type"));
9618-
}
9619-
9620-
function getTypeOfSetAccessor(symbol: Symbol): Type | undefined {
9621-
const links = getSymbolLinks(symbol);
9622-
return links.writeType || (links.writeType = getTypeOfAccessorsWorker(symbol, /*writing*/ true));
9623-
}
9624-
9625-
function getTypeOfAccessorsWorker(symbol: Symbol, writing = false): Type | undefined {
9626-
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
9627-
return errorType;
9628-
}
9629-
9630-
let type = resolveTypeOfAccessors(symbol, writing);
9631-
if (!popTypeResolution()) {
9622+
if (!links.type) {
9623+
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
9624+
return errorType;
9625+
}
96329626
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
9633-
if (getter) {
9634-
if (getEffectiveTypeAnnotationNode(getter)) {
9635-
error(getter.name, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
9627+
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
9628+
// We try to resolve a getter type annotation, a setter type annotation, or a getter function
9629+
// body return type inference, in that order.
9630+
let type = getter && isInJSFile(getter) && getTypeForDeclarationFromJSDocComment(getter) ||
9631+
getAnnotatedAccessorType(getter) ||
9632+
getAnnotatedAccessorType(setter) ||
9633+
getter && getter.body && getReturnTypeFromBody(getter);
9634+
if (!type) {
9635+
if (setter && !isPrivateWithinAmbient(setter)) {
9636+
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
9637+
}
9638+
else if (getter && !isPrivateWithinAmbient(getter)) {
9639+
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
9640+
}
9641+
type = anyType;
9642+
}
9643+
if (!popTypeResolution()) {
9644+
if (getAnnotatedAccessorTypeNode(getter)) {
9645+
error(getter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
96369646
}
9637-
else if (noImplicitAny) {
9647+
else if (getAnnotatedAccessorTypeNode(setter)) {
9648+
error(setter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
9649+
}
9650+
else if (getter && noImplicitAny) {
96389651
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));
96399652
}
9653+
type = anyType;
96409654
}
9641-
type = anyType;
9655+
links.type = type;
96429656
}
9643-
return type;
9657+
return links.type;
96449658
}
96459659

9646-
function resolveTypeOfAccessors(symbol: Symbol, writing = false) {
9647-
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
9648-
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
9649-
9650-
// For write operations, prioritize type annotations on the setter
9651-
if (writing) {
9652-
const setterType = getAnnotatedAccessorType(setter);
9653-
if (setterType) {
9654-
return instantiateTypeIfNeeded(setterType, symbol);
9655-
}
9656-
}
9657-
// Else defer to the getter type
9658-
9659-
if (getter && isInJSFile(getter)) {
9660-
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
9661-
if (jsDocType) {
9662-
return instantiateTypeIfNeeded(jsDocType, symbol);
9663-
}
9664-
}
9665-
9666-
// Try to see if the user specified a return type on the get-accessor.
9667-
const getterType = getAnnotatedAccessorType(getter);
9668-
if (getterType) {
9669-
return instantiateTypeIfNeeded(getterType, symbol);
9670-
}
9671-
9672-
// If the user didn't specify a return type, try to use the set-accessor's parameter type.
9673-
const setterType = getAnnotatedAccessorType(setter);
9674-
if (setterType) {
9675-
return setterType;
9676-
}
9677-
9678-
// If there are no specified types, try to infer it from the body of the get accessor if it exists.
9679-
if (getter && getter.body) {
9680-
const returnTypeFromBody = getReturnTypeFromBody(getter);
9681-
return instantiateTypeIfNeeded(returnTypeFromBody, symbol);
9682-
}
9683-
9684-
// Otherwise, fall back to 'any'.
9685-
if (setter) {
9686-
if (!isPrivateWithinAmbient(setter)) {
9687-
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
9688-
}
9689-
return anyType;
9690-
}
9691-
else if (getter) {
9692-
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
9693-
if (!isPrivateWithinAmbient(getter)) {
9694-
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
9660+
function getWriteTypeOfAccessors(symbol: Symbol): Type {
9661+
const links = getSymbolLinks(symbol);
9662+
if (!links.writeType) {
9663+
if (!pushTypeResolution(symbol, TypeSystemPropertyName.WriteType)) {
9664+
return errorType;
96959665
}
9696-
return anyType;
9697-
}
9698-
return undefined;
9699-
9700-
function instantiateTypeIfNeeded(type: Type, symbol: Symbol) {
9701-
if (getCheckFlags(symbol) & CheckFlags.Instantiated) {
9702-
const links = getSymbolLinks(symbol);
9703-
return instantiateType(type, links.mapper);
9666+
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
9667+
let writeType = getAnnotatedAccessorType(setter);
9668+
if (!popTypeResolution()) {
9669+
if (getAnnotatedAccessorTypeNode(setter)) {
9670+
error(setter, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation, symbolToString(symbol));
9671+
}
9672+
writeType = anyType;
97049673
}
9705-
9706-
return type;
9674+
// Absent an explicit setter type annotation we use the read type of the accessor.
9675+
links.writeType = writeType || getTypeOfAccessors(symbol);
97079676
}
9677+
return links.writeType;
97089678
}
97099679

97109680
function getBaseTypeVariableOfClass(symbol: Symbol) {
@@ -9792,17 +9762,12 @@ namespace ts {
97929762

97939763
function getTypeOfInstantiatedSymbol(symbol: Symbol): Type {
97949764
const links = getSymbolLinks(symbol);
9795-
if (!links.type) {
9796-
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
9797-
return links.type = errorType;
9798-
}
9799-
let type = instantiateType(getTypeOfSymbol(links.target!), links.mapper);
9800-
if (!popTypeResolution()) {
9801-
type = reportCircularityError(symbol);
9802-
}
9803-
links.type = type;
9804-
}
9805-
return links.type;
9765+
return links.type || (links.type = instantiateType(getTypeOfSymbol(links.target!), links.mapper));
9766+
}
9767+
9768+
function getWriteTypeOfInstantiatedSymbol(symbol: Symbol): Type {
9769+
const links = getSymbolLinks(symbol);
9770+
return links.writeType || (links.writeType = instantiateType(getWriteTypeOfSymbol(links.target!), links.mapper));
98069771
}
98079772

98089773
function reportCircularityError(symbol: Symbol) {
@@ -9845,36 +9810,23 @@ namespace ts {
98459810
}
98469811

98479812
/**
9848-
* Distinct write types come only from set accessors, but union and intersection
9849-
* properties deriving from set accessors will either pre-compute or defer the
9850-
* union or intersection of the writeTypes of their constituents. To account for
9851-
* this, we will assume that any deferred type or transient symbol may have a
9852-
* `writeType` (or a deferred write type ready to be computed) that should be
9853-
* used before looking for set accessor declarations.
9813+
* Distinct write types come only from set accessors, but synthetic union and intersection
9814+
* properties deriving from set accessors will either pre-compute or defer the union or
9815+
* intersection of the writeTypes of their constituents.
98549816
*/
98559817
function getWriteTypeOfSymbol(symbol: Symbol): Type {
98569818
const checkFlags = getCheckFlags(symbol);
9857-
if (checkFlags & CheckFlags.DeferredType) {
9858-
const writeType = getWriteTypeOfSymbolWithDeferredType(symbol);
9859-
if (writeType) {
9860-
return writeType;
9861-
}
9862-
}
9863-
if (symbol.flags & SymbolFlags.Transient) {
9864-
const { writeType } = symbol as TransientSymbol;
9865-
if (writeType) {
9866-
return writeType;
9867-
}
9819+
if (symbol.flags & SymbolFlags.Property) {
9820+
return checkFlags & CheckFlags.SyntheticProperty ?
9821+
checkFlags & CheckFlags.DeferredType ?
9822+
getWriteTypeOfSymbolWithDeferredType(symbol) || getTypeOfSymbolWithDeferredType(symbol) :
9823+
(symbol as TransientSymbol).writeType || (symbol as TransientSymbol).type! :
9824+
getTypeOfSymbol(symbol);
98689825
}
9869-
return getSetAccessorTypeOfSymbol(symbol);
9870-
}
9871-
9872-
function getSetAccessorTypeOfSymbol(symbol: Symbol): Type {
98739826
if (symbol.flags & SymbolFlags.Accessor) {
9874-
const type = getTypeOfSetAccessor(symbol);
9875-
if (type) {
9876-
return type;
9877-
}
9827+
return checkFlags & CheckFlags.Instantiated ?
9828+
getWriteTypeOfInstantiatedSymbol(symbol) :
9829+
getWriteTypeOfAccessors(symbol);
98789830
}
98799831
return getTypeOfSymbol(symbol);
98809832
}
@@ -25237,7 +25189,7 @@ namespace ts {
2523725189
}
2523825190
}
2523925191
if (isDeclarationName(location) && isSetAccessor(location.parent) && getAnnotatedAccessorTypeNode(location.parent)) {
25240-
return resolveTypeOfAccessors(location.parent.symbol, /*writing*/ true)!;
25192+
return getWriteTypeOfAccessors(location.parent.symbol);
2524125193
}
2524225194
// The location isn't a reference to the given symbol, meaning we're being asked
2524325195
// a hypothetical question of what type the symbol would have if there was a reference

Diff for: ‎src/compiler/utilitiesPublic.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -1335,7 +1335,9 @@ namespace ts {
13351335
|| kind === SyntaxKind.CallSignature
13361336
|| kind === SyntaxKind.PropertySignature
13371337
|| kind === SyntaxKind.MethodSignature
1338-
|| kind === SyntaxKind.IndexSignature;
1338+
|| kind === SyntaxKind.IndexSignature
1339+
|| kind === SyntaxKind.GetAccessor
1340+
|| kind === SyntaxKind.SetAccessor;
13391341
}
13401342

13411343
export function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement {

Diff for: ‎tests/baselines/reference/circularAccessorAnnotations.errors.txt

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
tests/cases/compiler/circularAccessorAnnotations.ts(2,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
2+
tests/cases/compiler/circularAccessorAnnotations.ts(6,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
3+
tests/cases/compiler/circularAccessorAnnotations.ts(15,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
4+
tests/cases/compiler/circularAccessorAnnotations.ts(19,9): error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
5+
6+
7+
==== tests/cases/compiler/circularAccessorAnnotations.ts (4 errors) ====
8+
declare const c1: {
9+
get foo(): typeof c1.foo;
10+
~~~
11+
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
12+
}
13+
14+
declare const c2: {
15+
set foo(value: typeof c2.foo);
16+
~~~
17+
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
18+
}
19+
20+
declare const c3: {
21+
get foo(): string;
22+
set foo(value: typeof c3.foo);
23+
}
24+
25+
type T1 = {
26+
get foo(): T1["foo"];
27+
~~~
28+
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
29+
}
30+
31+
type T2 = {
32+
set foo(value: T2["foo"]);
33+
~~~
34+
!!! error TS2502: 'foo' is referenced directly or indirectly in its own type annotation.
35+
}
36+
37+
type T3 = {
38+
get foo(): string;
39+
set foo(value: T3["foo"]);
40+
}
41+

Diff for: ‎tests/baselines/reference/circularAccessorAnnotations.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//// [circularAccessorAnnotations.ts]
2+
declare const c1: {
3+
get foo(): typeof c1.foo;
4+
}
5+
6+
declare const c2: {
7+
set foo(value: typeof c2.foo);
8+
}
9+
10+
declare const c3: {
11+
get foo(): string;
12+
set foo(value: typeof c3.foo);
13+
}
14+
15+
type T1 = {
16+
get foo(): T1["foo"];
17+
}
18+
19+
type T2 = {
20+
set foo(value: T2["foo"]);
21+
}
22+
23+
type T3 = {
24+
get foo(): string;
25+
set foo(value: T3["foo"]);
26+
}
27+
28+
29+
//// [circularAccessorAnnotations.js]
30+
"use strict";
31+
32+
33+
//// [circularAccessorAnnotations.d.ts]
34+
declare const c1: {
35+
get foo(): typeof c1.foo;
36+
};
37+
declare const c2: {
38+
set foo(value: typeof c2.foo);
39+
};
40+
declare const c3: {
41+
get foo(): string;
42+
set foo(value: typeof c3.foo);
43+
};
44+
declare type T1 = {
45+
get foo(): T1["foo"];
46+
};
47+
declare type T2 = {
48+
set foo(value: T2["foo"]);
49+
};
50+
declare type T3 = {
51+
get foo(): string;
52+
set foo(value: T3["foo"]);
53+
};

0 commit comments

Comments
 (0)