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.

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

Keep accessors as accessors in emitted anonymous class declarations #60853

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 61 additions & 27 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ import {
IntroducesNewScopeNode,
isAccessExpression,
isAccessor,
isAccessorModifier,
isAliasableExpression,
isAmbientModule,
isArray,
Expand Down Expand Up @@ -7412,26 +7413,50 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

if (propertySymbol.flags & SymbolFlags.Accessor) {
const writeType = getWriteTypeOfSymbol(propertySymbol);
if (propertyType !== writeType && !isErrorType(propertyType) && !isErrorType(writeType)) {
const getterDeclaration = getDeclarationOfKind<GetAccessorDeclaration>(propertySymbol, SyntaxKind.GetAccessor)!;
const getterSignature = getSignatureFromDeclaration(getterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(getterSignature, SyntaxKind.GetAccessor, context, { name: propertyName }) as GetAccessorDeclaration,
getterDeclaration,
),
);
const setterDeclaration = getDeclarationOfKind<SetAccessorDeclaration>(propertySymbol, SyntaxKind.SetAccessor)!;
const setterSignature = getSignatureFromDeclaration(setterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(setterSignature, SyntaxKind.SetAccessor, context, { name: propertyName }) as SetAccessorDeclaration,
setterDeclaration,
),
);
return;
if (!isErrorType(propertyType) && !isErrorType(writeType)) {
const propDeclaration = getDeclarationOfKind<PropertyDeclaration>(propertySymbol, SyntaxKind.PropertyDeclaration);
if (propertyType !== writeType || propertySymbol.parent!.flags & SymbolFlags.Class && !propDeclaration) {
const getterDeclaration = getDeclarationOfKind<GetAccessorDeclaration>(propertySymbol, SyntaxKind.GetAccessor);
if (getterDeclaration) {
const getterSignature = getSignatureFromDeclaration(getterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(getterSignature, SyntaxKind.GetAccessor, context, { name: propertyName }) as GetAccessorDeclaration,
getterDeclaration,
),
);
}
const setterDeclaration = getDeclarationOfKind<SetAccessorDeclaration>(propertySymbol, SyntaxKind.SetAccessor);
if (setterDeclaration) {
const setterSignature = getSignatureFromDeclaration(setterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(setterSignature, SyntaxKind.SetAccessor, context, { name: propertyName }) as SetAccessorDeclaration,
setterDeclaration,
),
);
}
return;
}
if (propertySymbol.parent!.flags & SymbolFlags.Class && propDeclaration && find(propDeclaration.modifiers, isAccessorModifier)) {
const fakeGetterSignature = createSignature(/*declaration*/ undefined, /*typeParameters*/ undefined, /*thisParameter*/ undefined, emptyArray, propertyType, /*resolvedTypePredicate*/ undefined, 0, SignatureFlags.None);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(fakeGetterSignature, SyntaxKind.GetAccessor, context, { name: propertyName }) as GetAccessorDeclaration,
propDeclaration,
),
);
const setterParam = createSymbol(SymbolFlags.FunctionScopedVariable, "arg" as __String);
setterParam.links.type = writeType;
const fakeSetterSignature = createSignature(/*declaration*/ undefined, /*typeParameters*/ undefined, /*thisParameter*/ undefined, [setterParam], voidType, /*resolvedTypePredicate*/ undefined, 0, SignatureFlags.None);
typeElements.push(
signatureToSignatureDeclarationHelper(fakeSetterSignature, SyntaxKind.SetAccessor, context, { name: propertyName }) as SetAccessorDeclaration,
);
return;
}
}
}

Expand Down Expand Up @@ -12288,13 +12313,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
*/
function getWriteTypeOfSymbol(symbol: Symbol): Type {
const checkFlags = getCheckFlags(symbol);
if (checkFlags & CheckFlags.SyntheticProperty) {
return checkFlags & CheckFlags.DeferredType ?
getWriteTypeOfSymbolWithDeferredType(symbol) || getTypeOfSymbolWithDeferredType(symbol) :
// NOTE: cast to TransientSymbol should be safe because only TransientSymbols can have CheckFlags.SyntheticProperty
(symbol as TransientSymbol).links.writeType || (symbol as TransientSymbol).links.type!;
}
if (symbol.flags & SymbolFlags.Property) {
return checkFlags & CheckFlags.SyntheticProperty ?
checkFlags & CheckFlags.DeferredType ?
getWriteTypeOfSymbolWithDeferredType(symbol) || getTypeOfSymbolWithDeferredType(symbol) :
// NOTE: cast to TransientSymbol should be safe because only TransientSymbols can have CheckFlags.SyntheticProperty
(symbol as TransientSymbol).links.writeType || (symbol as TransientSymbol).links.type! :
removeMissingType(getTypeOfSymbol(symbol), !!(symbol.flags & SymbolFlags.Optional));
return removeMissingType(getTypeOfSymbol(symbol), !!(symbol.flags & SymbolFlags.Optional));
}
if (symbol.flags & SymbolFlags.Accessor) {
return checkFlags & CheckFlags.Instantiated ?
Expand Down Expand Up @@ -14913,6 +14939,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
let propFlags = SymbolFlags.None;
let singleProp: Symbol | undefined;
let propSet: Map<SymbolId, Symbol> | undefined;
let indexTypes: Type[] | undefined;
Expand All @@ -14939,6 +14966,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
if (!singleProp) {
singleProp = prop;
propFlags = (prop.flags & SymbolFlags.Accessor) || SymbolFlags.Property;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the accessor case should be handled through checkFlags. I'm worried that it would require looking carefully at all call sites that handle accessors to cover for this new case - but maybe I'm worried about it prematurely. Anyway, let me know what you think about it ;)

}
else if (prop !== singleProp) {
const isInstantiation = (getTargetSymbol(prop) || prop) === (getTargetSymbol(singleProp) || singleProp);
Expand All @@ -14961,6 +14989,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
propSet.set(id, prop);
}
}
// classes created by mixins are represented as intersections and overriding a property in a derived class redefines it completely at runtime
// so a get accessor can't be merged with a set accessor in a base class, for that reason the accessor flags are only used when they are the same in all consistuents
Comment on lines +14992 to +14993
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps if I'd use a new CheckFlags.SyntheticAccessor that wouldn't really matter as much? I'm not sure if it's even that important of a rule to begin with, I just erred on the safer side of things and maintained the current behavior for this ambiguous case

if (propFlags & SymbolFlags.Accessor && (prop.flags & SymbolFlags.Accessor) !== (propFlags & SymbolFlags.Accessor)) {
propFlags = (propFlags & ~SymbolFlags.Accessor) | SymbolFlags.Property;
}
}
if (isUnion && isReadonlySymbol(prop)) {
checkFlags |= CheckFlags.Readonly;
Expand All @@ -14979,6 +15012,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
else if (isUnion) {
const indexInfo = !isLateBoundName(name) && getApplicableIndexInfoForName(type, name);
if (indexInfo) {
propFlags = (propFlags & ~SymbolFlags.Accessor) | SymbolFlags.Property;
checkFlags |= CheckFlags.WritePartial | (indexInfo.isReadonly ? CheckFlags.Readonly : 0);
indexTypes = append(indexTypes, isTupleType(type) ? getRestTypeOfTupleType(type) || undefinedType : indexInfo.type);
}
Expand Down Expand Up @@ -15057,7 +15091,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
propTypes.push(type);
}
addRange(propTypes, indexTypes);
const result = createSymbol(SymbolFlags.Property | (optionalFlag ?? 0), name, syntheticFlag | checkFlags);
const result = createSymbol(propFlags | (optionalFlag ?? 0), name, syntheticFlag | checkFlags);
result.links.containingType = containingType;
if (!hasNonUniformValueDeclaration && firstValueDeclaration) {
result.valueDeclaration = firstValueDeclaration;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//// [tests/cases/conformance/declarationEmit/anonymousClassAccessorsDeclarationEmit1.ts] ////

//// [anonymousClassAccessorsDeclarationEmit1.ts]
export abstract class Base {
accessor a = 1;
}

export function middle(Super = Base) {
abstract class Middle extends Super {}
return Middle;
}

class A {
constructor(...args: any[]) {}
}

export function Mixin<T extends typeof A>(Super: T) {
return class B extends Super {
get myName(): string {
return "B";
}
set myName(arg: string) {}
};
}


//// [anonymousClassAccessorsDeclarationEmit1.js]
export class Base {
accessor a = 1;
}
export function middle(Super = Base) {
class Middle extends Super {
}
return Middle;
}
class A {
constructor(...args) { }
}
export function Mixin(Super) {
return class B extends Super {
get myName() {
return "B";
}
set myName(arg) { }
};
}


//// [anonymousClassAccessorsDeclarationEmit1.d.ts]
export declare abstract class Base {
accessor a: number;
}
export declare function middle(Super?: typeof Base): abstract new () => {
get a(): number;
set a(arg: number);
};
declare class A {
constructor(...args: any[]);
}
export declare function Mixin<T extends typeof A>(Super: T): {
new (...args: any[]): {
get myName(): string;
set myName(arg: string);
};
} & T;
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//// [tests/cases/conformance/declarationEmit/anonymousClassAccessorsDeclarationEmit1.ts] ////

=== anonymousClassAccessorsDeclarationEmit1.ts ===
export abstract class Base {
>Base : Symbol(Base, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 0, 0))

accessor a = 1;
>a : Symbol(Base.a, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 0, 28))
}

export function middle(Super = Base) {
>middle : Symbol(middle, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 2, 1))
>Super : Symbol(Super, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 4, 23))
>Base : Symbol(Base, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 0, 0))

abstract class Middle extends Super {}
>Middle : Symbol(Middle, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 4, 38))
>Super : Symbol(Super, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 4, 23))

return Middle;
>Middle : Symbol(Middle, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 4, 38))
}

class A {
>A : Symbol(A, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 7, 1))

constructor(...args: any[]) {}
>args : Symbol(args, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 10, 14))
}

export function Mixin<T extends typeof A>(Super: T) {
>Mixin : Symbol(Mixin, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 11, 1))
>T : Symbol(T, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 13, 22))
>A : Symbol(A, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 7, 1))
>Super : Symbol(Super, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 13, 42))
>T : Symbol(T, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 13, 22))

return class B extends Super {
>B : Symbol(B, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 14, 8))
>Super : Symbol(Super, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 13, 42))

get myName(): string {
>myName : Symbol(B.myName, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 14, 32), Decl(anonymousClassAccessorsDeclarationEmit1.ts, 17, 5))

return "B";
}
set myName(arg: string) {}
>myName : Symbol(B.myName, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 14, 32), Decl(anonymousClassAccessorsDeclarationEmit1.ts, 17, 5))
>arg : Symbol(arg, Decl(anonymousClassAccessorsDeclarationEmit1.ts, 18, 15))

};
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
//// [tests/cases/conformance/declarationEmit/anonymousClassAccessorsDeclarationEmit1.ts] ////

=== anonymousClassAccessorsDeclarationEmit1.ts ===
export abstract class Base {
>Base : Base
> : ^^^^

accessor a = 1;
>a : number
> : ^^^^^^
>1 : 1
> : ^
}

export function middle(Super = Base) {
>middle : (Super?: typeof Base) => typeof Middle
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>Super : typeof Base
> : ^^^^^^^^^^^
>Base : typeof Base
> : ^^^^^^^^^^^

abstract class Middle extends Super {}
>Middle : Middle
> : ^^^^^^
>Super : Base
> : ^^^^

return Middle;
>Middle : typeof Middle
> : ^^^^^^^^^^^^^
}

class A {
>A : A
> : ^

constructor(...args: any[]) {}
>args : any[]
> : ^^^^^
}

export function Mixin<T extends typeof A>(Super: T) {
>Mixin : <T extends typeof A>(Super: T) => { new (...args: any[]): B; prototype: Mixin<any>.B; } & T
> : ^ ^^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>A : typeof A
> : ^^^^^^^^
>Super : T
> : ^

return class B extends Super {
>class B extends Super { get myName(): string { return "B"; } set myName(arg: string) {} } : { new (...args: any[]): B; prototype: Mixin<any>.B; } & T
> : ^^^^^^^^^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>B : { new (...args: any[]): B; prototype: Mixin<any>.B; } & T
> : ^^^^^^^^^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>Super : A
> : ^

get myName(): string {
>myName : string
> : ^^^^^^

return "B";
>"B" : "B"
> : ^^^
}
set myName(arg: string) {}
>myName : string
> : ^^^^^^
>arg : string
> : ^^^^^^

};
}

6 changes: 4 additions & 2 deletions tests/baselines/reference/autoAccessor8.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ declare class C2 {
}
declare function f(): {
new (): {
a: any;
get a(): any;
set a(arg: any);
};
b: any;
get b(): any;
set b(arg: any);
};
Loading
Loading