From 0612c893cae788a3d6a81eaf7b3c2b6e0c500b4c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 9 Jul 2019 16:30:37 -0700 Subject: [PATCH 01/16] Start prototyping addMissingAwait codefix --- src/compiler/diagnosticMessages.json | 13 ++++ src/services/codefixes/addMissingAwait.ts | 92 +++++++++++++++++++++++ src/services/tsconfig.json | 1 + 3 files changed, 106 insertions(+) create mode 100644 src/services/codefixes/addMissingAwait.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 16b167326942d..8285c7dbcb167 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5087,6 +5087,19 @@ "category": "Message", "code": 95082 }, + "Add 'await'": { + "category": "Message", + "code": 95083 + }, + "Add 'await' to initializer for '{0}'": { + "category": "Message", + "code": 95084 + }, + "Fix all expressions possibly missing 'await'": { + "category": "Message", + "code": 95085 + }, + "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", "code": 18004 diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts new file mode 100644 index 0000000000000..311c2cf482165 --- /dev/null +++ b/src/services/codefixes/addMissingAwait.ts @@ -0,0 +1,92 @@ +/* tslint:disable */ + +/* @internal */ +namespace ts.codefix { + const fixId = "addMissingAwait"; + const errorCodes = [ + Diagnostics.An_arithmetic_operand_must_be_of_type_any_number_bigint_or_an_enum_type.code, + Diagnostics.The_left_hand_side_of_an_arithmetic_operation_must_be_of_type_any_number_bigint_or_an_enum_type.code, + Diagnostics.The_right_hand_side_of_an_arithmetic_operation_must_be_of_type_any_number_bigint_or_an_enum_type.code, + Diagnostics.Operator_0_cannot_be_applied_to_type_1.code, + Diagnostics.Operator_0_cannot_be_applied_to_types_1_and_2.code, + Diagnostics.This_condition_will_always_return_0_since_the_types_1_and_2_have_no_overlap.code, + Diagnostics.Type_0_must_have_a_Symbol_iterator_method_that_returns_an_iterator.code, + Diagnostics.Type_0_must_have_a_Symbol_asyncIterator_method_that_returns_an_async_iterator.code, + Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, + Diagnostics.This_expression_is_not_callable.code, + Diagnostics.This_expression_is_not_constructable.code, + ]; + + registerCodeFix({ + fixIds: [fixId], + errorCodes, + getCodeActions: context => { + const { sourceFile, span, program } = context; + const expression = getAwaitableExpression(sourceFile, span); + if (!expression) { + return; + } + + const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, expression)); + const fixes = [createCodeFixAction(fixId, changes, Diagnostics.Add_await, fixId, Diagnostics.Fix_all_expressions_possibly_missing_await)]; + const awaitableInitializer = findAwaitableInitializer(expression, sourceFile, program); + if (awaitableInitializer) { + const initializerChanges = textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, awaitableInitializer)); + fixes.push(createCodeFixActionNoFixId( + "addMissingAwaitToInitializer", + initializerChanges, + [Diagnostics.Add_await_to_initializer_for_0, expression.getText(sourceFile)])); + } + + return fixes; + }, + }); + + function getAwaitableExpression(sourceFile: SourceFile, span: TextSpan) { + const token = getTokenAtPosition(sourceFile, span.start); + return findAncestor(token, node => { + if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) { + return "quit"; + } + return textSpansEqual(span, createTextSpanFromNode(node, sourceFile)) && isExpressionNode(node); + }); + } + + function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, program: Program): Node | undefined { + if (!isIdentifier(expression)) { + return; + } + + const checker = program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(expression); + if (!symbol) { + return; + } + + const declaration = tryCast(symbol.valueDeclaration, isVariableDeclaration); + const variableName = tryCast(declaration && declaration.name, isIdentifier); + const variableStatement = getAncestor(declaration, SyntaxKind.VariableStatement); + if (!declaration || !variableStatement || + declaration.type || + !declaration.initializer || + variableStatement.getSourceFile() !== sourceFile || + hasModifier(variableStatement, ModifierFlags.Export) || + !variableName) { + return; + } + + const isUsedElsewhere = FindAllReferences.Core.eachSymbolReferenceInFile(variableName, checker, sourceFile, identifier => { + return identifier !== expression; + }); + + if (isUsedElsewhere) { + return; + } + + return declaration.initializer; + } + + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, insertionSite: Node) { + changeTracker.insertModifierBefore(sourceFile, SyntaxKind.AwaitKeyword, insertionSite); + } +} diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 5e6de0f15caed..05c1b0b482ea0 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -45,6 +45,7 @@ "codeFixProvider.ts", "refactorProvider.ts", "codefixes/addConvertToUnknownForNonOverlappingTypes.ts", + "codefixes/addMissingAwait.ts", "codefixes/addMissingConst.ts", "codefixes/addMissingInvocationForDecorator.ts", "codefixes/addNameToNamelessParameter.ts", From 9013ce33060aa8c93b7588ade2a48923de652356 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 9 Jul 2019 16:52:13 -0700 Subject: [PATCH 02/16] Filter by diagnostics that have missing-await related info --- src/services/codefixes/addMissingAwait.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 311c2cf482165..846b56eb6c344 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -1,5 +1,3 @@ -/* tslint:disable */ - /* @internal */ namespace ts.codefix { const fixId = "addMissingAwait"; @@ -23,7 +21,7 @@ namespace ts.codefix { getCodeActions: context => { const { sourceFile, span, program } = context; const expression = getAwaitableExpression(sourceFile, span); - if (!expression) { + if (!expression || !isMissingAwaitError(context)) { return; } @@ -42,6 +40,16 @@ namespace ts.codefix { }, }); + function isMissingAwaitError({ sourceFile, cancellationToken, program, errorCode, span }: CodeFixContext) { + const checker = program.getDiagnosticsProducingTypeChecker(); + const diagnostics = checker.getDiagnostics(sourceFile, cancellationToken); + return some(diagnostics, ({ start, length, relatedInformation, code }) => + isNumber(start) && isNumber(length) && textSpansEqual({ start, length }, span) && + code === errorCode && + !!relatedInformation && + some(relatedInformation, related => related.code === Diagnostics.Did_you_forget_to_use_await.code)); + } + function getAwaitableExpression(sourceFile: SourceFile, span: TextSpan) { const token = getTokenAtPosition(sourceFile, span.start); return findAncestor(token, node => { From 5a5b15feb195e91c15cf86b6e7806f44885b568c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 13:07:39 -0700 Subject: [PATCH 03/16] Start writing tests and checking precedence --- src/services/codefixes/addMissingAwait.ts | 20 ++++++++++----- src/services/textChanges.ts | 4 +++ ...codeFixAddMissingAwait_unaryExpressions.ts | 25 +++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 846b56eb6c344..2e82454091b06 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -1,5 +1,6 @@ /* @internal */ namespace ts.codefix { + const awaitPrecedence = getOperatorPrecedence(SyntaxKind.AwaitExpression, undefined!); const fixId = "addMissingAwait"; const errorCodes = [ Diagnostics.An_arithmetic_operand_must_be_of_type_any_number_bigint_or_an_enum_type.code, @@ -50,17 +51,17 @@ namespace ts.codefix { some(relatedInformation, related => related.code === Diagnostics.Did_you_forget_to_use_await.code)); } - function getAwaitableExpression(sourceFile: SourceFile, span: TextSpan) { + function getAwaitableExpression(sourceFile: SourceFile, span: TextSpan): Expression | undefined { const token = getTokenAtPosition(sourceFile, span.start); return findAncestor(token, node => { if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) { return "quit"; } - return textSpansEqual(span, createTextSpanFromNode(node, sourceFile)) && isExpressionNode(node); - }); + return isExpression(node) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile)); + }) as Expression | undefined; } - function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, program: Program): Node | undefined { + function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, program: Program): Expression | undefined { if (!isIdentifier(expression)) { return; } @@ -94,7 +95,14 @@ namespace ts.codefix { return declaration.initializer; } - function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, insertionSite: Node) { - changeTracker.insertModifierBefore(sourceFile, SyntaxKind.AwaitKeyword, insertionSite); + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, insertionSite: Expression) { + const parentExpression = tryCast(insertionSite.parent, isExpression); + const awaitExpression = createAwait(insertionSite); + changeTracker.replaceRange( + sourceFile, + rangeOfNode(insertionSite), + parentExpression && getExpressionPrecedence(parentExpression) > awaitPrecedence + ? createParen(awaitExpression) + : awaitExpression); } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index f6af03aa2c5cc..f3e5bf9bfb57d 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -704,6 +704,10 @@ namespace ts.textChanges { } } + public parenthesizeExpression(sourceFile: SourceFile, expression: Expression) { + this.replaceRange(sourceFile, rangeOfNode(expression), createParen(expression)); + } + private finishClassesWithNodesInsertedAtStart(): void { this.classesWithNodesInsertedAtStart.forEach(({ node, sourceFile }) => { const [openBraceEnd, closeBraceEnd] = getClassOrObjectBraceEnds(node, sourceFile); diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts b/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts new file mode 100644 index 0000000000000..16fa8d779b391 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts @@ -0,0 +1,25 @@ +/// +////async function fn(x: Promise) { +//// x++; +//// --x; +////} + +verify.codeFix({ + description: "Add 'await'", + index: 0, + newFileContent: +`async function fn(x: Promise) { + (await x)++; + --x; +}` +}); + +verify.codeFix({ + description: "Add 'await'", + index: 0, + newFileContent: +`async function fn(x: Promise) { + (await x)++; + --(await x); +}` +}); From a873d6b788c7e08b17b3520cc19fc53c5e4cd87b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 16:00:18 -0700 Subject: [PATCH 04/16] Implement codeFixAll, add test for binary expressions --- src/services/codefixes/addMissingAwait.ts | 88 +++++++++++++------ ...odeFixAddMissingAwait_binaryExpressions.ts | 50 +++++++++++ ...codeFixAddMissingAwait_unaryExpressions.ts | 25 ------ 3 files changed, 113 insertions(+), 50 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_binaryExpressions.ts delete mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 2e82454091b06..612b89fa2e70c 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -1,5 +1,6 @@ /* @internal */ namespace ts.codefix { + type ContextualTrackChangesFunction = (cb: (changeTracker: textChanges.ChangeTracker) => void) => FileTextChanges[]; const awaitPrecedence = getOperatorPrecedence(SyntaxKind.AwaitExpression, undefined!); const fixId = "addMissingAwait"; const errorCodes = [ @@ -19,29 +20,54 @@ namespace ts.codefix { registerCodeFix({ fixIds: [fixId], errorCodes, - getCodeActions: context => { - const { sourceFile, span, program } = context; - const expression = getAwaitableExpression(sourceFile, span); - if (!expression || !isMissingAwaitError(context)) { - return; - } - - const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, expression)); - const fixes = [createCodeFixAction(fixId, changes, Diagnostics.Add_await, fixId, Diagnostics.Fix_all_expressions_possibly_missing_await)]; - const awaitableInitializer = findAwaitableInitializer(expression, sourceFile, program); - if (awaitableInitializer) { - const initializerChanges = textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, awaitableInitializer)); - fixes.push(createCodeFixActionNoFixId( - "addMissingAwaitToInitializer", - initializerChanges, - [Diagnostics.Add_await_to_initializer_for_0, expression.getText(sourceFile)])); - } - - return fixes; + getCodeActions: getAllPossibleFixesForError, + getAllCodeActions: context => { + const { sourceFile, program, cancellationToken } = context; + const checker = context.program.getTypeChecker(); + return codeFixAll(context, errorCodes, (t, diagnostic) => { + const expression = getAwaitableExpression(sourceFile, diagnostic.code, diagnostic, cancellationToken, program); + if (!expression) { + return; + } + const trackChanges: ContextualTrackChangesFunction = cb => (cb(t), []); + return getDeclarationSiteFix(context, expression, checker, trackChanges) + || getUseSiteFix(context, expression, checker, trackChanges); + }); }, }); - function isMissingAwaitError({ sourceFile, cancellationToken, program, errorCode, span }: CodeFixContext) { + function getAllPossibleFixesForError(context: CodeFixContext) { + const { sourceFile, errorCode, span, cancellationToken, program } = context; + const expression = getAwaitableExpression(sourceFile, errorCode, span, cancellationToken, program); + if (!expression) { + return; + } + + const checker = context.program.getTypeChecker(); + const trackChanges: ContextualTrackChangesFunction = cb => textChanges.ChangeTracker.with(context, cb); + return compact([ + getDeclarationSiteFix(context, expression, checker, trackChanges), + getUseSiteFix(context, expression, checker, trackChanges)]); + } + + function getDeclarationSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { + const { sourceFile } = context; + const awaitableInitializer = findAwaitableInitializer(expression, sourceFile, checker); + if (awaitableInitializer) { + const initializerChanges = trackChanges(t => makeChange(t, sourceFile, checker, awaitableInitializer)); + return createCodeFixActionNoFixId( + "addMissingAwaitToInitializer", + initializerChanges, + [Diagnostics.Add_await_to_initializer_for_0, expression.getText(sourceFile)]); + } + } + + function getUseSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { + const changes = trackChanges(t => makeChange(t, context.sourceFile, checker, expression)); + return createCodeFixAction(fixId, changes, Diagnostics.Add_await, fixId, Diagnostics.Fix_all_expressions_possibly_missing_await); + } + + function isMissingAwaitError(sourceFile: SourceFile, errorCode: number, span: TextSpan, cancellationToken: CancellationToken, program: Program) { const checker = program.getDiagnosticsProducingTypeChecker(); const diagnostics = checker.getDiagnostics(sourceFile, cancellationToken); return some(diagnostics, ({ start, length, relatedInformation, code }) => @@ -51,22 +77,23 @@ namespace ts.codefix { some(relatedInformation, related => related.code === Diagnostics.Did_you_forget_to_use_await.code)); } - function getAwaitableExpression(sourceFile: SourceFile, span: TextSpan): Expression | undefined { + function getAwaitableExpression(sourceFile: SourceFile, errorCode: number, span: TextSpan, cancellationToken: CancellationToken, program: Program): Expression | undefined { const token = getTokenAtPosition(sourceFile, span.start); - return findAncestor(token, node => { + const expression = findAncestor(token, node => { if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) { return "quit"; } return isExpression(node) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile)); }) as Expression | undefined; + + return isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program) ? expression : undefined; } - function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, program: Program): Expression | undefined { + function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, checker: TypeChecker): Expression | undefined { if (!isIdentifier(expression)) { return; } - const checker = program.getTypeChecker(); const symbol = checker.getSymbolAtLocation(expression); if (!symbol) { return; @@ -95,7 +122,18 @@ namespace ts.codefix { return declaration.initializer; } - function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, insertionSite: Expression) { + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, checker: TypeChecker, insertionSite: Expression) { + if (isBinaryExpression(insertionSite)) { + const { left, right } = insertionSite; + const leftType = checker.getTypeAtLocation(left); + const rightType = checker.getTypeAtLocation(right); + const newLeft = checker.getPromisedTypeOfPromise(leftType) ? createAwait(left) : left; + const newRight = checker.getPromisedTypeOfPromise(rightType) ? createAwait(right) : right; + changeTracker.replaceNode(sourceFile, left, newLeft); + changeTracker.replaceNode(sourceFile, right, newRight); + return; + } + const parentExpression = tryCast(insertionSite.parent, isExpression); const awaitExpression = createAwait(insertionSite); changeTracker.replaceRange( diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_binaryExpressions.ts b/tests/cases/fourslash/codeFixAddMissingAwait_binaryExpressions.ts new file mode 100644 index 0000000000000..eae2b0bdf363c --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_binaryExpressions.ts @@ -0,0 +1,50 @@ +/// +////async function fn(a: Promise, b: number) { +//// a | b; +//// b + a; +//// a + a; +////} + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 0, + newFileContent: +`async function fn(a: Promise, b: number) { + await a | b; + b + a; + a + a; +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 1, + newFileContent: +`async function fn(a: Promise, b: number) { + a | b; + b + await a; + a + a; +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 2, + newFileContent: +`async function fn(a: Promise, b: number) { + a | b; + b + a; + await a + await a; +}` +}); + +verify.codeFixAll({ + fixAllDescription: ts.Diagnostics.Fix_all_expressions_possibly_missing_await.message, + fixId: "addMissingAwait", + newFileContent: +`async function fn(a: Promise, b: number) { + await a | b; + b + await a; + await a + await a; +}` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts b/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts deleted file mode 100644 index 16fa8d779b391..0000000000000 --- a/tests/cases/fourslash/codeFixAddMissingAwait_unaryExpressions.ts +++ /dev/null @@ -1,25 +0,0 @@ -/// -////async function fn(x: Promise) { -//// x++; -//// --x; -////} - -verify.codeFix({ - description: "Add 'await'", - index: 0, - newFileContent: -`async function fn(x: Promise) { - (await x)++; - --x; -}` -}); - -verify.codeFix({ - description: "Add 'await'", - index: 0, - newFileContent: -`async function fn(x: Promise) { - (await x)++; - --(await x); -}` -}); From c692209b285a674db9ac230c520c91e8b0a1f947 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 16:57:24 -0700 Subject: [PATCH 05/16] Add test for iterables --- src/services/codefixes/addMissingAwait.ts | 5 ++ .../codeFixAddMissingAwait_iterables.ts | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_iterables.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 612b89fa2e70c..6332bb5d501b7 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -10,6 +10,11 @@ namespace ts.codefix { Diagnostics.Operator_0_cannot_be_applied_to_type_1.code, Diagnostics.Operator_0_cannot_be_applied_to_types_1_and_2.code, Diagnostics.This_condition_will_always_return_0_since_the_types_1_and_2_have_no_overlap.code, + Diagnostics.Type_0_is_not_an_array_type.code, + Diagnostics.Type_0_is_not_an_array_type_or_a_string_type.code, + Diagnostics.Type_0_is_not_an_array_type_or_a_string_type_Use_compiler_option_downlevelIteration_to_allow_iterating_of_iterators.code, + Diagnostics.Type_0_is_not_an_array_type_or_a_string_type_or_does_not_have_a_Symbol_iterator_method_that_returns_an_iterator.code, + Diagnostics.Type_0_is_not_an_array_type_or_does_not_have_a_Symbol_iterator_method_that_returns_an_iterator.code, Diagnostics.Type_0_must_have_a_Symbol_iterator_method_that_returns_an_iterator.code, Diagnostics.Type_0_must_have_a_Symbol_asyncIterator_method_that_returns_an_async_iterator.code, Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_iterables.ts b/tests/cases/fourslash/codeFixAddMissingAwait_iterables.ts new file mode 100644 index 0000000000000..80ca234e815ec --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_iterables.ts @@ -0,0 +1,50 @@ +/// +////async function fn(a: Promise) { +//// [...a]; +//// for (const c of a) { c; } +//// for await (const c of a) { c; } +////} + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 0, + newFileContent: +`async function fn(a: Promise) { + [...await a]; + for (const c of a) { c; } + for await (const c of a) { c; } +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 1, + newFileContent: +`async function fn(a: Promise) { + [...a]; + for (const c of await a) { c; } + for await (const c of a) { c; } +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 2, + newFileContent: +`async function fn(a: Promise) { + [...a]; + for (const c of a) { c; } + for await (const c of await a) { c; } +}` +}); + +verify.codeFixAll({ + fixAllDescription: ts.Diagnostics.Fix_all_expressions_possibly_missing_await.message, + fixId: "addMissingAwait", + newFileContent: +`async function fn(a: Promise) { + [...await a]; + for (const c of await a) { c; } + for await (const c of await a) { c; } +}` +}); From 8237f1208fea666530f8431c99fb6297dc99f442 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 22:14:23 -0700 Subject: [PATCH 06/16] Add test for passing argument --- src/services/codefixes/addMissingAwait.ts | 10 +--------- .../fourslash/codeFixAddMissingAwait_argument.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_argument.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 6332bb5d501b7..2383c2351c41e 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -1,7 +1,6 @@ /* @internal */ namespace ts.codefix { type ContextualTrackChangesFunction = (cb: (changeTracker: textChanges.ChangeTracker) => void) => FileTextChanges[]; - const awaitPrecedence = getOperatorPrecedence(SyntaxKind.AwaitExpression, undefined!); const fixId = "addMissingAwait"; const errorCodes = [ Diagnostics.An_arithmetic_operand_must_be_of_type_any_number_bigint_or_an_enum_type.code, @@ -139,13 +138,6 @@ namespace ts.codefix { return; } - const parentExpression = tryCast(insertionSite.parent, isExpression); - const awaitExpression = createAwait(insertionSite); - changeTracker.replaceRange( - sourceFile, - rangeOfNode(insertionSite), - parentExpression && getExpressionPrecedence(parentExpression) > awaitPrecedence - ? createParen(awaitExpression) - : awaitExpression); + changeTracker.replaceNode(sourceFile, insertionSite, createAwait(insertionSite)); } } diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_argument.ts b/tests/cases/fourslash/codeFixAddMissingAwait_argument.ts new file mode 100644 index 0000000000000..600e1338080f2 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_argument.ts @@ -0,0 +1,13 @@ +/// +////async function fn(a: Promise, b: string) { +//// fn(a, a); +////} + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 0, + newFileContent: +`async function fn(a: Promise, b: string) { + fn(a, await a); +}` +}); From 7be4e0aeda62537499dda27f5cb884ba1a8343b8 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 22:26:09 -0700 Subject: [PATCH 07/16] Add test for call/construct signatures --- src/services/codefixes/addMissingAwait.ts | 34 +++++++------ .../codeFixAddMissingAwait_signatures.ts | 50 +++++++++++++++++++ 2 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_signatures.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 2383c2351c41e..50a10c52ad62b 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -2,6 +2,10 @@ namespace ts.codefix { type ContextualTrackChangesFunction = (cb: (changeTracker: textChanges.ChangeTracker) => void) => FileTextChanges[]; const fixId = "addMissingAwait"; + const callableConstructableErrorCodes = [ + Diagnostics.This_expression_is_not_callable.code, + Diagnostics.This_expression_is_not_constructable.code, + ]; const errorCodes = [ Diagnostics.An_arithmetic_operand_must_be_of_type_any_number_bigint_or_an_enum_type.code, Diagnostics.The_left_hand_side_of_an_arithmetic_operation_must_be_of_type_any_number_bigint_or_an_enum_type.code, @@ -17,8 +21,7 @@ namespace ts.codefix { Diagnostics.Type_0_must_have_a_Symbol_iterator_method_that_returns_an_iterator.code, Diagnostics.Type_0_must_have_a_Symbol_asyncIterator_method_that_returns_an_async_iterator.code, Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, - Diagnostics.This_expression_is_not_callable.code, - Diagnostics.This_expression_is_not_constructable.code, + ...callableConstructableErrorCodes, ]; registerCodeFix({ @@ -34,8 +37,8 @@ namespace ts.codefix { return; } const trackChanges: ContextualTrackChangesFunction = cb => (cb(t), []); - return getDeclarationSiteFix(context, expression, checker, trackChanges) - || getUseSiteFix(context, expression, checker, trackChanges); + return getDeclarationSiteFix(context, expression, diagnostic.code, checker, trackChanges) + || getUseSiteFix(context, expression, diagnostic.code, checker, trackChanges); }); }, }); @@ -50,15 +53,15 @@ namespace ts.codefix { const checker = context.program.getTypeChecker(); const trackChanges: ContextualTrackChangesFunction = cb => textChanges.ChangeTracker.with(context, cb); return compact([ - getDeclarationSiteFix(context, expression, checker, trackChanges), - getUseSiteFix(context, expression, checker, trackChanges)]); + getDeclarationSiteFix(context, expression, errorCode, checker, trackChanges), + getUseSiteFix(context, expression, errorCode, checker, trackChanges)]); } - function getDeclarationSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { + function getDeclarationSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, errorCode: number, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { const { sourceFile } = context; const awaitableInitializer = findAwaitableInitializer(expression, sourceFile, checker); if (awaitableInitializer) { - const initializerChanges = trackChanges(t => makeChange(t, sourceFile, checker, awaitableInitializer)); + const initializerChanges = trackChanges(t => makeChange(t, errorCode, sourceFile, checker, awaitableInitializer)); return createCodeFixActionNoFixId( "addMissingAwaitToInitializer", initializerChanges, @@ -66,8 +69,8 @@ namespace ts.codefix { } } - function getUseSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { - const changes = trackChanges(t => makeChange(t, context.sourceFile, checker, expression)); + function getUseSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, errorCode: number, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { + const changes = trackChanges(t => makeChange(t, errorCode, context.sourceFile, checker, expression)); return createCodeFixAction(fixId, changes, Diagnostics.Add_await, fixId, Diagnostics.Fix_all_expressions_possibly_missing_await); } @@ -126,7 +129,7 @@ namespace ts.codefix { return declaration.initializer; } - function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, checker: TypeChecker, insertionSite: Expression) { + function makeChange(changeTracker: textChanges.ChangeTracker, errorCode: number, sourceFile: SourceFile, checker: TypeChecker, insertionSite: Expression) { if (isBinaryExpression(insertionSite)) { const { left, right } = insertionSite; const leftType = checker.getTypeAtLocation(left); @@ -135,9 +138,12 @@ namespace ts.codefix { const newRight = checker.getPromisedTypeOfPromise(rightType) ? createAwait(right) : right; changeTracker.replaceNode(sourceFile, left, newLeft); changeTracker.replaceNode(sourceFile, right, newRight); - return; } - - changeTracker.replaceNode(sourceFile, insertionSite, createAwait(insertionSite)); + else if (contains(callableConstructableErrorCodes, errorCode) && isCallOrNewExpression(insertionSite.parent)) { + changeTracker.replaceNode(sourceFile, insertionSite, createParen(createAwait(insertionSite))); + } + else { + changeTracker.replaceNode(sourceFile, insertionSite, createAwait(insertionSite)); + } } } diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_signatures.ts b/tests/cases/fourslash/codeFixAddMissingAwait_signatures.ts new file mode 100644 index 0000000000000..f4cfda8480774 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_signatures.ts @@ -0,0 +1,50 @@ +/// +////async function fn(a: Promise<() => void>, b: Promise<() => void> | (() => void), C: Promise<{ new(): any }>) { +//// a(); +//// b(); +//// new C(); +////} + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 0, + newFileContent: +`async function fn(a: Promise<() => void>, b: Promise<() => void> | (() => void), C: Promise<{ new(): any }>) { + (await a)(); + b(); + new C(); +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 1, + newFileContent: +`async function fn(a: Promise<() => void>, b: Promise<() => void> | (() => void), C: Promise<{ new(): any }>) { + a(); + (await b)(); + new C(); +}` +}); + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 2, + newFileContent: +`async function fn(a: Promise<() => void>, b: Promise<() => void> | (() => void), C: Promise<{ new(): any }>) { + a(); + b(); + new (await C)(); +}` +}); + +verify.codeFixAll({ + fixAllDescription: ts.Diagnostics.Fix_all_expressions_possibly_missing_await.message, + fixId: "addMissingAwait", + newFileContent: +`async function fn(a: Promise<() => void>, b: Promise<() => void> | (() => void), C: Promise<{ new(): any }>) { + (await a)(); + (await b)(); + new (await C)(); +}` +}); From 01e2e53a319615076b2b67a04525aaca9686c045 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 22:42:50 -0700 Subject: [PATCH 08/16] Add test for awaiting initializer --- .../codeFixAddMissingAwait_initializer.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_initializer.ts diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_initializer.ts b/tests/cases/fourslash/codeFixAddMissingAwait_initializer.ts new file mode 100644 index 0000000000000..7c86b140edae5 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_initializer.ts @@ -0,0 +1,28 @@ +/// +////async function fn(a: string, b: Promise) { +//// const x = b; +//// fn(x, b); +//// fn(b, b); +////} + +verify.codeFix({ + description: "Add 'await' to initializer for 'x'", + index: 0, + newFileContent: +`async function fn(a: string, b: Promise) { + const x = await b; + fn(x, b); + fn(b, b); +}` +}); + +verify.codeFixAll({ + fixAllDescription: ts.Diagnostics.Fix_all_expressions_possibly_missing_await.message, + fixId: "addMissingAwait", + newFileContent: +`async function fn(a: string, b: Promise) { + const x = await b; + fn(x, b); + fn(await b, b); +}` +}); From fce502dc56275c47990d0f71a05627991c4c1eb3 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 10 Jul 2019 22:46:44 -0700 Subject: [PATCH 09/16] Improve assertion error --- src/harness/fourslash.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index ffc0759e92ebf..eee7a4e72e1f0 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2829,7 +2829,7 @@ Actual: ${stringify(fullActual)}`); } public verifyCodeFixAvailable(negative: boolean, expected: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined): void { - assert(!negative || !expected); + assert(!(negative && expected), "Cannot provide an expected code fix with a negated assertion"); const codeFixes = this.getCodeFixes(this.activeFile.fileName); const actuals = codeFixes.map((fix): FourSlashInterface.VerifyCodeFixAvailableOptions => ({ description: fix.description, commands: fix.commands })); this.assertObjectsEqual(actuals, negative ? ts.emptyArray : expected); From f160336b1136ac262b4fc8a351ecd1d811c48a2f Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 09:58:47 -0700 Subject: [PATCH 10/16] Replace specific property access error with general one and add await related info --- src/compiler/checker.ts | 3 ++- src/compiler/diagnosticMessages.json | 4 ---- src/services/codefixes/addMissingAwait.ts | 1 + .../reference/operationsAvailableOnPromisedType.errors.txt | 5 +++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 978f841a29220..de3ee545fce9e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20714,7 +20714,8 @@ namespace ts { else { const promisedType = getPromisedTypeOfPromise(containingType); if (promisedType && getPropertyOfType(promisedType, propNode.escapedText)) { - errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_forget_to_use_await, declarationNameToString(propNode), typeToString(containingType)); + errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1, declarationNameToString(propNode), typeToString(containingType)); + relatedInfo = createDiagnosticForNode(propNode, Diagnostics.Did_you_forget_to_use_await); } else { const suggestion = getSuggestedSymbolForNonexistentProperty(propNode, containingType); diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 8285c7dbcb167..7fc2fdd4237e1 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -2076,10 +2076,6 @@ "category": "Error", "code": 2569 }, - "Property '{0}' does not exist on type '{1}'. Did you forget to use 'await'?": { - "category": "Error", - "code": 2570 - }, "Object is of type 'unknown'.": { "category": "Error", "code": 2571 diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 50a10c52ad62b..102d058da0448 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -21,6 +21,7 @@ namespace ts.codefix { Diagnostics.Type_0_must_have_a_Symbol_iterator_method_that_returns_an_iterator.code, Diagnostics.Type_0_must_have_a_Symbol_asyncIterator_method_that_returns_an_async_iterator.code, Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, + Diagnostics.Property_0_does_not_exist_on_type_1.code, ...callableConstructableErrorCodes, ]; diff --git a/tests/baselines/reference/operationsAvailableOnPromisedType.errors.txt b/tests/baselines/reference/operationsAvailableOnPromisedType.errors.txt index 2378274121554..ffa16497ad2eb 100644 --- a/tests/baselines/reference/operationsAvailableOnPromisedType.errors.txt +++ b/tests/baselines/reference/operationsAvailableOnPromisedType.errors.txt @@ -8,7 +8,7 @@ tests/cases/compiler/operationsAvailableOnPromisedType.ts(17,5): error TS2367: T tests/cases/compiler/operationsAvailableOnPromisedType.ts(18,9): error TS2461: Type 'Promise' is not an array type. tests/cases/compiler/operationsAvailableOnPromisedType.ts(19,21): error TS2495: Type 'Promise' is not an array type or a string type. tests/cases/compiler/operationsAvailableOnPromisedType.ts(20,12): error TS2345: Argument of type 'Promise' is not assignable to parameter of type 'number'. -tests/cases/compiler/operationsAvailableOnPromisedType.ts(21,11): error TS2570: Property 'prop' does not exist on type 'Promise<{ prop: string; }>'. Did you forget to use 'await'? +tests/cases/compiler/operationsAvailableOnPromisedType.ts(21,11): error TS2339: Property 'prop' does not exist on type 'Promise<{ prop: string; }>'. tests/cases/compiler/operationsAvailableOnPromisedType.ts(23,27): error TS2495: Type 'Promise' is not an array type or a string type. tests/cases/compiler/operationsAvailableOnPromisedType.ts(24,5): error TS2349: This expression is not callable. Type 'Promise<() => void>' has no call signatures. @@ -72,7 +72,8 @@ tests/cases/compiler/operationsAvailableOnPromisedType.ts(27,5): error TS2349: T !!! related TS2773 tests/cases/compiler/operationsAvailableOnPromisedType.ts:20:12: Did you forget to use 'await'? d.prop; ~~~~ -!!! error TS2570: Property 'prop' does not exist on type 'Promise<{ prop: string; }>'. Did you forget to use 'await'? +!!! error TS2339: Property 'prop' does not exist on type 'Promise<{ prop: string; }>'. +!!! related TS2773 tests/cases/compiler/operationsAvailableOnPromisedType.ts:21:11: Did you forget to use 'await'? } for await (const s of c) {} ~ From 4643d273c47834f2723c7028d640fee13186f2b1 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 10:05:02 -0700 Subject: [PATCH 11/16] Add test for property access --- src/services/codefixes/addMissingAwait.ts | 9 ++++++++- .../codeFixAddMissingAwait_propertyAccess.ts | 13 +++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_propertyAccess.ts diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 102d058da0448..24b7a9b3a3317 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -2,6 +2,7 @@ namespace ts.codefix { type ContextualTrackChangesFunction = (cb: (changeTracker: textChanges.ChangeTracker) => void) => FileTextChanges[]; const fixId = "addMissingAwait"; + const propertyAccessCode = Diagnostics.Property_0_does_not_exist_on_type_1.code; const callableConstructableErrorCodes = [ Diagnostics.This_expression_is_not_callable.code, Diagnostics.This_expression_is_not_constructable.code, @@ -21,7 +22,7 @@ namespace ts.codefix { Diagnostics.Type_0_must_have_a_Symbol_iterator_method_that_returns_an_iterator.code, Diagnostics.Type_0_must_have_a_Symbol_asyncIterator_method_that_returns_an_async_iterator.code, Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, - Diagnostics.Property_0_does_not_exist_on_type_1.code, + propertyAccessCode, ...callableConstructableErrorCodes, ]; @@ -140,6 +141,12 @@ namespace ts.codefix { changeTracker.replaceNode(sourceFile, left, newLeft); changeTracker.replaceNode(sourceFile, right, newRight); } + else if (errorCode === propertyAccessCode && isPropertyAccessExpression(insertionSite.parent)) { + changeTracker.replaceNode( + sourceFile, + insertionSite.parent.expression, + createParen(createAwait(insertionSite.parent.expression))); + } else if (contains(callableConstructableErrorCodes, errorCode) && isCallOrNewExpression(insertionSite.parent)) { changeTracker.replaceNode(sourceFile, insertionSite, createParen(createAwait(insertionSite))); } diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_propertyAccess.ts b/tests/cases/fourslash/codeFixAddMissingAwait_propertyAccess.ts new file mode 100644 index 0000000000000..fd1b7d65779bb --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_propertyAccess.ts @@ -0,0 +1,13 @@ +/// +////async function fn(a: Promise<{ x: string }>) { +//// a.x; +////} + +verify.codeFix({ + description: ts.Diagnostics.Add_await.message, + index: 0, + newFileContent: +`async function fn(a: Promise<{ x: string }>) { + (await a).x; +}` +}); From 355469e57055147ad8fa93ec5e81e42001c2c4d3 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 12:06:42 -0700 Subject: [PATCH 12/16] Require code to be inside a function body to offer await --- src/harness/fourslash.ts | 25 ++++++++++++++++--- src/services/codefixes/addMissingAwait.ts | 22 ++++++++++++++-- .../codeFixAddMissingAwait_topLevel.ts | 10 ++++++++ tests/cases/fourslash/fourslash.ts | 2 +- 4 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_topLevel.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index eee7a4e72e1f0..0febfab11e259 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2828,11 +2828,28 @@ Actual: ${stringify(fullActual)}`); } } - public verifyCodeFixAvailable(negative: boolean, expected: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined): void { - assert(!(negative && expected), "Cannot provide an expected code fix with a negated assertion"); + public verifyCodeFixAvailable(negative: boolean, expected: FourSlashInterface.VerifyCodeFixAvailableOptions[] | string | undefined): void { const codeFixes = this.getCodeFixes(this.activeFile.fileName); - const actuals = codeFixes.map((fix): FourSlashInterface.VerifyCodeFixAvailableOptions => ({ description: fix.description, commands: fix.commands })); - this.assertObjectsEqual(actuals, negative ? ts.emptyArray : expected); + if (negative) { + if (typeof expected === "undefined") { + this.assertObjectsEqual(codeFixes, ts.emptyArray); + } + else if (typeof expected === "string") { + if (codeFixes.some(fix => fix.fixName === expected)) { + this.raiseError(`Expected not to find a fix with the name '${expected}', but one exists.`); + } + } + else { + assert(typeof expected === "undefined" || typeof expected === "string", "With a negated assertion, 'expected' must be undefined or a string value of a codefix name."); + } + } + else if (typeof expected === "string") { + this.assertObjectsEqual(codeFixes.map(({ fixName }) => fixName), [expected]); + } + else { + const actuals = codeFixes.map((fix): FourSlashInterface.VerifyCodeFixAvailableOptions => ({ description: fix.description, commands: fix.commands })); + this.assertObjectsEqual(actuals, negative ? ts.emptyArray : expected); + } } public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) { diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 24b7a9b3a3317..187a84b4d0da5 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -88,6 +88,9 @@ namespace ts.codefix { function getAwaitableExpression(sourceFile: SourceFile, errorCode: number, span: TextSpan, cancellationToken: CancellationToken, program: Program): Expression | undefined { const token = getTokenAtPosition(sourceFile, span.start); + // Checker has already done work to determine that await might be possible, and has attached + // related info to the node, so start by finding the expression that exactly matches up + // with the diagnostic range. const expression = findAncestor(token, node => { if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) { return "quit"; @@ -95,7 +98,11 @@ namespace ts.codefix { return isExpression(node) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile)); }) as Expression | undefined; - return isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program) ? expression : undefined; + return expression + && isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program) + && isInsideAwaitableBody(expression) + ? expression + : undefined; } function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, checker: TypeChecker): Expression | undefined { @@ -116,7 +123,8 @@ namespace ts.codefix { !declaration.initializer || variableStatement.getSourceFile() !== sourceFile || hasModifier(variableStatement, ModifierFlags.Export) || - !variableName) { + !variableName || + !isInsideAwaitableBody(declaration.initializer)) { return; } @@ -131,6 +139,16 @@ namespace ts.codefix { return declaration.initializer; } + function isInsideAwaitableBody(node: Node) { + return !!findAncestor(node, ancestor => + ancestor.parent && isArrowFunction(ancestor.parent) && ancestor.parent.body === ancestor || + isBlock(ancestor) && ( + ancestor.parent.kind === SyntaxKind.FunctionDeclaration || + ancestor.parent.kind === SyntaxKind.FunctionExpression || + ancestor.parent.kind === SyntaxKind.ArrowFunction || + ancestor.parent.kind === SyntaxKind.MethodDeclaration)); + } + function makeChange(changeTracker: textChanges.ChangeTracker, errorCode: number, sourceFile: SourceFile, checker: TypeChecker, insertionSite: Expression) { if (isBinaryExpression(insertionSite)) { const { left, right } = insertionSite; diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_topLevel.ts b/tests/cases/fourslash/codeFixAddMissingAwait_topLevel.ts new file mode 100644 index 0000000000000..9f524b90d7c7e --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_topLevel.ts @@ -0,0 +1,10 @@ +/// +////declare function getPromise(): Promise; +////const p = getPromise(); +////while (true) { +//// p/*0*/.toLowerCase(); +//// getPromise()/*1*/.toLowerCase(); +////} + +verify.not.codeFixAvailable("addMissingAwait"); +verify.not.codeFixAvailable("addMissingAwaitToInitializer"); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 05799a156420a..d74523ea7fd67 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -185,7 +185,7 @@ declare namespace FourSlashInterface { applyChanges?: boolean, commands?: {}[], }); - codeFixAvailable(options?: ReadonlyArray): void; + codeFixAvailable(options?: ReadonlyArray | string): void; applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; From 9e5f261951afd5a89269bc3ee48dc7f2e3bc132e Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 14:46:45 -0700 Subject: [PATCH 13/16] Accept suggestion Co-Authored-By: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> --- src/harness/fourslash.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 0febfab11e259..5d5338db1f433 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2844,7 +2844,7 @@ Actual: ${stringify(fullActual)}`); } } else if (typeof expected === "string") { - this.assertObjectsEqual(codeFixes.map(({ fixName }) => fixName), [expected]); + this.assertObjectsEqual(codeFixes.map(fix => fix.fixName), [expected]); } else { const actuals = codeFixes.map((fix): FourSlashInterface.VerifyCodeFixAvailableOptions => ({ description: fix.description, commands: fix.commands })); From 3b90f5b3633f3836cdb83f8a1ba3b429e90eef63 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 14:54:27 -0700 Subject: [PATCH 14/16] Add explicit test for code fix being not available unless something is a Promise --- .../codeFixAddMissingAwait_notAvailableWithoutPromise.ts | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/cases/fourslash/codeFixAddMissingAwait_notAvailableWithoutPromise.ts diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_notAvailableWithoutPromise.ts b/tests/cases/fourslash/codeFixAddMissingAwait_notAvailableWithoutPromise.ts new file mode 100644 index 0000000000000..c2ae62fcab073 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingAwait_notAvailableWithoutPromise.ts @@ -0,0 +1,6 @@ +/// +////async function fn(a: {}, b: number) { +//// a + b; +////} + +verify.not.codeFixAvailable("addMissingAwait"); From 657daa496aa5daa993de7322f95b37336f28058b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 15:00:46 -0700 Subject: [PATCH 15/16] Skip looking for function body if already in AwaitContext flags --- src/services/codefixes/addMissingAwait.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 187a84b4d0da5..3890084dc61c7 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -140,7 +140,7 @@ namespace ts.codefix { } function isInsideAwaitableBody(node: Node) { - return !!findAncestor(node, ancestor => + return node.kind & NodeFlags.AwaitContext || !!findAncestor(node, ancestor => ancestor.parent && isArrowFunction(ancestor.parent) && ancestor.parent.body === ancestor || isBlock(ancestor) && ( ancestor.parent.kind === SyntaxKind.FunctionDeclaration || From 09853bec9b2e42e00e2359bff79ec1b30c1ab55d Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 15:02:35 -0700 Subject: [PATCH 16/16] Inline getCodeActions function for symmetry --- src/services/codefixes/addMissingAwait.ts | 28 +++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/services/codefixes/addMissingAwait.ts b/src/services/codefixes/addMissingAwait.ts index 3890084dc61c7..3545c65faf9a0 100644 --- a/src/services/codefixes/addMissingAwait.ts +++ b/src/services/codefixes/addMissingAwait.ts @@ -29,7 +29,19 @@ namespace ts.codefix { registerCodeFix({ fixIds: [fixId], errorCodes, - getCodeActions: getAllPossibleFixesForError, + getCodeActions: context => { + const { sourceFile, errorCode, span, cancellationToken, program } = context; + const expression = getAwaitableExpression(sourceFile, errorCode, span, cancellationToken, program); + if (!expression) { + return; + } + + const checker = context.program.getTypeChecker(); + const trackChanges: ContextualTrackChangesFunction = cb => textChanges.ChangeTracker.with(context, cb); + return compact([ + getDeclarationSiteFix(context, expression, errorCode, checker, trackChanges), + getUseSiteFix(context, expression, errorCode, checker, trackChanges)]); + }, getAllCodeActions: context => { const { sourceFile, program, cancellationToken } = context; const checker = context.program.getTypeChecker(); @@ -45,20 +57,6 @@ namespace ts.codefix { }, }); - function getAllPossibleFixesForError(context: CodeFixContext) { - const { sourceFile, errorCode, span, cancellationToken, program } = context; - const expression = getAwaitableExpression(sourceFile, errorCode, span, cancellationToken, program); - if (!expression) { - return; - } - - const checker = context.program.getTypeChecker(); - const trackChanges: ContextualTrackChangesFunction = cb => textChanges.ChangeTracker.with(context, cb); - return compact([ - getDeclarationSiteFix(context, expression, errorCode, checker, trackChanges), - getUseSiteFix(context, expression, errorCode, checker, trackChanges)]); - } - function getDeclarationSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, errorCode: number, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction) { const { sourceFile } = context; const awaitableInitializer = findAwaitableInitializer(expression, sourceFile, checker);