microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-13 16:34:39 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

Merge pull request #26930 from uniqueiniquity/onlyReportExpectedPromiseArgs

Browse Source 
Only perform async code fix if it can successfully refactor all parts
This commit is contained in:
Benjamin Lichtman 2018-09-14 09:41:34 -07:00 committed by GitHub
commit 13deedf841
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 196 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/services/codefixes/convertToAsyncFunction.ts
View File

@ -2,11 +2,13 @@
namespace ts.codefix {
const fixId = "convertToAsyncFunction";
const errorCodes = [Diagnostics.This_may_be_converted_to_an_async_function.code];
let codeActionSucceeded = true;
registerCodeFix({
errorCodes,
getCodeActions(context: CodeFixContext) {
codeActionSucceeded = true;
const changes = textChanges.ChangeTracker.with(context, (t) => convertToAsyncFunction(t, context.sourceFile, context.span.start, context.program.getTypeChecker(), context));
return [createCodeFixAction(fixId, changes, Diagnostics.Convert_to_async_function, fixId, Diagnostics.Convert_all_to_async_functions)];
return codeActionSucceeded ? [createCodeFixAction(fixId, changes, Diagnostics.Convert_to_async_function, fixId, Diagnostics.Convert_all_to_async_functions)] : [];
},
fixIds: [fixId],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, err) => convertToAsyncFunction(changes, err.file, err.start, context.program.getTypeChecker(), context)),
@ -252,6 +254,7 @@ namespace ts.codefix {
}
// 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[] {
if (!node) {
return [];
@ -273,6 +276,7 @@ namespace ts.codefix {
return transformPromiseCall(node, transformer, prevArgName);
}
codeActionSucceeded = false;
return [];
}
@ -381,13 +385,18 @@ namespace ts.codefix {
(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: Node, prevArgName: SynthIdentifier | undefined, argName: SynthIdentifier, parent: CallExpression, transformer: Transformer): NodeArray<Statement> {
const hasPrevArgName = prevArgName && prevArgName.identifier.text.length > 0;
const hasArgName = argName && argName.identifier.text.length > 0;
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(parent).toString());
switch (func.kind) {
case SyntaxKind.NullKeyword:
// do not produce a transformed statement for a null argument
break;
case SyntaxKind.Identifier:
// identifier includes undefined
if (!hasArgName) break;
const synthCall = createCall(getSynthesizedDeepClone(func) as Identifier, /*typeArguments*/ undefined, [argName.identifier]);
@ -443,6 +452,9 @@ namespace ts.codefix {
return createNodeArray([createReturn(getSynthesizedDeepClone(funcBody) as Expression)]);
}
}
default:
// We've found a transformation body we don't know how to handle, so the refactoring should no-op to avoid deleting code.
codeActionSucceeded = false;
break;
}
return createNodeArray([]);
@ -492,14 +504,6 @@ namespace ts.codefix {
return innerCbBody;
}
function hasPropertyAccessExpressionWithName(node: CallExpression, funcName: string): boolean {
if (!isPropertyAccessExpression(node.expression)) {
return false;
}
return node.expression.name.text === funcName;
}
function getArgName(funcNode: Node, transformer: Transformer): SynthIdentifier {
const numberOfAssignmentsOriginal = 0;

39
src/services/suggestionDiagnostics.ts
View File

@ -160,7 +160,7 @@ namespace ts {
}
function addHandlers(returnChild: Node) {
if (isPromiseHandler(returnChild)) {
if (isFixablePromiseHandler(returnChild)) {
returnStatements.push(child as ReturnStatement);
}
}
@ -170,8 +170,39 @@ namespace ts {
return returnStatements;
}
function isPromiseHandler(node: Node): boolean {
return (isCallExpression(node) && isPropertyAccessExpression(node.expression) &&
(node.expression.name.text === "then" || node.expression.name.text === "catch"));
// Should be kept up to date with transformExpression in convertToAsyncFunction.ts
function isFixablePromiseHandler(node: Node): boolean {
// ensure outermost call exists and is a promise handler
if (!isPromiseHandler(node) || !node.arguments.every(isFixablePromiseArgument)) {
return false;
}
// ensure all chained calls are valid
let currentNode = node.expression;
while (isPromiseHandler(currentNode) || isPropertyAccessExpression(currentNode)) {
if (isCallExpression(currentNode) && !currentNode.arguments.every(isFixablePromiseArgument)) {
return false;
}
currentNode = currentNode.expression;
}
return true;
}
function isPromiseHandler(node: Node): node is CallExpression {
return isCallExpression(node) && (hasPropertyAccessExpressionWithName(node, "then") || hasPropertyAccessExpressionWithName(node, "catch"));
}
// should be kept up to date with getTransformationBody in convertToAsyncFunction.ts
function isFixablePromiseArgument(arg: Expression): boolean {
switch (arg.kind) {
case SyntaxKind.NullKeyword:
case SyntaxKind.Identifier: // identifier includes undefined
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
return true;
default:
return false;
}
}
}

8
src/services/utilities.ts
View File

@ -226,6 +226,14 @@ namespace ts {
return undefined;
}
export function hasPropertyAccessExpressionWithName(node: CallExpression, funcName: string): boolean {
if (!isPropertyAccessExpression(node.expression)) {
return false;
}
return node.expression.name.text === funcName;
}
export function isJumpStatementTarget(node: Node): node is Identifier & { parent: BreakOrContinueStatement } {
return node.kind === SyntaxKind.Identifier && isBreakOrContinueStatement(node.parent) && node.parent.label === node;
}

168
src/testRunner/unittests/convertToAsyncFunction.ts
View File

@ -1,68 +1,4 @@
namespace ts {
interface Range {
pos: number;
end: number;
name: string;
}
interface Test {
source: string;
ranges: Map<Range>;
}
function getTest(source: string): Test {
const activeRanges: Range[] = [];
let text = "";
let lastPos = 0;
let pos = 0;
const ranges = createMap<Range>();
while (pos < source.length) {
if (source.charCodeAt(pos) === CharacterCodes.openBracket &&
(source.charCodeAt(pos + 1) === CharacterCodes.hash || source.charCodeAt(pos + 1) === CharacterCodes.$)) {
const saved = pos;
pos += 2;
const s = pos;
consumeIdentifier();
const e = pos;
if (source.charCodeAt(pos) === CharacterCodes.bar) {
pos++;
text += source.substring(lastPos, saved);
const name = s === e
? source.charCodeAt(saved + 1) === CharacterCodes.hash ? "selection" : "extracted"
: source.substring(s, e);
activeRanges.push({ name, pos: text.length, end: undefined! });
lastPos = pos;
continue;
}
else {
pos = saved;
}
}
else if (source.charCodeAt(pos) === CharacterCodes.bar && source.charCodeAt(pos + 1) === CharacterCodes.closeBracket) {
text += source.substring(lastPos, pos);
activeRanges[activeRanges.length - 1].end = text.length;
const range = activeRanges.pop()!;
if (range.name in ranges) {
throw new Error(`Duplicate name of range ${range.name}`);
}
ranges.set(range.name, range);
pos += 2;
lastPos = pos;
continue;
}
pos++;
}
text += source.substring(lastPos, pos);
function consumeIdentifier() {
while (isIdentifierPart(source.charCodeAt(pos), ScriptTarget.Latest)) {
pos++;
}
}
return { source: text, ranges };
}
const libFile: TestFSWithWatch.File = {
path: "/a/lib/lib.d.ts",
content: `/// <reference no-default-lib="true"/>
@ -319,19 +255,22 @@ interface String { charAt: any; }
interface Array<T> {}`
};
function testConvertToAsyncFunction(caption: string, text: string, baselineFolder: string, diagnosticDescription: DiagnosticMessage, codeFixDescription: DiagnosticMessage, includeLib?: boolean) {
const t = getTest(text);
function testConvertToAsyncFunction(caption: string, text: string, baselineFolder: string, includeLib?: boolean, expectFailure = false) {
const t = extractTest(text);
const selectionRange = t.ranges.get("selection")!;
if (!selectionRange) {
throw new Error(`Test ${caption} does not specify selection range`);
}
[Extension.Ts, Extension.Js].forEach(extension =>
const extensions = expectFailure ? [Extension.Ts] : [Extension.Ts, Extension.Js];
extensions.forEach(extension =>
it(`${caption} [${extension}]`, () => runBaseline(extension)));
function runBaseline(extension: Extension) {
const path = "/a" + extension;
const program = makeProgram({ path, content: t.source }, includeLib)!;
const languageService = makeLanguageService({ path, content: t.source }, includeLib);
const program = languageService.getProgram()!;
if (hasSyntacticDiagnostics(program)) {
// Don't bother generating JS baselines for inputs that aren't valid JS.
@ -345,10 +284,6 @@ interface Array<T> {}`
};
const sourceFile = program.getSourceFile(path)!;
const host = projectSystem.createServerHost([f, libFile]);
const projectService = projectSystem.createProjectService(host);
projectService.openClientFile(f.path);
const languageService = projectService.inferredProjects[0].getLanguageService();
const context: CodeFixContext = {
errorCode: 80006,
span: { start: selectionRange.pos, length: selectionRange.end - selectionRange.pos },
@ -361,37 +296,45 @@ interface Array<T> {}`
};
const diagnostics = languageService.getSuggestionDiagnostics(f.path);
const diagnostic = find(diagnostics, diagnostic => diagnostic.messageText === diagnosticDescription.message);
assert.exists(diagnostic);
assert.equal(diagnostic!.start, context.span.start);
assert.equal(diagnostic!.length, context.span.length);
const diagnostic = find(diagnostics, diagnostic => diagnostic.messageText === Diagnostics.This_may_be_converted_to_an_async_function.message &&
diagnostic.start === context.span.start && diagnostic.length === context.span.length);
if (expectFailure) {
assert.isUndefined(diagnostic);
}
else {
assert.exists(diagnostic);
}
const actions = codefix.getFixes(context);
const action = find(actions, action => action.description === codeFixDescription.message)!;
assert.exists(action);
const action = find(actions, action => action.description === Diagnostics.Convert_to_async_function.message);
if (expectFailure) {
assert.isNotTrue(action && action.changes.length > 0);
return;
}
assert.isTrue(action && action.changes.length > 0);
const data: string[] = [];
data.push(`// ==ORIGINAL==`);
data.push(text.replace("[#|", "/*[#|*/").replace("|]", "/*|]*/"));
const changes = action.changes;
const changes = action!.changes;
assert.lengthOf(changes, 1);
data.push(`// ==ASYNC FUNCTION::${action.description}==`);
data.push(`// ==ASYNC FUNCTION::${action!.description}==`);
const newText = textChanges.applyChanges(sourceFile.text, changes[0].textChanges);
data.push(newText);
const diagProgram = makeProgram({ path, content: newText }, includeLib)!;
const diagProgram = makeLanguageService({ path, content: newText }, includeLib).getProgram()!;
assert.isFalse(hasSyntacticDiagnostics(diagProgram));
Harness.Baseline.runBaseline(`${baselineFolder}/${caption}${extension}`, data.join(newLineCharacter));
}
function makeProgram(f: { path: string, content: string }, includeLib?: boolean) {
function makeLanguageService(f: { path: string, content: string }, includeLib?: boolean) {
const host = projectSystem.createServerHost(includeLib ? [f, libFile] : [f]); // libFile is expensive to parse repeatedly - only test when required
const projectService = projectSystem.createProjectService(host);
projectService.openClientFile(f.path);
const program = projectService.inferredProjects[0].getLanguageService().getProgram();
return program;
return projectService.inferredProjects[0].getLanguageService();
}
function hasSyntacticDiagnostics(program: Program) {
@ -400,27 +343,6 @@ interface Array<T> {}`
}
}
function testConvertToAsyncFunctionFailed(caption: string, text: string, description: DiagnosticMessage) {
it(caption, () => {
const t = extractTest(text);
const selectionRange = t.ranges.get("selection");
if (!selectionRange) {
throw new Error(`Test ${caption} does not specify selection range`);
}
const f = {
path: "/a.ts",
content: t.source
};
const host = projectSystem.createServerHost([f, libFile]);
const projectService = projectSystem.createProjectService(host);
projectService.openClientFile(f.path);
const languageService = projectService.inferredProjects[0].getLanguageService();
const actions = languageService.getSuggestionDiagnostics(f.path);
assert.isUndefined(find(actions, action => action.messageText === description.message));
});
}
describe("convertToAsyncFunctions", () => {
_testConvertToAsyncFunction("convertToAsyncFunction_basic", `
function [#|f|](): Promise<void>{
@ -545,6 +467,12 @@ function [#|f|]():Promise<void | Response> {
function [#|f|]():Promise<void | Response> {
return fetch('https://typescriptlang.org').catch(rej => console.log(rej));
}
`
);
_testConvertToAsyncFunction("convertToAsyncFunction_NoRes4", `
function [#|f|]() {
return fetch('https://typescriptlang.org').then(undefined, rejection => console.log("rejected:", rejection));
}
`
);
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestion", `
@ -1157,7 +1085,7 @@ function [#|f|]() {
`
);
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NestedFunction", `
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_NestedFunctionWrongLocation", `
function [#|f|]() {
function fn2(){
function fn3(){
@ -1167,6 +1095,18 @@ function [#|f|]() {
}
return fn2();
}
`);
_testConvertToAsyncFunction("convertToAsyncFunction_NestedFunctionRightLocation", `
function f() {
function fn2(){
function [#|fn3|](){
return fetch("https://typescriptlang.org").then(res => console.log(res));
}
return fn3();
}
return fn2();
}
`);
_testConvertToAsyncFunction("convertToAsyncFunction_UntypedFunction", `
@ -1194,14 +1134,26 @@ const [#|foo|] = function () {
}
`);
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_thenArgumentNotFunction", `
function [#|f|]() {
return Promise.resolve().then(f ? (x => x) : (y => y));
}
`);
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_thenArgumentNotFunctionNotLastInChain", `
function [#|f|]() {
return Promise.resolve().then(f ? (x => x) : (y => y)).then(q => q);
}
`);
});
function _testConvertToAsyncFunction(caption: string, text: string) {
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", Diagnostics.This_may_be_converted_to_an_async_function, Diagnostics.Convert_to_async_function, /*includeLib*/ true);
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true);
}
function _testConvertToAsyncFunctionFailed(caption: string, text: string) {
testConvertToAsyncFunctionFailed(caption, text, Diagnostics.Convert_to_async_function);
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true);
}
}

24
tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NestedFunctionRightLocation.js Normal file
View File

@ -0,0 +1,24 @@
// ==ORIGINAL==
function f() {
function fn2(){
function /*[#|*/fn3/*|]*/(){
return fetch("https://typescriptlang.org").then(res => console.log(res));
}
return fn3();
}
return fn2();
}
// ==ASYNC FUNCTION::Convert to async function==
function f() {
function fn2(){
async function fn3(){
const res = await fetch("https://typescriptlang.org");
return console.log(res);
}
return fn3();
}
return fn2();
}

24
tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NestedFunctionRightLocation.ts Normal file
View File

@ -0,0 +1,24 @@
// ==ORIGINAL==
function f() {
function fn2(){
function /*[#|*/fn3/*|]*/(){
return fetch("https://typescriptlang.org").then(res => console.log(res));
}
return fn3();
}
return fn2();
}
// ==ASYNC FUNCTION::Convert to async function==
function f() {
function fn2(){
async function fn3(){
const res = await fetch("https://typescriptlang.org");
return console.log(res);
}
return fn3();
}
return fn2();
}

16
tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoRes4.js Normal file
View File

@ -0,0 +1,16 @@
// ==ORIGINAL==
function /*[#|*/f/*|]*/() {
return fetch('https://typescriptlang.org').then(undefined, rejection => console.log("rejected:", rejection));
}
// ==ASYNC FUNCTION::Convert to async function==
async function f() {
try {
await fetch('https://typescriptlang.org');
}
catch (rejection) {
return console.log("rejected:", rejection);
}
}

16
tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoRes4.ts Normal file
View File

@ -0,0 +1,16 @@
// ==ORIGINAL==
function /*[#|*/f/*|]*/() {
return fetch('https://typescriptlang.org').then(undefined, rejection => console.log("rejected:", rejection));
}
// ==ASYNC FUNCTION::Convert to async function==
async function f() {
try {
await fetch('https://typescriptlang.org');
}
catch (rejection) {
return console.log("rejected:", rejection);
}
}