Package ghcide code actions

[berberman]

Motivation

Previously, functions implement ghcide code actions were required to return [(T.Text, [TextEdit])], and were wired together in suggestAction. In plugin handler, we used many shake rules and retrieved values used in concrete implementation of code actions, then called suggestAction here, passing those values. However, with more and more code actions implemented by contributors, now suggestAction already has 11 parameters, which we need name each of two times and pass all of by hand. So I think it's time to define a record type to gather them together. On the other hand, we introduced another potential return type Rewrite to represent the result of the code action and then used rewrite to convert it to TextEdit, packaging it in suggestAction. With this abstraction, it's hard to express some cases that the code action returns either TextEdit or Rewrite, which depends on the runtime (like suggestImportDisambiguation and suggestNewOrExtendImportForClassMethod). Thus we could use type class to relax the return type of functions.

Overview

First of all, we use a new record type CodeActionArgs to grab them up:

data CodeActionArgs = CodeActionArgs
  { caaExportsMap   :: ExportsMap
  , caaIdeOptions   :: IdeOptions
  , caaParsedModule :: Maybe ParsedModule
  , caaContents     :: Maybe Text
  , caaDf           :: Maybe DynFlags
  , caaAnnSource    :: Maybe (Annotated ParsedSource)
  , caaTmr          :: Maybe TcModuleResult
  , caaHar          :: Maybe HieAstResult
  , caaBindings     :: Maybe Bindings
  , caaGblSigs      :: Maybe GlobalBindingTypeSigsResult
  , caaDiagnostics  :: Diagnostic
  }

Then we have a type class ToCodeAction:

class ToCodeAction a where
  toCodeAction :: CodeActionArgs -> a -> [(T.Text, [TextEdit])]

where a can be converted into [(T.Text, [TextEdit])] (representation of code actions we use in suggestAction) under the context of CodeActionArgs. And we can define some instances of acceptable return types:

instance ToCodeAction [(T.Text, [TextEdit])] where
  toCodeAction _ = id
instance ToCodeAction [(T.Text, [Rewrite])] where
  toCodeAction CodeActionArgs{..} = rewrite caaDf caaAnnSource
instance ToCodeAction [(T.Text, [TextEdit |? Rewrite])] where
  toCodeAction CodeActionArgs{..} r = second (concatMap go) <$> r
   where
    go (InL te) = [te]
    go (InR rw)
      | Just df <- caaDf
        , Just ps <- caaAnnSource
        , Right x <- rewriteToEdit df (annsA ps) rw =
        x
      | otherwise = []

For each field fld of CodeActionArgs, we can make an instance instance ToCodeAction r => ToCodeAction (fld -> r) (fields of CodeActionArgs don't have overlapped types). In such instances, we take fld from CodeActionArgs then feed it into (fld -> r) to get r. If fld is Maybe a, we make ToCodeAction (Maybe a -> r) and ToCodeAction (a -> r). Take caaDf as an example:

instance ToCodeAction r => ToCodeAction (Maybe DynFlags -> r) where
  toCodeAction caa@CodeActionArgs {caaDf = x} f = toCodeAction caa $ f x
instance ToCodeAction r => ToCodeAction (DynFlags -> r) where
  toCodeAction caa@CodeActionArgs {caaDf = Just x} f = toCodeAction caa $ f x
  toCodeAction _ _  = []

Once we have those instances, suggestX (functions providing code actions) could be free to use values from CodeActionArgs implicitly, with Maybe or without Maybe. And we keep backward compatibility -- this PR doesn't change any these kind of functions. Moreover, they could use [TextEdit |? Rewrite] as the return type, which addresses the above issue.

[pepeiborra]

There are many ways to skin this cat, I just don't know if this is the best one.

Maybe it would help if you updated the PR description with an overview of the goals and rationale behind this change (and move the other remarks about newImport and Rewrite to a separate issue).

[pepeiborra]

I haven't had time to review this yet, but I think the new description is awesome

[pepeiborra]

Now that I understand what is going on, I think this is very cool. Nice Job!

[berberman]

I run ormolu first and then used stylish-haskell, but the later one didn't revert the changes done by the former one... Is that what we expect? @Ailrun

[Ailrun]

Yes in this case, as record formatting is disabled by default. Maybe it's better to enable it though.

[pepeiborra]

What does the Bool mean? A type synonym would go a long way:

type Meaning = Bool
type GhcideCodeActions = [ ..Meaning, ..]