Add config properties for each ghcide code action

[berberman]

Switch off any ghcide code actions you don't want. For example, with the help of Wingman, most people may not want to use code actions like replace _ with <...> that provided by GHC (#1804). Now they can simply disable suggestFillHole in the plugin config:

      "ghcide-code-actions": {
        "globalOn": true,
        "config": {
          "removeRedundantImport": true,
          "suggestModuleTypo": true,
          "suggestAddTypeAnnotationToSatisfyContraints": true,
          "suggestImportDisambiguation": true,
          "suggestExtendImport": true,
          "suggestImplicitParameter": true,
          "suggestFillHole": true,
          "suggestNewImport": true,
          "suggestSignature": true,
          "suggestDeleteUnusedBinding": true,
          "suggestReplaceIdentifier": true,
          "importClassMethod": true,
          "removeRedundantConstraints": true,
          "suggestHideShadow": true,
          "suggestNewDefinition": true,
          "removeInvalidExport": true,
          "suggestExportUnusedTopBinding": true,
          "suggestFixConstructorImport": true,
          "suggestConstraint": true,
          "suggestFillTypeWildcard": true
        }
      }

[jneira]

Nice, i am only a little bit worried with the size of the config, but this makes sense and maybe worths it

[Ailrun]

It would be great if you add some tests for these configs
(like these actually disable some actions/etc.)

[pepeiborra]

What is the connection between the names introduced here to enable/disable the code action, and the names used in the providers to identify the quick fixes? It would be a bit messy if we ended up with two different names for every code action.

[berberman]

The names of properties in config here are exactly the names of code actions implementation functions. IMHO code action kind should not be related to this: a single code action implementation function may provide many results with different kinds, which are not known until the computation finished; clients are always able to filter those results given specific kinds. However, those properties are designed to disable the computations.

[pepeiborra]

If we were to split the big monolithic code action into multiple plugins, one for each "function", then the problem that this PR solves would no longer exist. There are other advantages too:

• all the providers would run in parallel,
• (hopefully) error handling would be done per provider, preventing a single buggy code action from breaking them all

Is that direction worth pursuing?

[berberman]

I agree with breaking down this big code action provider, but I am not sure how hard it will be. And If we run providers in parallel, is there any way to implement the priorities? I believe we don't want to see code action to disable error messages appear before the code action of the real fix...

[pepeiborra]

And If we run providers in parallel, is there any way to implement the priorities? I believe we don't want to see code action to disable error messages appear before the code action of the real fix...

That shouldn't be a problem. The handlers are called concurrently, but their responses are combined in the order they are declared:

haskell-language-server/ghcide/src/Development/IDE/Plugin/HLS.hs

Lines 158 to 164 in e0a4642

	es <- runConcurrently msg (show m) fs ide params
	let (errs,succs) = partitionEithers $ toList es
	case nonEmpty succs of
	Nothing -> pure $ Left $ combineErrors errs
	Just xs -> do
	caps <- LSP.getClientCapabilities
	pure $ Right $ combineResponses m config caps params xs

[pepeiborra]

I agree with breaking down this big code action provider, but I am not sure how hard it will be.

My concern is that it will never happen if we keep making it harder and harder. So every time we make changes to this code, I mentally weigh the benefits of the changes against the cost of unwinding them when/if we clean it up.

[jneira]

And what about split them in several plugins, grouping related code actions if possible, maybe one plugin by action would create too much plugins? they have a non trivial maintenance burden

[pepeiborra]

And what about split them in several plugins, grouping related code actions if possible, maybe one plugin by action would create too much plugins? they have a non trivial maintenance burden

Why do you think they have a maintenance burden? We don't need to extract the plugins (or should call them providers instead) into Cabal packages, they would still live in the ghcide package.

[jneira]

mmm if we don't publish them the maintenance work is quite lower but it exists. The top level configuration will grow though, and we will have to add them to vscode and other editors config. In general the plugin surface namespace will grow linearly with each new code action. So to have a balance between the size of top plugins an the size of its subcomponents could be a better example for future design.

I can see several actions that are logically correlated: unused imports, constraints, etc. So grouping it will help to reveal that logical relation too.

[pepeiborra]

The top level HLS configuration will not change, as the ghcide descriptors are bundled together:

https://github.com/haskell/haskell-language-server/blob/master/ghcide/src/Development/IDE/Plugin/HLS/GhcIde.hs#L22-L29

I am not sure how much work is involved in adding them to vscode config, I thought that was now automated.

That said, I agree that logically correlated code actions should live in the same provider.

[berberman]

Let's move on to splitting the provider before it becomes harder

[jneira]

I am not sure how much work is involved in adding them to vscode config, I thought that was now automated.

Yeah it is automated thanks to @berberman but i was thinking in usability, less top level configuration would make it more usable (as you can show it in a tree etc)