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: errors in go.mod file prevents initial workspace load #36531

Closed
ridersofrohan opened this issue Jan 13, 2020 · 7 comments
Closed
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@ridersofrohan
Copy link

ridersofrohan commented Jan 13, 2020

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

$ go version
go version devel +8ac98e7b3f Thu Jan 9 18:00:06 2020 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

-- go.mod --
module mod.com

require golang.org/x/tools v0.0.0-20200107184032-11e9d9cc0042

yo

-- main.go --
package main

import "golang.org/x/tools/go/packages"

func Yo() {
	var _ packages.Config
}

What did you expect to see?

I expected the go.mod error to surface with the initial workspace load.

What did you see instead?

I saw that gopls logged the error due to the initial call packages.load call when the view is created. However, since there was an error with that initial load all the LSP features break. The error shows up once you save a .go file or start making edits in the go.mod file.

/cc @stamblerre

@gopherbot gopherbot added this to the Unreleased milestone Jan 13, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 13, 2020
@gopherbot
Copy link

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jan 13, 2020
@ridersofrohan
Copy link
Author

There is another place where the error message should be checked if it is related to a go.mod parse error: internal/lsp/cache/load.go#L72. Adding the same check as on internal/lsp/cache/builtin.go#L40 here would cause the go.mod error to show up on initial load of the workspace.

However, even after the error is addressed and the go.mod file is saved, gopls does not seem to call packages.load again and the error diagnostic does not disappear from the go.mod file.

@stamblerre
Copy link
Contributor

As a follow-up to https://golang.org/cl/214417, I will look into making one go/packages.Load call for "builtin" and the workspace. This should at least reduce the amount of special-cases needed. We do still have to figure out (1) what gopls should do when it sees an invalid go.mod file on start, and (2) if gopls should try to re-initialize if initialization failed.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Jan 14, 2020
@gopherbot
Copy link

Change https://golang.org/cl/214877 mentions this issue: internal/lsp/cache: refactor initialization for builtins

@stamblerre stamblerre modified the milestones: gopls v1.0, gopls/v0.3.0 Jan 15, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Jan 15, 2020
This change combines the two packages.Load calls that happen on view
creation. Builtins can be loaded along with the rest of the workspace.

To avoid race conditions, create a builtinPackageHandle type for
builtins and use it to create the data.

Updates golang/go#36531

Change-Id: I7aa342c463a0b7718e1ad5fee507622310d8443b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@stamblerre
Copy link
Contributor

Now that the above CL has been merged, the main question remains: How should gopls behave when your go.mod is broken. Realistically, we can't do anything until the initial workspace load succeeds, but we have to recover gracefully.

The problem is that we don't want to keep trying to initialize if it keeps failing. I propose that, if we have an initialization error, we retry after every change to the go.mod.

@stamblerre stamblerre added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 16, 2020
@ridersofrohan
Copy link
Author

Yeah, I think that approach works, since we will be able to surface go.mod errors even when the initial workspace load breaks.

@gopherbot
Copy link

Change https://golang.org/cl/216037 mentions this issue: internal/lsp: recreate the view when needed

@golang golang locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants