1
0
mirror of https://github.com/golang/go synced 2024-09-30 16:08:36 -06:00
Commit Graph

5043 Commits

Author SHA1 Message Date
Rob Findley
1c30660f9f gopls/integration/govim: enable running at main
We're going to switch to running govim tests at main as post-submit CI
rather than presubmit, and will also switch to running them via Kokoro
using the run_local script rather than cloud build.

Enable this by changing the semantics of run_local.sh to default to
main.

For golang/go#40451

Change-Id: I9c311dea8326a36a3f8335eddbfae0ce7f02f6bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245539
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-29 17:39:47 +00:00
Rebecca Stambler
6b78e25f47 internal/lsp: fix a few staticcheck suggestions, some cleanup
There were a few merge conflict-related issues in the GC optimization
details CL. Also fixed a few things I noticed after the fact, like
separating out a new mutex.

Staticcheck caught a few things, and I also fixed a bug I noticed
in the cache package.

Change-Id: I3fc519373253418586dca08fdec3114b30a247ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245399
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-07-29 17:11:59 +00:00
Rebecca Stambler
8afe7e0893 internal/lsp: don't show diagnostic in go.mod for direct dependencies
Fixes golang/go#40470.

Change-Id: I4831b837e6db5cc344c20480abde1e688ddab0dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-07-29 17:04:04 +00:00
Rebecca Stambler
449c585133 internal/lsp: properly check for nil on i.enclosing
Fixes golang/go#40479

Change-Id: If827a86fd603574d3adde295c1165bb6267732eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245486
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-29 16:35:35 +00:00
Josh Baum
1c1ec420e5 internal/lsp/source: move initial extract function logic to lsp/command
In the previous implementation, the initial verification in lsp/command
for whether extract function was relavant to the given range did not
contain much of the initial logic for extract function. This meant
that "canExtractFunction" produced many false positives (i.e. the
lightbulb would appear when it should not have in VSCode). This CL
moves more of the verification process from "extractFunction"
(lsp/source) to "canExtractFunction" (lsp/command).

Change-Id: If2683dc9ac3f4bfa8c3444418cf88edd8cbe73e6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245398
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-29 16:09:07 +00:00
Rebecca Stambler
df70183b18 internal/lsp/source: add extra nil check in searchForEnclosing
I still keep seeing this crash too, even after CL 244841.

Fixes golang/go#40464

Change-Id: Ic587045e65f34c24bd6df452e24517fd90e36bbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245440
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-29 04:18:21 +00:00
Peter Weinbergr
e8769ccb43 internal/lsp: fix logged message for gc_details
Removes leftover debugging string from the message logged when
gc_details executes.

Change-Id: I802b276d1590e0b1e9793190990a1e576dc893ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245417
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 23:52:36 +00:00
Rebecca Stambler
0dd562f374 internal/lsp: remove uses of crypto/sha1 in gopls
SHA1 can have collisions. Replaced with crypto/sha256.

Change-Id: I54e8c042ae1a4eb41760ccbe26a7c59f68309477
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245326
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 22:26:17 +00:00
Peter Weinbergr
95780ea8b3 internal/lsp: show compiler optimization decisions
The gc compiler will report its decisions about inlining, escapes, etc.
This can be turned on and off with a new optional code lens gc_details.
When enabled, the code lens will be displayed above the package
statement. The compiler's decisions are shown as information diagnostics.
(Other diagnostics have been errors and warnings.)

Change-Id: I7d1d5b5b5cf8acd7ff08f683e537ea618e269547
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243119
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 21:59:21 +00:00
Rebecca Stambler
edd3c8e9e2 internal/lsp: de-duplicate logic for canExtractVariable
We'd like to call canExtractVariable in extractVariable without
duplicating logic. The same needs to be done for canExtractFunction.

Change-Id: Ia99befabbafffcf13dd3bc12355f9ddb81a71002
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245135
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-28 19:08:22 +00:00
Rebecca Stambler
342ee1054f internal/lsp: check URIs of all workspace folders
We can prevent crashing on non-file URIs by checking the URIs of the
workspace folders, as well as the root URI.

Updates golang/go#40272

Change-Id: Ieddc6d6053fbb3d61e4c26fc8831c092328f6f33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244602
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 19:06:24 +00:00
Rob Findley
e7a7e3a8a0 internal/lsp/source: add a unit test for searchForEnclosing
The logic to resolve the enclosing type for an identifier is somewhat
tricky. Add a unit test to exercise a few edge cases.

This would probably be easier to read and write using a hybrid approach
that extracts markers from the source.

This test uncovered a bug, that on the SelectorExpr branch we were
accidentally returning a nil *Named types.Type, rather than a nil
types.Type.

Change-Id: I43e096f51999b2a6e109c09d3805ad70a4780398
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244841
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 18:51:24 +00:00
Heschi Kreinick
b6476686b7 internal/lsp: remove PackageHandle
Just like ParseGoHandle, PackageHandle isn't very useful as part of the
public API. Remove it.

Having PackagesForFile take a URI rather than a FileHandle seems
reasonable, and made me wonder if that logic applies to other calls like
ParseGo. For now I'm going to stop here. I could also revert that part
of the change.

Change-Id: Idba8e9fdba0b0c48e841a698eb97e47fd5f23cf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244637
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:35:25 +00:00
Heschi Kreinick
a9439ae9c1 internal/lsp: replace ParseGoHandle with concrete data
ParseGoHandles serve two purposes: they pin cache entries so that
redundant calculations are cached, and they allow users to obtain the
actual parsed AST. The former is an implementation detail, and the
latter turns out to just be an annoyance.

Parsed Go files are obtained from two places. By far the most common is
from a type checked package. But a type checked package must by
definition have already parsed all the files it contains, so the PGH
is already computed and cannot have failed. Type checked packages can
simply return the parsed file without requiring a separate Check
operation. We do want to pin the cache entries in this case, which I've
done by holding on to the PGH in cache.pkg.

There are some cases where we directly parse a file, such as for the
FoldingRange LSP call, which doesn't need type information. Those parses
can actually fail, so we do need an error check. But we don't need the
PGH; in all cases we are immediately using and discarding it.

So it turns out we don't actually need the PGH type at all, at least not
in the public API. Instead, we can pass around a concrete struct that
has the various pieces of data directly available.

This uncovered a bug in typeCheck: it should fail if it encounters any
real errors.

Change-Id: I203bf2dd79d5d65c01392d69c2cf4f7744fde7fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244021
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:35:11 +00:00
Heschi Kreinick
4d1d9acccf internal/lsp/cache: fix parseKey
The FileIdentity struct mixes information about the file itself
(filename, hash) with information about the LSP references to that file
(session ID, version). When we create a cache key using it, we only want
the former, as returned by the String method. Otherwise we split the
cache whenever those irrelevant fields are different.

We also use FileIdentity as an element of diagnosticsKey, but I believe
that use is appropriate.

Change-Id: I094e00d2700e05778da635effbb69d0ebcb6726e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244020
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:57 +00:00
Heschi Kreinick
72051f7961 internal/lsp: pass snapshot/view to memoize.Functions
Due to the runtime's inability to collect cycles involving finalizers,
we can't close over handles in memoize.Functions without causing memory
leaks. Up until now we've dealt with that by closing over all the bits
of the snapshot that we want, but it distorts the design of all the code
used in the Functions.

We can solve the problem another way: instead of closing over the
snapshot/view, we can force the caller to pass it in. This is somewhat
scary: there is no requirement that the argument matches the data that
we're working with. But the reality is that this is not a new problem:
the Function used to calculate a cache value is not necessarily the one
that the caller expects. As long as the cache key fully identifies all
the inputs to the Function, the output should be correct. And since the
caller used the snapshot/view to calculate that cache key, it should
always be safe to pass in that snapshot/view. If it's not, then we
already had a bug.

The Arg type in memoize is clumsy, but I thought it would be nice to
have at least a little bit of type safety. I'm open to suggestions.

Change-Id: I23f546638b0c66a4698620a986949087211f4762
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244019
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:46 +00:00
Heschi Kreinick
60da08ac03 internal/lsp/cache: remove files from the memoize.Store
The memoize cache buys us little for files: the cache value is not
really a function of the inputs, but rather the filesystem state. It's
pretty much just as easy to manage them explicitly, and it's a start at
simplifying our caching strategy.

We do lose one small feature: if we try to read the same file
concurrently, reads will not be deduplicated. I suspect that doesn't
matter.

Change-Id: I75e219467fb7a512d9cfdf87443d012c85f03df9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243197
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-28 17:34:34 +00:00
Heschi Kreinick
051e64e62c internal/lsp: minimize PackageHandle interface
The PackageHandle interface is fairly redundant with the Package
interface. As much as convenient, move users to Package and
weaken/remove methods from PackageHandle.

I would like to get rid of CompiledGoFiles too but
NarrowestPackageHandle is a little annoying. I think this is
unambiguously a step forward so I figured we can get it in and keep
iterating.

Change-Id: I6c5a3f462b1f19cbca6a267fedc36ce54613b6fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244018
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:24 +00:00
Rebecca Stambler
2ad651e9e2 internal/lsp: handle bad formatting with CRLF line endings
The importPrefix logic is complicated by Windows line endings, since
go/ast isn't aware of different line endings in comment text. I made a
few changes to the way that import prefixes are computed to handle this.

Specifically, for comments, we try to make sure the range ends on a full
line as much as possible, because that addresses the line ending issue.

Fixes golang/go#40355

Change-Id: I84c1cfa0d0bae532e52ed181e8a5383157feef24
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244897
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 16:05:17 +00:00
Josh Baum
3c048e20c6 internal/lsp: support return statements in extract function
Previously, users could not extract code that contained a return
statement. Now, users can extract code with return statements, as long
as the statements are nested within an if, case, or other control
flow statement.

Updates golang/go#37170

Change-Id: I2df52d0241517472decabce3666a32392ff257bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243650
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 16:04:03 +00:00
Peter Weinbergr
cd83430bb0 internal/lsp: update code for LSP protocol
1. Change code.ts so it generates []json.RawMessage in place of []interface{}
for Command-related arguments. As usual, vscode introduces a lot of
whitespace-only changes.
2. Generate code based on the July 28 version of vscode-languageserver.
The changes are mostly related to SemanticToken, and didn't require
any changes to gopls, other than in the generated code.

Change-Id: I673e29e2fbc097409683dfe7af911d8f66e25c5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245134
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-28 15:42:44 +00:00
Pontus Leitzler
55644ead90 internal/lsp: allow narrower scope for convenience CodeActions
Code actions that apply convenience fixes were filtered by the start
line so it wasn't possible to narrow the scope to a specific range.

This change allows clients to send a specific range (or cursor position)
to filter all fixes where the range doesn't intersect with the provided
range. It also widens the diagnostic returned by fillstruct analysis.

The idea is to provide a way to narrow the scope without breaking
clients that do want to ask for code actions using the entire line.

Updates golang/go#40438

Change-Id: Ifd984a092a4a3bf0b3a2a5426d3e65023ba4eebc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244519
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-27 23:36:28 +00:00
Rebecca Stambler
7b4c4ad3dc internal/lsp: correctly marshal arguments for upgrade code lens
I'm not sure how the regtest didn't catch this - is it possible that
it could unmarshal a single string a slice of string? Either way, I'd
like to get the fix in quicker - I'll try to add more regtests for this
later.

Also, validate the upgrade results more thoroughly.

Change-Id: I812a3fecd9f0642a1408c0a9c0376bb98d50b397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245065
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 21:52:59 +00:00
Rebecca Stambler
59c6fc0b54 internal/lsp: correctly invalidate metadata for batched changes
I finally spent the time to understand why branch changes were causing
unexpected errors. There may be other bugs, but this is the first I
spotted. For batched invalidations, we were overriding the value of
invalidateMetadata for each file, so the results depended on the order
of files in the didChangeWatchedFiles notification.

Change-Id: Id3ca7a758af0115c46dcd74ede590a0be3f8307d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244606
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 19:55:46 +00:00
Rebecca Stambler
9267083701 internal/lsp: support refactor.extract through commands
The logic for extracting a function is quite signficant, and the code
is expensive enough that we should only call it when requested by the
user. This means that we should support extracting through a command
rather than text edits in the code action.

To that end, we create a new struct for commands. Features like extract
variable and extract function can supply functions to determine if they
are relevant to the given range, and if so, to generate their text
edits. source.Analyzers now point to Commands, rather than
SuggestedFixFuncs. The "canExtractVariable" and "canExtractFunction"
functions still need improvements, but I think that can be done in a
follow-up.

Change-Id: I9ec894c5abdbb28505a0f84ad7c76aa50977827a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244598
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 19:25:51 +00:00
Rebecca Stambler
eaaaedc6af internal/lsp: fix hover link for embedded fields and methods
Our logic to generate documentation links did not account for embedded
fields and methods. The types.Info.ObjectOf an embedded field returns
the *types.Var created for the field, not its types.TypeName, so we have
to navigate back to the actual definition of the field. This requires
traversing through all of the named types in the top-level type.

Fixes golang/go#40294

Change-Id: Ia6573aebe66b7f60e2d6861a381cd7b07e7d7eaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244178
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-27 15:46:26 +00:00
Rebecca Stambler
102e7d3570 internal/lsp: fix GlobPattern for watching files
I noticed this wasn't working correctly when debugging issues with file
changes on disk. I think I misunderstood how to correctly match on
zero or more path segments. Confirmed that this is working as expected
with VS Code, but we really need regression tests for this in VS Code
(it seems to be based directly on VS Code's API). Filed
golang/vscode-go#404.

Change-Id: Ib906f73a97317dfa9bd30099877c90d4072651cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244605
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-25 20:09:36 +00:00
Rebecca Stambler
b5fc9d354d internal/lsp: add parse errors as diagnostics even when parsing fails
When a package consists of files that only fail to parse, we fail to
associate parse errors with the package, and therefore return no
diagnostics. To address this, we need to associate the errors with the
package. This involves adding a *token.File to the parseGoData so that
error messages and positions can be properly extracted, even when there
is no associated AST.

Fixes golang/go#39763

Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-24 17:29:32 +00:00
Rebecca Stambler
7017fd6b13 internal/lsp: fix hover for implicit type switch variable declarations
There was a bug in the hover for type switch variables. For example:

var x interface{}
switch y := x.(type) {
    case string:
    case int:
}

Hovering over y would previously show "var y string", because y's object
would be mapped to the first types.Object in the type switch. Now we
show the hover for y as "var y interface{}", since it's not yet in the
cases.

Change-Id: Ia9bd0afc4ddbb9d33bbd0c78fa32ffa75836a326
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-24 02:27:22 +00:00
Rebecca Stambler
cbb3c69a37 internal/lsp: add errors for missing modules that don't map to an import
Some modules may need to be added to the go.mod without being associated
with an import statement. In such cases, we show the missing module
diagnostic on the whole go.mod file. This isn't ideal, but mapping to
the full require statement isn't that simple, and this is an easy enough
starting point. The code in mod_tidy.go is becoming more unwieldy - I
think I will clean it up in a follow-up.

Fixes golang/go#39784

Change-Id: Ib32ec1fd74c455ce42ba778ea6cba0a475cf245a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243218
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-24 00:18:24 +00:00
Rebecca Stambler
37a045f3b9 internal/lsp: move undeclaredname suggested fix out of analysis
This CL is a follow-up from CL 241983. I didn't realize that the
undeclaredname analysis was also using the go/printer.Fprint trick,
which we decided was both incorrect and inefficient. This CL does
approximately the same things as CL 241983, with a few changes to make
the approach more general.

source.Analyzer now has a field to indicate if its suggested fix needs
to be computed separately, and that is used to determine which
code actions get commands. We also make helper functions to map
analyses to their commands.

I figured out a neater way to test suggested fixes in this CL, so I
reversed the move to source_test back to lsp_test (which was the right
place all along).

Change-Id: I505bf4790481d887edda8b82897e541ec73fb427
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242366
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 23:54:27 +00:00
Rob Findley
0e1f6f5c09 internal/lsp/regtest: fix WithoutWorkspaceFolders option
This option was previous a no-op.

Change-Id: Id28f6802c8f5b020fb3ae71f5b941c0517ca1725
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244548
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-23 23:38:05 +00:00
Rob Findley
93b64502c3 internal/lsp/regtest: add a simple stress test
Using the new run options to configure a testing environment suitable
for long-running tests, a trivial stress test is added to type
arbitrarily in a known problematic repository
(github.com/pilosa/pilosa).

Change-Id: I2c8237843111f17ff5a096515cb4704c62513ed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244441
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 23:37:47 +00:00
Rob Findley
84d0e3d1cc internal/lsp/regtest: add run options to support stress testing
A bunch of options are added to enable long-running performance-oriented
tests in existing directories. They will be used in a later CL to
implement a simple stress test, as an example of what is possible.

Change-Id: I531b201b415362ea135978238b3d64b903226359
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 23:37:37 +00:00
Rob Findley
a5e28d8dab internal/lsp/fake: move to a struct for configuring the sandbox
The Sandbox constructor was getting out of control, and this allows
binding regtest options directly to the sandbox configuration struct.

Change-Id: I18da4ddf43c86b3b652516c3ddca7726cd35e3b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244439
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 23:36:26 +00:00
Rob Findley
1ff154e68b internal/lsp: lift env out of the sandbox into the editor
This removes some unncessary mangling of strings, and allows the editor
to fully own its configuration.

Change-Id: I0fde206824964124182c9138ee06fb670461d486
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244360
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 21:58:45 +00:00
Rebecca Stambler
d2c1b4b270 internal/lsp: show errors in indirect dependencies as diagnostics
This change is a follow-up from CL 242477. We now also surface errors
in indirect dependencies as diagnostics in the go.mod file. Any errors
we cannot match are surfaced as diagnostics on the entire go.mod file.
This isn't the most user-friendly way, but it seems simplest.

Change-Id: Ife92c68c09b74a0b53de29d95b6c4c2a9c78d3b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 21:53:52 +00:00
Rebecca Stambler
0cdb17d11b internal/lsp: treat the module root as the workspace root, if available
This change expands the scope of a workspace to the whole module, if the
user is in module mode. This means that diagnostics will appear and will
be updated for the whole module, even if the user only opens a
subdirectory. Similarly, references and other such queries will always
return consistent results, no matter which directory the user opens.

A new "root" field is added to the view. This is either the view's
folder or its module root. Almost all uses of view.folder have been
changed to view.root.

Updates golang/go#32394

Change-Id: I46f401f7c44b1b8429505aa032e0c15e88c4e2ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 21:33:49 +00:00
Rebecca Stambler
b3d141ddef internal/lsp/cache: copy *packages.Config environment completely
I noticed a race in these logs:
https://build.golang.org/log/ec2915e97319de219284ed022338f2ebc549aff6.

We need to copy the environment and build flags for each config, since
we're treating the config as single use.

Change-Id: I9e717e688def088cb60f2b23b71d731e2b20b259
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244118
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-23 19:57:34 +00:00
Rebecca Stambler
80cb79702c internal/lsp: don't publish duplicate diagnostics in one report
CL 242579 changed the mechanism of reporting diagnostics so that we now
combine reports from multiple sources. Previously, we overwrote existing
diagnostics if new ones were reported. This was fine because
source.Diagnostics generated all of the diagnostics for Go files, and
mod.Diagnostics generated all of the diagnostics for go.mod files.

Now, we combine diagnostics from both sources -- mod.Diagnostics can
generate reports for Go files. We may get duplicate reports for packages
with test variants, so now, we check for those and dedupe.

Change-Id: I42e98079b4eead380058dd029a3a0c72a1796ebb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 19:34:40 +00:00
Noah Goldman
188b38280e go/packages: add failing test for golang/go#37098
When SWIG is used in a package, the returned "(*package.Package)" from
"packages.Load" contains C++ sources in "CompiledGoFiles".  This adds a
skipped failing test case that reproduces this issue.

Updates golang/go#37098.

Change-Id: I56a875c6ad9d18508413b00c34d6b6ac0b116fda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-07-23 16:59:20 +00:00
Rebecca Stambler
a7c6fd066f go/packages: find filenames in error strings if not in position
In the case of an invalid package name (the package name is a keyword),
`go list` doesn't report any filenames associated with the package.
As we have done previously, we can parse these out of the error message.
However, it seems like, in this case, the filename is in the error
string itself, not the error position.

Updates golang/go#39986
Updates golang/go#39763

Change-Id: Id9c68a93ac4f545e4e2152540ca85b92f1df4640
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244028
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-07-23 00:09:07 +00:00
Brayden Cloud
bd1e9de8d8 internal/lsp: separate test and benchmark codelens
This CL adds benchmarking by using the "-bench" flag for the test
command.

Change-Id: Idf714de1c6c3350d26a6874e7b4278927d0336fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243159
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-22 18:17:40 +00:00
KimMachineGun
7041913083 go/analysis/passes/unmarshal: Add check for asn1.Unmarshal
Unmarshal package checks whether `Unmarshal` and(or) `Decode` call of `encoding/json`, `encoding/xml` and `encoding/gob` package is valid or not. However, it didn't check `encoding/asn1` package's one.

This change makes it check whether `asn1.Unmarshal` call is valid or not as well.

Change-Id: If32d243f3a82ebb5ca3ca80657f4616e5daced2f
GitHub-Last-Rev: e61a96941ee64d7d141e635b374fe07104751283
GitHub-Pull-Request: golang/tools#243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243397
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-07-22 15:42:47 +00:00
Rebecca Stambler
6123e77877 internal/lsp: support go mod tidy on save without diagnostics
This change adds support for `go mod tidy` on save when users opt into
import organization on save. Previously, we supported this with the
go mod tidy command, but there's no need to do this when we already
have a ModTidyHandle available.

Change-Id: Ibb47b6a7611fd823dac2a3b4f3a43b9fbb3289b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 22:32:18 +00:00
Rebecca Stambler
b42efcd11c internal/lsp: try to parse diagnostics out of go list errors
This change attempts to parse diagnostics out of `go list` error
messages so that we can present them in a better way to the user. This
approach is definitely tailored to the unknown revision error described
in golang/go#38232, but we can modify it to handle other cases as well.

Fixes golang/go#38232

Change-Id: I0b0a8c39a189a127dc36894a25614535c804a3f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 22:03:47 +00:00
Rebecca Stambler
0a5cd10191 internal/lsp: handle unknown revision in go.mod file
This change ensures that, when the initial workspace load fails, we
re-run it if the go.mod file changes. Previously, if a user opened a
workspace with a corrupt go.mod file, we never recovered.

To reinitialize the workspace on-demand, we use the initializeOnce field
as an indicator of whether or not we should reinitialize. Every call to
awaitInitialized (which is called by all functions that need the IWL),
passes through the initialization code. If a retry isn't necessary,
this is a no-op, but if it is, we will call the initialization logic.
Only the first attempt uses a detached context; subsequent attempts can
be canceled by their contexts.

To indicate that we should reinitialize, we call maybeReinitialize.
Right now, we only call this when the go.mod file changes. In the
future, we may need it in other cases.

Fixes golang/go#38232

Change-Id: I77eefebb0ac38fbd0fe2c7da09c864eba45b075f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242159
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 21:40:18 +00:00
Rebecca Stambler
acdb8c158a internal/lsp: handle on-disk file deletions for opened files
Previously, we only updated the opened file's overlay, but not the
snapshot. This meant that the snapshot was still operating with stale
data. Invalidating the snapshot creates a new snapshot with the correct
set of overlays.

The test is skipped because it will flake until we have a better caching
strategy for `go mod tidy` results.

Updates golang/go#40269

Change-Id: Ia8d1ae75127a1d18d8877923e7a5b25b7bd965ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 21:30:00 +00:00
Rebecca Stambler
5ea363182e internal/lsp: change the way that we pass arguments to command
Our approach to commands and their arguments has been ad-hoc until this
point. This CL creates a standard way of defining and passing the
arguments to different commands. The arguments to a command are now
json.RawMessages, so that we don't have to double encode. This also
allows us to check the expected number of arguments without defining
a struct for every command.

Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-21 16:30:27 +00:00
Pontus Leitzler
b8e13e1a4d internal/lsp: return the actual range from convenience code actions
Convenience code actions operates at line-by-line, but a line can
contain more than one fix.

To be able to tell them apart each response now return the actual
range as reported by the analyzer.

Fixes golang/go#40328

Change-Id: I9ffe4737653d2a5a6dafc3574e74975a71c71937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243877
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-21 15:44:06 +00:00