Skip to content
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

x/tools/gopls: support quick fixes for type errors #34644

Open
asankov opened this issue Oct 1, 2019 · 4 comments
Open

x/tools/gopls: support quick fixes for type errors #34644

asankov opened this issue Oct 1, 2019 · 4 comments

Comments

@asankov
Copy link

@asankov asankov commented Oct 1, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

MacOS Mojave 10.14.5

go env Output
$ go env
GOOS="darwin"
GOARCH="amd64"

What did you do?

  1. I am using VSCode for Go development
  2. open/create a golang file
  3. write a function without specifying a return type
  4. return inside the function
  5. VS Code shows error - no result values expected (naturally)

Screen Shot 2019-10-01 at 15 15 47

Screen Shot 2019-10-01 at 15 15 40

What did you expect to see?

I would expect to see option like Specify return value, which when clicked would append the return value to method definition

What did you see instead?

No quick fixes available

I tried to troubleshoot the issue myself. From the MS LSP specification I found out that these actions are controlled by the textDocument/codeAction request. I grep-ed that in the /go/tools code and got to the (h serverHandler) Deliver method. The handler of this request calls (s *Server) CodeAction, which in terms calles the private (s *Server) codeAction. This method is quite long and I could not continue investigation further.

I am willing to help with the design/implentation of this, if you think it makes sense to have something like this build into the tools. Also, I would be thankful if you point me to some guidelines for building/running the language server locally, so I can debug a sample request and see what is happening where.

@gopherbot gopherbot added this to the Unreleased milestone Oct 1, 2019
@stamblerre stamblerre changed the title x/tools/gopls: No quick fixes for `no return values expected` x/tools/gopls: support quick fixes for compile errors Oct 2, 2019
@stamblerre stamblerre changed the title x/tools/gopls: support quick fixes for compile errors x/tools/gopls: support quick fixes for type errors Oct 2, 2019
@golang golang deleted a comment from gopherbot Oct 2, 2019
@asankov

This comment has been minimized.

Copy link
Author

@asankov asankov commented Oct 7, 2019

any update here?

as mentioned - I am willing to work on this myself and contribute it to gopls.

Any help will be appreciated.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Oct 7, 2019

Hi @asankov - thank you for filing this detailed issue report! You are correct in saying that these types of quick fixes are provided as code actions.

To add such a fix, we would first need to write an analysis with a suggested fix. You may find this talk given by @matloob helpful when getting started with go/analysis. Once the analysis is written, it can be added to gopls quite simply - just by adding it to this list.

You can run your own version of gopls simply by cding into golang.org/x/tools/gopls and running go install on your machine. To avoid clobbering your stable version of gopls, you can rename the development binary. You can also have per-workspace configurations for VSCode which differentiate which version of gopls to use based on the workspace.

@asankov

This comment has been minimized.

Copy link
Author

@asankov asankov commented Nov 9, 2019

Hello @stamblerre

Thank you for the detailed explanation. Michael's talk you refereed to and your talk from the same conference were very useful to me in getting some grasp on how goplsand analyzers work.

Regarding this issue - I was wondering whether such analyzer does not already exist somewhere. The reason being that I already get an error message in my IDE, I just don't get 'quick fixes'. I found no such analyzer in the defaultAnalyzers slice in the code you refereed, but after a bit more digging it turned out that something of the sort indeed exists:

~/go/src/github.com/golang 
$ grep -ri 'no result values expected' .
....
./go/src/go/types/stmt.go: check.error(s.Results[0].Pos(), "no result values expected")

this is the result of a grep for the error msg in the go codebase. Turns out this check is built in go vet and running go vet on the file, which I used for the screenshot yields the same error I see in my IDE. The problem is that when I looked into the code this is not an analyzer, and I don't think it will be trivial to extend it in a way that will be able to show the 'quick actions'.

So I was thinking that I can try to write a brand new analyzer from scratch and add it to the defaultAnalyzers slice in the go/tools codebase.

What's your take on that? Do you think it makes sense to write something brand new, when there already is something similar in the open, and do you think that adding such checker to gopls would conflict in some way with go vet and cause any problems to the users?

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Nov 9, 2019

Thanks for following up! The codebase that you were looking at here is the type-checker (https://golang.org/pkg/go/types/), which gopls uses to type-check your code. Because the type-checker is in the standard library, it isn't likely that we will be able to modify it to support suggested fixes anytime soon. I think writing analyses to provide suggested fixes is likely the best approach, and it won't conflict with go vet because vet doesn't deal with syntax or type errors.

@stamblerre stamblerre modified the milestones: Unreleased, gopls unplanned Dec 4, 2019
@stamblerre stamblerre modified the milestones: gopls unplanned, Unreleased Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.