TS Server: Return description of why a given refactoring cannot be applied

[mjbvz]

Follow up on #34930

Search terms

• refactor / refactoring

Problem

In VS Code, I often try to extract something to a function only to discover that this is not possible for the my current selection. The current UX for this is poor, since we offer no explanation of why the selection is not extractable.

Proposal

To fix this, we would like to have TS Server return a human readable message about why a refactoring cannot be applied. Client could then surface this information to users

With this proposal, TS Server would no silently longer dropping invalid refactorings like it does today and instead always return them but with an additional error message attached..

Protocol proposal

I believe this simplest approach is to add a new errorDescription field on ApplicableRefactorInfo:

interface ApplicableRefactorInfo {
    name: string;
    description: string;
    inlineable?: boolean;
    actions: RefactorActionInfo[];

    errorDescription?: string;
}

Client would be expected to not to try to resolve invalid refactorings.

To implement this in a way that won't break existing clients, we also need a way for a client to express that they can handle these invalid refactorings:

interface GetApplicableRefactorsRequestArgs extends FileLocationOrRangeRequestArgs {
    // better name?
    returnInvalidRefactorings?: boolean;
}

If returnInvalidRefactorings is not set, the TS Server should preserve its existing behavior, i.e. drop invalid refactorings

[amcasey]

As we discussed offline, this seems likely to be of interest primarily when the user has explicitly requested a particular refactoring or family of refactoring (e.g. extraction). If that's the case, it might be preferable to treat the presence of a refactoring/family identifier as an indicator that errors are desirable.

[amcasey]

For experimental purposes, we might want to add such a flag to the protocol and let the editor decide which portion of the flood of information is exposed to the user.

[mjbvz]

A few other points from our discussion last Thursday:

• As @amcasey notes, this proposal may be tied to TSServer: Refactor trigger reason #35096

• We likely only care about why a general class of refactorings are not valid. I.e. for extract constant we don't want to list out every possible scope that we could extract to and why we can't extract to it, instead: if any extract constant is possible then return that, otherwise return a single extract constant item that states why the selected expression could not be extracted (the error would probably come from the closest scope)

• We currently have no good way to surface this information in VS Code. I'm looking into how to do this this month

[mjbvz]

Here's the VS Code side of the proposal and some thoughts on how we would surface the error information to users: microsoft/vscode#85160