Explore improving UX / API for refactorings

[mjbvz]

Refactoring like extract method don't seem like a great fit for the code actions or any other API that VSCode offers. This issue tracks an investigation of what a better ux/api for refactorings could look like

Problems / Background Info

• @Gama11 makes some very good points about why refactorings don't seem to fit well with the current code action model: Annoying "lightbulb" whenever placing cursor into constant js/ts expression #33241 (comment)

• No way to configure a keybinding for specific code actions / refactoring: Keybinding for applying a specific code action  #33049

• No feedback on why a refactoring is not available: Extract method / functiom must give feedback when a selection is not extractable and why #33501

Any other problems you've noticed or thoughts on refactoring support?

[Gama11]

don't seem like a great fit for the code actions or any other API that VSCode offers

Not sure about that last part. If "extract method" was implemented as a command (explicitly selected by the user from the command palette), all of the mentioned issues would be resolved:

• no annoying light bulbs
• commands support keybindings
• a command can call showErrorMessage() in case of failure

On the flip side, you might end up with different languages all using different commands, unlike the "Rename Symbol" refactoring, which has explicit VSCode / language server support and uses the same command for all languages.

Edit: the Python extension already seems to follow this pattern:

[jrieken]

If "extract method" was implemented as a command (explicitly selected by the user from the command palette),

Wait, everything here is a command. The provideCodeAction-function is spec'd to return a set of Commands. Now, there is some confusing naming going on, let me explain:

• You call registerCommand to associate a string-identifier to a piece of code (function)
• You reference that string-identifier from package.json to make it appear in the command palette or other menus
• Or, you reference that string-identifier via the Command-interface to make that command appear in the UI. That naming is bad and Command should have been named MenuItem because that's what it actually is

Now, it depends on your extension. You can either reference you "xyzRefactor"-command exclusively from Commands returned from your provider, from package.json, or from both. And because code actions are just commands they also have the freedom to do whatever, e.g. show an error message etc.

unlike the "Rename Symbol" refactoring, which has explicit VSCode / language server support

Yeah, rename is a special case. With our provider-pattern we try to generalise and align things such that the editor owns the UI and that extensions just provide data. Finding the smallest common denominator isn't always easy and for more complex refactorings we haven't been brave enough to try.

[Gama11]

Yes, "command" is indeed a confusing term here - I meant "implemented as a menu item", hence the added "selected by the user from the command palette".
And I guess you're right, the lack of a proper error message is not due to a limitation of the code actions API.

[mjbvz]

While commands work and have some advantages, I think using regular commands for refactoring leaves something to be desired:

• Each language extension has to provided its own refactoring commands. We don't want to end up with a python.extractMethod command and a php.extractMethod command and a typescript.extractMethod command and so on.

  I'd really prefer something akin to a single vscode.refactor.extractMethod command, with a single keybinding that works across languages

• No standard UI for presenting refactoring commands

• Discoverability. The lightbulb may sometimes annoy users currently, but I feel it does help with discoverability. Commands on their own would not be very discoverable

• Ask vs tell. This one is debatable:

  In the code action model, VSCode tells the user which actions can be performed at a given location. We do not display actions that cannot be performed.

  In the command model, the user asks VS Code to perform an action. If that action cannot be performed, the user only gets this feedback after they have already asked for the action to be performed. One benefit of the ask/command based model is that the user can then get feedback on why an action cannot be performed

[eamodio]

Could there maybe be 2 types of code actions -- ones that are in @mjbvz terms "Tell" and ones that are "Ask". Meaning some code actions will cause the lightbulb to show up automagically, but refactoring and other similar code actions will only show if asked for (i.e. ctrl+.). It also would be nice, if the "Tell" code actions actually showed in the editor close in proximity to the "issue", while the "Ask" code actions could show the bulb in the gutter area like today.

[jrieken]

Since it results in a snippet without a final tab stop, VS Code appends a final tab stop at the end.

Yeah, it is a fine line. Generally, things that don't require UI, like "invert if-else", "insert semi-colon", or "inline variable" are good candidates for being a code action. However, it's not always that easy and when more complex interactions and user-input is required we usually carve out a provider-api including a data model, think of rename, completions etc.

I don't want this issue to be the kitchen sink for all possible refactoring UIs, but would look at them case by case. We did try this in the past with "Extract Method/Function" but weren't confident enough that we cover all cases for all languages.

We are also trying to keep our UI light and free from dialogs/pop-overs etc.

/cc @stevencl

[bmewburn]

A multi select would be nice for rename where you can choose to apply the rename to selected elements. It's not uncommon to have name collisions in PHP.

[mjbvz]

Closing out this investigation. Outcome:

• After some discussions, we decided to focus on improving the existing code action provider to make it suitable rather than introducing a new refactoring provider

• This culminated in the new CodeAction type and the CodeActionKind type which now can be used to differentiate refactorings from quick fixes (see vscode.d.ts for details)

• Extensions that provide refactorings can now use CodeActionContext.only to decide if they should return refactorings for the lightbulb menu (see TS extension for example)

• Code action kinds also enable keybindings for specific refactorings or specific classes of refactorings (Keybinding for applying a specific code action  #33049 (comment))

• Added new Refactor command that only shows code actions that have been marked as refactorings.

• Extensions that really want a user pull model for refactorings can use custom commands for this. This also lets the extension provide feedback on why a refactoring is not available. The same if a refactoring requires complex UI interaction

• We plan to continue improving the refactorings UI in upcoming iterations. Let us know if you have any ideas around this