Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

How to autocomplete unimported packages? #271

Open
JeanMertz opened this issue May 7, 2018 · 11 comments
Open

How to autocomplete unimported packages? #271

JeanMertz opened this issue May 7, 2018 · 11 comments

Comments

@JeanMertz
Copy link

I'd like something like strconv. to give me auto completion, but that only works when the strconv package is already imported.

Any way to make go-langserver work with unimported packages?

I found some references here:

//-------------------------------------------------------------------------
// config
//
// Structure represents persistent config storage of the gocode daemon. Usually
// the config is located somewhere in ~/.config/gocode directory.
//-------------------------------------------------------------------------
type config struct {
ProposeBuiltins bool `json:"propose-builtins"`
LibPath string `json:"lib-path"`
CustomPkgPrefix string `json:"custom-pkg-prefix"`
CustomVendorDir string `json:"custom-vendor-dir"`
Autobuild bool `json:"autobuild"`
ForceDebugOutput string `json:"force-debug-output"`
PackageLookupMode string `json:"package-lookup-mode"`
CloseTimeout int `json:"close-timeout"`
UnimportedPackages bool `json:"unimported-packages"`
}

But I couldn't find any other references/issues/documentation, so just wanted to make sure this is still correct (and that I need to configure gocode manually, to make this work).

@JeanMertz
Copy link
Author

Ah, in fact, I already have this configuration file, but it doesn't seem to be working?

{
  "propose-builtins": true,
  "lib-path": "",
  "custom-pkg-prefix": "",
  "custom-vendor-dir": "",
  "autobuild": true,
  "force-debug-output": "",
  "package-lookup-mode": "go",
  "close-timeout": 1800,
  "unimported-packages": true,
  "partials": true,
  "ignore-case": false,
  "class-filtering": true
}

I'm using go-langserver together with https://github.com/tomv564/LSP, but as far as I know, the client doesn't need to do anything special to support this, does it?

@JeanMertz
Copy link
Author

JeanMertz commented May 7, 2018

I just tried this in my terminal, and gocode is definitely returning values as expected:

echo -e "package main\n\nfunc main() {\n  strconv.At\n}\n" | gocode -f json autocomplete . 40 | jq .[1]
[
  {
    "class": "func",
    "name": "Atoi",
    "type": "func(s string) (int, error)",
    "package": "strconv"
  }
]

@JeanMertz
Copy link
Author

JeanMertz commented May 7, 2018

When enabling debug logging in the client, I see the following response returned by the server:

{'isIncomplete': False, 'items': []}

@JeanMertz
Copy link
Author

After digging some more, I guess the reason is that the code that I found regarding gocode is vendored code, and it looks like go-langserver doesn't use the file-based gocode configuration.

Looking at the InitDaemon code, it seems there are some hard-coded preferences, that can't be changed:

func InitDaemon(bc *build.Context) {
bctx = pack_build_context(bc)
g_config.ProposeBuiltins = true
g_daemon = new(daemon)
g_daemon.drop_cache()
}

Would you be willing to accept a PR to enable auto-completing unimported package by default? Or, if I'm off the mark and there is a way to enable this, let me know!

@JeanMertz
Copy link
Author

I validated that this diff works as expected:

diff --git a/langserver/internal/gocode/export.go b/langserver/internal/gocode/export.go
index ac533ff..00a0b70 100644
--- a/langserver/internal/gocode/export.go
+++ b/langserver/internal/gocode/export.go
@@ -9,6 +9,7 @@ var bctx go_build_context
 func InitDaemon(bc *build.Context) {
        bctx = pack_build_context(bc)
        g_config.ProposeBuiltins = true
+       g_config.UnimportedPackages = true
        g_daemon = new(daemon)
        g_daemon.drop_cache()
 }

obviously this would be a big change to make, so I'm unsure if this is something you want in a PR, or if more of this should be configurable through flags...

@keegancsmith
Copy link
Member

cc @Contextualist Any reason why we shouldn't enable unimportedpackages?

@JeanMertz
Copy link
Author

I just noticed that this breaks if you trigger autocomplete on a package that doesn't exist.

Using this file:

package main

func main() {

}

If I write fmt. in the main function, things work as expected, but if I instead write fm., then it crashes.

Here's the log:

https://gist.github.com/JeanMertz/eb2db980229d5caef496c9cdbd7a69b5

And here's the offending request/response:

--> request #5: textDocument/completion: {"textDocument":{"uri":"file:///Users/jean/code/go/src/github.com/blendle/importtest/main.go"},"position":{"line":3,"character":4}}
<-- result #5: textDocument/completion: {"isIncomplete":false,"items":[{"label":"PANIC","detail":"PANIC","insertText":"PANIC","insertTextFormat":1,"textEdit":{"range":{"start":{"line":3,"character":4},"end":{"line":3,"character":4}},"newText":"PANIC"}}]}

@JeanMertz
Copy link
Author

Seems to originate from here:

case decl_invalid:
return "PANIC"

@keegancsmith
Copy link
Member

FYI We do plan on switching our vendored in gocode to a new upstream implementation. So that sort of issues may disappear / belong in the upstream repo. See the discussion in #268

@JeanMertz
Copy link
Author

In the end, I copy/pasted the latest master source of gocode into go-langserver (and removed some unrelated changes/code), it now works as expected.

Full diff here:

https://gist.github.com/JeanMertz/aeb524db3b6eabfa46d9298ac228f1a5

I suspect this is the relevant change that fixed the above bug:

@@ -361,15 +362,29 @@ func (c *auto_complete_context) apropos(file []byte, filename string, cursor int
        // And we're ready to Go. ;)
 
        if !ok {
                var d *decl
                if ident, ok := cc.expr.(*ast.Ident); ok && g_config.UnimportedPackages {
                        p := resolveKnownPackageIdent(ident.Name, c.current.name, c.current.context)
-                       c.pcache[p.name] = p
-                       d = p.main
+                       if p != nil {
+                               c.pcache[p.name] = p
+                               d = p.main
+                       }
                }
                if d == nil {
                        return nil, 0

@Contextualist
Copy link
Contributor

@JeanMertz 👍 Wow, such a trek of digging! You're right. Sorry that I didn't include the config capability of gocode for simplicity sake. Also, nice finding on the behavior of nonexistent package. However, I am a little bit hesitated on whether to enable the option. See my second concern below.

@keegancsmith For why I drop that option is largely opinionated. I was planning to bring that option in together with the "auto import on formatting" (as being brought up recently by #272) as a integrated workflow. Anyway, there is really no harm to just enable it. My second concern is that mdempsky/gocode we are switching to (see #268) seems lack of support for both gocode config file and unimportedpackages. I haven't thought of how to implement that on our side.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants