Skip to content

Commit 683bbd9

Browse files
committed
Filter code actions based on prefix, not equality
It's quite unclear in the spec, but in microsoft/language-server-protocol#970 it's suggested that the intention is that the kinds given in `only` should be used as *prefix* filters of the generated code action kinds. That is to say, if the client asks for `only = [ CodeActionRefactor ]`, we should give them all kinds of refactoring code actions, including those whose kind is `CodeActionRefactorInline` (because as "hierarchical strings" they are represented as `"refactor"` and `"refactor.inline"`). This is quite important for the client: e.g. I hit this because I wanted to ask for all the import quickfixes so I could present them to the user to pick one. But they use various subkinds of `"quickfix.import"`, so currently you cannot ask for them all (asking for `"quickfix.import"` currentl returns nothing!). The ipmlemention is a little ugly: this needs some helper funcitons in `lsp`, which I'll make a PR for separately, but I didn't want to block this.
1 parent 12e7742 commit 683bbd9

File tree

2 files changed

+20
-13
lines changed

2 files changed

+20
-13
lines changed

Diff for: hls-plugin-api/src/Ide/Types.hs

+10-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,16 @@ instance PluginMethod TextDocumentCodeAction where
192192
wasRequested (InR ca)
193193
| Nothing <- _only context = True
194194
| Just (List allowed) <- _only context
195-
, Just caKind <- ca ^. kind = caKind `elem` allowed
195+
-- See https://github.com/microsoft/language-server-protocol/issues/970
196+
-- This is somewhat vague, but due to the hierarchical nature of action kinds, we
197+
-- should check whether the requested kind is a *prefix* of the action kind.
198+
-- That means, for example, we will return actions with kinds `quickfix.import` and
199+
-- `quickfix.somethingElse` if the requested kind is `quickfix`.
200+
-- TODO: add helpers in `lsp` for handling code action hierarchies
201+
-- For now we abuse the fact that the JSON representation gives us the hierarchical string.
202+
, Just caKind <- ca ^. kind
203+
, String caKindStr <- toJSON caKind =
204+
any (\k -> k `T.isPrefixOf` caKindStr) [kstr | k <- allowed, let String kstr = toJSON k ]
196205
| otherwise = False
197206

198207
instance PluginMethod TextDocumentCodeLens where

Diff for: test/functional/FunctionalCodeAction.hs

+10-12
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Control.Monad
99
import Data.Aeson
1010
import qualified Data.HashMap.Strict as HM
1111
import Data.List
12-
import qualified Data.Map as M
12+
import qualified Data.Map as M
1313
import Data.Maybe
1414
import qualified Data.Text as T
1515
import Ide.Plugin.Config
@@ -527,31 +527,29 @@ unusedTermTests = testGroup "unused term code actions" [
527527
_ <- waitForDiagnosticsFrom doc
528528
diags <- getCurrentDiagnostics doc
529529
let params = CodeActionParams Nothing Nothing doc (Range (Position 1 0) (Position 4 0)) caContext
530-
caContext = CodeActionContext (List diags) (Just (List [CodeActionRefactorInline]))
530+
caContext = CodeActionContext (List diags) (Just (List [CodeActionRefactor]))
531531
caContextAllActions = CodeActionContext (List diags) Nothing
532532
-- Verify that we get code actions of at least two different kinds.
533-
ResponseMessage _ _ (Right (List allCodeActions))
533+
ResponseMessage _ _ (Right (List res))
534534
<- request STextDocumentCodeAction (params & L.context .~ caContextAllActions)
535535
liftIO $ do
536-
redundantId <- inspectCodeAction allCodeActions ["Redundant id"]
537-
redundantId ^. L.kind @?= Just CodeActionQuickFix
538-
unfoldFoo <- inspectCodeAction allCodeActions ["Unfold foo"]
539-
unfoldFoo ^. L.kind @?= Just CodeActionRefactorInline
536+
let cas = map fromAction res
537+
kinds = map (^. L.kind) cas
538+
nub kinds @?= [Just CodeActionRefactorInline, Just CodeActionRefactorExtract, Just CodeActionQuickFix]
540539
-- Verify that that when we set the only parameter, we only get actions
541540
-- of the right kind.
542541
ResponseMessage _ _ (Right (List res)) <- request STextDocumentCodeAction params
543-
let cas = map fromAction res
544-
kinds = map (^. L.kind) cas
545542
liftIO $ do
546-
not (null kinds) @? "We found an action of kind RefactorInline"
547-
all (Just CodeActionRefactorInline ==) kinds @? "All CodeActionRefactorInline"
543+
let cas = map fromAction res
544+
kinds = map (^. L.kind) cas
545+
nub kinds @?= nub [Just CodeActionRefactorInline, Just CodeActionRefactorExtract]
548546
]
549547

550548
expectFailIfGhc9 :: String -> TestTree -> TestTree
551549
expectFailIfGhc9 reason =
552550
case ghcVersion of
553551
GHC90 -> expectFailBecause reason
554-
_ -> id
552+
_ -> id
555553

556554
disableWingman :: Session ()
557555
disableWingman =

0 commit comments

Comments
 (0)