Skip to content

Port async fixes #27671

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 3 commits into from
Oct 11, 2018
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
97 changes: 53 additions & 44 deletions src/services/codefixes/convertToAsyncFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@ namespace ts.codefix {
});

interface SynthIdentifier {
identifier: Identifier;
types: Type[];
readonly identifier: Identifier;
readonly types: Type[];
numberOfAssignmentsOriginal: number; // number of times the variable should be assigned in the refactor
}

interface SymbolAndIdentifier {
identifier: Identifier;
symbol: Symbol;
readonly identifier: Identifier;
readonly symbol: Symbol;
}

interface Transformer {
checker: TypeChecker;
synthNamesMap: Map<SynthIdentifier>; // keys are the symbol id of the identifier
allVarNames: SymbolAndIdentifier[];
setOfExpressionsToReturn: Map<true>; // keys are the node ids of the expressions
constIdentifiers: Identifier[];
originalTypeMap: Map<Type>; // keys are the node id of the identifier
isInJSFile: boolean;
readonly checker: TypeChecker;
readonly synthNamesMap: Map<SynthIdentifier>; // keys are the symbol id of the identifier
readonly allVarNames: ReadonlyArray<SymbolAndIdentifier>;
readonly setOfExpressionsToReturn: ReadonlyMap<true>; // keys are the node ids of the expressions
readonly constIdentifiers: Identifier[];
readonly originalTypeMap: ReadonlyMap<Type>; // keys are the node id of the identifier
readonly isInJSFile: boolean;
}

function convertToAsyncFunction(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, checker: TypeChecker, context: CodeFixContextBase): void {
Expand Down Expand Up @@ -61,14 +61,14 @@ namespace ts.codefix {
const functionToConvertRenamed: FunctionLikeDeclaration = renameCollidingVarNames(functionToConvert, checker, synthNamesMap, context, setOfExpressionsToReturn, originalTypeMap, allVarNames);
const constIdentifiers = getConstIdentifiers(synthNamesMap);
const returnStatements = getReturnStatementsWithPromiseHandlers(functionToConvertRenamed);
const transformer = { checker, synthNamesMap, allVarNames, setOfExpressionsToReturn, constIdentifiers, originalTypeMap, isInJSFile: isInJavascript };
const transformer: Transformer = { checker, synthNamesMap, allVarNames, setOfExpressionsToReturn, constIdentifiers, originalTypeMap, isInJSFile: isInJavascript };

if (!returnStatements.length) {
return;
}

// add the async keyword
changes.insertModifierBefore(sourceFile, SyntaxKind.AsyncKeyword, functionToConvert);
changes.insertLastModifierBefore(sourceFile, SyntaxKind.AsyncKeyword, functionToConvert);

function startTransformation(node: CallExpression, nodeToReplace: Node) {
const newNodes = transformExpression(node, transformer, node);
Expand All @@ -88,7 +88,7 @@ namespace ts.codefix {
}

// Returns the identifiers that are never reassigned in the refactor
function getConstIdentifiers(synthNamesMap: Map<SynthIdentifier>): Identifier[] {
function getConstIdentifiers(synthNamesMap: ReadonlyMap<SynthIdentifier>): Identifier[] {
const constIdentifiers: Identifier[] = [];
synthNamesMap.forEach((val) => {
if (val.numberOfAssignmentsOriginal === 0) {
Expand Down Expand Up @@ -249,18 +249,18 @@ namespace ts.codefix {
}
}

function getNewNameIfConflict(name: Identifier, originalNames: Map<Symbol[]>): SynthIdentifier {
const numVarsSameName = (originalNames.get(name.text) || []).length;
function getNewNameIfConflict(name: Identifier, originalNames: ReadonlyMap<Symbol[]>): SynthIdentifier {
const numVarsSameName = (originalNames.get(name.text) || emptyArray).length;
const numberOfAssignmentsOriginal = 0;
const identifier = numVarsSameName === 0 ? name : createIdentifier(name.text + "_" + numVarsSameName);
return { identifier, types: [], numberOfAssignmentsOriginal };
}

// dispatch function to recursively build the refactoring
// should be kept up to date with isFixablePromiseHandler in suggestionDiagnostics.ts
function transformExpression(node: Expression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): Statement[] {
function transformExpression(node: Expression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
if (!node) {
return [];
return emptyArray;
}

const originalType = isIdentifier(node) && transformer.originalTypeMap.get(getNodeId(node).toString());
Expand All @@ -280,10 +280,10 @@ namespace ts.codefix {
}

codeActionSucceeded = false;
return [];
return emptyArray;
}

function transformCatch(node: CallExpression, transformer: Transformer, prevArgName?: SynthIdentifier): Statement[] {
function transformCatch(node: CallExpression, transformer: Transformer, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
const func = node.arguments[0];
const argName = getArgName(func, transformer);
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(node).toString());
Expand Down Expand Up @@ -336,7 +336,7 @@ namespace ts.codefix {
return newSynthName;
}

function transformThen(node: CallExpression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): Statement[] {
function transformThen(node: CallExpression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
const [res, rej] = node.arguments;

if (!res) {
Expand All @@ -356,18 +356,18 @@ namespace ts.codefix {
const catchArg = argNameRej ? argNameRej.identifier.text : "e";
const catchClause = createCatchClause(catchArg, createBlock(transformationBody2));

return [createTry(tryBlock, catchClause, /* finallyBlock */ undefined) as Statement];
return [createTry(tryBlock, catchClause, /* finallyBlock */ undefined)];
}

return transformExpression(node.expression, transformer, node, argNameRes).concat(transformationBody);
}

function getFlagOfIdentifier(node: Identifier, constIdentifiers: Identifier[]): NodeFlags {
function getFlagOfIdentifier(node: Identifier, constIdentifiers: ReadonlyArray<Identifier>): NodeFlags {
const inArr: boolean = constIdentifiers.some(elem => elem.text === node.text);
return inArr ? NodeFlags.Const : NodeFlags.Let;
}

function transformPromiseCall(node: Expression, transformer: Transformer, prevArgName?: SynthIdentifier): Statement[] {
function transformPromiseCall(node: Expression, transformer: Transformer, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(node).toString());
// the identifier is empty when the handler (.then()) ignores the argument - In this situation we do not need to save the result of the promise returning call
const originalNodeParent = node.original ? node.original.parent : node.parent;
Expand All @@ -381,23 +381,23 @@ namespace ts.codefix {
return [createReturn(getSynthesizedDeepClone(node))];
}

function createTransformedStatement(prevArgName: SynthIdentifier | undefined, rightHandSide: Expression, transformer: Transformer): MutableNodeArray<Statement> {
function createTransformedStatement(prevArgName: SynthIdentifier | undefined, rightHandSide: Expression, transformer: Transformer): ReadonlyArray<Statement> {
if (!prevArgName || prevArgName.identifier.text.length === 0) {
// if there's no argName to assign to, there still might be side effects
return createNodeArray([createStatement(rightHandSide)]);
return [createStatement(rightHandSide)];
}

if (prevArgName.types.length < prevArgName.numberOfAssignmentsOriginal) {
// if the variable has already been declared, we don't need "let" or "const"
return createNodeArray([createStatement(createAssignment(getSynthesizedDeepClone(prevArgName.identifier), rightHandSide))]);
return [createStatement(createAssignment(getSynthesizedDeepClone(prevArgName.identifier), rightHandSide))];
}

return createNodeArray([createVariableStatement(/*modifiers*/ undefined,
(createVariableDeclarationList([createVariableDeclaration(getSynthesizedDeepClone(prevArgName.identifier), /*type*/ undefined, rightHandSide)], getFlagOfIdentifier(prevArgName.identifier, transformer.constIdentifiers))))]);
return [createVariableStatement(/*modifiers*/ undefined,
(createVariableDeclarationList([createVariableDeclaration(getSynthesizedDeepClone(prevArgName.identifier), /*type*/ undefined, rightHandSide)], getFlagOfIdentifier(prevArgName.identifier, transformer.constIdentifiers))))];
}

// should be kept up to date with isFixablePromiseArgument in suggestionDiagnostics.ts
function getTransformationBody(func: Expression, prevArgName: SynthIdentifier | undefined, argName: SynthIdentifier | undefined, parent: CallExpression, transformer: Transformer): NodeArray<Statement> {
function getTransformationBody(func: Expression, prevArgName: SynthIdentifier | undefined, argName: SynthIdentifier | undefined, parent: CallExpression, transformer: Transformer): ReadonlyArray<Statement> {

const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(parent).toString());
switch (func.kind) {
Expand All @@ -410,9 +410,9 @@ namespace ts.codefix {
break;
}

const synthCall = createCall(getSynthesizedDeepClone(func) as Identifier, /*typeArguments*/ undefined, argName ? [argName.identifier] : []);
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, argName ? [argName.identifier] : emptyArray);
if (shouldReturn) {
return createNodeArray([createReturn(synthCall)]);
return [createReturn(synthCall)];
}

const type = transformer.originalTypeMap.get(getNodeId(func).toString()) || transformer.checker.getTypeAtLocation(func);
Expand Down Expand Up @@ -450,15 +450,19 @@ namespace ts.codefix {
}
}

return shouldReturn ? getSynthesizedDeepClones(createNodeArray(refactoredStmts)) :
removeReturns(createNodeArray(refactoredStmts), prevArgName!.identifier, transformer, seenReturnStatement);
return shouldReturn ? refactoredStmts.map(s => getSynthesizedDeepClone(s)) :
removeReturns(
refactoredStmts,
prevArgName === undefined ? undefined : prevArgName.identifier,
transformer,
seenReturnStatement);
}
else {
const innerRetStmts = getReturnStatementsWithPromiseHandlers(createReturn(funcBody));
const innerCbBody = getInnerTransformationBody(transformer, innerRetStmts, prevArgName);

if (innerCbBody.length > 0) {
return createNodeArray(innerCbBody);
return innerCbBody;
}

if (!shouldReturn) {
Expand All @@ -473,7 +477,7 @@ namespace ts.codefix {
return transformedStatement;
}
else {
return createNodeArray([createReturn(getSynthesizedDeepClone(funcBody))]);
return [createReturn(getSynthesizedDeepClone(funcBody))];
}
}
}
Expand All @@ -482,7 +486,7 @@ namespace ts.codefix {
codeActionSucceeded = false;
break;
}
return createNodeArray([]);
return emptyArray;
}

function getLastCallSignature(type: Type, checker: TypeChecker): Signature | undefined {
Expand All @@ -491,14 +495,19 @@ namespace ts.codefix {
}


function removeReturns(stmts: NodeArray<Statement>, prevArgName: Identifier, transformer: Transformer, seenReturnStatement: boolean): NodeArray<Statement> {
function removeReturns(stmts: ReadonlyArray<Statement>, prevArgName: Identifier | undefined, transformer: Transformer, seenReturnStatement: boolean): ReadonlyArray<Statement> {
const ret: Statement[] = [];
for (const stmt of stmts) {
if (isReturnStatement(stmt)) {
if (stmt.expression) {
const possiblyAwaitedExpression = isPromiseReturningExpression(stmt.expression, transformer.checker) ? createAwait(stmt.expression) : stmt.expression;
ret.push(createVariableStatement(/*modifiers*/ undefined,
(createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, possiblyAwaitedExpression)], getFlagOfIdentifier(prevArgName, transformer.constIdentifiers)))));
if (prevArgName === undefined) {
ret.push(createExpressionStatement(possiblyAwaitedExpression));
}
else {
ret.push(createVariableStatement(/*modifiers*/ undefined,
(createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, possiblyAwaitedExpression)], getFlagOfIdentifier(prevArgName, transformer.constIdentifiers)))));
}
}
}
else {
Expand All @@ -507,20 +516,20 @@ namespace ts.codefix {
}

// if block has no return statement, need to define prevArgName as undefined to prevent undeclared variables
if (!seenReturnStatement) {
if (!seenReturnStatement && prevArgName !== undefined) {
ret.push(createVariableStatement(/*modifiers*/ undefined,
(createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, createIdentifier("undefined"))], getFlagOfIdentifier(prevArgName, transformer.constIdentifiers)))));
}

return createNodeArray(ret);
return ret;
}


function getInnerTransformationBody(transformer: Transformer, innerRetStmts: Node[], prevArgName?: SynthIdentifier) {
function getInnerTransformationBody(transformer: Transformer, innerRetStmts: ReadonlyArray<Node>, prevArgName?: SynthIdentifier) {

let innerCbBody: Statement[] = [];
for (const stmt of innerRetStmts) {
forEachChild(stmt, function visit(node: Node) {
forEachChild(stmt, function visit(node) {
if (isCallExpression(node)) {
const temp = transformExpression(node, transformer, node, prevArgName);
innerCbBody = innerCbBody.concat(temp);
Expand Down
10 changes: 10 additions & 0 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,16 @@ namespace ts.textChanges {
this.replaceRange(sourceFile, { pos, end: pos }, createToken(modifier), { suffix: " " });
}

public insertLastModifierBefore(sourceFile: SourceFile, modifier: SyntaxKind, before: Node): void {
if (!before.modifiers) {
this.insertModifierBefore(sourceFile, modifier, before);
return;
}

const pos = before.modifiers.end;
this.replaceRange(sourceFile, { pos, end: pos }, createToken(modifier), { prefix: " " });
}

public insertCommentBeforeLine(sourceFile: SourceFile, lineNumber: number, position: number, commentText: string): void {
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
Expand Down
20 changes: 19 additions & 1 deletion src/testRunner/unittests/convertToAsyncFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,24 @@ _testConvertToAsyncFunction("convertToAsyncFunction_nestedPromises", `
function [#|f|]() {
return fetch('https://typescriptlang.org').then(x => Promise.resolve(3).then(y => Promise.resolve(x.statusText.length + y)));
}
`);
_testConvertToAsyncFunction("convertToAsyncFunction_noArgs", `
function delay(millis: number): Promise<void> {
throw "no"
}

function [#|main2|]() {
console.log("Please wait. Loading.");
return delay(500)
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
}
`);
_testConvertToAsyncFunction("convertToAsyncFunction_exportModifier", `
export function [#|foo|]() {
return fetch('https://typescriptlang.org').then(s => console.log(s));
}
`);
});

Expand All @@ -1251,4 +1269,4 @@ function [#|f|]() {
function _testConvertToAsyncFunctionFailed(caption: string, text: string) {
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ==ORIGINAL==

export function /*[#|*/foo/*|]*/() {
return fetch('https://typescriptlang.org').then(s => console.log(s));
}

// ==ASYNC FUNCTION::Convert to async function==

export async function foo() {
const s = await fetch('https://typescriptlang.org');
return console.log(s);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ==ORIGINAL==

export function /*[#|*/foo/*|]*/() {
return fetch('https://typescriptlang.org').then(s => console.log(s));
}

// ==ASYNC FUNCTION::Convert to async function==

export async function foo() {
const s = await fetch('https://typescriptlang.org');
return console.log(s);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// ==ORIGINAL==

function delay(millis) {
throw "no"
}

function /*[#|*/main2/*|]*/() {
console.log("Please wait. Loading.");
return delay(500)
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
}

// ==ASYNC FUNCTION::Convert to async function==

function delay(millis) {
throw "no"
}

async function main2() {
console.log("Please wait. Loading.");
await delay(500);
console.log(".");
await delay(500);
console.log(".");
await delay(500);
console.log(".");
return delay(500);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// ==ORIGINAL==

function delay(millis: number): Promise<void> {
throw "no"
}

function /*[#|*/main2/*|]*/() {
console.log("Please wait. Loading.");
return delay(500)
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
.then(() => { console.log("."); return delay(500); })
}

// ==ASYNC FUNCTION::Convert to async function==

function delay(millis: number): Promise<void> {
throw "no"
}

async function main2() {
console.log("Please wait. Loading.");
await delay(500);
console.log(".");
await delay(500);
console.log(".");
await delay(500);
console.log(".");
return delay(500);
}