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

1589 Commits

Author SHA1 Message Date
Ian Cottrell
c0791ff00b internal/fakenet: add a fake network connection
This adds the ability to make a net.Conn based on a
reader/writer pair.
This is primarily useful for pretending stdin/stdout are a network
connection, but can be used for any reader/writer pair.

Change-Id: Ib398e62e0fac423db04aed27cfc48070bda2b93b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231697
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-27 14:27:59 +00:00
Ian Cottrell
dd5f9d1033 internal/stack: add the leak test
This adds a package for dealing with stacks in tests.
The only function at this time is NoLeak which verifies that a test
does not leak any goroutines, and prints a stack summary if it does.

Change-Id: I284b846f422334b745bef0e96d84138295120a62
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232681
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-27 14:25:42 +00:00
Ian Cottrell
a02cf32866 internal/stack: adding an internal stack dump parsing library
This can be used either to directly parse runtime.Stack output or
process text that includes stack dumps, like test timeouts or panics.
It includes a binary, gostacks that processes stdin to stdout replacing
stack dumps in place.

Change-Id: Id7b1cfd69b8aea36c66f12ec0bdf38b68cba5afb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232658
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-27 14:25:21 +00:00
pjw
253fce384c internal/lsp: fix formatting edge cases (36824)
One line legal code like `package x; import "os"; func f() {}` was
being misformatted. In these cases the parse flag ImportsOnly loses
important parts of the code, while full parsing works. Presumably
all these cases are short enough that there is no appreciable penalty
from the extra parsing.

Fixes https://github.com/golang/go/issues/36824

Change-Id: I9a4581d67c590578f8fdea5ed2a2a58e0bc3c40b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234900
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-26 20:56:00 +00:00
Bryan C. Mills
2b542361a4 internal/fastwalk: attempt Symlink tests on Windows
Windows does actually support symlinks, but older versions of
Windows only support symlinks when running as an administrator.
Newer versions of Windows support symlinks for all users.

Instead of skipping based on GOOS, first try the Symlink operation.
If it succeeds, we can proceed with the test; otherwise, we can try to
write a regular file to determine whether the problem was the symlink
operation itself or the destination path.

For golang/go#38772

Change-Id: Idaa9592011473de7f514b889859e420a84db6d01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234537
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-05-21 21:19:27 +00:00
Rebecca Stambler
91d71f6c2f internal/lsp/regtest: add a t.Skip for golang/go#36824 regtest
Switching to using a t.Skip means we are more likely to remember to
actually re-enable the test at some point.

Also picked up a staticcheck fix along the way.

Change-Id: I382eaa8d204bee74a7ff46e8a1b11dab567b83ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-05-21 15:57:04 +00:00
Muir Manders
cf2d1e09c8 internal/lsp/source: offer smart "append()" completions
Assigning a slice to the appendage of itself is common and tedious
enough to warrant a special case completion candidate. We now offer
smarter "append()" candidates:

    var foo []int
    foo = app<> // offer "append(foo, <>)"
    fo<> // offer "foo = append(foo, <>)"

The latter is only offered if the best completion candidate is a
slice. It is inserted as the second-best candidate because it seems
impossible to avoid annoying false positives if it is ranked first.

I added a new debug option to disable literal completions. This was to
clean up some test logic that was disabling snippets for all tests
just to defeat literal completions. My tests were failing mysteriously
due to having snippets disabled, and it was hard to figure out why.

Change-Id: I3e8313e00a1409840cb99d5d71c593435a7aeb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221777
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-20 22:05:37 +00:00
pjw
57a9e4404b internal/lsp: fix new bug duplicating comments after includes
Fixes https://github.com/golang/go/issues/39147

Change-Id: I6f78efccbabf21dbb00e56a49d88e26ff4733fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234584
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-19 20:57:26 +00:00
Emrecan BATI
7521f6f425 internal/lsp/cache: show update codelens in go.mod when -mod=vendor
Ignore vendor folder while checking module updates to be able to capture
updates.

Fixes golang/go#38711

Change-Id: I522246459f98c238c2b5cd28e6028f2b94cde7d9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-19 17:58:26 +00:00
Michael Matloob
10921354bc go/packages: add a Module field to the Package struct
This change introduces Module and ModuleError struct types to the
packages package with the same types as defined in the cmd/go
documentation for module information output by go list (with the
exception of the Module type's Versions and Update fields).
go/packages will fill the module struct with the module information
output by go list. Drivers that support modules can also provide
module information by filling the Module fields in the packages in
their driverResponses.

Fixes golang/go#35921

Change-Id: Icbdf79869f09d26f6a01c3670146ace4f6ffa25e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234219
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-19 14:27:18 +00:00
Rob Findley
0d0afa43d5 internal/lsp/fake: fix broken build due to conflicting changes
fake.Editor.Server was exported, but CL 233117 was rebased on top while
still using the unexported field.

Update the rebased code.

Change-Id: Ie8f1f2c3948a1122b9e30a843cc33c79ef623ea4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234494
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-05-19 01:57:57 +00:00
pjw
8979540587 internal/lsp: simplify and correct fixing imports for CodeAction
The code was introducting syntax errors for some edge cases (example in
regtest/import_test.go), and I found it hard to follow.

The new code passes all the tests. There are new regtests to guarantee
no CodeActions are returned for some cases that vim testing noticed.

Change-Id: Ia09da667f74673c11bfe185e4f76a76c66940105
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233117
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-18 22:54:12 +00:00
Rebecca Stambler
8018eb2c26 gopls: update dependencies in the go.mod
Change-Id: Iaf3f9d9d23f3977138978caac800c67c78bc92bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234338
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-18 20:39:08 +00:00
Rebecca Stambler
39aadb5b76 internal/lsp: fix docs on hover for var/const blocks
The priorities for which comment to show should be 1) documentation
directly above the var/const, 2) documentation for the var/const block,
3) line comments.

See https://github.com/microsoft/vscode-go/issues/3240.

Change-Id: Ie136f0f25ac8208147070682bb1f3a663d6da25f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234101
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-18 19:53:06 +00:00
Heschi Kreinick
259583f2d8 internal/testenv: tolerate missing cgo as needed
-nocgo builders and platforms where cgo doesn't work should skip cgo
tests.

Fixes golang/go#39131.

Change-Id: If596043dd25f824762d782c260f102008562ed42
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234479
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-18 19:41:03 +00:00
Rob Findley
4bde419ae8 internal/lsp/regtest: Skip failing/flaky TestRegenerateCgo
Something is making this test deterministically fail in some
environments, such as @bcmills' desktop.

Skip it while I build go at tip and debug.

Updates golang/go#39135

Change-Id: I1bf8c55c5cfca471a904de85936f504313094807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234480
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-18 19:26:01 +00:00
Heschi Kreinick
c79c01b1c5 all: consolidate cgo requirement checks
Many tools test check for the ability to compile cgo programs.
Consolidate them all into testenv.NeedsTool("cgo").

Change-Id: I62c96e7b4dc72df34b8fdbf10326c7d19e0613e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234108
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-18 17:24:58 +00:00
Heschi Kreinick
d3bf790afa internal/lsp: add Regenerate Cgo code lens
Now that we support authoring cgo packages better, we need a way to
regenerate the C definitions. Doing it automatically is very difficult;
in particular, referencing a new symbol from the C package may require
regeneration, but we don't want to do that for every typo.

For now, give the user a button and make them push it. We attach a
code lens to the import "C" line. This is vulnerable to the usual
user-didn't-save glitches.

Updates golang/go#35721.

Change-Id: Iaa3540a9a12bbd8705e7f0e43ad0be1b22e87067
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234103
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-15 22:01:28 +00:00
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
Rob Findley
0aa9f2fd80 internal/lsp/cmd: clean up remote flag descriptions
The format of descriptions for the `-remote.*` flags were inconsistent.
Clean them up.

Also remove a TODO in lsprpc.go about adding a test for telemetry. That
is enough of a separate concern (and one that is rapidly changing) that
I no longer think this TODO makes sense.

Change-Id: Id14796000634ae4be4b480d0386b1da9069737c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234113
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-15 20:48:22 +00:00
Rob Findley
8ddc06776e internal/lsp/source: don't link to packages matching GOPRIVATE in hover
Currently, our hover text by default links point to public documentation
sites (e.g. pkg.go.dev). This doesn't make sense for private repos, so
hide the hovertext link when the import path matches GOPRIVATE.

Implementing this was a little messy. To be optimal I had to thread
the value of goprivate through cache.view, and to be correct I had to
duplicate some code from cmd/go internal.

Regtest will follow after https://golang.org/cl/232983 is submitted.

Updates golang/go#36998

Change-Id: I1e556471bf919fea30132d9642426a08fdb7f434
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233524
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-15 19:36:02 +00:00
Heschi Kreinick
7d3b6ebf13 internal/lsp/cache: pass UsesCgo to go/types
In CL 229779 I enabled Cgo type checking for go/packages, but we don't
actually type check there. We need to enable it in our own type checking
too.

No test updates because the negative effects are relatively subtle and
caught by an upcoming regtest.

Change-Id: I31691d69eb104cdabfd4fbe0a14b1f3c9741eabb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234102
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-15 01:05:26 +00:00
Heschi Kreinick
0951661448 internal/lsp: use TypecheckCgo when possible
When on 1.15+, enable TypecheckCgo. This improves cgo support
significantly, but we'll still have trouble with newly-referenced C
identifiers and changes to the magic comment.

Updates golang/go#35721.

Change-Id: I44dc95ce2d91d552e1e66e3722dc4230ab59fedd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229779
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-13 17:53:51 +00:00
Richard Miller
866d71a317 internal/lsp/regtest: don't build unix_test for Plan 9
TestBadGOPATH tests for a empty element in the GOPATH list using
"GOPATH=:/path/to/gopath", assuming that ':' is the path list
separator. On Windows the test fails, because Windows path list
separator is ';'. Therefore this test isn't built for Windows.

On Plan 9, os.PathListSeparator is '\000', so the test fails
there too, and should not be built for Plan 9.

Change-Id: Icdc2f8de098c1415103ec6124906ad6c578ad183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233718
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-13 12:28:04 +00:00
Martin Asquino
2bc93b1c0c internal/lsp: add run test code lens
Change-Id: I2c47fa038c81851b2c1e689adc3812b23af55461
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231959
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-12 13:19:52 +00:00
Rebecca Stambler
aaeff5de67 internal/lsp/regtest: add regression test for golang/go#36960
It's in the build tagged tests because the fix is only in 1.14.

Fixes golang/go#36960

Change-Id: I606dfb4f73735c6db8c48f67f19720d340be8361
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232991
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-12 00:15:01 +00:00
Daisuke Suzuki
5485bfc441 internal/lsp/cmd: fix not displaying symbols result
When run in CLI, "DocumentSymbol()" returns "[]protocol.DocumentSymbol"
or "[]protocol.SymbolInformation", so need to handle that as well.

Change-Id: I7885d3c53899103553df57f7f0ceceb2a33ec021
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232557
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-11 23:26:04 +00:00
Paul Jolly
1762287ae9 internal/lsp: rename workspace symbol test symbols to avoid clash
In a later CL we include the fully qualified path to a symbol in the
Name field of SymbolInformation. This means that we end up with
matches like:

golang.org/x/tools/internal/lsp/workspacesymbol/b.WorkspaceSymbolVariableB

A fuzzy match against this name using the query "wsym" would match the
"workspacesymbol" of the import path as well as the
"WorkspaceSymbolVariableB" that is the symbol name itself.

Therefore we rename the symbols in the:

    internal/lsp/testdata/lsp/primarymod/workspacesymbol/...

from WorkspaceSymbol* to RandomGopher*, which allows our fuzzy matches
to be more precise.

Change-Id: Idbeb663f5750cae4835b0fdaa77531e30353fb89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228759
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 20:27:23 +00:00
Paul Jolly
0bd3dbed90 internal/lsp/fake: define Symbol method on Editor
In preparation for later changes to the implementation of the workspace
Symbol method, we add the Symbol method to fake.Editor. This requires
the definition of a number of associated fake types (editor-friendly,
byte-offset-based versions of protocol UTF16-based types) for example
fake.SymbolInformation and the types it references.

We also implement a basic regtest for the Symbol method, exposing Symbol
on regtest.Env like other LSP server methods. To aid with the writing of
Symbol result assertions, we provide some helper functions to simplify
the process of defining matches that are evaluated against the result
set.

Change-Id: If73b493e1e791c8201423a303af8041f5a15ccfc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228121
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 20:27:07 +00:00
Paul Jolly
ca5866bcf9 internal/lsp: add config option for SymbolMatch
In preparation for later changes to the workspace Symbol method, we add
a separate configuration option keyed by "symbolMatcher" that specifies
the type of matcher to use for workspace symbol requests. We also define
a new type SymbolMatcher, the type of this new option. We require
SymbolMatcher to be a separate type from Matcher because a later CL adds
a type of symbol matcher that does not make sense in the context of
other uses of Matcher, e.g. completion.

Change-Id: Icde7d270b9efb64444f35675a8d0b48ad3b8b3dd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228122
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 19:58:58 +00:00
Rebecca Stambler
8b0f8a7919 internal/lsp/source: handle nil pointer in package name hover
Updates golang/go#38977.

Change-Id: I8cbf0b058d77e749285cfe41b4b49de3764be861
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-11 19:44:01 +00:00
Paul Jolly
e48fac377d internal/lsp: change workspace symbols to use session config for matcher
WorkspaceSymbols matches symbols across views using the given query,
according to the matcher Matcher.

The workspace symbol method is defined in the spec as follows:

 > The workspace symbol request is sent from the client to the server to
 > list project-wide symbols matching the query string.

It is unclear what "project-wide" means here, but given the parameters
of workspace/symbol do not include any workspace identifier, then it has
to be assumed that "project-wide" means "across all workspaces".  Hence
why WorkspaceSymbols receives the views []View.

However, it then becomes unclear what it would mean to call
WorkspaceSymbols with a different configured Matcher per View.

Therefore we assume that Session level configuration will define the
Matcher to be used for the WorkspaceSymbols method.

As part of this change we also tidy up lsp_test.go and source_test.go to
remove some repetition.

Change-Id: I444f9a78303ac9d2c8d8ac6496603b5758e4aafd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228763
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-11 19:21:19 +00:00
Rob Findley
da4261a3d0 internal/lsp/cmd: use JSON output for the inspect subcommand
I've been using the inspect command to find data about the daemon and
its various sessions while debugging gopls. In practice, however, I
don't simply want to view the debug information: I want to script it.
This change removes the custom output formatting in favor of indented
JSON, so that we can do things like the following:

  tail -f $(gopls inspect sessions | gq -r .logfile)

Which tails the daemon logs for the current gopls binary version.

Change-Id: I8895644b1493862f027e6c4b06e32612a4f3927d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233357
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-11 18:25:40 +00:00
Ian Cottrell
01e0872ccf internal/event: improve the logging of events
This extracts the printing code from the log writer so it can be re-used
and then changes the loggers in the lsp to use it.
This means that messages that used to look like

  date:
    message=text

now print as

  date: text

which makes the logs a lot easier to read.

Change-Id: I9dfbae47cdc9aeb7d3ca3279e445f39f2e590827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232989
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 17:49:55 +00:00
Rebecca Stambler
2212a7e161 internal/lsp: return in the default case in cloneExpr
As a follow-up to CL 232990, return in the default case so that the
compiler will complain if we fail to return.

Change-Id: Ib771dfcd1a67b33fd51508ac183b861c00417efb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232992
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-09 03:07:07 +00:00
Heschi Kreinick
058404a2dd internal/lsp/source: fix cloneExpr for SelectorExprs
A missing return in the SelectorExpr case meant that cloneExpr would
return the original node, resulting in AST corruption when the caller
modified it.

It might be nice to panic in the default case to prevent this from
happening again, but for now let's just fix it.

Fixes golang/go#38927.

Change-Id: Ib99f2dadecf52007ac9319c480fbd2d636a0474a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232990
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-08 22:40:54 +00:00
Hana (Hyang-Ah) Kim
9b82503631 internal/lsp/source: return nothing for empty workspace symbol queries
In VS Code, a workspace symbol query with an empty query parameter
is issued as soon as users open the symbol search box. There are many
symbols in a reasonably sized project and the chance that a user finds
a result in the randomly chosen 100 items out of those many symbols is
low. Thus, this first query is often useless.

Ignore this query and return an empty result immediately.

Change-Id: Idc7703c8e460c9115ecbcf198907acc9c82add4d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232986
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 20:51:52 +00:00
Richard Miller
19e4049dcd internal/testenv: check that external 'diff' tool is the GNU version
TestVerifyUnified in internal/lsp/diff/difftest requires specific
behaviour of the 'diff' command which is known to be satisfied by
GNU diff. The plan9 'diff' command has no '-u' option, and the
illumos 'diff -u' produces output in a different format. Checking
specifically for the GNU version in the HasTool function ensures
the expected behaviour, and otherwise causes the test to be skipped.

Updates golang/go#38772

Change-Id: I5493fa8cfc48a112dc0b7356618c62d3ccb0366f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232479
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-08 20:46:49 +00:00
Rob Findley
b8469989bc internal/lsp/fake: fix some messiness around client hooks
While writing the fake editor, I added some state tracking without using
it (log messages, events etc). We have since duplicated this logic in
the regtest package using client hooks.

Fix two messy aspects of this:
 - remove the state tracking in the editor
 - pass in the client hooks when connecting, so that they may be used
   without locking, and so that we do not miss any hooks that may fire
   during session initialization.

Change-Id: I24c17a28e9cfa4fca32b7ddd17c7bf01cbb12e0f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232744
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:54 +00:00
Rob Findley
88bf40a80d internal/lsp/source: use the setString method when setting options
A couple string options were not using the asString helper. Update them,
and also add a setString helper to be consistent with setBool.

Change-Id: I38caef5b1595a3535b759e5bf1c8d350a8bf061e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232741
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:48 +00:00
Rob Findley
cb8d9cd245 internal/lsp: support configurable codeLens
Some code lenses may be undesirable for certain users or editors -- for
example a code lens that runs tests, when VSCode already supports this
functionality outside of the LSP. To handle such situations, support
configuring code lenses via a new 'codelens' gopls option.

Add support for code lens in regtests, and use this to test the new
configuration. To achieve this, thread through a new 'EditorConfig' type
that configures the fake editor's LSP session. It made sense to move the
test Env overlay onto this config object as well.

While looking at them, document some types in source.Options.

Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:35 +00:00
Rebecca Stambler
480da3ebd7 internal/lsp: return early in completion where possible
This a pure cut-and-paste.

Fixes golang/go#38868

Change-Id: I2ff07134a8b9f6186bab737caceab8a34a8e2f44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232677
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-07 20:50:54 +00:00
Rebecca Stambler
6441d34c3f internal/lsp: fix caching issue with duplicate handles
In (*snapshot).addPackage, we return early if the package handle is
already cached, but we continue building the dependency graph with a
handle passed into addPackage.

This seems fine since both handles should have the same cache key,
but if we clone the snapshot, we will end up dropping the handle that
had the type information on it. It will then have to be recomputed,
causing the skew in the types.Package.

Fixes golang/go#38403

Change-Id: I0e360447a428123fcac444fbea3c2a3232ef941a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-07 19:23:25 +00:00
Rob Findley
08cbf656ce internal/lsp/cache: add an UnsavedFiles method to Session
It is useful to know whether the session has any unsaved files, for
example to warn/error when executing a command that interacts only with
files on disk.

Add a new UnsavedFiles method to the Session.

Change-Id: Iea4bf472e3ed6897979306b670eb974a2ee0d3bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232747
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07 17:51:45 +00:00
Richard Miller
ff647943c7 internal/testenv: make ExitIfSmallMachine apply to plan9-arm builders
Some x/tools tests use too much memory (virtual or real) or other
resources to run on "small" builders. Originally only linux-arm
was classified as small. This change addes plan9-arm to the list,
since it normally runs on Raspberry Pi boards with 1GB of RAM.

Partial workaround for golang/go#38772

Change-Id: I93307af10cccf7b1e26d57b9af914c85cf676d43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232478
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-07 17:29:48 +00:00
Hana (Hyang-Ah) Kim
625332f3c5 internal/lsp/protocol: send responses for cancelled requests
LSP https://microsoft.github.io/language-server-protocol/specifications/specification-current/#cancelRequest
expects the server to send back the response even when the request is cancelled.

Gopls LSP protocol implements the cancellation using the context cancellation.
That is, upon a cancellation request from the client, the server calls the
corresponding canceller that cancels the context passed to the handler.
Reusing this cancelled context for the replyer is not safe because code in any layer
can decide to shortcircuit and skip sending the data back to the client.
E.g. https://cs.opensource.google/go/tools/+/master:internal/jsonrpc2/stream.go;l=63

This CL make sure to pass the detached context to the replier.

Alternative, or a better approach to avoid any unexpected side-effect of
using a detached context is to send out the response at the point of cancellation
with a separate context. But that requires more significant code change.

Testing is currently hard, but maybe doable once the current refactoring is
done. Test is left as a TODO.

Change-Id: I4611af00ad913e96b62c6b7180c6673b0465daf9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232300
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-05-07 15:26:07 +00:00
smasher164
a1532b81a2 internal/lsp/cache: avoid string(int) conversion
LoadMode and ParseMode are currently hashed with a string(int)
conversion, but this conversion is now discouraged (see the vet check
'stringintconv' for more information). Since the hash uses the code
point, this change replaces these instances with string(rune(int)).

Updates golang/go#32479.

Change-Id: I0d8e91d073fc34ac9faafe75a0d86cf71a5327d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232679
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-07 05:02:07 +00:00
Rob Findley
be0c89d0d3 internal/lsp/fake: check for file changes after running the Go command
Our fake.Workdir generates synthetic file events for "watched" files
when using its file API, but we have no such hooks for file changes
originating from an external process, in this case the go command.

Previously, we detected a file event by checking go command logs to see
if a go.mod file was created. This is bound to be fragile, so we replace
this with a helper function that walks the working directory looking for
changes.

Change-Id: I2c2176099b97f3a3b25f4aed2b18de13ae05544a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232637
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07 03:33:52 +00:00
Rebecca Stambler
88e38c1d8d internal/lsp: make sure diagnostics only refer to existing files
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.

Fixes golang/go#38602

Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 02:01:22 +00:00
Rebecca Stambler
002d754683 internal/lsp/regtest: add test for a GOPATH that's missing an element
Test the case described in
https://github.com/fatih/vim-go/issues/2673#issuecomment-622307211.

Change-Id: I55ff3b8719fc255ec0901cf3778e68b48630323d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232360
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 01:58:00 +00:00