Use custom CodeActionKinds to support client-side filtering

[michaelpj]

The CodeActionKind enum is actually an open enumeration of strings. There's nothing stopping us from adding our own kinds.

The main advantage of this is if we want to do some client-side filtering this is a much nicer thing to use than, say, matching on the action title.

We might also want plugins to tell us about custom kinds they support so that we can return the set that we use in CodeActionOptions.

Real-world usecase: emacs-lsp/lsp-haskell#112 (comment)

[michaelpj]

It also occurred to me that the "Apply all hints" action is rather odd: arguably this should be something the client could do generically for a particular code action kind (if we put hlints in quickfix.hlint or something). Even just "apply all quickfixes" should be something clients can do for us.

[pepeiborra]

VScode can apply all the code actions of the kinds that you select (in configuration), on save. It would be great if we enriched our code actions with kind metadata so that we could, for example, insert all the missing imports on save.

[TOTBWF]

@isovector for the wingman stuff, the following kinds would probably be useful

• refine
• caseSplit
• homomorphicCaseSplit
• auto

[michaelpj]

I would suggest nesting them somewhat, maybe something like refactor.wingman.refine or something. That way e.g. VSCode will show them in a "refactoring" menu, and you can filter down to just the wingman actions, and you can filter down to a specific type of wingman action.

[isovector]

Wow, this is a great improvement in wingman. I'd suggest we also add kinds to the import code actions, so that people can only see the imports for their favorite styles.

[jneira]

@michaelpj that is a really great idea, many thanks for the suggestion, will do for hlint asap

[jneira]

• There had been two pr's adding kinds for code actions, they can be used by editors to filter them nicely
  • See Add kind and preferred flag for all Wingman code actions #1499 for wingman code actions
  • See Add custom code action kinds for import related code actions #1570 for import related code actions from ghcide
• Imo all hls code actions should have its kind informed
• In Add custom code action kinds for import related code actions #1570 @berberman suggested that they should be documented, a really good idea

Could we made up such document in this pr, adding the code actions with kind informed and which have to be added? Taking as base the starting point in #1570 (comment) by @michaelpj

[michaelpj]

TODOS:

• check which plugins are using them
• make the plugins which aren't using them use them
• document all the code actions and their kinds

[andys8]

@michaelpj I started looking over code action kinds and their usages. Here my current findings.

Code Action Kinds

Defined in https://hackage.haskell.org/package/lsp-types-1.5.0.0/docs/Language-LSP-Types.html

CodeActionQuickFix

Seems to be what is used the most. Used in:

hls-haddock-comments-plugin
hls-class-plugin
hls-qualify-imported-names-plugin
hls-explicit-imports-plugin
hls-pragmas-plugin
hls-hlint-plugin

CodeActionRefactor*

Used in hls-gadt-plugin, hls-retrie-plugin and hls-splice-plugin

CodeActionSource

Source code actions apply to the entire file.

With this explanation I think this could make sense for changes to pragmas

haskell-language-server/plugins/hls-pragmas-plugin/src/Ide/Plugin/Pragmas.hs

Line 73 in 344323b

J.InR $ J.CodeAction title (Just J.CodeActionQuickFix) (Just (J.List [])) Nothing Nothing (Just edit) Nothing Nothing

or disabling an hlint rule for the whole file

haskell-language-server/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs

Line 455 in 344323b

, Just (mkCodeAction suppressHintTitle diagnostic (Just suppressHintWorkspaceEdit) Nothing)

CodeActionSourceOrganizeImports

Could be interesting for "remove (all) unused imports" (ghcide)

https://github.com/alanz/ghcide/blob/c1678223bbe9ec73628888ef466e0e471258040c/src/Development/IDE/Plugin/CodeAction.hs#L329-L375

CodeActionUnknown

This is currently in use to express details, but if or how the given information is leveraged.

"quickfix.import.refine"
"quickfix.literals.style"
"refactor.wingman.<x>"

https://github.com/alanz/ghcide/blob/c1678223bbe9ec73628888ef466e0e471258040c/src/Development/IDE/Plugin/CodeAction.hs#L1932-L1938

[michaelpj]

Great, thanks!

With this explanation I think this could make sense for changes to pragmas

Honestly, the intended meaning of these kinds is very unclear :/ I wonder if we can look at what other people do to figure out a sensible rubric?

For pragmas, I think it makes sense to keep them as a quickfix since it's an action that appears in response to a diagnostic, i.e. it fixes a compilation error.

or disabling an hlint rule for the whole file

Maybe? Apply all hints might also fit!

This is currently in use to express details, but if or how the given information is leveraged.

I think that's the correct way to do it. The lsp-types model of CodeActionKind doesn't really expose their hierarchical nature, but my understanding is that they are supposed to be hierarchical, so having a "custom" code action kind of quickfix.X.Y is totally legitimate.

If anything, I think we should use this more, and that was what the ticket was originally about. If we can group the code actions we provide nicely using hierarchical kinds, then on the client side it's relatively easy for someone to have e.g. a keybinding to trigger a specific code action or subgroup of code actions (e.g. people have asked for this for the wingman code actions quite a few times).

[andys8]

I think that's the correct way to do it. The lsp-types model of CodeActionKind doesn't really expose their hierarchical nature, but my understanding is that they are supposed to be hierarchical, so having a "custom" code action kind of quickfix.X.Y is totally legitimate.

Thanks, this is very interesting. Judging from the constructor name I assumed CodeActionUnknown is for .. unknown, non-matching or maybe legacy actions. But looking at the implementation it's actually more of a CodeActionCustom. Not sure about the details, but maybe it's possible (and worth) touching the types. It doesn't look like the term "Unknown" is used in the specification but rather defined in Haskell's lsp-types.

CodeActionSourceOrganizeImports

source.organizeImports is currently not used. There are code actions that modify / fix imports though. It assume that using this kind would lead prominently defined "organize import" shortcuts to work probably in several tools.

E.g. in Vim/Coc this is in the default configuration example in the README but doesn't work for HLS. Not sure if this is also the case in other editors.

https://github.com/neoclide/coc.nvim

[michaelpj]

I'm revamping lsp-types substantially and it's going to be CodeActionKind_Custom or similar indeed :)

source.organizeImports is currently not used. There are code actions that modify / fix imports though. It assume that using this kind would lead prominently defined "organize import" shortcuts to work probably in several tools.

Well, I think it's not 100% clear what "organizing imports" would entail. As you say, we have several actions that do part of that, but tastes differ. I would expect "organize imports" to do some or all of:

• Remove redundant imports
• Make imports explicit (if that's something I want as a user)
• Format the import stanzas

Hard to say, really! At the moment we provide a bunch of such actions, and if you triggered them all with organizeImports then you might e.g. get all your imports made explicit even if you don't want that. Maybe we need a specific organize imports action that is configurable to do what the user wants? Not sure.