add completion for promise context

[Kingwl]

Fixes #31450

This PR added a new SymbolOriginInfo for Promise and create await on createCompletionEntry if the node is in AwaitContext.
I'm not sure the current insertText behavior is totally correct, and as the result of completionOfAwaitPromise3.ts, everything seems ok, but I cannot do it in my local vscode

unsupported:

• add async modifier to container function
• top level await

[andrewbranch]

My suspicion is that most of the time when these completions appear, my desired fix is like this:

async function fetchThings(fetchOptions: RequestInfo): Thing[] {
  const rawJson = (await fetch(fetchOptions)).json(); // accidentally a Promise<any>
  return deserializeThingsResponse(rawJson);
}

// should become

async function fetchThings(fetchOptions: RequestInfo): Thing[] {
  const rawJson = await (await fetch(fetchOptions)).json();
  return deserializeThingsResponse(rawJson);
}

If I have a variable declaration a) without a declared type, b) inside an async function, c) whose resolved type is a Promise, and d) I never successfully access any members of that Promise, then chances are, I meant to use await at the declaration site, not the use site.

Of course, that’s more semantic criteria to check. It also increases the likelihood that the codefix will take place offscreen. Let me see if I can try to find some data on how frequently we see parenthesized await expressions vs. as the initializer of a variable declaration.

[andrewbranch]

Ok, so in our user tests (123,359 TS/JS/TSX/JSX files), I looked for the forms:

• any await expression
• const x = await something (AwaitExpression as VariableDeclaration initializer)
• (await foo).bar (AwaitExpression as expression of ParenthesizedExpression as expression of PropertyAccessExpression)

The results were:

• total await: 3,983 occurrences in 358 files
• initializer: 1,391 occurrences in 271 files (35% of all awaits)
• parenthesized then property access: 76 occurrences in 26 files (2% of all awaits)

This isn't a huge sample size (of files that actually contain await)—but 2% vs. 35% is a big enough gap that I’m skeptical that this is the refactor users expect when choosing a completion. /cc @DanielRosenwasser

[Kingwl]

That should be a quick fix and trigger by type mismatch.
The difference is property access is usual when you are writing some fresh code.

I‘m not sure but I think you cannot find too many property access is caused people always write the following code rather than parens and parens：

async function fetchThings(fetchOptions: RequestInfo): Thing[] {
  const resp = await fetch(fetchOptions)
  const rawJson = await resp.json();
  return deserializeThingsResponse(rawJson);
}

[andrewbranch]

Sure, maybe my example wasn’t very good. But I think there will generally be cases like

declare function someAsyncThing(): Promise<{ foo: string }>;

async function whatever() {
  const x = someAsyncThing(); // meant to await
  x./*|*/
}

and if we complete foo there, it would be better to place the await on await someAsyncThing(). There are many other cases where that wouldn’t work, like if x was a function parameter, or if x was already used elsewhere, or if the property access was occurring on a non-identifier expression. In those cases, I think this completion behavior makes sense, but I want to consider and get feedback on whether it makes sense to try to offer the declaration site await where relevant.

[Kingwl]

It's also a bit weird that we request autocomplete here and the leading code has been modified.

if x was already used elsewhere

And I'm not sure that is acceptable what request find all reference in autocomplete?

[andrewbranch]

I'm not sure that is acceptable what request find all reference in autocomplete?

I’m working on something similar for the codefix scenario:

Over the next couple days I’ll be testing this out and seeing how the performance is. We don’t need to do quite a full find-all-references, and the scope should be limited to the source file at largest.

I think we should get this merged, and if it seems reasonable to add the initializer await, I can make it pull in the same code I’m working on now.

Have you addressed Sheetal’s feedback?

[andrewbranch]

completionOfAwaitPromise3.ts, everything seems ok, but I cannot do it in my local vscode

Yeah, it doesn’t work for me either—I stepped through on the VS Code side and confirmed the response from TSServer looks good, and I couldn’t find where it was getting filtered out. It seems like it makes it all the way out of the typescript-language-features extension. Will likely need some help from the VS Code folks to figure this one out, but I don’t think it’s a problem for this PR.

👇 stupid MacBook trackpad clicked the button while I was typing 😑

[andrewbranch]

Shouldn't this be something like if (!insertAwait || preferences.includeCompletionsWithInsertText)? Seems like addSymbol would now reject all completions if the preference is disabled 🙃

[Kingwl]

I still felt it's hard to get an await-missing expression when you are coding.
But it's not a big deal.

[andrewbranch]

Looks like just a lint error now.

[Kingwl]

Hope I have not delay your release.

[sheetalkamat]

I don't think this needs to be passed around since this check can easily be performed as usage sites of needsConvertAwait