Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revise accessor resolution logic and error reporting #48459

Merged
merged 4 commits into from
Mar 28, 2022
Merged

Revise accessor resolution logic and error reporting #48459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1369e07
Revise accessor resolution logic and error reporting
ahejlsberg Mar 28, 2022
c5e9b37
Accept new baselines
ahejlsberg Mar 28, 2022
5e53099
Update isTypeElement
ahejlsberg Mar 28, 2022
f941933
Add tests
ahejlsberg Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
192 changes: 72 additions & 120 deletions src/compiler/checker.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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"]);
};