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

Jump to bottom

x/tools/gopls: show errors on all files with import cycles #52904

Open
findleyr opened this issue May 13, 2022 · 1 comment
Open

x/tools/gopls: show errors on all files with import cycles #52904

findleyr opened this issue May 13, 2022 · 1 comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@findleyr
Copy link
Contributor

Investigating failures of gopls/internal/regtest/diagnostics.TestResolveImportCycles with @matloob, we discovered that gopls is not locating import cycle errors on all packages involved in an import cycle.

In go list json output, the Error field is only set for one package, but the rest can be inferred from the cycle information.
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels May 13, 2022
@gopherbot gopherbot added this to the Unreleased milestone May 13, 2022
@gopherbot
Copy link

Change https://go.dev/cl/406274 mentions this issue: internal/lsp/regtest: make TestResolveImportCycle robust to error order

gopherbot pushed a commit to golang/tools that referenced this issue May 13, 2022
@findleyr
internal/lsp/regtest: make TestResolveImportCycle robust to error order
090b14e 
In JSON output, the Go command was only setting the Error field for one
package involved in an import cycle. As a result, TestResolveImportCycle
was dependent on the chosen package, causing flakes when the chosen
package is not deterministic.

Arguably this behavior should be fixed, both in the go command and in
gopls, but for now make the test resilient to choice by asserting on any
of the possible errors.

Add a new AnyOf expectation to support this type of assertion and tweak the
test output formatting. Also update the test to eagerly fail once the
didOpen notification has been fully processed, so that we don't have to
wait for the assertion timeout.

Updates golang/go#52904

Change-Id: Ic209d8fdcb7308c041b287a8f122c47e96d29a96
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406274
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
@findleyr findleyr modified the milestones: Unreleased, gopls/later May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
2 participants
@gopherbot @findleyr