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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: show diagnostic for for circular dependency #33085

Closed
inliquid opened this issue Jul 12, 2019 · 7 comments
Closed

x/tools/gopls: show diagnostic for for circular dependency #33085

inliquid opened this issue Jul 12, 2019 · 7 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsFix The path to resolution is known, but the work has not been done.

Comments

@inliquid
Copy link

VS Code. Say, we have a circular dependency. In this case go vet would warn that it's not allowed. However it seems that gopls doesn't check this and just stops working (not providing hover outputs, references, what so ever) with these non-informative errors in output:

[Error - 21:27:10] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:10] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:10] Request textDocument/hover failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:12] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:13] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:13] Request textDocument/definition failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:13] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:14] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:14] Request textDocument/hover failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:15] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:16] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:16] Request textDocument/definition failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:16] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:17] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:17] Request textDocument/hover failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:18] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:18] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:18] Request textDocument/definition failed.
  Message: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
  Code: 0 
[Error - 21:27:19] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:20] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
[Error - 21:27:20] no highlight for c:\Users\***\go\src\path\to\workdir\pkg\client\ui\ui.go:21:25: no AST for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go
[Error - 21:27:21] unable to check package for file:///c:/Users/***/go/src/path/to/workdir/pkg/client/ui/ui.go: package path/to/workdir/pkg/client/ui has errors, skipping type-checking
@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2019
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jul 12, 2019
@stamblerre stamblerre changed the title x/cmd/gopls: gopls doesn't vet package for circular dependency x/tools/gopls: show diagnostic for for circular dependency Jul 12, 2019
@stamblerre
Copy link
Contributor

Good catch, thank you for noticing this. You are right, gopls will not even bother to check your package if it notices a circular dependency (https://github.com/golang/tools/blob/d5f455491e2f028578d3349b43426ca65e2e0ee0/internal/lsp/cache/check.go#L51), so we should show a diagnostic for those cases.

This was referenced Jul 12, 2019
Closed
Closed
@stamblerre stamblerre added NeedsFix The path to resolution is known, but the work has not been done. help wanted Suggested Issues that may be good for new contributors looking for work to do. and removed Suggested Issues that may be good for new contributors looking for work to do. labels Aug 6, 2019
@urisimchoni urisimchoni mentioned this issue Sep 4, 2019
Closed
@jbszczepaniak
Copy link

I would like to claim this one ;)

@stamblerre
Copy link
Contributor

Great! Let me know if you need any advice on how to tackle it, or please feel free to reach out to me directly on Gophers Slack.

@jbszczepaniak
Copy link

As @stamblerre suggested I will take something easier for first task. Leaving this for now.

@stamblerre stamblerre modified the milestones: Unreleased, gopls unplanned Dec 4, 2019
@gopherbot
Copy link

Change https://golang.org/cl/209857 mentions this issue: internal/lsp: add error handling for self imports

@ridersofrohan
Copy link

After #35964 gets fixed. We need to adjust/write the diagnostic tests inside (tools/internal/lsp/testdata/circular/) to test the behavior and ensure that it is working.

gopherbot pushed a commit to golang/tools that referenced this issue Dec 6, 2019
@ridersofrohan
internal/lsp: add error handling for self imports
cec9580 
This change will provide a more useful error when you
are self importing a package. It has TODOs in place to propagate the
"import cycle not allowed" error from go list to the user.

Updates golang/go#33085

Change-Id: Ia868a7c688b0f0a7a9689cfda5ea8cea8ae1faff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209857
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/210942 mentions this issue: internal/lsp: add diagnostic on import causing import cycle

@golang golang locked and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ridersofrohan @stamblerre @inliquid @gopherbot @jbszczepaniak