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: 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
gopls/v0.4.0

Comments

@stamblerre
Copy link
Contributor

stamblerre commented May 13, 2019 •
edited
Loading

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 mentioned this issue Jul 8, 2019
Closed
@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
@jayconrod jayconrod mentioned this issue Sep 24, 2019
Closed
@stamblerre stamblerre mentioned this issue Nov 14, 2019
Closed
This was referenced Nov 22, 2019
Closed
Closed
@stamblerre stamblerre mentioned this issue Nov 25, 2019
Closed
@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.

@myitcv myitcv mentioned this issue Dec 2, 2019
Merged
@stamblerre stamblerre mentioned this issue Dec 4, 2019
Closed
@gopherbot
Copy link
Contributor

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

@myitcv myitcv mentioned this issue Dec 9, 2019
Open
@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp: invalidate workspace packages when go.mod file changes
ac2db28 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Dec 11, 2019
@ridersofrohan
internal/lsp: add handling for go.mod files in internal/lsp functions
ad473c0 
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 <[email protected]>
Run-TryBot: Rohan Challa <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp: add warning diagnostics for unused dependencies in go.m…
f270e23 
…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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
@stamblerre stamblerre mentioned this issue Jan 10, 2020
Closed
gopherbot pushed a commit to golang/tools that referenced this issue Jan 13, 2020
@ridersofrohan
internal/lsp: surface diagnostics for invalid go.mod files
30cae5f 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp: add mapper for go.mod files
579903b 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 15, 2020
@ridersofrohan
internal/lsp: show dependency quick fixes for go.mod diagnostics
de0b176 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/214700 mentions this issue: internal/lsp: add diagnostics for imports not in go.mod

@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp/cache: create infra for caching go.mod diagnostics
bd79bb7 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 27, 2020
@ridersofrohan
internal/lsp: surface missing dependencies for imports not in go.mod
dd312ab 
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 <[email protected]>
Run-TryBot: Rohan Challa <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp: add diagnostics in .go files for missing deps in go.mod
c4cbd3b 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
@gopherbot
Copy link
Contributor

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
@ridersofrohan
internal/lsp: add quickfixes for missing dependencies in go.mod
6fdc577 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
@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 🎉

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020
@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 •
edited
Loading

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 stamblerre mentioned this issue Aug 10, 2020
Open
@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.
Assignees
No one assigned
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
Milestone
gopls/v0.4.0
Development

No branches or pull requests
8 participants
@andybons @ridersofrohan @myitcv @stamblerre @suzmue @gopherbot @heschi @segevfiner