Skip to content

Implement declaration emit for accessors in the js declaration emitter #33649

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

Merged
merged 2 commits into from
Sep 30, 2019
Merged
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
35 changes: 32 additions & 3 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2797,9 +2797,38 @@ namespace ts {
(namespaceSymbol.members || (namespaceSymbol.members = createSymbolTable())) :
(namespaceSymbol.exports || (namespaceSymbol.exports = createSymbolTable()));

const isMethod = isFunctionLikeDeclaration(getAssignedExpandoInitializer(declaration)!);
const includes = isMethod ? SymbolFlags.Method : SymbolFlags.Property;
const excludes = isMethod ? SymbolFlags.MethodExcludes : SymbolFlags.PropertyExcludes;
let includes = SymbolFlags.None;
let excludes = SymbolFlags.None;
// Method-like
if (isFunctionLikeDeclaration(getAssignedExpandoInitializer(declaration)!)) {
includes = SymbolFlags.Method;
excludes = SymbolFlags.MethodExcludes;
}
// Maybe accessor-like
else if (isCallExpression(declaration) && isBindableObjectDefinePropertyCall(declaration)) {
if (some(declaration.arguments[2].properties, p => {
const id = getNameOfDeclaration(p);
return !!id && isIdentifier(id) && idText(id) === "set";
})) {
// We mix in `SymbolFLags.Property` so in the checker `getTypeOfVariableParameterOrProperty` is used for this
// symbol, instead of `getTypeOfAccessor` (which will assert as there is no real accessor declaration)
includes |= SymbolFlags.SetAccessor | SymbolFlags.Property;
excludes |= SymbolFlags.SetAccessorExcludes;
}
if (some(declaration.arguments[2].properties, p => {
const id = getNameOfDeclaration(p);
return !!id && isIdentifier(id) && idText(id) === "get";
})) {
includes |= SymbolFlags.GetAccessor | SymbolFlags.Property;
excludes |= SymbolFlags.GetAccessorExcludes;
}
}

if (includes === SymbolFlags.None) {
includes = SymbolFlags.Property;
excludes = SymbolFlags.PropertyExcludes;
}

declareSymbol(symbolTable, namespaceSymbol, declaration, includes | SymbolFlags.Assignment, excludes & ~SymbolFlags.Assignment);
}

Expand Down
61 changes: 55 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4916,8 +4916,8 @@ namespace ts {
}

function symbolTableToDeclarationStatements(symbolTable: SymbolTable, context: NodeBuilderContext, bundled?: boolean): Statement[] {
const serializePropertySymbolForClass = makeSerializePropertySymbol<ClassElement>(createProperty, SyntaxKind.MethodDeclaration);
const serializePropertySymbolForInterfaceWorker = makeSerializePropertySymbol<TypeElement>((_decorators, mods, name, question, type, initializer) => createPropertySignature(mods, name, question, type, initializer), SyntaxKind.MethodSignature);
const serializePropertySymbolForClass = makeSerializePropertySymbol<ClassElement>(createProperty, SyntaxKind.MethodDeclaration, /*useAcessors*/ true);
const serializePropertySymbolForInterfaceWorker = makeSerializePropertySymbol<TypeElement>((_decorators, mods, name, question, type, initializer) => createPropertySignature(mods, name, question, type, initializer), SyntaxKind.MethodSignature, /*useAcessors*/ false);

// TODO: Use `setOriginalNode` on original declaration names where possible so these declarations see some kind of
// declaration mapping
Expand Down Expand Up @@ -5734,7 +5734,23 @@ namespace ts {
questionOrExclamationToken: QuestionToken | undefined,
type: TypeNode | undefined,
initializer: Expression | undefined
) => T, methodKind: SyntaxKind): (p: Symbol, isStatic: boolean, baseType: Type | undefined) => (T | T[]) {
) => T, methodKind: SyntaxKind, useAccessors: true): (p: Symbol, isStatic: boolean, baseType: Type | undefined) => (T | AccessorDeclaration | (T | AccessorDeclaration)[]);
function makeSerializePropertySymbol<T extends Node>(createProperty: (
decorators: readonly Decorator[] | undefined,
modifiers: readonly Modifier[] | undefined,
name: string | PropertyName,
questionOrExclamationToken: QuestionToken | undefined,
type: TypeNode | undefined,
initializer: Expression | undefined
) => T, methodKind: SyntaxKind, useAccessors: false): (p: Symbol, isStatic: boolean, baseType: Type | undefined) => (T | T[]);
function makeSerializePropertySymbol<T extends Node>(createProperty: (
decorators: readonly Decorator[] | undefined,
modifiers: readonly Modifier[] | undefined,
name: string | PropertyName,
questionOrExclamationToken: QuestionToken | undefined,
type: TypeNode | undefined,
initializer: Expression | undefined
) => T, methodKind: SyntaxKind, useAccessors: boolean): (p: Symbol, isStatic: boolean, baseType: Type | undefined) => (T | AccessorDeclaration | (T | AccessorDeclaration)[]) {
return function serializePropertySymbol(p: Symbol, isStatic: boolean, baseType: Type | undefined) {
if (isStatic && (p.flags & (SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias))) {
// Only value-only-meaning symbols can be correctly encoded as class statics, type/namespace/alias meaning symbols
Expand All @@ -5750,7 +5766,40 @@ namespace ts {
const staticFlag = isStatic ? ModifierFlags.Static : 0;
const rawName = unescapeLeadingUnderscores(p.escapedName);
const name = getPropertyNameNodeForSymbolFromNameType(p, context) || createIdentifier(rawName);
if (p.flags & (SymbolFlags.Property | SymbolFlags.Accessor | SymbolFlags.Variable)) {
const firstPropertyLikeDecl = find(p.declarations, or(isPropertyDeclaration, isAccessor, isVariableDeclaration, isPropertySignature, isBinaryExpression, isPropertyAccessExpression));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const firstPropertyLikeDecl = find(p.declarations, d => isPropertyDeclaration(d) || isAccessor(d) || isVariableDeclaration(d) || isPropertySignature(d) || isBinaryExpression(d) || isPropertyAccessExpression(d));
const firstPropertyLikeDecl = find(p.declarations, or(isPropertyDeclaration, isAccessor, isVariableDeclaration, isPropertySignature, isBinaryExpression, isPropertyAccessExpression));

unless you need narrowing?

if (p.flags & SymbolFlags.Accessor && useAccessors) {
const result: AccessorDeclaration[] = [];
if (p.flags & SymbolFlags.SetAccessor) {
result.push(setTextRange(createSetAccessor(
/*decorators*/ undefined,
createModifiersFromModifierFlags(staticFlag),
name,
[createParameter(
/*decorators*/ undefined,
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"arg",
/*questionToken*/ undefined,
serializeTypeForDeclaration(getTypeOfSymbol(p), p)
)],
/*body*/ undefined
), find(p.declarations, isSetAccessor) || firstPropertyLikeDecl));
}
if (p.flags & SymbolFlags.GetAccessor) {
result.push(setTextRange(createGetAccessor(
/*decorators*/ undefined,
createModifiersFromModifierFlags(staticFlag),
name,
[],
serializeTypeForDeclaration(getTypeOfSymbol(p), p),
/*body*/ undefined
), find(p.declarations, isGetAccessor) || firstPropertyLikeDecl));
}
return result;
}
// This is an else/if as accessors and properties can't merge in TS, but might in JS
// If this happens, we assume the accessor takes priority, as it imposes more constraints
else if (p.flags & (SymbolFlags.Property | SymbolFlags.Variable)) {
return setTextRange(createProperty(
/*decorators*/ undefined,
createModifiersFromModifierFlags((isReadonlySymbol(p) ? ModifierFlags.Readonly : 0) | staticFlag),
Expand All @@ -5760,7 +5809,7 @@ namespace ts {
// TODO: https://github.com/microsoft/TypeScript/pull/32372#discussion_r328386357
// interface members can't have initializers, however class members _can_
/*initializer*/ undefined
), filter(p.declarations, d => isPropertyDeclaration(d) || isAccessor(d) || isVariableDeclaration(d) || isPropertySignature(d) || isBinaryExpression(d) || isPropertyAccessExpression(d))[0]);
), find(p.declarations, or(isPropertyDeclaration, isVariableDeclaration)) || firstPropertyLikeDecl);
}
if (p.flags & (SymbolFlags.Method | SymbolFlags.Function)) {
const type = getTypeOfSymbol(p);
Expand Down Expand Up @@ -7340,7 +7389,7 @@ namespace ts {
}
}
else {
Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function");
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
errorOrSuggestion(noImplicitAny, getter!, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
return anyType;
Expand Down
20 changes: 14 additions & 6 deletions tests/baselines/reference/jsDeclarationsClasses.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,18 +479,22 @@ export class E<T, U> {
*/
static staticReadonlyField: string;
static staticInitializedField: number;
/**
* @param {string} _p
*/
static set s1(arg: string);
/**
* @return {string}
*/
static s1: string;
static get s1(): string;
/**
* @return {string}
*/
static readonly s2: string;
static get s2(): string;
/**
* @param {string} _p
*/
static s3: string;
static set s3(arg: string);
/**
* @param {T} a
* @param {U} b
Expand All @@ -506,18 +510,22 @@ export class E<T, U> {
*/
readonlyField: T & U;
initializedField: number;
/**
* @param {U} _p
*/
set f1(arg: U);
/**
* @return {U}
*/
f1: U;
get f1(): U;
/**
* @return {U}
*/
readonly f2: U;
get f2(): U;
/**
* @param {U} _p
*/
f3: U;
set f3(arg: U);
}
/**
* @template T,U
Expand Down
12 changes: 10 additions & 2 deletions tests/baselines/reference/jsDeclarationsFunctionLikeClasses2.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,16 @@ export class Point2D {
* @param {number} y
*/
constructor(x: number, y: number);
x: number;
y: number;
/**
* @param {number} x
*/
set x(arg: number);
get x(): number;
/**
* @param {number} y
*/
set y(arg: number);
get y(): number;
__proto__: typeof Vec;
}
//// [referencer.d.ts]
Expand Down
122 changes: 122 additions & 0 deletions tests/baselines/reference/jsDeclarationsGetterSetter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
//// [index.js]
export class A {
get x() {
return 12;
}
}

export class B {
/**
* @param {number} _arg
*/
set x(_arg) {
}
}

export class C {
get x() {
return 12;
}
set x(_arg) {
}
}

export class D {}
Object.defineProperty(D.prototype, "x", {
get() {
return 12;
}
});

export class E {}
Object.defineProperty(E.prototype, "x", {
/**
* @param {number} _arg
*/
set(_arg) {}
});

export class F {}
Object.defineProperty(F.prototype, "x", {
get() {
return 12;
},
/**
* @param {number} _arg
*/
set(_arg) {}
});


//// [index.js]
export class A {
get x() {
return 12;
}
}
export class B {
/**
* @param {number} _arg
*/
set x(_arg) {
}
}
export class C {
get x() {
return 12;
}
set x(_arg) {
}
}
export class D {
}
Object.defineProperty(D.prototype, "x", {
get() {
return 12;
}
});
export class E {
}
Object.defineProperty(E.prototype, "x", {
/**
* @param {number} _arg
*/
set(_arg) { }
});
export class F {
}
Object.defineProperty(F.prototype, "x", {
get() {
return 12;
},
/**
* @param {number} _arg
*/
set(_arg) { }
});


//// [index.d.ts]
export class A {
get x(): number;
}
export class B {
/**
* @param {number} _arg
*/
set x(arg: number);
}
export class C {
set x(arg: number);
get x(): number;
}
export class D {
get x(): number;
}
export class E {
set x(arg: number);
}
export class F {
set x(arg: number);
get x(): number;
}
Copy link
Member

Choose a reason for hiding this comment

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

This is very cool

Loading