Skip to content
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

Add quick fix to add missing 'await' #32356

Merged
Merged
Show file tree
Hide file tree
Changes from 12 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
3 changes: 2 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 13 additions & 4 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -5087,6 +5083,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
Expand Down
25 changes: 21 additions & 4 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2828,11 +2828,28 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyCodeFixAvailable(negative: boolean, expected: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined): void {
assert(!negative || !expected);
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]);
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
}
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) {
Expand Down
175 changes: 175 additions & 0 deletions src/services/codefixes/addMissingAwait.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
/* @internal */
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,
];
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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will show for any old type combination, like number + Event, right? What happens in that case? Does it just fail to do anything when you try to "add missing await"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's supposed to be filtered out further in getCodeActions (it doesn’t show anything if you return undefined / empty array in there). This is back to the tradeoffs we were discussing about using related info as a “bonus error message” vs. copying and pasting all these into new, unique messages. I wrote this down as a larger LS design/API change to consider at some later point.

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,
propertyAccessCode,
...callableConstructableErrorCodes,
];

registerCodeFix({
fixIds: [fixId],
errorCodes,
getCodeActions: getAllPossibleFixesForError,
getAllCodeActions: context => {
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
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, diagnostic.code, checker, trackChanges)
|| getUseSiteFix(context, expression, diagnostic.code, checker, trackChanges);
});
},
});

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);
if (awaitableInitializer) {
const initializerChanges = trackChanges(t => makeChange(t, errorCode, sourceFile, checker, awaitableInitializer));
return createCodeFixActionNoFixId(
"addMissingAwaitToInitializer",
initializerChanges,
[Diagnostics.Add_await_to_initializer_for_0, expression.getText(sourceFile)]);
}
}

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);
}

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 }) =>
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, 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";
}
return isExpression(node) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile));
}) as Expression | undefined;

return expression
&& isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program)
&& isInsideAwaitableBody(expression)
? expression
: undefined;
}

function findAwaitableInitializer(expression: Node, sourceFile: SourceFile, checker: TypeChecker): Expression | undefined {
if (!isIdentifier(expression)) {
return;
}

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 ||
!isInsideAwaitableBody(declaration.initializer)) {
return;
}

const isUsedElsewhere = FindAllReferences.Core.eachSymbolReferenceInFile(variableName, checker, sourceFile, identifier => {
return identifier !== expression;
});

if (isUsedElsewhere) {
return;
}

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;
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);
}
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)));
}
else {
changeTracker.replaceNode(sourceFile, insertionSite, createAwait(insertionSite));
}
}
}
4 changes: 4 additions & 0 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"codeFixProvider.ts",
"refactorProvider.ts",
"codefixes/addConvertToUnknownForNonOverlappingTypes.ts",
"codefixes/addMissingAwait.ts",
"codefixes/addMissingConst.ts",
"codefixes/addMissingInvocationForDecorator.ts",
"codefixes/addNameToNamelessParameter.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string[]>' is not an array type.
tests/cases/compiler/operationsAvailableOnPromisedType.ts(19,21): error TS2495: Type 'Promise<string[]>' is not an array type or a string type.
tests/cases/compiler/operationsAvailableOnPromisedType.ts(20,12): error TS2345: Argument of type 'Promise<number>' 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<string[]>' 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.
Expand Down Expand Up @@ -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) {}
~
Expand Down
13 changes: 13 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingAwait_argument.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path="fourslash.ts" />
////async function fn(a: Promise<string>, b: string) {
//// fn(a, a);
////}

verify.codeFix({
description: ts.Diagnostics.Add_await.message,
index: 0,
newFileContent:
`async function fn(a: Promise<string>, b: string) {
fn(a, await a);
}`
});
50 changes: 50 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingAwait_binaryExpressions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/// <reference path="fourslash.ts" />
////async function fn(a: Promise<number>, b: number) {
//// a | b;
//// b + a;
//// a + a;
////}

verify.codeFix({
description: ts.Diagnostics.Add_await.message,
index: 0,
newFileContent:
`async function fn(a: Promise<number>, 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<number>, 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<number>, 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<number>, b: number) {
await a | b;
b + await a;
await a + await a;
}`
});
28 changes: 28 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingAwait_initializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path="fourslash.ts" />
////async function fn(a: string, b: Promise<string>) {
//// 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<string>) {
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<string>) {
const x = await b;
fn(x, b);
fn(await b, b);
}`
});
Loading