Skip to content

Commit 6f7faa7

Browse files
Jack-WorksJack-Works
authored
Fix: convertFunctionToEs6Class cannot recognize x.prototype = {} pattern (microsoft#35219)
* fix: convertFunctionToEs6Class cannot recognize `x.prototype = {}` pattern * test: add test for convert fn to es6 * chore: add more tests * chore: move tests around * chore: make code more clear
1 parent 10fc8c3 commit 6f7faa7

6 files changed

+263
-54
lines changed

‎src/services/codefixes/convertFunctionToEs6Class.ts

+126-54
Original file line numberDiff line numberDiff line change
@@ -60,95 +60,161 @@ namespace ts.codefix {
6060
const memberElements: ClassElement[] = [];
6161
// all instance members are stored in the "member" array of symbol
6262
if (symbol.members) {
63-
symbol.members.forEach(member => {
63+
symbol.members.forEach((member, key) => {
64+
if (key === "constructor") {
65+
// fn.prototype.constructor = fn
66+
changes.delete(sourceFile, member.valueDeclaration.parent);
67+
return;
68+
}
6469
const memberElement = createClassElement(member, /*modifiers*/ undefined);
6570
if (memberElement) {
66-
memberElements.push(memberElement);
71+
memberElements.push(...memberElement);
6772
}
6873
});
6974
}
7075

7176
// all static members are stored in the "exports" array of symbol
7277
if (symbol.exports) {
7378
symbol.exports.forEach(member => {
74-
const memberElement = createClassElement(member, [createToken(SyntaxKind.StaticKeyword)]);
75-
if (memberElement) {
76-
memberElements.push(memberElement);
79+
if (member.name === "prototype") {
80+
const firstDeclaration = member.declarations[0];
81+
// only one "x.prototype = { ... }" will pass
82+
if (member.declarations.length === 1 &&
83+
isPropertyAccessExpression(firstDeclaration) &&
84+
isBinaryExpression(firstDeclaration.parent) &&
85+
firstDeclaration.parent.operatorToken.kind === SyntaxKind.EqualsToken &&
86+
isObjectLiteralExpression(firstDeclaration.parent.right)
87+
) {
88+
const prototypes = firstDeclaration.parent.right;
89+
const memberElement = createClassElement(prototypes.symbol, /** modifiers */ undefined);
90+
if (memberElement) {
91+
memberElements.push(...memberElement);
92+
}
93+
}
94+
}
95+
else {
96+
const memberElement = createClassElement(member, [createToken(SyntaxKind.StaticKeyword)]);
97+
if (memberElement) {
98+
memberElements.push(...memberElement);
99+
}
77100
}
78101
});
79102
}
80103

81104
return memberElements;
82105

83-
function shouldConvertDeclaration(_target: PropertyAccessExpression, source: Expression) {
84-
// Right now the only thing we can convert are function expressions - other values shouldn't get
85-
// transformed. We can update this once ES public class properties are available.
86-
return isFunctionLike(source);
106+
function shouldConvertDeclaration(_target: PropertyAccessExpression | ObjectLiteralExpression, source: Expression) {
107+
// Right now the only thing we can convert are function expressions, get/set accessors and methods
108+
// other values like normal value fields ({a: 1}) shouldn't get transformed.
109+
// We can update this once ES public class properties are available.
110+
if (isPropertyAccessExpression(_target)) {
111+
if (isConstructorAssignment(_target)) return true;
112+
return isFunctionLike(source);
113+
}
114+
else {
115+
return every(_target.properties, property => {
116+
// a() {}
117+
if (isMethodDeclaration(property) || isGetOrSetAccessorDeclaration(property)) return true;
118+
// a: function() {}
119+
if (isPropertyAssignment(property) && isFunctionExpression(property.initializer) && !!property.name) return true;
120+
// x.prototype.constructor = fn
121+
if (isConstructorAssignment(property)) return true;
122+
return false;
123+
});
124+
}
87125
}
88126

89-
function createClassElement(symbol: Symbol, modifiers: Modifier[] | undefined): ClassElement | undefined {
127+
function createClassElement(symbol: Symbol, modifiers: Modifier[] | undefined): readonly ClassElement[] {
90128
// Right now the only thing we can convert are function expressions, which are marked as methods
91-
if (!(symbol.flags & SymbolFlags.Method)) {
92-
return;
129+
// or { x: y } type prototype assignments, which are marked as ObjectLiteral
130+
const members: ClassElement[] = [];
131+
if (!(symbol.flags & SymbolFlags.Method) && !(symbol.flags & SymbolFlags.ObjectLiteral)) {
132+
return members;
93133
}
94134

95-
const memberDeclaration = symbol.valueDeclaration as PropertyAccessExpression;
135+
const memberDeclaration = symbol.valueDeclaration as PropertyAccessExpression | ObjectLiteralExpression;
96136
const assignmentBinaryExpression = memberDeclaration.parent as BinaryExpression;
137+
const assignmentExpr = assignmentBinaryExpression.right;
97138

98-
if (!shouldConvertDeclaration(memberDeclaration, assignmentBinaryExpression.right)) {
99-
return;
139+
if (!shouldConvertDeclaration(memberDeclaration, assignmentExpr)) {
140+
return members;
100141
}
101142

102143
// delete the entire statement if this expression is the sole expression to take care of the semicolon at the end
103144
const nodeToDelete = assignmentBinaryExpression.parent && assignmentBinaryExpression.parent.kind === SyntaxKind.ExpressionStatement
104145
? assignmentBinaryExpression.parent : assignmentBinaryExpression;
105146
changes.delete(sourceFile, nodeToDelete);
106147

107-
if (!assignmentBinaryExpression.right) {
108-
return createProperty([], modifiers, symbol.name, /*questionToken*/ undefined,
109-
/*type*/ undefined, /*initializer*/ undefined);
148+
if (!assignmentExpr) {
149+
members.push(createProperty([], modifiers, symbol.name, /*questionToken*/ undefined,
150+
/*type*/ undefined, /*initializer*/ undefined));
151+
return members;
110152
}
111153

112-
switch (assignmentBinaryExpression.right.kind) {
113-
case SyntaxKind.FunctionExpression: {
114-
const functionExpression = assignmentBinaryExpression.right as FunctionExpression;
115-
const fullModifiers = concatenate(modifiers, getModifierKindFromSource(functionExpression, SyntaxKind.AsyncKeyword));
116-
const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined,
117-
/*typeParameters*/ undefined, functionExpression.parameters, /*type*/ undefined, functionExpression.body);
118-
copyLeadingComments(assignmentBinaryExpression, method, sourceFile);
119-
return method;
120-
}
154+
// f.x = expr
155+
if (isPropertyAccessExpression(memberDeclaration) && (isFunctionExpression(assignmentExpr) || isArrowFunction(assignmentExpr))) {
156+
return createFunctionLikeExpressionMember(members, assignmentExpr, memberDeclaration.name);
157+
}
158+
// f.prototype = { ... }
159+
else if (isObjectLiteralExpression(assignmentExpr)) {
160+
return flatMap(
161+
assignmentExpr.properties,
162+
property => {
163+
if (isMethodDeclaration(property) || isGetOrSetAccessorDeclaration(property)) {
164+
// MethodDeclaration and AccessorDeclaration can appear in a class directly
165+
return members.concat(property);
166+
}
167+
if (isPropertyAssignment(property) && isFunctionExpression(property.initializer)) {
168+
return createFunctionLikeExpressionMember(members, property.initializer, property.name);
169+
}
170+
// Drop constructor assignments
171+
if (isConstructorAssignment(property)) return members;
172+
return [];
173+
}
174+
);
175+
}
176+
else {
177+
// Don't try to declare members in JavaScript files
178+
if (isSourceFileJS(sourceFile)) return members;
179+
if (!isPropertyAccessExpression(memberDeclaration)) return members;
180+
const prop = createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, /*type*/ undefined, assignmentExpr);
181+
copyLeadingComments(assignmentBinaryExpression.parent, prop, sourceFile);
182+
members.push(prop);
183+
return members;
184+
}
121185

122-
case SyntaxKind.ArrowFunction: {
123-
const arrowFunction = assignmentBinaryExpression.right as ArrowFunction;
124-
const arrowFunctionBody = arrowFunction.body;
125-
let bodyBlock: Block;
186+
type MethodName = Parameters<typeof createMethod>[3];
126187

127-
// case 1: () => { return [1,2,3] }
128-
if (arrowFunctionBody.kind === SyntaxKind.Block) {
129-
bodyBlock = arrowFunctionBody as Block;
130-
}
131-
// case 2: () => [1,2,3]
132-
else {
133-
bodyBlock = createBlock([createReturn(arrowFunctionBody)]);
134-
}
135-
const fullModifiers = concatenate(modifiers, getModifierKindFromSource(arrowFunction, SyntaxKind.AsyncKeyword));
136-
const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined,
137-
/*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock);
138-
copyLeadingComments(assignmentBinaryExpression, method, sourceFile);
139-
return method;
140-
}
188+
function createFunctionLikeExpressionMember(members: readonly ClassElement[], expression: FunctionExpression | ArrowFunction, name: MethodName) {
189+
if (isFunctionExpression(expression)) return createFunctionExpressionMember(members, expression, name);
190+
else return createArrowFunctionExpressionMember(members, expression, name);
191+
}
141192

142-
default: {
143-
// Don't try to declare members in JavaScript files
144-
if (isSourceFileJS(sourceFile)) {
145-
return;
146-
}
147-
const prop = createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined,
148-
/*type*/ undefined, assignmentBinaryExpression.right);
149-
copyLeadingComments(assignmentBinaryExpression.parent, prop, sourceFile);
150-
return prop;
193+
function createFunctionExpressionMember(members: readonly ClassElement[], functionExpression: FunctionExpression, name: MethodName) {
194+
const fullModifiers = concatenate(modifiers, getModifierKindFromSource(functionExpression, SyntaxKind.AsyncKeyword));
195+
const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, name, /*questionToken*/ undefined,
196+
/*typeParameters*/ undefined, functionExpression.parameters, /*type*/ undefined, functionExpression.body);
197+
copyLeadingComments(assignmentBinaryExpression, method, sourceFile);
198+
return members.concat(method);
199+
}
200+
201+
function createArrowFunctionExpressionMember(members: readonly ClassElement[], arrowFunction: ArrowFunction, name: MethodName) {
202+
const arrowFunctionBody = arrowFunction.body;
203+
let bodyBlock: Block;
204+
205+
// case 1: () => { return [1,2,3] }
206+
if (arrowFunctionBody.kind === SyntaxKind.Block) {
207+
bodyBlock = arrowFunctionBody as Block;
151208
}
209+
// case 2: () => [1,2,3]
210+
else {
211+
bodyBlock = createBlock([createReturn(arrowFunctionBody)]);
212+
}
213+
const fullModifiers = concatenate(modifiers, getModifierKindFromSource(arrowFunction, SyntaxKind.AsyncKeyword));
214+
const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, name, /*questionToken*/ undefined,
215+
/*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock);
216+
copyLeadingComments(assignmentBinaryExpression, method, sourceFile);
217+
return members.concat(method);
152218
}
153219
}
154220
}
@@ -192,4 +258,10 @@ namespace ts.codefix {
192258
function getModifierKindFromSource(source: Node, kind: SyntaxKind): readonly Modifier[] | undefined {
193259
return filter(source.modifiers, modifier => modifier.kind === kind);
194260
}
261+
262+
function isConstructorAssignment(x: ObjectLiteralElementLike | PropertyAccessExpression) {
263+
if (!x.name) return false;
264+
if (isIdentifier(x.name) && x.name.text === "constructor") return true;
265+
return false;
266+
}
195267
}

‎tests/cases/fourslash/convertFunctionToEs6Class-propertyMember.ts

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// @allowNonTsExtensions: true
2+
// @Filename: test123.js
3+
4+
/// <reference path="./fourslash.ts" />
5+
6+
//// // Comment
7+
//// function /*1*/fn() {
8+
//// this.baz = 10;
9+
//// }
10+
//// fn.prototype = {
11+
//// /** JSDoc */
12+
//// bar: function () {
13+
//// console.log('hello world');
14+
//// }
15+
//// }
16+
17+
verify.codeFix({
18+
description: "Convert function to an ES2015 class",
19+
newFileContent:
20+
`// Comment
21+
class fn {
22+
constructor() {
23+
this.baz = 10;
24+
}
25+
/** JSDoc */
26+
bar() {
27+
console.log('hello world');
28+
}
29+
}
30+
`,
31+
});

‎tests/cases/fourslash/convertFunctionToEs6Class-prototypeAccessor.ts

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// @allowNonTsExtensions: true
2+
// @Filename: test123.js
3+
4+
/// <reference path="./fourslash.ts" />
5+
6+
//// // Comment
7+
//// function /*1*/fn() {
8+
//// this.baz = 10;
9+
//// }
10+
//// fn.prototype = {
11+
//// /** JSDoc */
12+
//// get bar() {
13+
//// return this.baz;
14+
//// }
15+
//// }
16+
17+
verify.codeFix({
18+
description: "Convert function to an ES2015 class",
19+
newFileContent:
20+
`// Comment
21+
class fn {
22+
constructor() {
23+
this.baz = 10;
24+
}
25+
/** JSDoc */
26+
get bar() {
27+
return this.baz;
28+
}
29+
}
30+
`,
31+
});

‎tests/cases/fourslash/convertFunctionToEs6Class-prototypeMethod.ts

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// @allowNonTsExtensions: true
2+
// @Filename: test123.js
3+
4+
/// <reference path="./fourslash.ts" />
5+
6+
//// // Comment
7+
//// function /*1*/fn() {
8+
//// this.baz = 10;
9+
//// }
10+
//// fn.prototype = {
11+
//// bar() {
12+
//// console.log('hello world');
13+
//// }
14+
//// }
15+
16+
verify.codeFix({
17+
description: "Convert function to an ES2015 class",
18+
newFileContent:
19+
`// Comment
20+
class fn {
21+
constructor() {
22+
this.baz = 10;
23+
}
24+
bar() {
25+
console.log('hello world');
26+
}
27+
}
28+
`,
29+
});

‎tests/cases/fourslash/convertFunctionToEs6Class-removeConstructor.ts

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// @allowNonTsExtensions: true
2+
// @Filename: test123.js
3+
4+
/// <reference path="./fourslash.ts" />
5+
6+
//// // Comment
7+
//// function /*1*/fn() {
8+
//// this.baz = 10;
9+
//// }
10+
//// fn.prototype = {
11+
//// constructor: fn
12+
//// }
13+
14+
verify.codeFix({
15+
description: "Convert function to an ES2015 class",
16+
newFileContent:
17+
`// Comment
18+
class fn {
19+
constructor() {
20+
this.baz = 10;
21+
}
22+
}
23+
`,
24+
});

‎tests/cases/fourslash/convertFunctionToEs6Class-removeConstructor2.ts

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @allowNonTsExtensions: true
2+
// @Filename: test123.js
3+
4+
/// <reference path="./fourslash.ts" />
5+
6+
//// // Comment
7+
//// function /*1*/fn() {
8+
//// this.baz = 10;
9+
//// }
10+
//// fn.prototype.constructor = fn
11+
12+
verify.codeFix({
13+
description: "Convert function to an ES2015 class",
14+
newFileContent:
15+
`// Comment
16+
class fn {
17+
constructor() {
18+
this.baz = 10;
19+
}
20+
}
21+
`,
22+
});

0 commit comments

Comments
 (0)