1
0
mirror of https://github.com/golang/go synced 2024-11-18 13:14:47 -07:00
Commit Graph

83 Commits

Author SHA1 Message Date
Heschi Kreinick
444c5ef18e internal/imports: only check first segment for .
An import path like "foo/bar.v1" is still a local path, not an external
package, and should be grouped as such.

Change-Id: I06be3c01076f616a3cdc8e23bc9c056643035ad1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234111
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-15 21:25:07 +00:00
smasher164
978e26b7c3 internal/imports: update stdlib index for 1.14
$ go run mkstdlib.go

Fixes golang/go#38464.

Change-Id: I97cbd0abf6b9d874739d236429aba8ef271c48ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228884
Run-TryBot: Akhil Indurti <aindurti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-20 00:18:25 +00:00
Rohan Challa
46bd65c853 internal/lsp/cache: add concurrency error check for go cmds
This change attempts to fix a concurrency error that would cause
textDocument/CodeLens, textDocument/Formatting, textDocument/DocumentLink,
and textDocument/Hover from failing on go.mod files.

The issue was that the go command would return a potential concurrency
error since the ModHandle and the ModTidyHandle are both using the
temporary go.mod file.

Updates golang/go#37824

Change-Id: I6cd63c1f75817c7308e033aec473966536a2a3bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224917
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-08 03:22:09 +00:00
Rebecca Stambler
2b0b585e22 internal/imports: don't set a logger unless the user has provided it
Not sure why I thought it was a good idea to enable extra logging
by default, but this was added in CL 184198.

Fixes golang/go#37636

Change-Id: I1840a9e53625db99c9097f2c23500ae20d6dac1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221918
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-03-03 21:46:25 +00:00
Heschi Kreinick
5bcca83a78 internal: rationalize debug logging
In all cases, use a Logf field to configure debug logging. Non-nil means
that logging is enabled through the given function.

Fixes accidental debug spam from goimports, which had a separate Debug
flag that was supposed to guard logging, but wasn't used when creating
the gocommand.Invocation.

Change-Id: I448fa282111db556ac2e49801268d0affc19ae30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221557
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-03 16:59:18 +00:00
Heschi Kreinick
c5cec6710e all: consolidate invokeGo implementations
Over time we have accumulated a disturbing number of ways to run the Go
command, each of which supported slightly different features and
workarounds. Combine all of them.

This unavoidably introduces some changes in debug logging behavior; I
think we should try to fix them incrementally and consistently.

Updates golang/go#37368.

Change-Id: I664ca8685bf247a226be3cb807789c2fcddf233d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220737
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-25 21:33:46 +00:00
Heschi Kreinick
6bd7a38086 internal/imports: filepath.Clean module Dirs
Various bits of the module code use HasPrefix, which only works if all
paths are clean. Make sure they are.

No test; this is pretty esoteric and it's not easy to test.

Fixes golang/go#36193.

Change-Id: I8c8e87d1882d149a1a129d8b7123bc95c115b2ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220659
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-25 18:45:58 +00:00
Jay Conrod
1ace956b0e internal/imports: import packages from x/mod instead of internal copy
This CL deletes ./internal/module and ./internal/semver, which are
copies of packages in golang.org/x/mod. The copies were created before
x/mod was a separate module.

./internal/imports is updated to use the packages in x/mod.

Change-Id: Id434f5f0a240de97d18505cb7c925c2e062f6231
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218897
Run-TryBot: Jay Conrod <jayconrod@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-10 19:23:13 +00:00
Rohan Challa
3868802a52 internal/imports: change processEnv to use buildflags
This change adds a buildFlags variable to the processEnv struct rather than appending them to the GOFLAGS by using a space separator.

Fixes golang/go#37108

Change-Id: I4331066c30fa51f0133504d723132527b00ce74a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218857
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-10 18:52:02 +00:00
Heschi Kreinick
9f574694fd internal/imports: prevent self-imports in the stdlib
goimports has a fast path for stdlib imports that was not checking for
import cycles. Add the check.

Fixes golang/go#37063.

Change-Id: I46c98c317d8f06f83018fef9ef7edf9222e6b3f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217958
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 00:08:43 +00:00
Heschi Kreinick
cf5ba95194 internal/imports: stop leaking listeners
(*dirInfoCache).ScanAndListen saves a listener when it's called. That
listener is a chain of callbacks that leads all the way back up to the
original caller, i.e. the gopls completion code. It needs to unregister
that listener, but in cases where the context was cancelled before it
finished its initial walk of the cache, it failed to. In gopls' case,
that means retaining the entire state of the workspace as of completion
triggering.

Return a real stop function on all paths.

Change-Id: Iff3046e627b1fa89f576371c4742fee6fdca7589
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217677
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-04 05:47:48 +00:00
Rebecca Stambler
207d3de1fa all: fix some staticcheck errors
Updates golang/go#35718

Change-Id: I10bfd5421cd44bb58b8bcaa6e9205040c25f51be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-29 04:53:41 +00:00
Heschi Kreinick
d456b1cd8c internal/imports: pass dummy source for completion functions
The goimports code tries to read source from disk if none is supplied,
but the completion functions don't need source. Pass a dummy slice.

Fixes golang/go#36671.

Change-Id: Ieecc9ee75d39f8f165c7b9764ff49c8334a7fcd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215681
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 20:03:11 +00:00
Heschi Kreinick
354bea8ca8 internal/lsp/cache: let gopls track go.mod files
We used to read the go.mod file information out of the imports.Resolver.
Now that gopls tracks go.mod itself, we can use that instead. This is a
slight regression, in that go.mods in replace targets will no longer be
watched, but I don't think that's too important.

This allows us to stop reading the ModuleResolver's internals, which
were not sufficiently locked.

Updates golang/go#36605.

Change-Id: I42939e0248cba1f6b3850a003de67fcc11ab10b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215319
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 21:20:11 +00:00
Heschi Kreinick
6edc0a871e internal/lsp/source: score in-memory unimported candidates
We were assuming that all in-memory packages were equally useful. That's
not true for projects with a large dependency tree. Call into the
imports code to score them.

While I'm here, score the main module above direct deps.

Updates golang/go#36591.

Change-Id: I07c56dd3ff7338e76f3643e18d35abc1b52d6763
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215023
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 01:23:04 +00:00
Heschi Kreinick
1c4842a210 internal/lsp/cache: refresh imports cache in the background
Now that we can detach scans, it's easy to kick off a background refresh
that doesn't block the user. Performance may be a bit worse until the
scan finishes, but that's the price we pay for freshness.

Adaptively rate-limit the refresh rate so that we don't turn the user's
computer into a hot plate.

Change-Id: Icbe8f384f44a219b57465da22d9becc19001eab8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212022
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 20:36:08 +00:00
Heschi Kreinick
3d14842334 internal/imports: load test exports of package under test
x_tests can access exports from the test variant of the package under
test. Change loadExportsFromFiles to understand that mode, and use it
where appropriate.

I didn't want to come up with a cache key for for the test variant, so
for now we bypass the cache in these situations.

Fixes golang/go#29979.

Change-Id: I9959a08da97bbee64c5bcd56e06f548486693156
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213221
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 23:54:10 +00:00
Rebecca Stambler
e2f26524b7 internal/imports: create listener map after clearing for new scan
The module resolver needs to recreate the listener map if it's cleared
for a new scan.

Change-Id: If5e945d4f2059f2a79aef3129f963a2c50e90229
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 04:28:03 +00:00
Heschi Kreinick
563860d11d internal/imports: fix use of uninitialized resolvers
Resolvers are lazy initialized. That worked fine until the addition of
the scan semaphore -- it's not a good idea to create that lazily, since
you can't synchronize on a channel that doesn't exist.

Specifically, this caused a gopls hang when completion finished without
needing to use the resolver. In that case, we'd call ClearForNewScan/Mod
on an uninitialized resolver, and then hang receiving from a nil
channel.

Instead, eagerly initialize where convenient, and particularly the scan
semaphore.

Change-Id: I3adb1ae76b751650995e50f50346e06fd9c9f88d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214258
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-10 00:40:31 +00:00
Heschi Kreinick
5d34a75004 internal/imports: don't block completions on walks
Filesystem walks of large GOPATHs/module caches can take seconds,
especially on systems with slow filesystems like MacOS and WSL. We don't
want to block completion requests on walks finishing. At the same time,
cancelling a walk midway through results in an unusable cache, where we
don't know which parts have been scanned so far.

The best option is to run the walks in a separate goroutine. Then we can
detach and let them finish. On the other side, we need to be able to
reattach for the next completion request.

Introduce a new method on caches, ScanAndListen, which first processes
all the items in the cache, then notifies of any new items. This allows
us to reattach to an existing scan without missing anything.

The background scan introduces concurrency to the resolvers where there
wasn't any before. We can't use mutexes, because there's no way to stop
Lock() when a context expires. Use a 1-element semaphore channel to
accomplish the same effect.

Along the way: Only rescan GOPATH if the resolver has been cleared. None
of this makes sense for GOPATH without that. Fix a bug where we were
scanning the main module twice in module mode. Stop loading exports in
module tests, it slows them down a ton.

Change-Id: I978efae733ccba0c0cdc8e8fe6892bf5f15feac8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213217
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-09 22:24:46 +00:00
Heschi Kreinick
7bda30096d internal/imports: actually skip things in scan
An important part of letting the callback choose what to load is...not
loading the stuff it doesn't want.

Change-Id: I4048d7aed756b6ebc26fb6f8e384f44c64281f90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213129
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-03 21:11:27 +00:00
Heschi Kreinick
ba16e80ae2 internal/imports: filter out self-import completions
Fixes golang/go#36321.

Change-Id: Ic6cdad4b611e5a16e086743f53f85bcb71070a21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212897
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:44:38 +00:00
Heschi Kreinick
1e586a538e internal/imports: clean up dead code
Now that we're storing module information per-directory, we don't need
to pre-compute the need for a replace statement. And we never have the
package name in the place it used to be set.

Change-Id: I3b0845dc49f52f8c449840410dbb786fe903d29d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212861
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:44 +00:00
Heschi Kreinick
2f3125dfbf internal/imports: filter candidates on directory name
When finding completion candidates, we can use the same tricks
goimports uses to ignore directories that look irrelevant.

Change-Id: I114a3d4e487aed7f59fc48b2f86d42129baf5183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212859
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:29 +00:00
Heschi Kreinick
872f4f411e internal/imports: filter roots with callback
Now that we have all these callbacks, it's strange to have a list of
root types to exclude on the side. Merge that into the callback.

Change-Id: I8dc88e095362a8d2e180196ad9b81e17d4d34949
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212858
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:40 +00:00
Heschi Kreinick
7ec15289dd internal/imports: optimize scan implementations
In scan implementations, stop after cancellation, and swallow the
context's error for convenience.

In the module implementation specifically, try to avoid scanning if the
cache is enough to satisfy the user. When we do have to scan, prioritize
module dependencies before the whole cache.

Change-Id: I23dc98df016f9fca4f31c7ded3d11bc257c29b94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212857
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:33 +00:00
Heschi Kreinick
c2a8f45ada internal/imports,lsp: use callbacks for completion functions
We only need to return a relatively small number of completions to the
user. There's no point continuing once we have those, so switch the
completion functions to be callback-based, and cancel once we've got
what we want.

Change-Id: Ied199fb1f41346819c7237dfed8251fa3ac73ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212634
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:27 +00:00
Heschi Kreinick
50c778fb86 internal/imports: redesign scan API
We have multiple use cases for scanning: goimports, import completion,
and unimported completions. All three need slightly different features,
and the latter have very different performance considerations. Scanning
everything all at once and returning it was not good enough for them.

Instead, design the API as a series of callbacks for each
directory/package: first we discover its existence, then we load its
package name, then we load its exports. At each step the caller can
choose whether to proceed with the package. Import completion can stop
before loading exports, goimports can apply its directory name
heuristics, and in the future we'll be able to stop the scan short once
we've found all the results we want for completions.

I don't intend any significant changes here but there may be some little
ones around the edges.

Change-Id: I39c3aa08cc0e4793c280242c342770f62e101364
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212631
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:22 +00:00
Heschi Kreinick
ac3e9e73d1 internal/imports: remove go/packages support
We don't use the go/packages resolver in goimports, and as we develop
gopls it only becomes harder to use there. Give up.

Change-Id: Ic8b566c6dd730b23b0c81d7d34a41f16fe0be7e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212630
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:16 +00:00
Heschi Kreinick
62b9674312 internal/imports: consider direct mod deps more relevant
As a followup to CL 204203, prefer direct dependencies over indirect.
This should improve results for common names like "log" and "errors".

Updates golang/go#36077.

Change-Id: I3f8cfa070832c2035aec60c4e583ee1c0abf5085
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212021
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-19 20:57:09 +00:00
Bryan C. Mills
01c78d57fd internal/imports: set the Dir field on the build.Context (instead of WorkingDir) if present
Updates golang/go#36168

Change-Id: Ibdb277176a28da72d85a4c7c8ed7c903c278125e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211599
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-12-17 14:41:53 +00:00
Heschi Kreinick
621d4eef75 imports: pass more of the environment
goimports now wants various module flags, but I forgot to set them up in
the many places we create environments. Do so.

Change-Id: Ic3817caeb8fc4d564b49006ef6ca6842b2498eaf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211581
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-17 00:36:01 +00:00
Heschi Kreinick
6d582d504c internal/lsp/source: optimize computeFixEdits
In the common case that a file has imports, we're going to diff just the
import block. That means that ApplyFixes doesn't need to produce the
whole formatted file, which is a huge speedup. We will do more work twice
for files with no imports, but those are presumably pretty short.

Hat tip to Muir for pointing towards this in
https://go-review.googlesource.com/c/tools/+/209579/2/internal/imports/imports.go#87
even if I didn't catch it.

Updates golang/go#36001.

Change-Id: Ibbeb4d88c6505eac26a36994de514813606c8c79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210200
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-06 20:21:26 +00:00
Heschi Kreinick
5a103c92be internal/imports: make ApplyFixes work despite syntax errors
ApplyFixes is only used by gopls, which cares a lot about files with syntax
errors, and not at all about files that aren't structurally valid. Use
the standard ParseFile function instead of goimports' weird one.

Adding a test is impractical because it seems to break type checking of
whatever package it's in.

Fixes golang/go#35915.

Change-Id: Iaf0e331978415428a422d942a1e0c5f6e66dc8a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209579
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-02 20:30:33 +00:00
Heschi Kreinick
66af5afb16 internal/lsp/source: give more imports names
Expose ImportPathToAssumedName (internally) and use it in an LSP
completion case that doesn't go through the usual imports code.

Fixes golang/go#35401.

Change-Id: If87912072e11e22c542f7474841e53467a33ef2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206890
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-25 20:50:14 +00:00
Jay Conrod
298f0cb188 txtar: promote from internal
Rather than copying this package to another repository, let's promote
this one out of internal.

Change-Id: I6f9cc1ada1577a720905271f7471c3afe05a2b41
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207905
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-11-19 22:48:55 +00:00
Heschi Kreinick
689d0f08e6 internal/imports: set import names on completion candidates
I forgot to set .Name on completion candidates. That meant that if the
package name didn't match the import path, you'd first get an import
without a name, then it would be added when you organized imports.

Change-Id: Ic374de872324effa6bc04c1440c659d7a182d17f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205503
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-05 23:13:37 +00:00
Heschi Kreinick
fda23558a1 internal/lsp/cache: only refresh imports cache every 30 seconds
Loading completion suggestions can be slow, especially in GOPATH mode
where basically anything can change at any time. As a compromise, cache
everything for 30 seconds. Specifically, after a completion operation
finishes, if the cache is more than 30 seconds old, refresh it
asynchronously. That keeps user-facing latency consistent, without
chewing up CPU when the editor isn't in use. It does mean that if you
walk away for an hour and come back, the first completion may be stale.

In module mode this is relatively benign. The only things the
longer caching affects are the main module and replace targets, and
relevant packages in those will generally be loaded by gopls, so they'll
have full, up-to-date type information regardless.

In GOPATH mode this may be more troublesome, since it affects
everything. In particular, go get -u of a package that isn't imported
yet won't be reflected until the cache period expires. I think that's a
rare enough case not to worry about.

Change-Id: Iaadfd0ff647cda2b1dcdead9254b5492b397e86e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205163
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-04 21:31:29 +00:00
Heschi Kreinick
c02c21e5e9 internal/imports: cache GOPATH, exports
We intend to use the GOPATH resolver's results during LSP
autocompletion. That means we have to be able to cache its data, same as
we do for modules. Convert it to use a dirInfoCache.

Cache exports in the dirInfoCache. Along the way, store exports as slices
rather than maps. We don't need the extra structure the vast majority of
the time, and the memory overhead is nontrivial.

Change-Id: If267d6b00da2163a960b93b2cf2088ec2538f73d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205162
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-04 21:31:17 +00:00
Heschi Kreinick
8266eea4ea internal/imports: provide export completions for unimported packages
Add a function that returns all the exported identifiers associated with
a package name that doesn't have an import yet. This will allow
completions like rand<> to return rand.Seed (from math/rand) and
rand.Prime (from crypto/rand).

Updates golang/go#31906

Change-Id: Iee290c786de263d42acbfabd76bf0edbf303afc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204204
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-04 21:30:58 +00:00
Heschi Kreinick
7871c2d767 internal/imports: sort import candidates by "relevance"
When proposing packages to import, we can propose more relevant packages
first. Introduce that concept to the pkg struct, and sort by it when
returning candidates.

In all cases we prefer stdlib packages first. Then, in module mode, we
prefer packages that are in the module's dependencies over those that
aren't. We could go further and prefer direct deps over indirect too,
but I didn't have the code for that handy.

I also changed the alphabetical sort from import path to package name,
because that's what the user sees first in the UI.

Updates golang/go#31906

Change-Id: Ia981ee9ffe3202e2a68eef3a36f65e81849a4ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204203
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-10-30 22:54:52 +00:00
Bryan C. Mills
e26a8c8a3c internal/imports: set ctx.WorkingDir if such a field exists
CL 203820 removes an assumption in go/build that srcDir is in the main
module, since in general it need not be. That requires the use of some
other mechanism for callers to communicate the correct location of the
main module.

Fortunately, we already have a WorkingDir field on the ProcessEnv
struct that does exactly that. We can simply propagate it through if
the corresponding field is present on go/build.Context.

Updates golang/go#34860

Change-Id: Idabf9ae06d8383a72772d5a589fae1d10f206c01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203857
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-10-29 18:52:34 +00:00
Heschi Kreinick
dc9d807def internal/imports: re-enable TestStdlibNotPrefixed
Vendor mode is fully supported now and I forgot to revert this.

Change-Id: I2a5b69818944e87cfb85e00f0486ffb3754fe6a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203280
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-24 21:22:50 +00:00
Heschi Kreinick
377bdac4e7 internal/imports: support vendoring in module mode
Previously, goimports half-supported vendor mode -- it searched the
module cache on some code paths and the vendor dir in others. That
seemed to work okay, probably because people happened to have a populated
module cache. In 1.14, it's much more likely that people will work
solely from the vendor directory.

In this CL we bite the bullet and fully support vendor mode. 1.14 makes
this particularly challenging by disabling list -m ... in vendor mode, and
by enabling it automatically under some circumstances. We need to mirror
that behavior, which means knowing whether we're running with 1.14, and
figuring out whether vendoring should be enabled given that. We collect
the information we need with a list -m -f query just on the main module.

If vendor mode is enabled, we throw away all the modules and replace
them with a single pseudo-module rooted at /vendor. Everything basically
works at that point.

Fixes golang/go#34826

Change-Id: Ia4030344d822d5a4a3bbc010912ab98bf2f5f95b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203017
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-10-24 21:04:39 +00:00
Heschi Kreinick
c825e86a85 internal/imports: skip TestStdlibNotPrefixed
At tip, $GOROOT/src/go.mod is 1.14, $GOROOT/src/vendor exists, and so
vendor mode is automatically enabled. That causes golang/go#34826.

This will be fixed by my upcoming vendoring CL, but for now skip.

Updates golang/go#34826

Change-Id: I5fff51fff54cf83e6369ae76bf3b19cfb7b5ac15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203257
Run-TryBot: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-24 19:18:02 +00:00
Heschi Kreinick
7178990c25 internal/imports: cache module information
To avoid proposing unusable completions, such as those in modules that
need a replace statement to be usable, we need to know what module a
directory is in. That involves walking up the directory tree to find a
go.mod file, which is expensive to do over and over. Really, we just
need to check if the directory we're in has a go.mod file, then use the
parent dir's results.

Add module information to the cache and use it when figuring out what
module a dir is in.

Change-Id: Ia74ba9b37d73fca5e6786a94c73c8fd71b591645
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202541
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-10-21 22:41:28 +00:00
Heschi Kreinick
d2fffb4b84 internal/imports: cache things outside the mod cache
Since a user's module cache is generally going to be much bigger than
their main module, one would expect that caching just information about
the module cache would be sufficient. It turns out that's not correct.
When we discover something in the module cache, we have to make sure
that a different version of it isn't already in scope. Doing that can
require information about the main module or replace targets, so that
needs to be cached too.

Concretely, when I'm working in x/tools, if a scan discovers a version
of x/tools in the module cache, it should usually ignore that version.
But that might not be true in more complicated cases, especially those
involving nested modules whose boundaries change.

So, cache everything except GOROOT. Since the new data is mutable,
we store it separately from the module cache data so that it can be
discarded easily between runs.

Change-Id: I47364f6c0270fee03af8898fec6c85d1b9c8d780
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202045
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-10-21 20:45:41 +00:00
Heschi Kreinick
eb46839a96 internal/imports: return non-stdlib candidates from GetAllCandidates
Scan most sources, including GOPATH, the module cache, the main module,
and replace targets as appropriate. Use the cached stdlib instead of
scanning GOROOT.

We heavily cache the contents of the module cache, so performance is
decent. But we have to look at all the modules not in the module cache
too to get the right versions of modules (see
(*ModuleResolver).canonicalize), which currently isn't cached at all,
even just for a single run. That ends up being pretty expensive.

The implementation changes are relatively small; add package name
loading to scan(), cache that result, and allow callers to control what
directories are scanned so that it can skip GOROOT.

I also cleared out most of the stdlib from the unimported completion
test and added a simple external completion to it for safety's sake.

Change-Id: Id50fd4703b1126be35a000fe90719e19c3ab84bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199178
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-10-21 20:45:15 +00:00
Ariel Mashraki
1f7a813c0d tools/internal/imports: fix data race in packageInfo
Before this commit, when running imports.Process concurrently, the program
panics with a fatal error due to concurrent map iterations and map writes.

This CL fixes this by adding a copy of the map to the packageInfo structure.

Fixed #34895

Change-Id: If009e6108813f86495c7e20e69739186b8b236d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200865
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-10-18 18:31:53 +00:00
Jay Conrod
8abd424004 internal/imports: skip TestModeGetmodeVendor
Test is broken by recent cmd/go vendoring changes. Skipping the test
case while we figure out what to do.

Updates golang/go#34826

Change-Id: I31a6f18e40420970251e25836edcf32c56c10ef5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200299
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-10-10 17:12:13 +00:00