Skip to content

Commit c83f769

Browse files
a-tarasyuka-tarasyuk
authored
fix(41405): allow using property access as reference to function (microsoft#41429)
1 parent 33ea6c5 commit c83f769

File tree

7 files changed

+202
-16
lines changed

7 files changed

+202
-16
lines changed

‎src/services/codefixes/convertToAsyncFunction.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ namespace ts.codefix {
6565
const isInJavascript = isInJSFile(functionToConvert);
6666
const setOfExpressionsToReturn = getAllPromiseExpressionsToReturn(functionToConvert, checker);
6767
const functionToConvertRenamed = renameCollidingVarNames(functionToConvert, checker, synthNamesMap);
68-
const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body) : emptyArray;
68+
const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body, checker) : emptyArray;
6969
const transformer: Transformer = { checker, synthNamesMap, setOfExpressionsToReturn, isInJSFile: isInJavascript };
7070
if (!returnStatements.length) {
7171
return;
@@ -90,10 +90,10 @@ namespace ts.codefix {
9090
}
9191
}
9292

93-
function getReturnStatementsWithPromiseHandlers(body: Block): readonly ReturnStatement[] {
93+
function getReturnStatementsWithPromiseHandlers(body: Block, checker: TypeChecker): readonly ReturnStatement[] {
9494
const res: ReturnStatement[] = [];
9595
forEachReturnStatement(body, ret => {
96-
if (isReturnStatementWithFixablePromiseHandler(ret)) res.push(ret);
96+
if (isReturnStatementWithFixablePromiseHandler(ret, checker)) res.push(ret);
9797
});
9898
return res;
9999
}
@@ -374,13 +374,14 @@ namespace ts.codefix {
374374
case SyntaxKind.NullKeyword:
375375
// do not produce a transformed statement for a null argument
376376
break;
377+
case SyntaxKind.PropertyAccessExpression:
377378
case SyntaxKind.Identifier: // identifier includes undefined
378379
if (!argName) {
379380
// undefined was argument passed to promise handler
380381
break;
381382
}
382383

383-
const synthCall = factory.createCallExpression(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []);
384+
const synthCall = factory.createCallExpression(getSynthesizedDeepClone(func as Identifier | PropertyAccessExpression), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []);
384385
if (shouldReturn(parent, transformer)) {
385386
return maybeAnnotateAndReturn(synthCall, parent.typeArguments?.[0]);
386387
}
@@ -410,7 +411,7 @@ namespace ts.codefix {
410411
for (const statement of funcBody.statements) {
411412
if (isReturnStatement(statement)) {
412413
seenReturnStatement = true;
413-
if (isReturnStatementWithFixablePromiseHandler(statement)) {
414+
if (isReturnStatementWithFixablePromiseHandler(statement, transformer.checker)) {
414415
refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName));
415416
}
416417
else {
@@ -432,7 +433,7 @@ namespace ts.codefix {
432433
seenReturnStatement);
433434
}
434435
else {
435-
const innerRetStmts = isFixablePromiseHandler(funcBody) ? [factory.createReturnStatement(funcBody)] : emptyArray;
436+
const innerRetStmts = isFixablePromiseHandler(funcBody, transformer.checker) ? [factory.createReturnStatement(funcBody)] : emptyArray;
436437
const innerCbBody = getInnerTransformationBody(transformer, innerRetStmts, prevArgName);
437438

438439
if (innerCbBody.length > 0) {
@@ -536,6 +537,9 @@ namespace ts.codefix {
536537
else if (isIdentifier(funcNode)) {
537538
name = getMapEntryOrDefault(funcNode);
538539
}
540+
else if (isPropertyAccessExpression(funcNode) && isIdentifier(funcNode.name)) {
541+
name = getMapEntryOrDefault(funcNode.name);
542+
}
539543

540544
// return undefined argName when arg is null or undefined
541545
// eslint-disable-next-line no-in-operator

‎src/services/suggestionDiagnostics.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ namespace ts {
114114
return !isAsyncFunction(node) &&
115115
node.body &&
116116
isBlock(node.body) &&
117-
hasReturnStatementWithPromiseHandler(node.body) &&
117+
hasReturnStatementWithPromiseHandler(node.body, checker) &&
118118
returnsPromise(node, checker);
119119
}
120120

@@ -129,25 +129,25 @@ namespace ts {
129129
return isBinaryExpression(commonJsModuleIndicator) ? commonJsModuleIndicator.left : commonJsModuleIndicator;
130130
}
131131

132-
function hasReturnStatementWithPromiseHandler(body: Block): boolean {
133-
return !!forEachReturnStatement(body, isReturnStatementWithFixablePromiseHandler);
132+
function hasReturnStatementWithPromiseHandler(body: Block, checker: TypeChecker): boolean {
133+
return !!forEachReturnStatement(body, statement => isReturnStatementWithFixablePromiseHandler(statement, checker));
134134
}
135135

136-
export function isReturnStatementWithFixablePromiseHandler(node: Node): node is ReturnStatement & { expression: CallExpression } {
137-
return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression);
136+
export function isReturnStatementWithFixablePromiseHandler(node: Node, checker: TypeChecker): node is ReturnStatement & { expression: CallExpression } {
137+
return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression, checker);
138138
}
139139

140140
// Should be kept up to date with transformExpression in convertToAsyncFunction.ts
141-
export function isFixablePromiseHandler(node: Node): boolean {
141+
export function isFixablePromiseHandler(node: Node, checker: TypeChecker): boolean {
142142
// ensure outermost call exists and is a promise handler
143-
if (!isPromiseHandler(node) || !node.arguments.every(isFixablePromiseArgument)) {
143+
if (!isPromiseHandler(node) || !node.arguments.every(arg => isFixablePromiseArgument(arg, checker))) {
144144
return false;
145145
}
146146

147147
// ensure all chained calls are valid
148148
let currentNode = node.expression;
149149
while (isPromiseHandler(currentNode) || isPropertyAccessExpression(currentNode)) {
150-
if (isCallExpression(currentNode) && !currentNode.arguments.every(isFixablePromiseArgument)) {
150+
if (isCallExpression(currentNode) && !currentNode.arguments.every(arg => isFixablePromiseArgument(arg, checker))) {
151151
return false;
152152
}
153153
currentNode = currentNode.expression;
@@ -171,16 +171,24 @@ namespace ts {
171171
}
172172

173173
// should be kept up to date with getTransformationBody in convertToAsyncFunction.ts
174-
function isFixablePromiseArgument(arg: Expression): boolean {
174+
function isFixablePromiseArgument(arg: Expression, checker: TypeChecker): boolean {
175175
switch (arg.kind) {
176176
case SyntaxKind.FunctionDeclaration:
177177
case SyntaxKind.FunctionExpression:
178178
case SyntaxKind.ArrowFunction:
179179
visitedNestedConvertibleFunctions.set(getKeyFromNode(arg as FunctionLikeDeclaration), true);
180180
// falls through
181181
case SyntaxKind.NullKeyword:
182-
case SyntaxKind.Identifier: // identifier includes undefined
183182
return true;
183+
case SyntaxKind.Identifier:
184+
case SyntaxKind.PropertyAccessExpression: {
185+
const symbol = checker.getSymbolAtLocation(arg);
186+
if (!symbol) {
187+
return false;
188+
}
189+
return checker.isUndefinedSymbol(symbol) ||
190+
some(skipAlias(symbol, checker).declarations, d => isFunctionLike(d) || hasInitializer(d) && !!d.initializer && isFunctionLike(d.initializer));
191+
}
184192
default:
185193
return false;
186194
}

‎src/testRunner/unittests/services/convertToAsyncFunction.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,7 @@ function [#|f|]():Promise<void | Response> {
539539
}
540540
`
541541
);
542+
542543
_testConvertToAsyncFunction("convertToAsyncFunction_NoRes3", `
543544
function [#|f|]():Promise<void | Response> {
544545
return fetch('https://typescriptlang.org').catch(rej => console.log(rej));
@@ -600,6 +601,7 @@ function [#|f|]():Promise<void> {
600601
}
601602
`
602603
);
604+
603605
_testConvertToAsyncFunction("convertToAsyncFunction_ResRef", `
604606
function [#|f|]():Promise<boolean> {
605607
return fetch('https://typescriptlang.org').then(res);
@@ -609,6 +611,75 @@ function res(result){
609611
}
610612
`
611613
);
614+
615+
_testConvertToAsyncFunction("convertToAsyncFunction_ResRef1", `
616+
class Foo {
617+
public [#|method|](): Promise<boolean> {
618+
return fetch('a').then(this.foo);
619+
}
620+
621+
private foo(res) {
622+
return res.ok;
623+
}
624+
}
625+
`);
626+
627+
_testConvertToAsyncFunction("convertToAsyncFunction_ResRef2", `
628+
class Foo {
629+
public [#|method|](): Promise<Response> {
630+
return fetch('a').then(this.foo);
631+
}
632+
633+
private foo = res => res;
634+
}
635+
`);
636+
637+
_testConvertToAsyncFunction("convertToAsyncFunction_ResRef3", `
638+
const res = (result) => {
639+
return result.ok;
640+
}
641+
function [#|f|](): Promise<boolean> {
642+
return fetch('https://typescriptlang.org').then(res);
643+
}
644+
`
645+
);
646+
647+
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef1", `
648+
const res = 1;
649+
function [#|f|]() {
650+
return fetch('https://typescriptlang.org').then(res);
651+
}
652+
`
653+
);
654+
655+
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef2", `
656+
class Foo {
657+
private foo = 1;
658+
public [#|method|](): Promise<boolean> {
659+
return fetch('a').then(this.foo);
660+
}
661+
}
662+
`
663+
);
664+
665+
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef3", `
666+
const res = undefined;
667+
function [#|f|]() {
668+
return fetch('https://typescriptlang.org').then(res);
669+
}
670+
`
671+
);
672+
673+
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef4", `
674+
class Foo {
675+
private foo = undefined;
676+
public [#|method|](): Promise<boolean> {
677+
return fetch('a').then(this.foo);
678+
}
679+
}
680+
`
681+
);
682+
612683
_testConvertToAsyncFunction("convertToAsyncFunction_ResRefNoReturnVal", `
613684
function [#|f|]():Promise<void> {
614685
return fetch('https://typescriptlang.org').then(res);
@@ -618,6 +689,19 @@ function res(result){
618689
}
619690
`
620691
);
692+
693+
_testConvertToAsyncFunction("convertToAsyncFunction_ResRefNoReturnVal1", `
694+
class Foo {
695+
public [#|method|](): Promise<void> {
696+
return fetch('a').then(this.foo);
697+
}
698+
699+
private foo(res) {
700+
console.log(res);
701+
}
702+
}
703+
`);
704+
621705
_testConvertToAsyncFunction("convertToAsyncFunction_NoBrackets", `
622706
function [#|f|]():Promise<void> {
623707
return fetch('https://typescriptlang.org').then(result => console.log(result));

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef1.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
class Foo {
4+
public /*[#|*/method/*|]*/(): Promise<boolean> {
5+
return fetch('a').then(this.foo);
6+
}
7+
8+
private foo(res) {
9+
return res.ok;
10+
}
11+
}
12+
13+
// ==ASYNC FUNCTION::Convert to async function==
14+
15+
class Foo {
16+
public async method(): Promise<boolean> {
17+
const res = await fetch('a');
18+
return this.foo(res);
19+
}
20+
21+
private foo(res) {
22+
return res.ok;
23+
}
24+
}
25+

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef2.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// ==ORIGINAL==
2+
3+
class Foo {
4+
public /*[#|*/method/*|]*/(): Promise<Response> {
5+
return fetch('a').then(this.foo);
6+
}
7+
8+
private foo = res => res;
9+
}
10+
11+
// ==ASYNC FUNCTION::Convert to async function==
12+
13+
class Foo {
14+
public async method(): Promise<Response> {
15+
const res = await fetch('a');
16+
return this.foo(res);
17+
}
18+
19+
private foo = res => res;
20+
}
21+

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef3.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// ==ORIGINAL==
2+
3+
const res = (result) => {
4+
return result.ok;
5+
}
6+
function /*[#|*/f/*|]*/(): Promise<boolean> {
7+
return fetch('https://typescriptlang.org').then(res);
8+
}
9+
10+
// ==ASYNC FUNCTION::Convert to async function==
11+
12+
const res = (result) => {
13+
return result.ok;
14+
}
15+
async function f(): Promise<boolean> {
16+
const result = await fetch('https://typescriptlang.org');
17+
return res(result);
18+
}
19+

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRefNoReturnVal1.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
class Foo {
4+
public /*[#|*/method/*|]*/(): Promise<void> {
5+
return fetch('a').then(this.foo);
6+
}
7+
8+
private foo(res) {
9+
console.log(res);
10+
}
11+
}
12+
13+
// ==ASYNC FUNCTION::Convert to async function==
14+
15+
class Foo {
16+
public async method(): Promise<void> {
17+
const res = await fetch('a');
18+
return this.foo(res);
19+
}
20+
21+
private foo(res) {
22+
console.log(res);
23+
}
24+
}
25+

0 commit comments

Comments
 (0)