git.iko.soy/ghcide
on master help sign in
Improve completion contexts (#495)

The completion context determines whether we show completions for types or completions for values. This is done by looking at the parsed module. This PR fixes two things: 1. While we only use the parsed module for getting the context previously we got the parsed module out of the typechecked module. This means that if you have a module that parses but doesn’t typecheck, we will use the parsed module at the point where it last typechecked which is out of date and produces incorrect (or just no) contexts. 2. When we could not find a context, we defaulted to assuming we are in a value context. Especially in combination with 1 but also just in general, this is rather annoying. If we aren’t sure we should show the user everything we have and not filter out some completions. Filtering out completions interacts particularly badly with VSCode’s default behavior of accepting the first completion when you press return.

commit45f97424619d6f78b1397f98f75f40e2c9462e65 authorMoritz Kiefer <moritz.kiefer@purelyfunctional.org> dateFri 20 Mar 2020, 14:42 (7 years ago) parent07a5d32
3 files changed+44−20
compare with master
unifiedsplit
src/Development/IDE/Plugin/Completions.hs+13−9 src/Development/IDE/Plugin/Completions/Logic.hs+10−11 test/exe/Main.hs+21
src/Development/IDE/Plugin/Completions.hs+13−9
@@ -2,6 +2,8 @@
22
33module Development.IDE.Plugin.Completions(plugin) where
44
5+import Control.Applicative
6+import Data.Maybe
57import Language.Haskell.LSP.Messages
68import Language.Haskell.LSP.Types
79import qualified Language.Haskell.LSP.Core as LSP
@@ -10,8 +12,6 @@ import Language.Haskell.LSP.Types.Capabilities
1012import Development.Shake.Classes
1113import Development.Shake
1214import GHC.Generics
13−import Data.Maybe
14−import HscTypes
1515
1616import Development.IDE.Plugin
1717import Development.IDE.Core.Service
@@ -37,14 +37,14 @@ produceCompletions =
3737 packageState <- fmap (hscEnv . fst) <$> useWithStale GhcSession file
3838 case (tm, packageState) of
3939 (Just tm', Just packageState') -> do
40− cdata <- liftIO $ cacheDataProducer packageState' (hsc_dflags packageState')
40+ cdata <- liftIO $ cacheDataProducer packageState'
4141 (tmrModule tm') parsedDeps
42− return ([], Just (cdata, tm'))
42+ return ([], Just cdata)
4343 _ -> return ([], Nothing)
4444
4545
4646-- | Produce completions info for a file
47−type instance RuleResult ProduceCompletions = (CachedCompletions, TcModuleResult)
47+type instance RuleResult ProduceCompletions = CachedCompletions
4848
4949data ProduceCompletions = ProduceCompletions
5050 deriving (Eq, Show, Typeable, Generic)
@@ -67,17 +67,21 @@ getCompletionsLSP lsp ide
6767 fmap Right $ case (contents, uriToFilePath' uri) of
6868 (Just cnts, Just path) -> do
6969 let npath = toNormalizedFilePath path
70− (ideOpts, compls) <- runAction ide ((,) <$> getIdeOptions <*> useWithStale ProduceCompletions npath)
70+ (ideOpts, compls) <- runAction ide $ do
71+ opts <- getIdeOptions
72+ compls <- useWithStale ProduceCompletions npath
73+ pm <- useWithStale GetParsedModule npath
74+ pure (opts, liftA2 (,) compls pm)
7175 case compls of
72− Just ((cci', tm'), mapping) -> do
73− let position' = fromCurrentPosition mapping position
76+ Just ((cci', _), (pm, mapping)) -> do
77+ let !position' = fromCurrentPosition mapping position
7478 pfix <- maybe (return Nothing) (flip VFS.getCompletionPrefix cnts) position'
7579 case (pfix, completionContext) of
7680 (Just (VFS.PosPrefixInfo _ "" _ _), Just CompletionContext { _triggerCharacter = Just "."})
7781 -> return (Completions $ List [])
7882 (Just pfix', _) -> do
7983 let fakeClientCapabilities = ClientCapabilities Nothing Nothing Nothing Nothing
80− Completions . List <$> getCompletions ideOpts cci' (tmrModule tm') pfix' fakeClientCapabilities (WithSnippets True)
84+ Completions . List <$> getCompletions ideOpts cci' pm pfix' fakeClientCapabilities (WithSnippets True)
8185 _ -> return (Completions $ List [])
8286 _ -> return (Completions $ List [])
8387 _ -> return (Completions $ List [])
src/Development/IDE/Plugin/Completions/Logic.hs+10−11
@@ -210,9 +210,10 @@ mkPragmaCompl label insertText =
210210 Nothing Nothing Nothing Nothing Nothing (Just insertText) (Just Snippet)
211211 Nothing Nothing Nothing Nothing Nothing
212212
213−cacheDataProducer :: HscEnv -> DynFlags -> TypecheckedModule -> [ParsedModule] -> IO CachedCompletions
214−cacheDataProducer packageState dflags tm deps = do
213+cacheDataProducer :: HscEnv -> TypecheckedModule -> [ParsedModule] -> IO CachedCompletions
214+cacheDataProducer packageState tm deps = do
215215 let parsedMod = tm_parsed_module tm
216+ dflags = hsc_dflags packageState
216217 curMod = moduleName $ ms_mod $ pm_mod_summary parsedMod
217218 Just (_,limports,_,_) = tm_renamed_source tm
218219
@@ -306,16 +307,13 @@ toggleSnippets ClientCapabilities { _textDocument } (WithSnippets with) x
306307 where supported = Just True == (_textDocument >>= _completion >>= _completionItem >>= _snippetSupport)
307308
308309-- | Returns the cached completions for the given module and position.
309−getCompletions :: IdeOptions -> CachedCompletions -> TypecheckedModule -> VFS.PosPrefixInfo -> ClientCapabilities -> WithSnippets -> IO [CompletionItem]
310+getCompletions :: IdeOptions -> CachedCompletions -> ParsedModule -> VFS.PosPrefixInfo -> ClientCapabilities -> WithSnippets -> IO [CompletionItem]
310311getCompletions ideOpts CC { allModNamesAsNS, unqualCompls, qualCompls, importableModules }
311− tm prefixInfo caps withSnippets = do
312+ pm prefixInfo caps withSnippets = do
312313 let VFS.PosPrefixInfo { VFS.fullLine, VFS.prefixModule, VFS.prefixText } = prefixInfo
313314 enteredQual = if T.null prefixModule then "" else prefixModule <> "."
314315 fullPrefix = enteredQual <> prefixText
315316
316− -- default to value context if no explicit context
317− context = fromMaybe ValueContext $ getCContext pos (tm_parsed_module tm)
318−
319317 {- correct the position by moving 'foo :: Int -> String -> '
320318 ^
321319 to 'foo :: Int -> String -> '
@@ -344,10 +342,11 @@ getCompletions ideOpts CC { allModNamesAsNS, unqualCompls, qualCompls, importabl
344342 where
345343 isTypeCompl = isTcOcc . occName . origName
346344 -- completions specific to the current context
347− ctxCompls' = case context of
348− TypeContext -> filter isTypeCompl compls
349− ValueContext -> filter (not . isTypeCompl) compls
350− _ -> filter (not . isTypeCompl) compls
345+ ctxCompls' = case getCContext pos pm of
346+ Nothing -> compls
347+ Just TypeContext -> filter isTypeCompl compls
348+ Just ValueContext -> filter (not . isTypeCompl) compls
349+ Just _ -> filter (not . isTypeCompl) compls
351350 -- Add whether the text to insert has backticks
352351 ctxCompls = map (\comp -> comp { isInfix = infixCompls }) ctxCompls'
353352
test/exe/Main.hs+21
@@ -1630,6 +1630,27 @@ completionTests
16301630 docId <- openDoc' "A.hs" "haskell" source
16311631 compls <- getCompletions docId (Position 1 9)
16321632 liftIO $ compls @?= [keywordItem "newtype"]
1633+ , testSessionWait "type context" $ do
1634+ let source = T.unlines
1635+ [ "{-# OPTIONS_GHC -Wunused-binds #-}"
1636+ , "module A () where"
1637+ , "f = f"
1638+ ]
1639+ docId <- openDoc' "A.hs" "haskell" source
1640+ expectDiagnostics [("A.hs", [(DsWarning, (2, 0), "not used")])]
1641+ changeDoc docId
1642+ [ TextDocumentContentChangeEvent Nothing Nothing $ T.unlines
1643+ [ "{-# OPTIONS_GHC -Wunused-binds #-}"
1644+ , "module A () where"
1645+ , "f = f"
1646+ , "g :: Intege"
1647+ ]
1648+ ]
1649+ -- At this point the module parses but does not typecheck.
1650+ -- This should be sufficient to detect that we are in a
1651+ -- type context and only show the completion to the type.
1652+ compls <- getCompletions docId (Position 3 11)
1653+ liftIO $ map dropDocs compls @?= [complItem "Integer"(Just CiStruct) (Just "*")]
16331654 ]
16341655 where
16351656 dropDocs :: CompletionItem -> CompletionItem