tigersoldier / company-lsp

Company completion backend for lsp-mode
GNU General Public License v3.0
251 stars 26 forks source link

Problems with gopls autocompleting partially written functions #103

Open storvik opened 5 years ago

storvik commented 5 years ago

Cannot get autocompletion of partially written functions to work with go-mode and gopls. completion-at-point seems to be working correctly.

I am able to reproduce using https://github.com/emacs-lsp/lsp-mode/blob/master/scripts/lsp-start-plain.el only adding go-mode to installed packages. Installed gopls by running go get -u golang.org/x/tools/cmd/goplsSee attached file for minimal example.tar.gz. Example must be placed in $GOPATH/src/test/ or similar in order to run.

What happens is when trying to autocomplete fmt. every possible candidate is showed, Printf, Print, Println, etc. But when trying to autocomplete fmt.Pri I get nothing. Checked output of *lsp-log* to find that candidate is returned from language server.

Output returned in *Messages* is:

No completion found user-error: Cannot complete at point

complation-at-point autocompletes fmt.Pri to fmt.Print however.

Checked (company-lsp--completion-prefix) to find that prefix is ("" . t) when autocompleting fmt. and (#("Pri" 0 3 (fontified t)) . t) when completing partially written function beginning with "Pri".

I also get loads of messages similar to LSP :: no object for ident Pri, but not sure this is related to company-lsp.

It seems to be working in other modes, so this may be related to Go and gopls?

Thanks in advance!

yyoncho commented 5 years ago

@storvik

(add-to-list 'company-lsp-filter-candidates '(gopls . nil))

This line will sove your problem.

@tigersoldier I was unable to track down what is causing this behaviour.

storvik commented 5 years ago

Thank you @yyoncho seems to be working perfectly now!

Not sure if this issue should be closed or if you want to keep it open?

yyoncho commented 5 years ago

@storvik it should be open, we need to make it work out of the box.

@tigersoldier the problem is that the firlterText is the remaining part, e. g. "int($1)" (from Print).I am really puzzled how it is all supposed to work...

muirdm commented 5 years ago

It seems like gopls is sending back a "progressive" filterText. The client sends a complete request after fmt.P has been typed, so gopls gives results with a filterText that doesn't include the leading P. gopls seems to be assuming (not unreasonably) that subsequent filtering will start from after the P that has already been entered.

@stamblerre do you know any details about filterText? Is it supposed to be progressive like this, or is it possible it is supposed to take into account the entire prefix the user has typed so far? The spec doesn't really make it clear.

I tried to compare to other language servers, but Java and Rust don't seem to send filterText. Does anyone know of another language server that sets filterText?

yyoncho commented 5 years ago

@muirrn you may try with clangd.

tigersoldier commented 5 years ago

Sorry for late response. I'll disable client-side filtering for gopls as a quick fix. A better solution will be comparing the filterText with and without the prefix. The best solution is to implement flx filtering.

yyoncho commented 5 years ago

A better solution will be comparing the filterText with and without the prefix. The best solution is to implement flx filtering.

I think that unifing the filtering with what vscode does is a good idea since pretty much everybody is testing only with vscode - https://github.com/Microsoft/vscode/blob/d13f20e019044747729ab0370893e666090299cb/src/vs/editor/contrib/suggest/completionModel.ts . As far as I can see it automatically determines whether to use client side filtering (not sure how).

muirdm commented 5 years ago

I'll disable client-side filtering for gopls as a quick fix.

Thanks, that did the trick.

I looked at other clients/servers and opened a bug for gopls golang/go#31552 since gopls is the only one I found that behaves like this. Still not sure if filterText is allowed/intended to work like this.

I think that unifing the filtering with what vscode does

Sounds good to me.

yyoncho commented 5 years ago

@tigersoldier we have problems with xml autocompletion.

The prefix is calculated incorrectly. If you have <tag the prefix will be calculated to tag while each of the items will be <tag which will remove all items from the list. At the same time, the workaround with disabling client-side filtering does not work since the xml server does not filter at all.

yyoncho commented 5 years ago

@tigersoldier I did some experimentation with company-lsp--completion-prefix, I changed it to company-grab-symbol and it seems to fix the issues with clojure and xml. Do you know a language server that won't work with that approach and requires prefix calculation using trigger chars?

tigersoldier commented 5 years ago

@yyoncho I introduced trigger chars calculation because it doesn't work for Java when annotation is involved. The prefix will be @Override instead of Override.

Flex matching is on its way. Stay tuned.

yyoncho commented 5 years ago

I did some debugging and narrowed down how vscode works. VScode fetches the current word(i. e. prefix) depending on the currently defined syntax. Guess what, if you are editing xml and you have <tag the prefix is <tag but when you are editing html using the html language server and you have <tag the prefix is tag. And since everybody is testing only against vscode that this behaviour is inherited by the servers. Note that flex matching wont solve that problem if the word that you have calculated is not the right one.

yyoncho commented 5 years ago

FYI https://github.com/Microsoft/vscode/blob/a3099d3e10e47d5060bf73beb5c6fd5bae0567a9/src/vs/editor/common/model/textModel.ts#L2017

tigersoldier commented 5 years ago

@yyoncho If we are calculating prefix as <tag, the HTML language server will be broken. However, if we are always removing the trigger character, the prefix may be a substring of the prefix expected by the server (in this case the XML slanguage server). In this case flex matching should work.

yyoncho commented 5 years ago

@tigersoldier I agree that it will work in this case. You know what is the problem - if the word separator is not considered as a trigger character this might cause failure client side. IMO the proper fix is server to return the match instead of letting the client to use some heuristics to guess what the server has used as a prefix...

muirdm commented 5 years ago

This issue has been fixed in gopls for a while. We can probably close this issue (and re-enable the client side filtering, if desired).