Ghcide: Ignore import lists for producing completions

Created on 22 May 2020  路  15Comments  路  Source: haskell/ghcide

Given a module header like the one below with explicit import lists, ghcide should provide completions for all of Control.Monad and not limit to just join:

module MyModule where

import Control.Monad (join)

It would be great if selecting a completion would also check and expand the corresponding import list if needed.

good first issue

All 15 comments

@pepeiborra I would like to take a look at this one, next. Should I be starting in Completions.hs to find out where the list is getting restricted?

Completions are driven by the LocalCompletions and NonLocalCompletions rules. To achieve the goal in this ticket, the NonLocalCompletions rule needs to be relaxed to generate completions for the unrestricted imports. This should be quite easy, the code is in:

https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Plugin/Completions.hs#L55-L81

Basically, use map unrestrictImport imps instead of imps

Great. Thanks for the pointer. Will review this item this week.

I did some preliminary research on this. Before, I take a undesired path approaching this, I wanted to confirm my understanding. Any feedback on this is appreciated.

Currently, the import list is being restricted because we map over [GlobalRdrElt] at https://github.com/haskell/ghcide/blob/863392b9b94777a069a2a31e9b909d1ce45e93d4/src/Development/IDE/Plugin/Completions/Logic.hs#L257.

My understanding is that GlobalRdrElt already only has the restricted elements based on the explicit import list.

To address this issue, we cannot use GlobalRdrElt anymore. Instead, we need to get all available completions, for each LImportDecl GhcPs. To accomplish that, we have to access the parsed module HsModule from each ImportDecl and then access the hsModExports values to build the CompItem.

This essentially means, the logic in cacheDataProducer will change entirely.

Is this a approach I can pursue?

Thanks!

This is not what I had in mind, almost no changes are needed if my suggestion below works.

We are in control of the import list used to generate the [GlobalRdrElt], since we explicitly call tcRnImportDecls. So just modify the import list before calling:

https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Plugin/Completions.hs#L74

Thanks. That makes sense.

Regarding automatically expanding the import list:

Will the command be part of the _command in Completion? (http://hackage.haskell.org/package/haskell-lsp-types-0.15.0.0/docs/Language-Haskell-LSP-Types.html#t:CompletionItem)

It could also happen that the same function comes from different imported modules. In that case, automatically extending would not work. Can the _command handle it?

I will try to understand how the _command works later this week, as well.

I think this is what _additionalTextEdits :: Maybe (List TextEdit) is for

Great. I will take a look at that. Thanks for the pointer.

Reopening for the automatic extension of import lists when needed.

Thanks for merging the previous changes. I will leave some notes here on how I am approaching the pending task.

I think the _additionalTextEdits is the way to go. For this I am thinking we need to do that following

  1. (Add back) Continue carrying the original import we filtered, as part of our earlier change(#919). We will need this to decide how to update the text.
  2. Update the CompItem record to capture this information by parsing the list in ImportDecl and generate a new Import statement.
  3. Transfer over these values to CompletionItem.

As a side note, this information is already available in the resulting CodeAction, but I think we may not be able to use it since this become available on the consecutive round trip between client and server.

We have some logic to extend import lists that would be appropriate to reuse:

https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Plugin/CodeAction.hs#L935-L977

You won't be able to directly reuse the existing CodeAction since that is driven by a GHC diagnostic so you would need to typecheck the module for every CompItem, which would take too long.

@pepeiborra I left some questions in the TODO list of the new PR. Can you or others provide suggestions on how to address those issues. Thanks!

Also, in general, is it desired to discuss these on the PR or over here on the issues ticket?

Can we close this or should we keep this open till we get user feedback on how the imports are automatically added?

Also, in general, is it desired to discuss these on the PR or over here on the issues ticket?

We have not a established rule so i usually link or quote the relevant comments in both

Can we close this or should we keep this open till we get user feedback on how the imports are automatically added?

I think we can close this one and open new ones with the concrete issues or suggestions from users

Was this page helpful?
0 / 5 - 0 ratings