Skip to content

Auto complete definitions within imports #2152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 35 commits into from
Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
47fd19d
auto complete functions from imports
alexnaspo Sep 1, 2021
34e3e7a
Merge branch 'master' into import-func-completions
alexnaspo Sep 2, 2021
31471c7
Merge branch 'master' into import-func-completions
jneira Sep 2, 2021
8b9310e
address PR comments
alexnaspo Sep 2, 2021
dcfdbfc
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspo Sep 2, 2021
863a483
clean up
alexnaspo Sep 2, 2021
9306c91
Merge branch 'master' into import-func-completions
jneira Sep 3, 2021
21af666
remove duplicate HashMap import
alexnaspo Sep 3, 2021
2111b5e
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspo Sep 3, 2021
f899d00
use lookupDefault
alexnaspo Sep 3, 2021
987119b
Merge branch 'master' into import-func-completions
alexnaspoleap Sep 3, 2021
b3a0ad7
fuzzy match filter
alexnaspoleap Sep 4, 2021
b54678d
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspoleap Sep 4, 2021
7a73509
add field to exportsMap
alexnaspoleap Sep 5, 2021
10919e0
generate map from modIFace
alexnaspoleap Sep 5, 2021
2c519c4
Update ghcide/src/Development/IDE/Types/Exports.hs
alexnaspo Sep 5, 2021
387e5d0
module name text alias
alexnaspoleap Sep 5, 2021
feaa4e9
use hashset; enable local modules
alexnaspoleap Sep 5, 2021
5bfff3d
local module imports now working
alexnaspoleap Sep 6, 2021
8a167a0
derive map from exportMap
alexnaspo Sep 6, 2021
4f85abc
generate maps from list
alexnaspo Sep 6, 2021
daf1915
Merge branch 'master' into import-func-completions
alexnaspo Sep 6, 2021
1c1012d
clean up
alexnaspo Sep 6, 2021
80820bb
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspo Sep 6, 2021
b79af6b
addressing PR comments
alexnaspo Sep 7, 2021
7454e6c
Merge branch 'master' into import-func-completions
alexnaspo Sep 7, 2021
f0e25c3
clean up
alexnaspo Sep 7, 2021
fa7763e
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspo Sep 7, 2021
1077480
clean up
alexnaspo Sep 7, 2021
88f2c56
Merge branch 'master' into import-func-completions
alexnaspoleap Sep 8, 2021
110ce17
useWithStaleFast
alexnaspo Sep 8, 2021
63fb733
Merge branch 'import-func-completions' of github.com:alexnaspo/haskel…
alexnaspo Sep 8, 2021
6df51ae
Merge branch 'master' into import-func-completions
pepeiborra Sep 9, 2021
440fb82
Merge branch 'master' into import-func-completions
mergify[bot] Sep 9, 2021
689b1b3
Merge branch 'master' into import-func-completions
mergify[bot] Sep 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions ghcide/src/Development/IDE/Plugin/Completions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import Data.Aeson
import qualified Data.HashMap.Strict as Map
import qualified Data.HashSet as Set
import Data.List (find)
import qualified Data.Map as DM (Map, fromListWith, empty)
import Data.Maybe
import qualified Data.Text as T
import Development.IDE.Core.PositionMapping
Expand Down Expand Up @@ -131,35 +132,49 @@ getCompletionsLSP ide plId
fmap Right $ case (contents, uriToFilePath' uri) of
(Just cnts, Just path) -> do
let npath = toNormalizedFilePath' path
(ideOpts, compls) <- liftIO $ runIdeAction "Completion" (shakeExtras ide) $ do
(ideOpts, compls, moduleExports) <- liftIO $ runIdeAction "Completion" (shakeExtras ide) $ do
opts <- liftIO $ getIdeOptionsIO $ shakeExtras ide
localCompls <- useWithStaleFast LocalCompletions npath
nonLocalCompls <- useWithStaleFast NonLocalCompletions npath
pm <- useWithStaleFast GetParsedModule npath
binds <- fromMaybe (mempty, zeroMapping) <$> useWithStaleFast GetBindings npath
exportsMapIO <- fmap(envPackageExports . fst) <$> useWithStaleFast GhcSession npath
exportsMap <- mapM liftIO exportsMapIO
let moduleExports = buildModouleExportMap exportsMap
let exportsCompItems = foldMap (map (fromIdentInfo uri) . Set.toList) . Map.elems . getExportsMap <$> exportsMap
exportsCompls = mempty{anyQualCompls = fromMaybe [] exportsCompItems}
let compls = (fst <$> localCompls) <> (fst <$> nonLocalCompls) <> Just exportsCompls
pure (opts, fmap (,pm,binds) compls)
case compls of
Just (cci', parsedMod, bindMap) -> do
pure (opts, fmap (,pm,binds) compls, moduleExports)
case (compls, moduleExports) of
(Just (cci', parsedMod, bindMap), mExports) -> do
pfix <- VFS.getCompletionPrefix position cnts
case (pfix, completionContext) of
(Just (VFS.PosPrefixInfo _ "" _ _), Just CompletionContext { _triggerCharacter = Just "."})
-> return (InL $ List [])
(Just pfix', _) -> do
let clientCaps = clientCapabilities $ shakeExtras ide
config <- getCompletionsConfig plId
allCompletions <- liftIO $ getCompletions plId ideOpts cci' parsedMod bindMap pfix' clientCaps config
allCompletions <- liftIO $ getCompletions plId ideOpts cci' parsedMod bindMap pfix' clientCaps config mExports
pure $ InL (List allCompletions)
_ -> return (InL $ List [])
_ -> return (InL $ List [])
_ -> return (InL $ List [])

----------------------------------------------------------------------------------------------------

identInfoToKeyVal :: IdentInfo -> (T.Text, T.Text)
identInfoToKeyVal IdentInfo {rendered, moduleNameText} =
(moduleNameText, rendered)

buildModouleExportMap:: Maybe (ExportsMap) -> DM.Map T.Text [T.Text]
buildModouleExportMap (Just exportsMap) = do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this function do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a typo in the name, however, it builds a map of "module.name" -> ["functions" .. ] which I use to look up the available functions for that module

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why is it needed? Can't you use the ExportsMap as it is?

Copy link
Contributor Author

@alexnaspo alexnaspo Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I need it because I have to group on "module.name" and use that as the key to look up all functions within that module. ExportsMap as it is seems to be a map of "function-name" -> [{moduleNameText: "module-name", name: "function-name" ...} ]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, you are right.

But why build a map to do only one lookup and then throw it away?

  • Cost of building the map: O(NlogN)
  • Cost of doing the lookup by scanning the original exports map: O(N)

Neither is good enough, you want O(logN). How? Probably by changing the representation of the exports map to add a second index, the module name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think the first suggestion would be a good first pass? With potential follow up commits to pull out a plugin?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ExportsMap only indexes package modules. You will need to fetch the ModIface for local modules. So both changes are needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a first pass at this. There is still more work to do. As of this comment, I am just generating the map based on the already existing map. I also see that this does not work for local modules, as you suggested.

Copy link
Contributor Author

@alexnaspo alexnaspo Sep 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass - I generate the map from the modIface rather than the existing map.. This still does not account for local modules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I now generate the map for local modules as well. I will take a look at your other comments.

sortAndGroup $ map identInfoToKeyVal $
concatMap (Set.toList . snd) $ toList $ getExportsMap exportsMap
buildModouleExportMap (Nothing) = DM.empty

sortAndGroup :: [(T.Text, T.Text)] -> DM.Map T.Text [T.Text]
sortAndGroup assocs = DM.fromListWith (++) [(k, [v]) | (k, v) <- assocs]

extendImportCommand :: PluginCommand IdeState
extendImportCommand =
PluginCommand (CommandId extendImportCommandId) "additional edits for a completion" extendImportHandler
Expand Down
27 changes: 23 additions & 4 deletions ghcide/src/Development/IDE/Plugin/Completions/Logic.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import Control.Monad
import Data.Aeson (ToJSON (toJSON))
import Data.Either (fromRight)
import Data.Functor
import qualified Data.Map as DM (Map)
import qualified Data.Set as Set
import Development.IDE.Core.Compile
import Development.IDE.Core.PositionMapping
Expand All @@ -54,7 +55,7 @@ import Development.IDE.Spans.LocalBindings
import Development.IDE.Types.Exports
import Development.IDE.Types.HscEnvEq
import Development.IDE.Types.Options
import GhcPlugins (flLabel, unpackFS)
import GhcPlugins (flLabel, unpackFS, lookupWithDefaultUFM)
import Ide.PluginUtils (mkLspCommand)
import Ide.Types (CommandId (..),
PluginId)
Expand All @@ -64,6 +65,7 @@ import qualified Language.LSP.VFS as VFS
import Outputable (Outputable)
import TyCoRep


-- From haskell-ide-engine/hie-plugin-api/Haskell/Ide/Engine/Context.hs

-- | A context of a declaration in the program
Expand Down Expand Up @@ -285,6 +287,13 @@ mkModCompl label =
Nothing Nothing Nothing Nothing Nothing Nothing Nothing
Nothing Nothing Nothing Nothing Nothing Nothing


mkModuleFunctionImport :: T.Text -> T.Text -> CompletionItem
mkModuleFunctionImport moduleName label =
CompletionItem label (Just CiFunction) Nothing (Just moduleName)
Nothing Nothing Nothing Nothing Nothing Nothing Nothing
Nothing Nothing Nothing Nothing Nothing Nothing

mkImportCompl :: T.Text -> T.Text -> CompletionItem
mkImportCompl enteredQual label =
CompletionItem m (Just CiModule) Nothing (Just label)
Expand Down Expand Up @@ -530,9 +539,10 @@ getCompletions
-> VFS.PosPrefixInfo
-> ClientCapabilities
-> CompletionsConfig
-> DM.Map T.Text [T.Text]
-> IO [CompletionItem]
getCompletions plId ideOpts CC {allModNamesAsNS, anyQualCompls, unqualCompls, qualCompls, importableModules}
maybe_parsed (localBindings, bmapping) prefixInfo caps config = do
maybe_parsed (localBindings, bmapping) prefixInfo caps config exportsMap = do
let VFS.PosPrefixInfo { fullLine, prefixModule, prefixText } = prefixInfo
enteredQual = if T.null prefixModule then "" else prefixModule <> "."
fullPrefix = enteredQual <> prefixText
Expand Down Expand Up @@ -619,9 +629,17 @@ getCompletions plId ideOpts CC {allModNamesAsNS, anyQualCompls, unqualCompls, qu
| s == c = ss
| otherwise = s:ss

if
if
| "import " `T.isPrefixOf` fullLine
&& (List.length (words (T.unpack fullLine)) >= 2)
&& "(" `isInfixOf` T.unpack fullLine
-> do
let moduleName = (words (T.unpack fullLine)) !! 1
funcs = Map.findWithDefault [] (T.pack moduleName) exportsMap
return (map (mkModuleFunctionImport (T.pack moduleName)) funcs)
| "import " `T.isPrefixOf` fullLine
-> return filtImportCompls
-> do
return filtImportCompls
-- we leave this condition here to avoid duplications and return empty list
-- since HLS implements this completion (#haskell-language-server/pull/662)
| "{-# language" `T.isPrefixOf` T.toLower fullLine
Expand Down Expand Up @@ -651,6 +669,7 @@ uniqueCompl x y =
then EQ
else compare (insertText x) (insertText y)
other -> other

-- ---------------------------------------------------------------------
-- helper functions for pragmas
-- ---------------------------------------------------------------------
Expand Down
25 changes: 25 additions & 0 deletions test/functional/Completion.hs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ tests = testGroup "completions" [
compls <- getCompletions doc (Position 5 7)
liftIO $ length compls @?= maxCompletions def

, testCase "import function completions" $ runSession hlsCommand fullCaps "test/testdata/completion" $ do
doc <- openDoc "FunctionCompletions.hs" "haskell"

let te = TextEdit (Range (Position 0 30) (Position 0 41)) "A"
_ <- applyEdit doc te

compls <- getCompletions doc (Position 0 31)
let item = head $ filter ((== "Alternative") . (^. label)) compls
liftIO $ do
item ^. label @?= "Alternative"
item ^. kind @?= Just CiFunction
item ^. detail @?= Just "Control.Applicative"

, testCase "import second function completion" $ runSession hlsCommand fullCaps "test/testdata/completion" $ do
doc <- openDoc "FunctionCompletions.hs" "haskell"

let te = TextEdit (Range (Position 0 41) (Position 0 42)) ", l"
_ <- applyEdit doc te

compls <- getCompletions doc (Position 0 41)
let item = head $ filter ((== "liftA") . (^. label)) compls
liftIO $ do
item ^. label @?= "liftA"
item ^. kind @?= Just CiFunction
item ^. detail @?= Just "Control.Applicative"
, contextTests
, snippetTests
]
Expand Down
8 changes: 8 additions & 0 deletions test/testdata/completion/FunctionCompletions.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Control.Applicative (Alternative)
import qualified Data.List

main :: IO ()
main = putStrLn "hello"

foo :: Either a b -> Either a b
foo = id