Code action order could prefer fixes over disabling warnings

[andys8]

TL;DR

Code actions are ordered in a way where the most likely option is on the bottom.

Issue

I'm often using code actions to organize imports and remove the ones that aren't necessary. The option that is use the most often ("Remove all redundant imports") is the last option of the menu. And I think most of the time "Disable ... warnings" is the first option. "Make all imports implicit" seems not relevant here.

One issue is that one needs to move to the end of the list each time. But for example in CoC (Vim) coc-fix-current defines the top code action as default and will execute it. So changing the order could be an improvement.

Potential Improvement

Change the order of code actions to (try to) match the most likely behavior. E.g. move "disable warnings" to the bottom of the suggested code actions by default.

Probe Tools

haskell-language-server version: 1.3.0.0 (GHC: 8.6.5) (PATH: /home/andreas/.local/bin/haskell-language-server-wrapper) (GIT hash: e7c5e90b6df5dff2760d76169eddaea3bdd6a831)
Tool versions found on the $PATH
cabal:          3.2.0.0
stack:          2.7.3
ghc:            8.10.4

Config

https://github.com/andys8/dotfiles/blob/3c1d32349ffaf4c1ab840f7b340746846fb5e522/coc-settings.json#L34-L55

[July541]

Hello, I can only get this hint, am I missing something?

[jneira]

Mmm i think in vim all the code actions are shown, not only the associated with the diagnostic, in vscode you could see them clicking in the light bulb at the start of the expression

[July541]

@jneira Seems no, all I can see while clicking the bulb is just the one hint.

[July541]

It turns out I don't know when my hls doesn't work as before, I remember it can suggest removing unused import.

[jneira]

Maybe related with #2013??, does add import suggestions still works for you?

[jneira]

The existence of remove unused imports is being exercised here afaics:

haskell-language-server/test/functional/FunctionalCodeAction.hs

Line 353 in 9207050

redundantImportTests :: TestTree

[pepeiborra]

You may need to enable -Wredundant-imports or similar in your project.

[jneira]

Oh yeah i forgot that "little" detail, thanks
It should be documented though (in an note adding the feature in readme?)

[July541]

@pepeiborra Yes, it works! My order is a little different from the author's.

@jneira I think enable -Wall to single file has more advantages.

[jneira]

@jneira I think enable -Wall to single file has more advantages.

Yeah we are doing it in hls itself for example, but i think we have to be precise in the documentation and note that this concrete warning (mentioning -Wall) is needed to make those code actions work at all.

[berberman]

If I understand correctly, the current display order of code actions is up to how handlers are concatenated, which I think is hard to track around ghcide and HLS plugins. Moreover, take pragmas plugin as example, we want adding pragmas be the top priority in most cases (though it‘s supposed to be now, since parsing errors or typechecking errors caused by missing pragmas will prevent ghc from producing warnings), but disabling warnings be the lowest. This doesn't seem feasible by simply reordering handlers.

[jneira]

Mmm so a proper solution could be change hls-plugin-api to being able to inform the priority of code actions? or it would need a even deeper change?
maybe it should be a editor feature (without exclude a default server ordering)

[July541]

How about a hack filter in

haskell-language-server/ghcide/src/Development/IDE/Main.hs

Line 215 in 047ffc3

plugins = hlsPlugin <> argsGhcidePlugin

😆

[wraithm]

HLS is absolutely amazing, and this would be a huge feature for the ergonomics of HLS. I'm firing off tons and tons of code actions every day, but it's often that the one I want is at the bottom of the suggested list.

Maybe a first step would be to categorize code actions and then assign priorities to them? Just to understand the high-level design of something like this.

Ones that I'm often reaching for are:

• Add a function/data type to an existing import statement
• hlint
• Turn on a LANGUAGE pragma

Things I'm often not trying to do that are frequently at the top:

• Disable/enable warning ghc options
• Import a new module (I usually have it in scope already, and I just want to add a function to an existing import)

Just some thoughts that might help. Anybody else have other ones?

[jneira]

code actions has a kind field, which can be used (and it is used in fact) to filter or order them, see #1458
as stated in the issue, once you have your actions labeled, it is easier for clients filtering (and ordering them)

[jneira]

I think it would be difficult to reach a ordering which will satisfy all or even most users. So it should be configurable and make sense that clients have the responsability of doing that.

[andys8]

There are CodeActionKinds and I've started to look into their current usages, but I'm not sure how they can be leveraged to filter or even sort actions. Would one only want to see quick fixes but no refactorings?

There is a property isPreferred that can be defined per CodeAction. It could be useful for sorting. It can be missing, preferred or not preferred (Maybe Bool).

A refactoring should be marked preferred if it is the most reasonable choice of actions to take.

Defined isPreferred for certain actions could help with issues like this one. E.g. if the user is using and has configured hlint, it's probably the "most reasonable choice" to fix an issue and less likely to add a rule to ignore it (#3018). There might be more cases like this where isPreferred could help ending up in a sort order that is more often nicer to use.

[michaelpj]

I'm not sure if we return the code actions in any particular order. We do for completions, but I think code actions are pretty random. We could try and implement some sorting, but using isPreferred for "definitely good" code actions seems like a good thing to do where we can.

[andys8]

Yes, I assume we could change order to be less random, but this would need a different way to make sure sorting is done across multiple plugins.

isPreferred itself seems to be taken into account for sorting on the client. At least this seems to be the case for CoC: https://github.com/neoclide/coc.nvim/blob/70f11e074f45bc1bed1c17e3b0c2cf687f5582b6/src/handler/codeActions.ts#L65

[andys8]

Changed in two cases:

• Hlint: CodeAction with isPreferred #3018
• Refactor plugin: Prefer code action #3167

Will close the issue since the original examples should be improved. Anyhow, there might be more places where we could mark code actions as isPreferred.