-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Add elaboration & quickfix for async-able arrow function #36342
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
/* @internal */ | ||
namespace ts.codefix { | ||
type ContextualTrackChangesFunction = (cb: (changeTracker: textChanges.ChangeTracker) => void) => FileTextChanges[]; | ||
const fixId = "addMissingAsync"; | ||
const errorCodes = [ | ||
Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code, | ||
Diagnostics.Type_0_is_not_assignable_to_type_1.code, | ||
Diagnostics.Type_0_is_not_comparable_to_type_1.code | ||
]; | ||
|
||
registerCodeFix({ | ||
fixIds: [fixId], | ||
errorCodes, | ||
getCodeActions: context => { | ||
const { sourceFile, errorCode, cancellationToken, program, span } = context; | ||
const diagnostic = find(program.getDiagnosticsProducingTypeChecker().getDiagnostics(sourceFile, cancellationToken), getIsMatchingAsyncError(span, errorCode)); | ||
const directSpan = diagnostic && diagnostic.relatedInformation && find(diagnostic.relatedInformation, r => r.code === Diagnostics.Did_you_mean_to_mark_this_function_as_async.code) as TextSpan | undefined; | ||
|
||
const decl = getFixableErrorSpanDeclaration(sourceFile, directSpan); | ||
if (!decl) { | ||
return; | ||
} | ||
|
||
const trackChanges: ContextualTrackChangesFunction = cb => textChanges.ChangeTracker.with(context, cb); | ||
return [getFix(context, decl, trackChanges)]; | ||
}, | ||
getAllCodeActions: context => { | ||
const { sourceFile } = context; | ||
const fixedDeclarations = createMap<true>(); | ||
return codeFixAll(context, errorCodes, (t, diagnostic) => { | ||
const span = diagnostic.relatedInformation && find(diagnostic.relatedInformation, r => r.code === Diagnostics.Did_you_mean_to_mark_this_function_as_async.code) as TextSpan | undefined; | ||
const decl = getFixableErrorSpanDeclaration(sourceFile, span); | ||
if (!decl) { | ||
return; | ||
} | ||
const trackChanges: ContextualTrackChangesFunction = cb => (cb(t), []); | ||
return getFix(context, decl, trackChanges, fixedDeclarations); | ||
}); | ||
}, | ||
}); | ||
|
||
type FixableDeclaration = ArrowFunction | FunctionDeclaration | FunctionExpression | MethodDeclaration; | ||
function getFix(context: CodeFixContext | CodeFixAllContext, decl: FixableDeclaration, trackChanges: ContextualTrackChangesFunction, fixedDeclarations?: Map<true>) { | ||
const changes = trackChanges(t => makeChange(t, context.sourceFile, decl, fixedDeclarations)); | ||
return createCodeFixAction(fixId, changes, Diagnostics.Add_async_modifier_to_containing_function, fixId, Diagnostics.Add_all_missing_async_modifiers); | ||
} | ||
|
||
function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, insertionSite: FixableDeclaration, fixedDeclarations?: Map<true>) { | ||
if (fixedDeclarations) { | ||
if (fixedDeclarations.has(getNodeId(insertionSite).toString())) { | ||
return; | ||
} | ||
} | ||
fixedDeclarations?.set(getNodeId(insertionSite).toString(), true); | ||
const cloneWithModifier = getSynthesizedDeepClone(insertionSite, /*includeTrivia*/ true); | ||
cloneWithModifier.modifiers = createNodeArray(createModifiersFromModifierFlags(getModifierFlags(insertionSite) | ModifierFlags.Async)); | ||
cloneWithModifier.modifierFlagsCache = 0; | ||
changeTracker.replaceNode( | ||
sourceFile, | ||
insertionSite, | ||
cloneWithModifier); | ||
} | ||
|
||
function getFixableErrorSpanDeclaration(sourceFile: SourceFile, span: TextSpan | undefined): FixableDeclaration | undefined { | ||
if (!span) return undefined; | ||
const token = getTokenAtPosition(sourceFile, span.start); | ||
// Checker has already done work to determine that async might be possible, and has attached | ||
// related info to the node, so start by finding the signature that exactly matches up | ||
// with the diagnostic range. | ||
const decl = findAncestor(token, node => { | ||
if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) { | ||
return "quit"; | ||
} | ||
return (isArrowFunction(node) || isMethodDeclaration(node) || isFunctionExpression(node) || isFunctionDeclaration(node)) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile)); | ||
}) as FixableDeclaration | undefined; | ||
|
||
return decl; | ||
} | ||
|
||
function getIsMatchingAsyncError(span: TextSpan, errorCode: number) { | ||
return ({ start, length, relatedInformation, code }: Diagnostic) => | ||
isNumber(start) && isNumber(length) && textSpansEqual({ start, length }, span) && | ||
code === errorCode && | ||
!!relatedInformation && | ||
some(relatedInformation, related => related.code === Diagnostics.Did_you_mean_to_mark_this_function_as_async.code); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,41 +37,51 @@ tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts(27,16): err | |
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:3:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:18:10: Did you mean to mark this function as 'async'? | ||
c: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:4:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:19:10: Did you mean to mark this function as 'async'? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is kind of a weird thing now that I think about it - why would I want to mark this as async? There's no If a user did write interface Stuff {
b: () => Promise<string>;
}
function foo(): Stuff | Date {
return {
b: _ =>
await "hello" // two error spans with duplicate related spans
}
} So this is only distinct actionable advice information for a user if they hadn't written There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because
More like, using |
||
d: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:5:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:20:10: Did you mean to mark this function as 'async'? | ||
e: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:6:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:21:10: Did you mean to mark this function as 'async'? | ||
f: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:7:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:22:10: Did you mean to mark this function as 'async'? | ||
g: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:8:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:23:10: Did you mean to mark this function as 'async'? | ||
h: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:9:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:24:10: Did you mean to mark this function as 'async'? | ||
i: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:10:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:25:10: Did you mean to mark this function as 'async'? | ||
j: () => "hello", | ||
~~~~~~~ | ||
!!! error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:11:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:26:10: Did you mean to mark this function as 'async'? | ||
k: () => 123 | ||
~~~ | ||
!!! error TS2322: Type 'number' is not assignable to type 'Promise<number>'. | ||
!!! related TS6502 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:12:8: The expected type comes from the return type of this signature. | ||
!!! related TS1356 tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts:27:10: Did you mean to mark this function as 'async'? | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/// <reference path="fourslash.ts" /> | ||
////interface Stuff { | ||
//// b: () => Promise<string>; | ||
////} | ||
//// | ||
////function foo(): Stuff | Date { | ||
//// return { | ||
//// b: () => "hello", | ||
//// } | ||
////} | ||
|
||
verify.codeFix({ | ||
description: ts.Diagnostics.Add_async_modifier_to_containing_function.message, | ||
index: 0, | ||
newFileContent: | ||
`interface Stuff { | ||
b: () => Promise<string>; | ||
} | ||
|
||
function foo(): Stuff | Date { | ||
return { | ||
b: async () => "hello", | ||
} | ||
}` | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/// <reference path="fourslash.ts" /> | ||
////interface Stuff { | ||
//// b: () => Promise<string>; | ||
////} | ||
//// | ||
////function foo(): Stuff | Date { | ||
//// return { | ||
//// b: _ => "hello", | ||
//// } | ||
////} | ||
|
||
verify.codeFix({ | ||
description: ts.Diagnostics.Add_async_modifier_to_containing_function.message, | ||
index: 0, | ||
newFileContent: | ||
`interface Stuff { | ||
b: () => Promise<string>; | ||
} | ||
|
||
function foo(): Stuff | Date { | ||
return { | ||
b: async (_) => "hello", | ||
} | ||
}` | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this code be shared with
isMissingAwaitError
inaddMissingAwait.ts
? It seems like finding a diagnostic with particular related information might be a general utility.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite! In the other quick fix, we're checking if the diagnostic's span matches the input span, then returning the diagnostic if it matches and has the target related span code. In this quick fix, we don't do the filtering on related span at the matching step, because we're looking to
find
the span of the related span (so we can use that instead). So, yes, there's a common iteration bit, but to handle the bits variable between the two is essentially just asome
with variable condition, onto which we then append an optional extra filter/map on the final result. Which seems like a bit too multifunctional for a single helper for my tastes.But it should totally feel similar, since I started writing this quick fix by copying the
addMissingAwait
quick fix, which is why the span-matching check is almost identical in structure.