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

4426 Commits

Author SHA1 Message Date
Rebecca Stambler
62545537a9 internal/lsp: use correct file identities when computing diagnostics
CL 214586 switched to using URIs in error messages instead of file
identities. It wasn't fully correct because the reports were being
allocated for the file identities in the package handle (which may be
outdated).

Fixes golang/go#36601

Change-Id: I4fcfc02df74d94fff49540c784ef816c357e7232
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215680
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 20:00:43 +00:00
Rohan Challa
bd79bb77d6 internal/lsp/cache: create infra for caching go.mod diagnostics
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>
2020-01-21 19:43:28 +00:00
Heschi Kreinick
9375b12bd8 internal/lsp/cache: fix mod file change check
The modfiles function only works with Go 1.14. When it is enabled,
it reenters the view, causing a deadlock. Stop using it, and move the
process env under a separate lock so to break the deadlock.

Change-Id: I34c528c2be1f32c06b423ead44e90155f60c2214
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215679
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 19:24:08 +00:00
Rob Findley
13c74800b9 internal/lsp/protocol: remove unused DocumentUri type
Change-Id: I8fbc8cbbb4e62c1eca4de767d054f5d098f27edd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 14:52:36 +00:00
Iskander Sharipov
dbc83e6dc0 internal/lsp/source: fix typeIsValid() inf recursion
typeIsValid() intended to stop on a named type, but
since we called Underlying(), switch case never caught any
named type. To avoid that, do an early check.

Fixes golang/go#36637

Change-Id: I2700afbb8f9678b4542e2e7dccc3be59b1d9ebdf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 04:27:40 +00:00
vovapi
eb0d8dd85b go/ssa: remove workaround for lack of vendoring in go/loader
Support for vendoring in go/loader was added in golang.org/cl/18053
Similar workaround was removed from go/loader tests in golang.org/cl/18214

Resolves TODO left by adonovan

Change-Id: I418463feb3e2dded191b6f1770dd7b9e44c2deca
GitHub-Last-Rev: b556acfce87ab1bac9a73b69202a1afdd6e641cc
GitHub-Pull-Request: golang/tools#199
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215458
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-19 21:55:04 +00:00
vovapi
e919f86751 go/ssa/interp: fix goroutines and channels leaks
rangeIter leaks goroutines and channels
on each incomplete map iteration.
So remove goroutines and channels iterators
and use reflect.(Value).MapRange() iteration instead.

Resolves TODO left by @adonovan

Change-Id: I8cc24b264ae782376c392fd9497be81bf50a415f
GitHub-Last-Rev: 5bba70b8658575b3e2e33b8941fce3ef75199a4f
GitHub-Pull-Request: golang/tools#198
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215457
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-19 20:12:43 +00:00
Heschi Kreinick
0cba7a3a9e internal/lsp/source: enable unimported completions by default
Fixes golang/go#31906.

Change-Id: I626ff1fe94a171d2280d9deeb4578b5abc759913
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214947
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 22:05:05 +00:00
Heschi Kreinick
fe56e63357 internal/lsp/source: fix ranking of untyped completions
Claiming that untyped candidates matched the type of whatever we were
looking for messed up rankings in found(). The only other places that
use it will all work better with false. Return false.

Updates golang/go#36591.

Change-Id: I5e1e8af7cc5c27422740cbb77f9a4a20edb1e447
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215322
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 21:50:04 +00:00
Rebecca Stambler
1486df0b25 internal/lsp: do not invoke the Go command when checking common errors
There's no need to do this more than once per view.

Change-Id: I3160adc602764204155dd0e506fd554aeb55d639
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 21:23:25 +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
Rebecca Stambler
633b092c1e internal/lsp: add recency check to avoid sending old diagnostics
This change uses snapshot IDs instead of file versions to determine if
diagnostics are stale.

Fixes golang/go#36601.

Change-Id: I7727a30222fecf2d7733fa013c6cee6338ffd968
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 20:46:10 +00:00
Rebecca Stambler
351fb220e1 internal/lsp: invalidate directories if we have no direct IDs
Previously, we were only invalidating packages in directories if we
didn't have a file handle. We should also invalidate if we have an
unparseable file handle - that is, a file with no content or no package
name.

Fixes golang/go#36608.

Change-Id: Ia12fad962c06ddeeac382185d3220961f5c584e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215318
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 20:35:50 +00:00
Heschi Kreinick
95ece921ff internal/lsp/source: trim file very carefully
Apparently the AST will sometimes give us offsets past the end of the
file. Don't crash when it does.

Fixes golang/go#36610.

Change-Id: I3cfbf8645bfcea94a5d87bca5bef4236d657b2c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215119
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 20:34:13 +00:00
Muir Manders
a20955124b internal/lsp: consolidate completion sorting
Move completion candidate sorting into internal/lsp/source so
source_test.go and internal/lsp don't have to duplicate the logic.

Change-Id: Ifbe7ca5ad6a5b74020fd1260b4d4f775709968cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 20:30:43 +00:00
Rebecca Stambler
7ad9cd8f31 internal/lsp/cache: invalidate metadata for x_tests and test variants
Invalidating metadata for reverse dependencies isn't necessary, except
for in the case of test variants and x_tests.

An example:

The only way to reload the metadata for
"golang.org/x/tools/internal/lsp/cache
[golang.org/x/tools/internal/lsp/source.test]" is by reloading
"golang.org/x/tools/internal/lsp/source" with "[-test -deps]". That
means we have to invalidate the metadata for that
"golang.org/x/tools/internal/lsp/source_test" when we invalidating
"golang.org/x/tools/internal/lsp/cache".

Fixes golang/go#36165

Change-Id: Iff0e03a7a46158fbdafaffa091f90ca434700a97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 17:36:07 +00:00
Rohan Challa
ade7f2547e internal/lsp: add highlighting for import statement
This change adds highlights for imports when the cursor is over the use of that import. It also adds it for the opposite direction when the cursor is on the import, it will highlight uses of that import.

Fixes golang/go#36590

Change-Id: Ifd04d81ec9b4fdf2be1b763f31b44d0ef7d92f47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215258
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 17:07:20 +00:00
itchyny
43d5027782 go/ast/astutil: fix DeleteImport SEGV when Rparen is invalid
Updates #36383

Change-Id: I04b33810c16f4fb7871f5a6a8207b1c159cbc65f
GitHub-Last-Rev: 791fb862709c9a210bedde524555c5a6a92d9c51
GitHub-Pull-Request: golang/tools#196
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214340
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-01-17 16:16:41 +00:00
Rebecca Stambler
39095c1d17 gopls/internal/hooks: ignore a duplicate analysis from staticcheck
Hardcoded for now. Will need a better approach if more of these come up.

Fixes golang/go#34494

Change-Id: Id442d68fa16d81e747ad5ec951fb6b80fdb65f94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215118
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 06:52:30 +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
Michael Matloob
84cebe1034 go/packages: fix incorrect needtypes and needsrcs logic
This logic is really fiddly and I'm not really 100% sure it's right, though
I've thought about it for quite abit (and added comments to help me reason
through it).

Also always request CompiledGoFiles when NeedTypes is true because we might
need to fall back to loading from source when type data is incorrect.

Fixes golang/go#36441
Fixes golang/go#36547

Change-Id: I1cc27ca2e4401a9abc8502990b0da7d0480f6f84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214943
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 22:59:55 +00:00
Rebecca Stambler
28ed04f882 go/packages: internally expose ForTests in go/packages
This change eliminates our need to guess what the package under test is
in gopls, since `go list` always knows the answer.

Change-Id: I16e482ed3b452bd57cd478b1f8280fcea56474d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215020
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 22:11:50 +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
Rebecca Stambler
872a348c38 internal/lsp: don't log context.Cancelation in diagnostics
Fixes golang/go#34103.

Change-Id: I76e713d54422d31b587cedd0e4359c8b3bc7845e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215019
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 18:16:51 +00:00
Muir Manders
0508ad4c83 internal/lsp/source: return obj decl first in find-references
Restore previous behavior where the object's declaration is returned
as the first reference in find-reference calls.

Fixes golang/go#36598.

Change-Id: Ibdeaf9971aa5cb1f3244f6a888fe77bdf386e563
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215057
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-16 16:57:51 +00:00
Muir Manders
473961ec04 internal/lsp/source: improve completion for "range" and "<-"
Now that we understand object "kind" for builtin generic functions, we
can apply it to a couple more places as well:

// prefer rangeable object kinds
for i := range <> {
}

// prefer channels
<- <>

Change-Id: If9cfba3a06b3abde073a9d397000bb3f3b0e9853
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214678
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:25 +00:00
Muir Manders
f80fb1dfa1 internal/lsp: refactor find-references and rename
The main goal is to push the package variant logic from internal/lsp
into internal/lsp/source so all users of internal/lsp/source benefit.

"references" and "rename" now have top-level source.References() and
source.Rename() entry points (as opposed to hanging off
source.Identifier()). I expanded objectsAtProtocolPos() to know about
implicit objects (type switch and import spec), and to
handle *ast.ImportSpec generically. This gets rid of special case
handling of *types.PkgName in various places.

The biggest practical benefit, though, is that "references" no longer
needs to compute the objectpath for every types.Object comparison it
does, instead using direct types.Object equality. This speeds up
"references" and "rename" a lot.

Two other notable improvements that fell out of not using
source.Identifier()'s logic:

- Finding references on an embedded field now shows references to the
  field, not the type being embedded.
- Finding references on an imported object now works
  correctly (previously it searched the importing package's dependents
  rather than the imported package's dependents).

Finally, I refactored findIdentifier() to use pathEnclosingObjNode()
instead of astutil.PathEnclosingInterval. Now we only need a single
call to get the path because pathEnclosingObjNode() has the
"try pos || try pos-1" logic built in.

Change-Id: I667be9bed6ad83912404b90257c5c1485b3a7025
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211999
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:15 +00:00
Heschi Kreinick
7042ee646e internal/lsp/source: always look up mapper when building ranges
Any file could have //line directives in it, which means that we should
never trust a mapper that was looked up for a whole file. Remove the
range conversion helpers that accepted a mapper and look it up on the
spot.

Change-Id: Ic518891fcc1a682b31cbc6d1d4e1e1af1ef21962
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214949
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 01:10:02 +00:00
Muir Manders
49797d04a8 internal/lsp/source: improve completion in type assertions
In cases like:

var foo *someType = bar.(some<>)

We will now complete "some" to "*someType". This involved two changes:

1. Properly detect expected type as *someType in above example. To do
   this I just removed *ast.TypeAssertExpr from
   breaksExpectedTypeInference() so we continue searching up the AST for
   the expected type.

2. If the given type name T doesn't match, also try *T. If *T does
   match, we mark the candidate as "makePointer=true" so we know to
   prepend the "*" when formatting the candidate.

Change-Id: I05859c68082a798141755b614673a1483d864e3e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212717
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 01:06:23 +00:00
Rebecca Stambler
96555e0fa5 internal/lsp/cache: initialize view before LookupBuiltin
This will crash otherwise.

Change-Id: I4fbce813283291792ed21fa5d83186ec59543ff1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214948
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 00:42:58 +00:00
Rebecca Stambler
a7dab0268b internal/lsp: diagnose the snapshot on every text synchronization event
This change moves to our ultimate approach of diagnostics the snapshot
on every file change, instead of carefully picking which files and
packages to diagnose. Analyses are shown for packages whose files are
open in the editor. Reverse dependencies are no longer needed for
source.Diagnostics because they will be invalidated when the snapshot is
cloned, so diagnosing the entire snapshot will bring them up to date.

This even works for go.mod files because all of workspace-level `go list`s
will be canceled as the user types, and then we trigger an uncancellable
go/packages.Load when the user saves. There is still room for improvement
here, but it will require much more careful invalidation of metadata for
go.mod files.

Change-Id: Id068505634b5e701c6f861a61b09a4c6704c565f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 23:07:48 +00:00
Rebecca Stambler
97cd989a76 internal/lsp: remove boolean for publishEmpty in diagnostics
This parameter was added to avoid sending a slew of empty diagnostics
for the initial workspace load. It's actually not needed anymore, as we
can just detect if we have previously sent diagnostics for the given
file, and if not we shouldn't send an empty diagnostic. This is true for
all cases, not just the initial workspace load.

Change-Id: I34a323bc0c0df79edd39630cd25577238b256266
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214287
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 22:25:09 +00:00
Lucas Bremgartner
0110d43865 cover: error on negative numbers in profiles
CL 179377 introduced an optimized parser for coverage profiles.
The parser replaces the following regex:
^(.+):([0-9]+)\.([0-9]+),([0-9]+)\.([0-9]+) ([0-9]+) ([0-9]+)$

With this regex, negative numbers in the coverage profiles resulted
in parsing errors. With the new parser in place, this is no longer
the case. This commit restores the old behavior.

Change-Id: Iaa72035f1f587ed186eaf5a84b209df88e67fb57
GitHub-Last-Rev: 07470fba9d587e6d3f383409fc8cf52bb882a984
GitHub-Pull-Request: golang/tools#195
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213357
Reviewed-by: Katharine Berry <ktbry@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-15 21:23:19 +00:00
Rebecca Stambler
f04f2c82d0 internal/lsp/cache: refactor initialization for builtins
This change combines the two packages.Load calls that happen on view
creation. Builtins can be loaded along with the rest of the workspace.

To avoid race conditions, create a builtinPackageHandle type for
builtins and use it to create the data.

Updates golang/go#36531

Change-Id: I7aa342c463a0b7718e1ad5fee507622310d8443b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 21:04:26 +00:00
Rebecca Stambler
20e5918576 gopls/doc: improve troubleshooting docs with specifics
Add the exact VS Code settings that users need to capture valuable logs.

Change-Id: If6b4874d1d3beab2fa6da054615dd7973b6e3fa9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214940
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 20:40:27 +00:00
Rebecca Stambler
497c7f156c internal/lsp/cache: construct package handles as part of IWL
Package handles should be cached on the snapshot as part of the initial
workspace load, otherwise this cached data will be repeatedly lost and
reconstructed during tests and regular execution.

Fixes golang/go#36556

Change-Id: If7676685db17519c998b857a812467c7f3cc6003
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214799
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 19:55:33 +00:00
Heschi Kreinick
3ded1b734d internal/lsp: add and use nonstandard gopls/diagnoseFiles
Switch the command line client, and its tests, away from the hardcoded
30-second timeout and to a newly-added custom request.

Inconveniently for us, the jsonrpc2 package only serializes requests,
not replies. (Notifications are requests for this purpose.) So, for a
flow like this:

diagnoseFiles -->
  <-- publishDiagnostics
  <-- publishDiagnostics
diagnoseFiles <-- (reply)

...there's actually no guarantee that the incoming requests will be
processed before the reply comes in -- it gets to jump the
serialization. The only way to guarantee previous notifications have
been processed is to send another request. I didn't feel like adding
nonstandard notification support, so I just send a fake diagnostic.

Error handling for untyped JSON is hideous so for now we just panic.
Nobody else should be calling these, or if they do it's at their own
risk.

Fixes golang/go#36518.

Change-Id: I63f8df5bb627c9783314a0d38d2dac8721b3c320
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214583
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 19:23:06 +00:00
Rohan Challa
de0b176007 internal/lsp: show dependency quick fixes for go.mod diagnostics
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>
2020-01-15 16:51:05 +00:00
Damien Lespiau
fdfa0def04 gopls/doc: fix it's/its typo
Change-Id: If8eecf41ae6d83bf4228e818aa1ef9ee3267db58
GitHub-Last-Rev: 73153ddfb8c86a145eef79997b18a5cdef21b8bf
GitHub-Pull-Request: golang/tools#197
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214858
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-15 15:48:44 +00:00
Rob Findley
98c82cf1f4 internal/lsp: add server instance to debug info
When debugging multiple instances of gopls simultaneously, it is useful
to be able to inspect stateful debugging information for each server
instance, such as the location of logfiles and server startup
information.

This CL adds an additional section to the /info http handler, that
formats additional information related to the gopls instance handling
the request.

Updates golang/go#34111

Change-Id: I6cb8073800ce52b0645f1898461a19e1ac980d2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214803
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-15 14:58:21 +00:00
Rob Findley
850357681a gopls/integration/govim: update build steps to capture artifacts
Govim integration tests generate a number of artifacts, including both
the govim and gopls logs, that can be useful in debugging failures. This
change updates our cloud build configuration to capture these artifacts,
along with several other minor improvements.

Notably artifacts are uploaded to GCS as a separate build step, so that
we have the potential to use its granular permission model for sharing
these artifacts. Right now, this requires temporarily swallowing the
exit code of `go test` so that the build can proceed.

Also:
 - Update govim to a newer version; we still can't use latest as there
   isn't a tagged version that contains the requisite flag change.
 - Alter the test harness to run tests from the github.com/govim/govim
   module root.
 - Switch use a major version label when referring to the test harness
   build step, to allow for breaking changes (such as the one made
   here).
 - Add a missing copyright header to run_local.sh.
 - Update run_local.sh to work with the modified harness.
 - Update documentation accordingly.

Change-Id: Ie5ddaf54e775371a36163f98c1beb90c217be931
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214577
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 14:49:07 +00:00
Rob Findley
7fd989f60c internal/lsp/tests: correct typo 'CompletionCaseSensitve'
CompletionCaseSensitve is renamed to CompletionCaseSensitive.

Change-Id: I060d02cd72d271d0a1f5595ca35c55e7e142c996
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214802
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-15 14:33:14 +00:00
Rob Findley
df87866820 internal/lsp,internal/telemetry: correct stale docstrings
Several docstrings reference earlier names for the symbols they
document. This CL corrects those that I noticed while reading the lsp
code.

Change-Id: I1968459feff7011e070333c99eb149e72d3302de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214801
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 14:25:42 +00:00
Rebecca Stambler
831fdb1e18 internal/lsp: push initialization tasks into one function
This change moves as much view initialization code into the
initialization function, instead of having it happen on view create.
Also, the `go env` variables that are collected at inconsistent times
are all collected on view creation. That is sufficient, since the view
is recreated if the environment changes.

I had originally hoped that the initial call to `go env` and the
-modfile detection could become part of this parallel initialization as
well, but you can't create a *packages.Config until the temporary
modfile has been set up, so it still makes sense to do that on view
create. This is, however, the reasoning behind the refactorings in
the -modfile detection in this CL. The main changes are a few renamings
and a split between snapshot.ModFiles and view.modFiles to maximize the
amount of work done in the view. I changed view.modfiles to moduleInformation
because I thought we might want to store additional information there at some
point. Rohan, please let me know if you disagree with any of the changes I made,
and I can revert them.

Fixes golang/go#36487

Change-Id: I504db5a4f41b79bee99ebd391e32e7b520a19569
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-01-15 04:46:56 +00:00
Rob Findley
7ae403b6b5 internal/lsp: finish renaming CheckPackageHandle to PackageHandle
In golang.org/cl/209419, CheckPackageHandle was renamed to
PackageHandle, but a number of references to CheckPackageHandle remained
in function names and comments.

This CL cleans up most of these, though there was at least one case
(internal/lsp/cache.checkPackageKey) where the obvious renaming
conflicted with another function, so I skipped it.

Change-Id: I517324279ff05bd5b1cab4eeb212a0090ca3e3ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214800
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-14 23:56:10 +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
Rob Findley
e23c44e711 gopls/doc: minor fixes in design.md and implementation.md
A couple sentences are clarified in design.md, and a broken link is
fixed in implementation.md.

Change-Id: Ibbb4c2f74ce9e46c565a7e90798047e7168b076e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214798
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 23:33:23 +00:00
Rebecca Stambler
f986e617de internal/lsp: make sure diagnostics with analyses are not overwritten
The initial workspace load does not send analyses with diagnostics, but
subsequent diagnostic requests do. If we've already sent diagnostics
with analysis for a file at a given version and snapshot, do not resend
diagnostics for the same file version with analyses.

Change-Id: I6979aa308e8846ff0cb66bd23e0dac488eae5446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214587
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 22:43:10 +00:00
Rohan Challa
579903b3ad internal/lsp: add mapper for go.mod files
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>
2020-01-14 22:04:35 +00:00
Heschi Kreinick
3be912efc3 internal/lsp/protocol: support nonstandard requests
Support arbitrary client->server requests in the generated code. This is
primitive, with no strong typing, but should be good enough for simple
requests. We can do something fancier later if we want.

Change-Id: Ib83ff6aa49418bda5222b37cde3b150a26552221
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214582
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 21:52:24 +00:00