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

4528 Commits

Author SHA1 Message Date
Muir Manders
49b8ac185c internal/lsp/cache: add file contents to ParseGoHandle
Currently there is no need for this because the file contents are part
of the file handle. This change is in preparation for an impending
improvement that tweaks the source code during the parse stage to fix
certain kind of terminal parse errors. Any code that wants to use
an *ast.File or *token.File in conjunction with the file contents
needs access to the doctored source code so things line up.

Change-Id: I59d83d3d6150aa1264761aa2c1f6c1269075a2ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218979
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 05:05:14 +00:00
Muir Manders
cbc0cc175f internal/lsp/source: fix completion crash in append()
We were crashing in cases like:

    var _ []byte = append([]byte{}, ""...<>)

We were type asserting the type of append's second param
to *types.Slice, but in this case it is a string (*types.Basic). Fix
by checking the type assert was successful.

Note that we still don't attempt to give string completions when
appending to a byte slice. We can add that special case later once
everyone is clamoring for it.

Change-Id: I1d2fbd7f538e580d33c2dab4ef127a88e16d7ced
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219144
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 02:33:03 +00:00
Rebecca Stambler
2ee7536ab1 internal/lsp: remove nilness analyzer
The nilness analyzer requires SSA, which is very expensive to build and
uses a lot of RAM. It also seems to really shoot up memory usage when it
hits certain cases, which is causing a lot of problems for users.
Disable this analysis - we'll leave SSA to staticcheck.

Updates golang/go#36639

Change-Id: I46e67a6fd7828a5fddcd42d1aa00876f17c79e3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219203
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-12 21:32:43 +00:00
Rebecca Stambler
6dd6151793 internal/lsp: ignore irrelevant on-disk changes
Change-Id: Ibdceadbfc822a64066d9370eefa9ff5f7988d0a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219202
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-12 21:15:05 +00:00
Muir Manders
ea181f53ac internal/lsp/source: filter candidates when type name required
Now when we expect a type name at the cursor, we omit non-type name
completion candidates. For example:

inch := 1
var foo in<> // don't offer "inch"

I also added expected type name detection for value specs:

// Expect a type name at <>
var foo <>

Fixes golang/go#32806.

Change-Id: I32477cb286d2050bac5ccc767f8a608124fa5acd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216400
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-12 15:05:39 +00:00
Andrew Bonventre
11eff242d1 Revert "go/analysis: add pass to check for impossible interface-to-interface type assertions"
This reverts commit 7a72f3f8e9.

Reason for revert: The 1.15 tree is not open yet.

Change-Id: I5b3e458748bb3d69950f6331672e8c883d6234fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219119
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 20:56:36 +00:00
Rebecca Stambler
6822f1ada4 internal/lsp: disable nilness analyzer, unless staticcheck enabled
Users with staticcheck enabled will be more tolerant of increased RAM
usage. We might be able to disable this check entirely once the next
version of staticcheck is released with a similar analysis (https://staticcheck.io/docs/checks#SA5011).

Change-Id: I1a844cc226e34e0f62222f12912797a6cc9d06e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-11 20:49:38 +00:00
Rebecca Stambler
e2a38c8363 internal/lsp: propagate file invalidations to all views
We were previously only invalidating files if they were contained within
a subdirectory of a view, but we should really be invalidating all files
that are known to a view.

Fixes golang/go#34955

Change-Id: I2eb1476e6b5f74a64dbb6d47459f4009648c720d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218859
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 18:37:05 +00:00
Rob Findley
f41547ceaf internal/lsp/debug: fix early closure of logfile
As of https://golang.org/cl/218457, logs are not being captured because
the logfile is prematurely closed due to the scope of the deferred
closure changing.

Change-Id: I1754e5555025c7b2a5da58f621184d6740fd03cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219080
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 18:05:03 +00:00
smasher164
7a72f3f8e9 go/analysis: add pass to check for impossible interface-to-interface type assertions
For a type assertion V.(T), where V and T are both interfaces, the
concrete type of V cannot possibly implement T if V and T contain a method
with the same name, but different signature. This change adds an
analysis pass to flag such cases.

Updates golang/go#4483.

Change-Id: Ib1e91794ca7079b3f450520cc1a57d91e46e42fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218779
Reviewed-by: Alan Donovan <adonovan@google.com>
2020-02-11 17:49:21 +00:00
Muir Manders
2de505fc53 internal/lsp: support multi-dereferencing completion candidates
Now we keep a count of how many times to dereference a candidate. For
example:

    var foo ***int
    var _ int = f<> // Now we offer "***foo" instead of "*foo".

Change-Id: I14edc40aeec6884399eceb3dd3b4f85dc74a773c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218580
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 04:52:51 +00:00
Muir Manders
1e13d9580f internal/lsp/source: fix type modifier detection in composite literals
When completing a composite literal value, we were returning from
candidate inference before we recorded type modifiers such as prefix
"&" or "*". This was causing funny completions like:

type myStruct struct { s *myStruct }
myStruct{s: &mySt<> // completed to "&&myStruct{}"

Now we properly pick up on the "&" prefix so we know our literal
"myStruct{}" candidate does not need a "&".

Change-Id: I908936698cfedfef81bc0c1cbcd93e14dc00e3a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218377
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 03:24:02 +00:00
Tomohiro Kusumoto
8feddd8b6a internal/lsp/tests: fix missing period
Change-Id: Idb0b6405aefec1dcef84958482325a68afec5348
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218957
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 03:20:47 +00:00
Muir Manders
0bc66720f3 internal/lsp/source: limit to 5 unimported package name candidates
Currently we show up to ~100 unimported package names matching the
completion prefix. This isn't useful since, assuming the user even
wants an unimported package, they will just type more to narrow it
down rather than scroll through 100 options. Having so many candidates
also slows things down due to per-candidate overhead in gopls and in
the LSP client. Now we instead limit to 5 unimported package names.

Unimported package members, on the other hand, make sense to list
many. The user may want to scroll through because they don't remember
the name of what they are looking for. I left the max value at 100.

Change-Id: I00e11fa0420758f8db6c7049f80fa156773a5ee6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218879
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 00:32:09 +00:00
Heschi Kreinick
b0390335f4 internal/lsp/debug: write debug info for large goplses
We've had various reports of high memory usage in gopls. Catching it in
the act can be difficult, so check every second and write relevant
profiles to os.TempDir every time the heap reaches a new watermark.

Note that the logging in this package is broken. I didn't fix it, so
nobody will know this is happening until we tell them. So it goes.

Change-Id: I972d7ccfe5308658f86dde717465f0e0151b396d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218858
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 22:37:59 +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
Ian Cottrell
bdd8440908 internal/lsp: move all debugging support to the debug package
We now carry around a debug instance in the app, and it manages the log file and
all telemetry

Change-Id: If4a51a36c38ef301b1b2bbda8e4c0a8d9bfc3c04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218457
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-10 18:44:17 +00:00
Jeremy Faller
a1f8cf0047 cmd/benchcmp: add deprecation notice
Change-Id: Ifa408a53f6a1edba3b8e53aae2048d999a5da45c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216843
Reviewed-by: Austin Clements <austin@google.com>
2020-02-10 18:42:41 +00:00
Muir Manders
2ef0387721 internal/lsp/source: fix unimported member completion ranking
When completing members on a type checked, unimported package, you get
fully typed members. That means you get deep completions. Because we
downrank the initial unimported package members so much, any deep
completions were dominating the rankings. For example

    context.Back<>

yielded "context.Background().Err" ranked above "context.Background".
Fix by scoring context.Background in this example as
stdScore+tinyRelevanceScore instead of just tinyRelevanceScore. I also
changed untyped candidate scores in the same way so they stay
competitive when you have both imported and unimported candidates.

The other option was to propagate the score penalty into deep
candidates, but that wasn't easy. In general I think you are better off
avoiding big score penalties because they complicate the interplay
between different kinds of candidates. Scoring needs an overhaul, but
at least we are building up our test suite in the meantime.

Change-Id: Ia5d32c057b04174229686cec6ac0542c30e186e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218378
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-10 18:17:33 +00:00
Rob Findley
9fbd0ccf67 internal/lsp/lsprpc: add test for definition outside of workspace
Add regression tests for GoToDefinition. In particular, exercise the
panic from golang/go#37045.

Updates golang/go#37045
Updates golang/go#36879

Change-Id: I67b562acd293f47907de0435c14b62c1a22cf2ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218322
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:53:45 +00:00
Rob Findley
babff93c04 internal/lsp/lsprpc: add test for empty diagnostics in deleted files
Add a test for the bug reported in golang/go#37049: we are missing empty
diagnostics for deleted files. Doing this involved added a missing
RemoveFile method on the fake.Watcher type.

Skip the test for now, as it is failing.

Updates golang/go#37049
Updates golang/go#36879

Change-Id: Ib3b6907455cc44a2e6af00c2254aa444e9480749
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218278
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:44:40 +00:00
Rob Findley
f9587291b6 internal/lsp/fake: add fakes for testing editor interaction
A lot of bug reports originating from LSP clients are related to either
the timing or sequence of editor interactions with gopls (or at least
they're originally reported this way). For example: "when I open a
package and then create a new file, I lose diagnostics for existing
files".  These conditions are often hard to reproduce, and to isolate as
either a gopls bug or a bug in the editor.

Right now we're relying on govim integration tests to catch these
regressions, but it's important to also have a testing framework that
can exercise this functionality in-process.  As a starting point this CL
adds test fakes that implement a high level API for scripting editor
interactions. A fake workspace can be used to sandbox file operations; a
fake editor provides an interface for text editing operations; a fake
LSP client can be used to connect the fake editor to a gopls instance.
Some tests are added to the lsprpc package to demonstrate the API.

The primary goal of these fakes should be to simulate an client that
complies to the LSP spec. Put another way: if we have a bug report that
we can't reproduce with our regression tests, it should either be a bug
in our test fakes or a bug in the LSP client originating the report.

I did my best to comply with the spec in this implementation, but it
will certainly develop as we write more tests. We will also need to add
to the editor API in the future for testing more language features.

Updates golang/go#36879
Updates golang/go#34111

Change-Id: Ib81188683a7066184b8a254275ed5525191a2d68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217598
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:44:19 +00:00
Muir Manders
61798d64f0 internal/lsp: fix crash completing recursive pointer types
We were recursing infinitely evaluating objects of recursive pointer
types such as "type foo *foo". Now we track named pointer types we
have already seen to avoid trying to dereference such objects forever.
I lazily initialized the "seen" map to avoid the allocation in the
normal case when you aren't dealing with named pointer types.

Fixes golang/go#37104.

Change-Id: I5f294cfc5a641e7b5fd24e1d9dc55520726ea560
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218579
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 22:44:06 +00:00
Rebecca Stambler
6dcdf1db2c internal/lsp/cache: refactor functions that return PackageHandles
Now that we build PackageHandles in load, we can simplify a lot of our
logic for rebuilding them and reloading metadata.

The only possibly-significant change is the missing imports logic. Now
that we always create package handles in load, we don't have to do the
extra "sameSet" logic. If the package handle hasn't been invalidated
since we last built it, we will keep the benefits of caching it. Otherwise,
it will be rebuilt in load.

Updates golang/go#36918

Change-Id: Ieb45898d248501e3cebdb95c8b518149ba7498e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217157
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 22:39:43 +00:00
Heschi Kreinick
009580c43b internal/lsp/source: export FindFileInpackage
And delete a copy of it.

Change-Id: Ice7b932327dbfe5e00f1d084fc6669f1e4059afe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218320
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 21:55:11 +00:00
Rebecca Stambler
8bb641bf89 internal/lsp/source: default to full documentation for hover
I'm starting to think that it might make more sense to show all
documentation on hover as a default. A number of people have requested
this behavior, and I think it would help ensure a more consistent
experience for users. We had originally defaulted to the synopsis
because VS Code Go had this behavior, but I see no reason to follow that
as a guideline.

Change-Id: I67aa530d253422550f59b5583e4c4a90ebd48f5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217727
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 21:32:44 +00:00
emahiro
b6336cbc8d internal/lsp/tests: move workspace symbol test helpers functions into util.go
Change-Id: I747c7a0ca9b41e82e33a39fc91ddc2afaf458be6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218139
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 21:23:39 +00:00
Rebecca Stambler
7cfd24942e internal/lsp/cache: hardcode parse modes instead of guessing them
This change largely reverts CL 217139, which attempted to guess a
package's parse mode based on whether or not it was in the user's
workspace. This ignored the fact that a user may jump to the definition
of a file outside of their workspace.

Fixes golang/go#37045

Change-Id: Icb6b9d055bd1f260013227db1a6a34873c45b680
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218499
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-07 20:00:15 +00:00
Tomohiro Kusumoto
112b90105c internal/lsp/source: eliminate setKind
Eliminate unnecessary function

Change-Id: I1b590961c3b2042f244eeb4c11c34fbf20b8b74a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218138
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 19:42:34 +00:00
Rebecca Stambler
3d51b05cfb internal/lsp: don't use overlays from the session in the snapshot
Hold the session's overlay mutex the whole time we compute new overlays,
and then pass these overlays directly into clone. This avoids us calling
s.session.GetFile, which can return overlays that the snapshot doesn't
yet "know" about.

Change-Id: I1a10c78e26f8fec64550bfe0a97b5975ea8f976b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 19:21:16 +00:00
Rebecca Stambler
b753a1ba74 internal/lsp: build overlays through the snapshot
This is the first in a series of changes to move overlay handling to the
snapshot instead of the session. We may not be able to fully get away
from managing overlays on the session, but we should be able to only use
overlays when they are known to the snapshot.

Change-Id: I88b125117cd2cfbd0ff9ef16a944a34297c81b10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218324
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 18:37:49 +00:00
Rob Findley
58fec203a2 internal/lsp/cache: don't type check types.Unsafe
Recent test runs exposed that we are racing to types.Unsafe, as a result
of parallel type checking. Example:
https://storage.googleapis.com/go-build-log/494dd1dd/linux-amd64-race_c978961e.log

We shouldn't have to type check unsafe, so we can return early instead.

Change-Id: I20143dbfb07925d85d7342b93360bdda1c45e4aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 18:36:39 +00:00
Rohan Challa
a9bd9c230f internal/lsp/cache: improve ModTidyHandle cache key
This change alters the key that is used to cache go.mod diagnostic changes, in particular it replaces using the snapshot ID with a string of all the imports used in the module and the hashed contents of the go.mod file. This reduces the number of times that we run "go mod tidy" to only when we detect import changes or the go.mod file is changed.

Change-Id: Icf49db34f44a4ae4772fff6dfb8b9a6955a8e2d6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218238
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 18:07:39 +00:00
Christopher Loessl
533eb26545 fix(readme): expected comma
Change-Id: I821783f364bcbc527ba4942d63c0aeb9ca473862
GitHub-Last-Rev: b0ac31a12214825f9ee8efd643d0bb976bf2c4ac
GitHub-Pull-Request: golang/tools#202
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218437
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-02-07 13:10:02 +00:00
Ian Cottrell
59bd84bb0c internal/lsp: change debug instance to a struct
We now build an instance and invoke methods on it.
This is a step towards removing some global state,
allowing multiple instances in one process and cleaning
up some telemetry handling.

Change-Id: I067469fed39c96653fffe96945c79193e86458d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218157
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-07 11:26:02 +00:00
Rohan Challa
6fdc5776f4 internal/lsp: add quickfixes for missing dependencies in go.mod
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>
2020-02-07 00:16:14 +00:00
Rob Findley
c29062fe1d internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).

This change reorganizes Server instantiation as follows:

 + The lsp.Server is now purely an implementation of the protocol.Server
   interface. It is no longer responsible for installing itself into the
   jsonrpc2 Stream, nor for running itself.

 + A new package 'lsprpc' is added, to implement the logic of binding an
   incoming connection to an LSP server session. This is put in a
   separate package for lack of a clear home: it didn't really
   philosophically belong in any of the lsp, cmd, or protocol packages.
   We can perhaps move it to cmd in the future, but I'd like to keep it
   as a separate package while I develop request forwarding.

   simplified import graph:

    jsonrpc2 ⭠ lsprpc ⭠ cmd
               ⭩           ⭦
            lsp           (t.b.d. client tests)
           ⭩   ⭨
     protocol  source

 + The jsonrpc2 package is extended to have a minimal API for running a
   'StreamServer': something analogous to an HTTP server that listens
   for new connections and delegates to a handler (but we couldn't use
   the word 'Handler' for this delegate as it was already taken).

After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.

This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.

To test this change, the following improvements are made:

 + A servertest package is added to make it easier to run a test against
   an in-process jsonrpc2 server. For now, this uses TCP but it could
   easily be modified to use io.Pipe.

 + cmd tests are updated to use the servertest package. Unfortunately it
   wasn't yet possible to eliminate the concept of `remote=internal` in
   favor of just using multiple sessions, because view initialization
   involves calling both `go env` and `packages.Load`, which slow down
   session startup significantly. See also golang.org/issue/35968.

   Instead, the syntax for `-remote=internal` is modified to be
   `-remote=internal@127.0.0.1:12345`.

 + An additional test for request cancellation is added for the
   sessionserver package. This test uncovered a bug: when calling
   Canceller.Cancel, we were using id rather than &id, which resulted in
   incorrect json serialization (as only the pointer receiver implements
   the json.Marshaller interface).

Updates golang/go#34111

Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 23:12:37 +00:00
Rebecca Stambler
37215997d4 internal/lsp: move all of the test helpers functions into one file
Each of these files had a couple of functions that were very similar in
nature. There's no need to have separate files for all of these.

Change-Id: I4ca648d1b7e90539f274871d45b7c97a8111631f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 20:47:26 +00:00
emahiro
f8c9a4c3ab internal/lsp: export DiffSymbols to avoid duplication
Change-Id: I9002d19067011e0fcff22782eebc593a124594c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-02-06 20:09:13 +00:00
Rebecca Stambler
222e5d4b1b internal/lsp: don't return references for builtins
https://github.com/fatih/vim-go/issues/2701 contains a report of a panic
when finding references on a builtin. Avoid this by simply not returning
any references.

Change-Id: I195fcb4502634201888f0c65022c9d16169cc1f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 19:49:06 +00:00
Rebecca Stambler
6224300ba8 internal/lsp: remove unnecessary source.SignatureInformation type
We should just use the protocol.SignatureInformation type, as it's
essentially the same thing. Refactor tests a bit to make use of the
shared type.

Change-Id: I169949f6e23757ce0a6f54de36560c4c8e0479ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217731
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-06 19:05:38 +00:00
Rohan Challa
64a0f23fc3 go/packages/packagestest: do not overwrite existing go.mod file
This change adds support for testing go.mod files within packagestest. Primarily, if there are markers in the go.mod file, this will copy the contents to a temporary file, build the modcache, then set the contents back.

Updates golang/go#36091

Change-Id: Icb707906eb7fc9e4a06fe043f94f34d9223d84c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216839
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 15:23:23 +00:00
Rohan Challa
2529d2857a internal/lsp/tests: standardize testdata folder format
This change standardizes the folder structure for testdata that are used for testing the lsp. In particular, it uses the following format:
- dir
  - primarymod
    - .go files
    - packages
    - go.mod (optional)
  - modules
    - repoa
      - mod1
        - .go files
        -  packages
        - go.mod (optional)

As we can see, any folder inside of testdata should be of this format, where the primary test files with the markers are all located inside the primarymod folder. The modules folder is used to hold any potential dependencies that are used for testing.

A consequence of this change is that we can have one directory separated by folders, where each folder is it's own module, this allows us to use internal/lsp/tests with go.mod files. Now, tests.Load() will return an array of Data objects, where each object corresponds to one of the directories structured above.

Updates golang/go#36091

Change-Id: I437cc2a2a9fc1bac93779845737aa74383fbf9c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217541
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 14:14:23 +00:00
Muir Manders
dd0d5d4851 internal/lsp: filter keyword completions in tests
Filter keywords out of completion results in tests similar to
builtins. You don't care about keyword completions unless you are
explicitly testing keyword completion.

Change-Id: I0caaaef8b0f5b08c4b15ba3ada1a963f35a14028
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217499
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:30 +00:00
Muir Manders
f66ef90017 internal/lsp/source: improve completion after accidental keywords
Completion often fails when the completion prefix happens to be a
keyword. We previously tried to fix this with AST surgery, but
often the accidental keyword is not apparent looking at the AST.
For example:

    chan<>
    foo()

parses as CallExpr{Fun: ChanType{Value: Ident{"foo"}}} with very few
hints that something is wrong, and:

    default
    foo()

is completely omitted from the AST.

Rather than look in the AST, we now instead manually look for a
keyword token that contains the completion position. If we find one,
we treat that as our surrounding identifier.

Updates golang/go#34332.

Change-Id: I68ed0dd905848c0eae61f39ecb8b73adb1e72746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216961
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:22 +00:00
Muir Manders
50d618665b internal/lsp/source: improve completion involving multiple return values
For example:

// Prefer functions that return one or two values. Previously
// we had no preference.
foo, bar := <>

// Prefer functions that return "(int)" or "(int, ??)". Previously we
// only preferred the former.
var foo int
foo, bar := <>

// Prefer functions that return "(int)" or "(int, int)". Previously we
// only preferred the former.
var foo func(int, int)
foo(<>)

In the above example, we don't handle "foo" being variadic yet.

I also took the liberty to break up matchingCandidate() into separate
functions since it was getting rather long.

Updates golang/go#36540.

Change-Id: I9140dd989dfde1ddcfcd9d2a14198045c02587f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215537
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:07:08 +00:00
Rebecca Stambler
531cc8856e gopls/doc: update VS Code settings to correspond with current defaults
Change-Id: Icf16808af9e476040b034ab5bd31e56fd0c0938e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218077
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 01:06:05 +00:00
Rebecca Stambler
403f1254bd internal/lsp: stop returning metadata from (*snapshot).load
This metadata was hardly being used, and it's not really necessary now
that we are creating package handles in load. There are still a number
of cases that can simplified because of this fact, but those will be
done in follow-ups.

Also, fix a stray staticcheck warning.

Change-Id: I12d1b4d568a8fd145d08397a926e7ba6f3428604
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 00:15:21 +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