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 go.mod files #31999

Closed
stamblerre opened this issue May 13, 2019 · 23 comments
Closed

x/tools/gopls: support go.mod files #31999

stamblerre opened this issue May 13, 2019 · 23 comments
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.
Milestone

Comments

@stamblerre
Copy link
Contributor

stamblerre commented May 13, 2019

Context: Beginning of #31712.

Some other features that fall under the umbrella of this issue:

@gopherbot gopherbot added this to the Unreleased milestone May 13, 2019
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label May 13, 2019
@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label May 13, 2019
@stamblerre stamblerre changed the title x/tools/internal/lsp: re-run go/packages.Load when go.mod file added x/tools/gopls: re-run go/packages.Load when go.mod file added Jul 2, 2019
@stamblerre stamblerre changed the title x/tools/gopls: re-run go/packages.Load when go.mod file added x/tools/gopls: re-run go/packages.Load when go.mod modified Jul 8, 2019
@stamblerre stamblerre changed the title x/tools/gopls: re-run go/packages.Load when go.mod modified x/tools/gopls: support go.mod files Aug 8, 2019
@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Aug 27, 2019
@suzmue suzmue assigned heschi and unassigned suzmue Aug 30, 2019
@myitcv
Copy link
Member

myitcv commented Nov 26, 2019

Noting a specific comment that in #35820 we report that changes to go.mod files do not trigger recalculation of diagnostics.

@gopherbot
Copy link

Change https://golang.org/cl/210557 mentions this issue: internal/lsp: invalidate open views when mod file changes

@gopherbot
Copy link

Change https://golang.org/cl/210757 mentions this issue: internal/lsp: add execption for go.mod files in cache/load

gopherbot pushed a commit to golang/tools that referenced this issue Dec 11, 2019
When the go.mod file changes, we should invalidate all the files that are
contained in the package for the mod file. This will allow the files to recheck
their packages in case new packages were added in the go.mod file.
This still does not fix issue where changes to go.mod files do not trigger recalculation of diagnostics.

Updates golang/go#31999

Change-Id: I6d8f1531f5c28ed2dca7fb8ad4ee0317fada787b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210557
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Dec 11, 2019
When we are processing a go.mod file, we are calling go/packages.load
when we should not be. It will always return 0 packages since it is
not a .go file. This CL adds branching inside each internal/lsp protocol
function and also adds a check in snapshot.PackageHandles for the file type
and returns an error. This will prevent `go list` from running on go.mod files for now.

Updates golang/go#31999

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

Change https://golang.org/cl/211303 mentions this issue: internal/lsp: create parseModHandle for storing go.mod data

gopherbot pushed a commit to golang/tools that referenced this issue Jan 9, 2020
…od files

This is the first step to surfacing potential fixes and suggestions to a user's go.mod file. Specifically, it will show a warning if you have a dependency that is not used, or if a dependency is declared as indirect when it should be direct and vice versa.

This CL adds functionality for version of Go that are >= 1.14.

Updates golang/go#31999

Change-Id: Id60fa0ee201dcd843f62e2659dda8e795bd671db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211937
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 13, 2020
This change will surface errors that come from the mod package. It will handle incorrect usages, invalid directives, and other errors that occur when parsing go.mod files.

Updates golang/go#31999

Change-Id: Icd817c02a4b656b2a71914ee60be4dbe2bea062d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213779
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/214699 mentions this issue: internal/lsp: add mapper for go.mod files

gopherbot pushed a commit to golang/tools that referenced this issue Jan 14, 2020
This change adds a protocol.ColumnMapper when parsing go.mod files. This will prevent us from having to worry about line and column offsets, specifically when converting from the x/mod/modfile position to a span.Span.

Updates golang/go#31999

Change-Id: Iacdfb42d61dfea9b5f70325cf5a87c9575f8f345
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214699
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 15, 2020
This change adds quickfixes for unused dependencies and dependencies that should not be marked as indirect. It also updates the positions for the diagnostics to make more sense relative to the warning message. There is a testing harness now for suggested fixes.

Updates golang/go#31999

Change-Id: I0db3382bf892fcc2d9ab3b633020d9167a0ad09b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213917
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/214700 mentions this issue: internal/lsp: add diagnostics for imports not in go.mod

@gopherbot
Copy link

Change https://golang.org/cl/215021 mentions this issue: internal/lsp/cache: create infra for caching go.mod diagnostics

gopherbot pushed a commit to golang/tools that referenced this issue Jan 21, 2020
This change creates the infrastructure for caching diagnostics. Right now it uses a naive key of the entire snapshot which does not really add any benefit from using the cache. It also moves some code from inside the internal/lsp/mod to internal/lsp/cache.

Updates golang/go#31999

Change-Id: I599ad281baa062448d33ca0ea88ae6e09e10d73e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215021
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 27, 2020
This change sets up the infrastructure to surface the dependencies that are missing in a go.mod file. In a follow up CL, we will use these to add diagnostics to the imports of .go files telling the user to add the dependency to their go.mod file.

Updates golang/go#31999

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

Change https://golang.org/cl/216277 mentions this issue: internal/lsp: add diagnostics in .go files for missing deps in go.mod

gopherbot pushed a commit to golang/tools that referenced this issue Feb 3, 2020
This change adds diagnostics to imports in .go files that are missing the dependency in the go.mod file. The quick fix that adds the appropriate require statement is coming in a follow up CL.

Updates golang/go#31999

Change-Id: I314cdbe8e3dd27da744ca0391e7a6e5dc1ebaa77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216277
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/215898 mentions this issue: internal/lsp: add quickfixes for missing dependencies in go.mod

gopherbot pushed a commit to golang/tools that referenced this issue Feb 7, 2020
This change adds quick fixes for diagnostics in .go files, specifically for diagnostics that deal with imported packages that are not declared in the go.mod file. These quick fixes will automatically add the dependency in the go.mod file and format the file if there are any issues.

Updates golang/go#31999

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

@ridersofrohan: Do you think we can close this issue?

@ridersofrohan
Copy link

I think we can 😃

@stamblerre
Copy link
Contributor Author

I'll let you do the honors 🎉

@segevfiner
Copy link
Contributor

@stamblerre Was automatic detection of the local module ever implemented, both this and the referenced issue are closed.

P.S. That's kind of a missing thing in the stock goimports, most editors just run it without flags and you have to specifically customize the editor configuration to pass the flag which is obviously different for different projects, it doesn't have a config file or detection logic for it, which is annoying.

@stamblerre
Copy link
Contributor Author

@segevfiner: This issue refers to adding support for various features for go.mod in gopls. The automatic detection of the module is partially implemented, but not yet complete - #32394 tracks that work.

As for goimports, neither you nor your editor need to run goimports directly if you are using gopls. All import organization should go through gopls. I'm not sure what flag you are referring to specifically - the only one I know that changes per-project is -local -- is that what you're referring to? In any case, please open a new issue to continue this discussion.

@segevfiner
Copy link
Contributor

segevfiner commented Aug 10, 2020

I was specifically complaining about standalone goimports not supporting automatic detection of the local import prefix, requiring to supply -local to it. I know gopls calls it internally, but I'm not sure if it implemented any such automatic detection either, which is what I'm asking about. Should I open an issue about this or is this tracked somewhere?

I think the issue you linked is about figuring out module roots independent of editor workspace folder roots?

@stamblerre
Copy link
Contributor Author

Ah I see, thanks for the clarification. This was an umbrella issue so I wasn't sure what feature you were referring to. It seems #32049 did not cover automatic detection of the local module. I opened #40660 to restart that discussion - thanks!

@stamblerre stamblerre reopened this Aug 10, 2020
@golang golang locked and limited conversation to collaborators Aug 10, 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.
Projects
None yet
Development

No branches or pull requests

8 participants