Add optionalReplacementSpan to completions response

[andrewbranch]

Closes #35602

Adds a top-level property optionalReplacementSpan to CompletionsInfo, which contains the TextSpan representing the identifier or string literal-like content where completions were completions were requested, if they were requested on such a node. E.g. when requesting completions at the caret in

console.clockwork
//        ^

the optionalReplacementSpan will be the span of the identifier clockwork. The editor can opt to use this span when accepting the completion entry clear to produce console.clear instead of console.clearockwork, as it would today. The span is also populated for string literal completions:

Array["toSpaghetti"]
//        ^

The optionalReplacementSpan here is the span of toSpaghetti, excluding the quotes.

As before, if an individual CompletionEntry specifies a replacementSpan, that span must be used. It is possible for an optionalReplacementSpan to be populated even when some individual entries have a replacementSpan that must receive priority.

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

[mjbvz]

The protocol changes look good to me

[sheetalkamat]

I am not sure why this has to be optionalReplacementSpan and not just replacementSpan if editor supports it (there is already preference for that no)

[andrewbranch]

My understanding is there is not a preference that allows users to opt out of using replacementSpan—in VS and VS Code, the presence of replacementSpan means the editor will always use it, which is good because it means users can’t end up in a situation where they trigger completions on style.| and end up with a syntax error like style.["background-color"]. This new span will be configurable by a preference.

That said, it does seem like using the new optionalReplacementSpan is almost always better than the current behavior, but I guess @mjbvz and co. have thought through the reasons someone might want to have both options more than I have.

[sheetalkamat]

But i dont see this being configured by preference? Even if this is configured by preference, why cant it be returned as part of replacementSpan since replacement span if present is always meant to override this one?

[andrewbranch]

I believe it’s supposed to be an editor preference, which could potentially be configured across multiple languages. If TS Server adopted this preference and VS Code sent it to us, that would be an option. That’s not something that was discussed in the issue; I personally don’t have strong feelings one way or another. I’ll let @mjbvz weigh in.

[mjbvz]

VS code lets users configure how suggestions are inserted using the editor.suggest.insertMode setting. The options are insert which always inserts the current suggestion without replacing the suffix, and replace which would try replacing the suffix word.

However users can always toggle to the opposite mode of the current one by holding down shift while accepting a completion. This means that TS always needs to return both spans to us in some way. I think we need a clear distinction between:

• Here's a span that should always be replaced regardless of insert mode (the current replacementSpan property)
• Here's a replacement span that you should use if the user accepts a completion in replace mode (which would be the new optionalRepalcementSpan)

I think the current proposal covers our needs but let me know if that doesn't sound correct based on my description

[sheetalkamat]

Got it. Now understand why you always want this property to be populated and separate. Thanks.

[andrewbranch]

Thanks for the clarification 👍

[jasonwilliams]

@andrewbranch is this correct? The examples above in the unittests show TextSpan being used. But this is pointing to here, is it using the correct interface?

[jasonwilliams]

https://github.com/microsoft/TypeScript/blob/main/src/testRunner/unittests/tsserver/metadataInResponse.ts#L83-L86 its still set to that interface also

[andrewbranch]

Yes, internally it should be a ts.TextSpan and over the wire it should be a ts.server.protocol.TextSpan. The conversion is done on the way out in session.ts. It’s possible we’re missing a conversion somewhere but the types are right. What’s going on?

[jasonwilliams]

What’s going on?

im having a nightmare with a typescript server plugin not having correct completion data, mainly range data, see here microsoft/vscode#134328

I thought maybe it needs to include This new field but TextSpan didn’t match with the examples here

[jasonwilliams]

I though the culprit was here https://github.com/microsoft/typescript-styled-plugin/blob/main/src/_language-service.ts#L121-L138 because item has the range, but its never converted. But the types seem correct.

Looking at the types for CompletionEntry and CompletionEntryDetails I don't see where the range would be passed through back to VSCode

[andrewbranch]

Here’s where it gets converted on its way to VS Code

TypeScript/src/server/session.ts

Line 1873 in 9766757

optionalReplacementSpan: completions.optionalReplacementSpan && toProtocolTextSpan(completions.optionalReplacementSpan, scriptInfo),

[jasonwilliams]

Thanks Andrew.
I don’t suppose you know of any typescript plugin that sets this? I’ve burnt out trying to find a solution for here. I’ve struggled to get the information of the token the cursor is setting next too (like offset and length).

[jasonwilliams]

@andrewbranch in case you were curious, this was the fix, took me days: microsoft/typescript-styled-plugin#156