Skip to content

Backward-compatible accessor emit #33880

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

Closed
wants to merge 1 commit into from
Closed
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
3 changes: 2 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31642,7 +31642,8 @@ namespace ts {
if (basePropertyFlags !== SymbolFlags.Property && derivedPropertyFlags === SymbolFlags.Property) {
errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_property;
}
else if (basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property) {
else if (basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property
&& !(base.valueDeclaration && base.valueDeclaration.flags & NodeFlags.Ambient && getJSDocTags(base.valueDeclaration).find(tag => tag.tagName.escapedText === "accessor"))) {
errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_accessor;
}
else {
Expand Down
81 changes: 64 additions & 17 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ namespace ts {
let emittedImports: readonly AnyImportSyntax[] | undefined; // must be declared in container so it can be `undefined` while transformer's first pass
const resolver = context.getEmitResolver();
const options = context.getCompilerOptions();
const newLine = getNewLineCharacter(options);
const { noResolve, stripInternal } = options;
return transformRoot;

Expand Down Expand Up @@ -864,25 +865,35 @@ namespace ts {
return cleanup(sig);
}
case SyntaxKind.GetAccessor: {
const isPrivate = hasModifier(input, ModifierFlags.Private);
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
return cleanup(updateGetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, isPrivate),
!isPrivate ? ensureType(input, accessorType) : undefined,
/*body*/ undefined));
if (options.useDefineForClassFields) {
const isPrivate = hasModifier(input, ModifierFlags.Private);
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
return cleanup(updateGetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, isPrivate),
!isPrivate ? ensureType(input, accessorType) : undefined,
/*body*/ undefined));
}
else {
return cleanup(ensureAccessor(input));
}
}
case SyntaxKind.SetAccessor: {
return cleanup(updateSetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, hasModifier(input, ModifierFlags.Private)),
/*body*/ undefined));
if (options.useDefineForClassFields) {
return cleanup(updateSetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, hasModifier(input, ModifierFlags.Private)),
/*body*/ undefined));
}
else {
return cleanup(ensureAccessor(input));
}
}
case SyntaxKind.PropertyDeclaration:
return cleanup(updateProperty(
Expand Down Expand Up @@ -1455,6 +1466,42 @@ namespace ts {
return accessorType;
}

function ensureAccessor(node: AccessorDeclaration): PropertyDeclaration | undefined {
const accessors = resolver.getAllAccessorDeclarations(node);
if (node.kind !== accessors.firstAccessor.kind) {
return;
}
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(node, accessors);
const prop = createProperty(/*decorators*/ undefined, maskModifiers(node, /*mask*/ undefined, (!accessors.setAccessor) ? ModifierFlags.Readonly : ModifierFlags.None), node.name, node.questionToken, ensureType(node, accessorType), /*initializer*/ undefined);
const leadingsSyntheticCommentRanges = accessors.secondAccessor && getLeadingCommentRangesOfNode(accessors.secondAccessor, currentSourceFile);
if (leadingsSyntheticCommentRanges) {
for (const range of leadingsSyntheticCommentRanges) {
if (range.kind === SyntaxKind.MultiLineCommentTrivia) {
let text = currentSourceFile.text.slice(range.pos + 2, range.end - 2);
const lines = text.split(/\r\n?|\n/g);
if (lines.length > 1) {
const lastLines = lines.slice(1);
const indentation = guessIndentation(lastLines);
text = [lines[0], ...map(lastLines, l => l.slice(indentation))].join(newLine);
}
addSyntheticLeadingComment(
prop,
range.kind,
text,
range.hasTrailingNewLine
);
}
}
}
addSyntheticLeadingComment(
prop,
SyntaxKind.MultiLineCommentTrivia,
"*@accessor",
/*hasTrailingNewLine*/ false
);
return prop;
}

function transformHeritageClauses(nodes: NodeArray<HeritageClause> | undefined) {
return createNodeArray(filter(map(nodes, clause => updateHeritageClause(clause, visitNodes(createNodeArray(filter(clause.types, t => {
return isEntityNameExpression(t.expression) || (clause.token === SyntaxKind.ExtendsKeyword && t.expression.kind === SyntaxKind.NullKeyword);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/accessorDeclarationEmitVisibilityErrors.ts(2,18): error TS2304: Cannot find name 'DoesNotExist'.
tests/cases/compiler/accessorDeclarationEmitVisibilityErrors.ts(2,18): error TS4106: Parameter 'arg' of accessor has or is using private name 'DoesNotExist'.
tests/cases/compiler/accessorDeclarationEmitVisibilityErrors.ts(2,18): error TS4037: Parameter type of public setter 'bet' from exported class has or is using private name 'DoesNotExist'.


==== tests/cases/compiler/accessorDeclarationEmitVisibilityErrors.ts (2 errors) ====
Expand All @@ -8,5 +8,5 @@ tests/cases/compiler/accessorDeclarationEmitVisibilityErrors.ts(2,18): error TS4
~~~~~~~~~~~~
!!! error TS2304: Cannot find name 'DoesNotExist'.
~~~~~~~~~~~~
!!! error TS4106: Parameter 'arg' of accessor has or is using private name 'DoesNotExist'.
!!! error TS4037: Parameter type of public setter 'bet' from exported class has or is using private name 'DoesNotExist'.
}
41 changes: 40 additions & 1 deletion tests/baselines/reference/accessorWithES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,21 @@ class D {
}
}

class E {
// comment 1
get x() { return 2; }
// comment 2
set x(v) { }
}

var x = {
get a() { return 1 }
}

var y = {
set b(v) { }
}
}


//// [accessorWithES5.js]
var C = /** @class */ (function () {
Expand All @@ -42,9 +50,40 @@ var D = /** @class */ (function () {
});
return D;
}());
var E = /** @class */ (function () {
function E() {
}
Object.defineProperty(E.prototype, "x", {
// comment 1
get: function () { return 2; },
// comment 2
set: function (v) { },
enumerable: true,
configurable: true
});
return E;
}());
var x = {
get a() { return 1; }
};
var y = {
set b(v) { }
};


//// [accessorWithES5.d.ts]
declare class C {
/**@accessor*/ readonly x: number;
}
declare class D {
/**@accessor*/ x: any;
}
declare class E {
/**@accessor*/ x: number;
}
declare var x: {
readonly a: number;
};
declare var y: {
b: any;
};
24 changes: 19 additions & 5 deletions tests/baselines/reference/accessorWithES5.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,31 @@ class D {
}
}

class E {
>E : Symbol(E, Decl(accessorWithES5.ts, 9, 1))

// comment 1
get x() { return 2; }
>x : Symbol(E.x, Decl(accessorWithES5.ts, 11, 9), Decl(accessorWithES5.ts, 13, 25))

// comment 2
set x(v) { }
>x : Symbol(E.x, Decl(accessorWithES5.ts, 11, 9), Decl(accessorWithES5.ts, 13, 25))
>v : Symbol(v, Decl(accessorWithES5.ts, 15, 10))
}

var x = {
>x : Symbol(x, Decl(accessorWithES5.ts, 11, 3))
>x : Symbol(x, Decl(accessorWithES5.ts, 18, 3))

get a() { return 1 }
>a : Symbol(a, Decl(accessorWithES5.ts, 11, 9))
>a : Symbol(a, Decl(accessorWithES5.ts, 18, 9))
}

var y = {
>y : Symbol(y, Decl(accessorWithES5.ts, 15, 3))
>y : Symbol(y, Decl(accessorWithES5.ts, 22, 3))

set b(v) { }
>b : Symbol(b, Decl(accessorWithES5.ts, 15, 9))
>v : Symbol(v, Decl(accessorWithES5.ts, 16, 10))
>b : Symbol(b, Decl(accessorWithES5.ts, 22, 9))
>v : Symbol(v, Decl(accessorWithES5.ts, 23, 10))
}

15 changes: 15 additions & 0 deletions tests/baselines/reference/accessorWithES5.types
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ class D {
}
}

class E {
>E : E

// comment 1
get x() { return 2; }
>x : number
>2 : 2

// comment 2
set x(v) { }
>x : number
>v : number
}

var x = {
>x : { readonly a: number; }
>{ get a() { return 1 }} : { readonly a: number; }
Expand All @@ -36,3 +50,4 @@ var y = {
>b : any
>v : any
}

12 changes: 4 additions & 8 deletions tests/baselines/reference/ambientAccessors(target=es3).js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@ declare class C {

//// [ambientAccessors.d.ts]
declare class C {
static get a(): string;
static set a(value: string);
private static get b();
private static set b(value);
get x(): string;
set x(value: string);
private get y();
private set y(value);
/**@accessor*/ static a: string;
/**@accessor*/ private static b;
/**@accessor*/ x: string;
/**@accessor*/ private y;
}
12 changes: 4 additions & 8 deletions tests/baselines/reference/ambientAccessors(target=es5).js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@ declare class C {

//// [ambientAccessors.d.ts]
declare class C {
static get a(): string;
static set a(value: string);
private static get b();
private static set b(value);
get x(): string;
set x(value: string);
private get y();
private set y(value);
/**@accessor*/ static a: string;
/**@accessor*/ private static b;
/**@accessor*/ x: string;
/**@accessor*/ private y;
}
6 changes: 2 additions & 4 deletions tests/baselines/reference/assertionTypePredicates1.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,7 @@ declare let Q1: new (x: unknown) => x is string;
declare let Q2: new (x: boolean) => asserts x;
declare let Q3: new (x: unknown) => asserts x is string;
declare class Wat {
get p1(): this is string;
set p1(x: this is string);
get p2(): asserts this is string;
set p2(x: asserts this is string);
/**@accessor*/ p1: this is string;
/**@accessor*/ p2: asserts this is string;
}
declare function f20(x: unknown): void;
7 changes: 3 additions & 4 deletions tests/baselines/reference/classdecl.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,13 @@ declare class a {
constructor(s: string);
pgF(): void;
pv: any;
get d(): number;
set d(a: number);
static get p2(): {
/**@accessor*/ d: number;
/**@accessor*/ static readonly p2: {
x: number;
y: number;
};
private static d2;
private static get p3();
/**@accessor*/ private static readonly p3;
private pv3;
private foo;
}
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/commentsdoNotEmitComments.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ declare class c {
constructor();
b: number;
myFoo(): number;
get prop1(): number;
set prop1(val: number);
prop1: number;
foo1(a: number): string;
foo1(b: string): string;
}
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/commentsemitComments.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,8 @@ declare class c {
/** function comment */
myFoo(): number;
/** getter comment*/
get prop1(): number;
/** setter comment*/
set prop1(val: number);
/**@accessor*/ prop1: number;
/** overload signature1*/
foo1(a: number): string;
/** Overload signature 2*/
Expand Down
Loading