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

748 Commits

Author SHA1 Message Date
Heschi Kreinick
4fd1c64487 internal/lsp/cache: fix ignored file check
The return of IgnoreFile. We continue using go list's ignore rules,
but only apply them to the part of the path underneath the source root.

This should be fixed to include replace targets once we have them.

Change-Id: I054fee8c12fc860a279b0d0c1fd670f44d78a63f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239288
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-06-22 19:29:24 +00:00
Pontus Leitzler
037be6a065 internal/lsp/source: support highlight of switch statements
Placing the cursor on a switch statement or corresponding break and call
document.Highlight will now highlight them.

Fixes golang/go#39275

Change-Id: Ib7e3ba0c6e78141ed3dd37cfd3b72b567b857247
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238478
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-19 02:36:21 +00:00
Rebecca Stambler
c7475b9d7f internal/lsp/source: refactor highlighting code
The current implementation has a number of helper functions that have to
thread through context, errors, converters, etc. Simplify the code by
factoring out the conversion to a protocol range into a final step.
There are probably further improvements that can be made too.

As part of debugging, I noticed that the cmdtest server does not get
shutdown once tests finish, which leads to excessive logging. Is closing
the test server connection enough to trigger shutdowns? It doesn't seem
to be.

Change-Id: Ia67666f6b8debccd120795bb24bd089620c0a87b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238377
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-18 15:59:44 +00:00
Rob Findley
20370b0cb4 internal/lsp: honor GOPRIVATE in documentLinks and go.mod hovers
Several fixes related to GOPRIVATE handling and links:
 + In Go source, fix links matching GOPRIVATE for external modules.
   Previously, in these cases we'd try to match <mod>@v1.2.3/<suffix>,
   which wasn't the correct input into the GOPRIVATE matching algorithm.
 + Similarly check GOPRIVATE for go.mod require statement hovers.
 + Likewise, for documentLink requests (both mod and source).
 + Move the existing hover regtest to link_test.go, and expand to cover
   all these cases.

Along the way, I encountered a couple apparent bugs, which I fixed:
 + Correctly handle the case where there is only one require in a go.mod
   file. This was exercised by the regtest, so took some debugging.
 + Only format links [like](this) if the requested format is actually
   markdown.

Fixes golang/go#36998

Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-18 13:42:42 +00:00
Rebecca Stambler
47c907e258 internal/lsp: use a new temporary go.mod for every go list call
Refactor internal/lsp/cache to use a new temporary go.mod file for each
go command invocation. This cleans up the abstraction in the source
package, as we no longer are aware of temporary go.mod files.

This will also fix the raciness of reusing the same temporary go.mod
file for each invocation.

Updates golang/go#37318.
Fixes golang/go#39504.

Change-Id: I90bc17a678b5df222ab598c8f7dbf6c6fdd393f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-18 02:37:23 +00:00
Pontus Leitzler
dc31b401ab internal/lsp/source: avoid panic in rename check
In a case where the type info for an ast.CompositeLit isn't found in
forEachLexicalRef(...) gopls will panic.

e.g. trying to rename "foo" where it is declared in this case:
func fn() {
	var foo bool
	make(map[string]bool
	if foo {
	}
}

Note the missing ')' after make.

This change will skip ast.CompositeLits if the type can't be found, and
that fixes the panic. But it also do rename the identifier as long as it
is possible, and I'm not convinced that we should allow rename at all if
the source can't be compiled.

Updates golang/go#39614

Change-Id: Ibb50b15ce4b31056f2f1da52a4dcab7b8b91a320
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238042
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-06-16 19:50:46 +00:00
Rebecca Stambler
6aa8f57aac internal/lsp: un-export (*snapshot).Config to limit it to cache
Limiting the Config to the view seems reasonable, considering that it is
only used to run the `go` command. I prefer just having the cache run
go commands, so that source doesn't have to deal with the environment.

This also enables CL 237517.

Change-Id: I639c082592de30e9682dc25cdd12c7751ddb4f97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-15 22:28:25 +00:00
Rebecca Stambler
e31b568ad1 internal/lsp: plumb fillstruct through analysis
Now that fillstruct is an analyzer, we can simplify the code that calls
it in code_action.go. We introduce a new class of analyzer --
convenience analyzers, which are closer to commands. These represent
suggestions that won't necessarily improve the quality or correctness of
your code, but they offer small helper functions for the user.

This CL also combines the refactor rewrite tests with the suggested fix
tests, since they are effectively the same.

For now, we only support convenience analyzers when a code action was
requested on the same line as the fix. I'm not sure how to otherwise
handle this without bothering the user with unnecessary diagnostics.

Change-Id: I7f0aa198b5ee9964a907d709bae6380093d4ef21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237687
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-15 21:22:08 +00:00
Heschi Kreinick
1725ffee6d internal/lsp/cache: hide diagnostics for ignored dirs
The go command ignores go files in specially-named directories like
testdata. We probably still want to try to offer services like
autocomplete and jump-to-definition there, but diagnostics are less
likely to be helpful. As a compromise, just squash diagnostics.

Note that the rules we use are slightly wrong; see the comment on
ignoreFiles for details.

Fixes golang/go#39563.

Change-Id: I0bc00639e68bd71ea55d867af36e07ef4ec780a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237638
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-15 17:27:23 +00:00
Josh Baum
a1e2396bbd internal/lsp: port fill struct into analysis framework
The current implementation of the fill struct tool is not a part of
the analysis framework. This commit moves the functionality from the
source directory to the analysis directory.

Change-Id: Ibe37b57f3e6680c8729932dbbe010a4642600e4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237258
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-12 20:33:59 +00:00
Rebecca Stambler
3c1b287bbd internal/lsp: await the initial workspace load in ModHandle
ModHandle races with the initial workspace load if the go.mod file does
not yet exist. We should await for the initial workspace load to
complete before proceeding with update codelenses, etc.

Part of trying to figure out the flakes in golang/go#39504.
Also a few staticcheck fixes, and fix the Windows line endings in
fill_struct.go, because `git gofmt` complains.

Change-Id: Ide21a47137390792d1afb924740cff0bb6f0b764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-12 18:21:55 +00:00
Rebecca Stambler
9001f16f58 internal/lsp: cancel the initial workspace load when view shuts down
The error messages from view cancellation clutter up the logs when
testing, especially if you're running a single subtest.

A few quick staticcheck fixes in the CL also.

Change-Id: Ia1ed5360ac754023c589ed526ec0ed3e94a79b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-12 18:06:40 +00:00
Heschi Kreinick
f520afa52e internal/lsp: remove Ignore feature
Ignore ignored the builtin package and files that start with _. The
latter should already be ignored by "go list". The former seems
like too much effort to me. People shouldn't edit random parts of the
stdlib, and ignoring changes to (e.g.) the Error interface seems like
the least of the trouble they can get themselves into.

Remove it for now. If we get complains I'll re-add it, probably by
rejecting the write entirely somewhere.

We incidentally relied on this in the identifier functions; change those
to treat the builtin package slightly more specially.

Change-Id: I005b02a66b1a987c50a3074d53a2d28ff07d3324
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-11 22:55:14 +00:00
Heschi Kreinick
ecd3fc4348 internal/lsp: read files eagerly
We use file identities pervasively throughout gopls. Prior to this
change, the identity is the modification date of an unopened file, or
the hash of an opened file. That means that opening a file changes its
identity, which causes unnecessary churn in the cache.

Unfortunately, there isn't an easy way to fix this. Changing the
cache key to something else, such as the modification time, means that
we won't unify cache entries if a change is made and then undone. The
approach here is to read files eagerly in GetFile, so that we know their
hashes immediately. That resolves the churn, but means that we do a ton
of file IO at startup.

Incidental changes:

Remove the FileSystem interface; there was only one implementation and
it added a fair amount of cruft. We have many other places that assume
os.Stat and such work.

Add direct accessors to FileHandle for URI, Kind, and Version. Most uses
of (FileHandle).Identity were for stuff that we derive solely from the
URI, and this helped me disentangle them. It is a *ton* of churn,
though. I can revert it if you want.

Change-Id: Ia2133bc527f71daf81c9d674951726a232ca5bc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237037
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-11 22:11:59 +00:00
Heschi Kreinick
1fdcbd1300 internal/lsp/source: format fill_struct.go
Change-Id: Iaf238dfba6f51aed0a15f6ef2b231e1a1816f683
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237421
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-11 03:21:20 +00:00
Pontus Leitzler
8d7dbee4c8 internal/lsp/source: support labeled statements when highlighting loops
When document.Highlight is called with the cursor on a loop statement or
branch statement, gopls doesn't look for labels. Placing the cursor at
the break statement below highlights the inner for loop:

Outer:
for {
    for {
        break Outer
    }
}

By making highlight label aware, and ensure that unlabeled "break" in
"switch"/"select" doesn't highlight the outer loop, this change fixes
loop highlighting.

Adding support for highlight of "switch" and "select" will be handled in
a separate CL.

Updates golang/go#39275

Change-Id: I7014aa7b0dfb1da871863ced611623be995f3944
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236524
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-10 05:20:24 +00:00
Ian Cottrell
ce53dc4445 internal/lsp: clean out the debug handling
This removes all the cache/session/view hooks from the lsp and instead
uses normal introspection from the client to find all the entities
rather than keeping shadow copies of the object graph in the debug page
handler.
This required the addition only of the ability to view the sessions open
on a cache and exposing the unique identifier for all the objects, both
of which are useful and reasonable things to have in the API anyway.

Change-Id: Ic19903e7b19832ca79290954ec373d4177789742
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236197
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:14 +00:00
Rebecca Stambler
4d5ea46c79 internal/lsp: support go mod vendor as a command
In addition to adding a `go mod vendor` command option, which can be
exposed via an editor client frontend, we show a suggestion to users who
experience the "inconsistent vendoring" error message.

The main change made here is that we save the view initialization error,
and we return it if the view has absolutely no metadata. This seems
reasonable enough, but my fear is that it may lead to us showing
outdated error messages. I will spend some time improving the handling
of initialization errors in follow-up CLs.

Updates golang/go#39100

Change-Id: Iba21fb3fbfa4bca956fdf63736b397c47fc7ae44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235619
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-04 18:33:45 +00:00
Josh Baum
dcff9671f6 internal/lsp: support code folding for composite literals
The existing implementation does not allow users to collapse composite literals.

Fixes golang/go#38360

Change-Id: If84de6b2c59218bd62fecb81efae2126547def1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236377
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-04 17:49:48 +00:00
Pei Xian Chee
9b20fe4cab internal/lsp: added a fill struct code action
This code action generates key-value pairs of fields and default values between a struct's enclosing braces.

Fixes #37576

Change-Id: Ia0555164d2164c2bc90bb9ecabbb55042cdd3846
GitHub-Last-Rev: 0e4db3effad7212b6c2b226079fa8fc5464eb0b9
GitHub-Pull-Request: golang/tools#220
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227437
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-04 04:23:27 +00:00
Pontus Leitzler
1c0c6cf43e x/tools/internal/lsp/source: avoid panic in failing Highlight test
When testing Highlight the highlight count is checked against expected
number of highlights. If it doesn't match t.Errorf(...) is called and
the test continues.

A few lines below the test ranges over results using the index for both
result and expected result leading to a panic if there are less then
expected highlights.

This change fails fast with t.Fatalf(...) instead to avoid the panic.

Change-Id: I33d0973f3145c307d9084d037ffbb73b244a3acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236099
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-03 17:03:22 +00:00
Rebecca Stambler
a64b766573 internal/lsp: fix a few staticcheck issues
Change-Id: I328d68d2250bc1cc975d8d9d14984c8f03fcd0e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235618
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-29 17:23:31 +00:00
Heschi Kreinick
7527cb292c internal/lsp/source: sort cached package completions by relevance
Cached packages are probably more relevant than uncached packages, but
we still need to go in relevance order, since we'll stop adding results
after we hit the cap.

Fixes golang/go#38461. (Hopefully.)

Change-Id: I555dd5f7568baa8d69760ed5836341a474e94346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231619
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-27 17:50:47 +00:00
Heschi Kreinick
6a7cf6184f internal/lsp/source: fix cached package name matching
I made a silly mistake and checked the prefix on the import path rather
than the package name, which obviously breaks everything other than
top-level stdlib packages.

Fix that, then tweak the ranking a bit. We now get deep completions, which
is nice, but filled up the results too fast. Now instead of 5 results of
any kind, we give up after 5 packages searched.

Change-Id: I15b293f68f17531077da9ffe791a38ccc0e129f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231617
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-27 17:50:42 +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
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
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
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
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
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
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
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
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
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
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
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
Ian Cottrell
535e1470ec internal/lsp: use %w in error wrappers
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.

Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 14:52:14 +00:00
Clint J Edwards
6b6965ac5d internal/lsp: add comment completions for remaining exported symbols
This should provide simple name completions for comments
above exported vars, constants, functions, and types.

Can be activated with `ctrl+space` within a comment.

Also fixes a panic introduced in the previous commit when completing comments that occur at the end of a file.

Fixes #34010
Fixes #38793

Demo: https://i.imgur.com/qN82CVA.mp4

Change-Id: If9aaec7ce03a3e085361144bce5c7a66535127d1
GitHub-Last-Rev: b9ac874c7ff3c9a164ba698d0d561141a59e2435
GitHub-Pull-Request: golang/tools#224
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230215
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 02:29:51 +00:00
Rebecca Stambler
ab2804fb9c internal/lsp: don't offer suggested fixes for generated files
As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.

Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.

Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-01 06:56:59 +00:00
Muir Manders
f26c0dd982 internal/lsp/source: improve unimported package member ranking
Untyped members from unimported packages are scored the same as typed
members from unimported packages. We depended on the unimported
package relevance to rank the probably-more-relevant typed members
higher. However, there are some unrelated score penalties that can
only be applied to typed candidates, so the untyped candidates ended
up being ranked higher. Fix by increasing the relevance coefficient so
the relevance score overpowers other less important scoring
adjustments.

Fixes golang/go#38104.

Change-Id: Ie43f769a41511f9cc3747ce6936130be7a29cd31
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231238
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-30 22:11:53 +00:00
Rebecca Stambler
2840dafb9e Revert "internal/lsp: hide analysis diagnostics from generated files"
This reverts commit e4881b2459.

Reason for revert: <CL 230999 has a different approach to this>

Change-Id: I9ec47d858e7db2a66ec8a93063ab950b8553e45b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231042
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-30 19:28:56 +00:00
Rebecca Stambler
b8428586f4 internal/lsp: handle hover documentation for package declarations
We had previously not been generating documentation on hover for
package declarations or import specs. We do this by adding a few special
cases, since package declarations don't appear in type information.

Throughout, we make the assumption that only one file in a package will
have the documentation for the package. go/doc just appends
documentation as it sees it. We may be able to do better by checking for
a "Package ..." but that still is not guaranteed. Not sure what the
right approach is, so this assumption may be the best option.

Fixes golang/go#38526

Change-Id: Ibc515f8729e1aba0d11090be62371be6b18d0cfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-29 20:57:41 +00:00
Rebecca Stambler
e4881b2459 internal/lsp: hide analysis diagnostics from generated files
Don't show non-vet analyses when they appear in generated files. Vet
analyzers will give useful reports even in generated files.

Fixes golang/go#38467

Change-Id: I0e628760b386553932de4cf1f5ba39784a205b53
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-29 20:34:34 +00:00
Rebecca Stambler
352a5409fa internal/lsp: handle different package names in signature help
There were a few cases where we were not properly qualifying package
names, particularly if the original package had a named import. Now,
we map between these names correctly - handling the case of multiple
packages that need to be qualified. This requires applying edits to
*ast.SelectorExprs, as well as *ast.Idents.

We still do not fully qualify unimported packages, and likely won't,
unless that's an issue for many users.

Updates golang/go#38591

Change-Id: I966a4d1f936f37ede89362d03da3ff98d8952a06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229979
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-27 20:59:12 +00:00
Rebecca Stambler
e0d5eebdf8 internal/lsp: fix docs on hover for ungrouped package variables
Fixes golang/go#38525

Change-Id: I8833c925663b67b2c82ad4cbf580d1c6f3c7a81d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230305
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-27 18:59:06 +00:00
Muir Manders
8463f397d0 internal/lsp/source: fix false positive "..." in completions
In cases like:

    var v interface{}
    fmt.Println(<>)

Completing to "v" would insert "v..." instead of "v". This was due to
a mixup where we were checking if the variadic type "[]interface{}"
was assignable to the candidate type "interface{}" instead of the
other way around.

Fixes golang/go#38652.

Change-Id: I27c0b50bbf4b895924c8ed2c0c9dd6785e98cbe1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229921
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-25 04:34:58 +00:00
Rob Findley
3585060312 internal/lsp/source: add option for verbose work progress reporting
In order to experiment with adding more progress reporting to gopls, add
a new experimental configuration for verbose work done reporting.

Also, pass configuration in InitializationOptions when initializing the
editor.

Change-Id: Id2336790719d9c0b6d663997e1aef672aec43d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229458
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-24 19:57:22 +00:00
Rebecca Stambler
59e73619c7 internal/lsp: correctly handle type aliases when formatting
This change improves our approach to handling type aliases. Previously,
we were not fully qualifying the names in the AST, making the code
inserted in completions incorrect at times. Now, we clone the relevant
AST expr and qualify it. We also add handling for the return values of a
function, instead of just the parameters.

Fixes golang/go#38230
Fixes golang/go#37283

Change-Id: Ib79f4636891c9b610ae848e9fa4dbae7c63db509
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-23 20:53:58 +00:00
Ian Cottrell
c81623a0cb internal/event: move event/core.Tag to event/label.Label
Also moves core.Key to label.Key, but leaves the implementations
behind for now.
After using for a while, the word Tag conveys slightly the wrong
concept, tagging implies the entire set of information, label maps
better to a single named piece of information.
A label is just a named key/value pair, it is not really tied to the
event package, separating it makes it much easier to understand the
public symbols of the event and core packages, and allows us to also
move the key implementations somewhere else, which otherwise dominate
the API.

Change-Id: I46275d531cec91e28af6ab1e74a2713505d52533
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229239
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23 18:13:33 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start

In order to support this core now exposes the MakeEvent and Export functions.

Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.

Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:20:48 +00:00
Rebecca Stambler
72e4a01eba internal/lsp: refactor code for formatting signatures
The code for formatting function signatures is fairly confusing.
Factoring out an unexported signature type simplifies things a bit.
Hopefully we'll be able to pull out more formatting logic from the other
features. Ideally, I'd like to return to the separation between
internal/lsp/source and internal/lsp so that a formatting package can be
pulled out and used in internal/lsp.

Change-Id: I7428db5004eab371e46402188e0dc6bb30f0c425
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229318
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-22 20:52:58 +00:00
Rebecca Stambler
c07e33ef32 internal/lsp/source: enable type error analyzers
I had intended to enable these after gopls/v0.4.0 was released so that
people who test at master can try these out. Also, mark "fillreturns" as
high confidence so users can get it applied on save, much like
goreturns.

Change-Id: I71aa0b723b2e9b69474ceed7cb1d7da7c929d65d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228724
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-17 14:00:56 +00:00
Rebecca Stambler
5744cfde56 internal/lsp: fix imports issue with duplicate package decl
We shouldn't apply formatting fixes when we are
organizing imports.

Fixes golang/go#38412

Change-Id: I082f4d9a7d1d1dd571304733c287b0d5e90ea448
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228264
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-16 06:17:24 +00:00
Rebecca Stambler
5d8e1897c7 internal/lsp: fix erroneous documentation for struct fields
The fix is just a missing return.
Also clean up a staticcheck thing, regenerate the golden files.

Fixes golang/go#38417

Change-Id: I290b63df9d97211c59d6399fda7a273bc700fbdb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228297
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-15 03:45:06 +00:00
Rebecca Stambler
79a7a3126e internal/lsp: add an extra bounds check to avoid nil pointers
A nil pointer was reported to the golang-tools group (see
https://groups.google.com/g/golang-tools/c/JrNTz8I6ifo/m/tcJRpek-AAAJ).
I think this bounds check should address it.

Updates golang/go#34433

Change-Id: I87352c269c65c844c86ebe9ee3fd2d041cc49ee9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227770
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10 19:49:07 +00:00
Rebecca Stambler
4d14fc9c00 internal/lsp: add type error fixes to existing diagnostics
This change is the first step in handling golang/go#38136. Instead of
creating multiple diagnostic reports for type error analyzers, we add
suggested fixes to the existing reports. To match the analyzers for
FindAnalysisError, we add an ErrorMatch function to source.Analyzer.

This is not an ideal solution, but it was the best one I could come up
with without modifying the go/analysis API. analysisinternal could be
used for this purpose, but it seemed to complicated to be worth it, and
this is fairly simple. I think that go/analysis itself might need to be
extended for type error analyzers, but these temporary measures will
help us understand the kinds of features we need for type error
analyzers.

A follow-up CL might be to not add reports for type error analyzers
until the end of source.Diagnostic, which would remove the need for the
look-up.

Fixes golang/go#38136

Change-Id: I25bc6396b09d49facecd918bf5591d2d5bdf1b3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-08 01:45:16 +00:00
Rebecca Stambler
a3568bac92 internal/lsp: disable unimported completions for snippet tests
Turns out that these were already disabled in the
golang.org/x/tools/internal/lsp tests, which is why we only ever save
the failures in golang.org/x/tools/internal/lsp/source tests. Discussing
the reasons that this is necessary on the issue, but I think completion
scoring in general needs to be centralized and documented, so hopefully
we can handle this case as part of that larger fix.

Fixes golang/go#38269

Change-Id: Ie1b615315e417ab8211a3580cf8f27cbdc3b74e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-07 14:37:52 +00:00
Rebecca Stambler
ee2abff5cf internal/lsp: use one context throughout completion
I originally made this change to see if it would help with the timeouts.
Based on the TryBot results, it doesn't -- but I still think it's more
correct to have the contexts this way. It was my mistake to put the
context on the completer in the first place, I think.

Change-Id: Ib77c8f0ac0b0d0922b82db4120820fb96cb664f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227303
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-06 16:51:37 +00:00
Rebecca Stambler
44a64ad78b internal/lsp, go/packages: don't log context cancellation errors
Instead of checking the context, check the error. This may expose some
errors that are not wrapped correctly. Replaced all uses of errors
with golang.org/x/xerrors.

Change-Id: Ia40160f8ea352e02618765f2a9415a4ece0dcd94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227036
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-03 19:08:13 +00:00
Rebecca Stambler
bcf690261a internal/lsp: use context with timeout in all RunProcessEnvFuncs
The modifications in CL 226559 introduced a new call to
RunProcessEnvFunc that was not using the context with a timeout, so deep
completion tests were hitting the timeout early (I think?).

Also, fix a staticcheck warning that's been bugging me today.

Change-Id: Iac894f630ebfa1cbc5588ae3a5490a93fa53aba2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227057
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-02 22:33:21 +00:00
Rebecca Stambler
226fa68e9d internal/lsp: fix references for transitive dependencies
We need to search all transitive dependencies of a package, not just its
immediate imports.

Fixes golang/go#38100

Change-Id: I15b4dbe226ba851691ca0c95460c3648ede32f04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227030
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-04-02 20:53:30 +00:00
Rebecca Stambler
9fc00b0a7f internal/lsp: fix panic in builtin completions
Fixes golang/go#38091

Change-Id: I88ac6d3413de3dd9d235e2f2fca9b4a3f7127e0a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227026
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-02 17:53:26 +00:00
Heschi Kreinick
0a46fa39b0 internal/lsp/source: propose loaded unimported package names first
When proposing unimported package members, we start with packages loaded
in-memory first, for three reasons: we know that they're fast, they're
fully typed, and they're probably pretty relevant. I hadn't bothered
doing that for package names, because the first two reasons aren't very
relevant. But the third still is -- loaded packages are a pretty good
approximation for in module scope, etc.

With this change we do package names the same as members, so relevance
should be about as good. Not perfect, but nobody's complained much yet.

Fair bit of copy-and-paste, but I don't want to extend the
getAllCandidates abstraction outside of the imports package yet.

Updates #38104.

Change-Id: Ia479181607dff898baee3cd6aa84d1ab61715d19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226559
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-02 17:48:25 +00:00
Rebecca Stambler
3304cfb00f internal/lsp: temporarily disable type error analyzers by default
If we release gopls/v0.4.0 soon, we should keep these new analyzers off
by default. They were just merged, so they haven't been used enough to
be enabled, I think. We'll turn them on by default for gopls/v0.5.0.

Also, ended up creating a helper function to check if analysis has been
abled (which fixed a small bug in FindAnalysisError), and another helper
function to enable all analyses for testing purposes.

Updates golang/go#38212

Change-Id: I5ee94b3582dfc0863978650fc6ce51bfa0606c13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226962
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-04-02 16:54:32 +00:00
Rebecca Stambler
69646383af internal/lsp/source: add a replacement for experimentalDisabledAnalyses
Change-Id: I5bff1db1d937ffdcb8bb4e36f05073238fbdeb5f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226961
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-04-02 14:48:50 +00:00
Rohan Challa
099440627f internal/lsp: add goreturns like functionality as quickfix
This change ports the functionality of https://github.com/sqs/goreturns
to be used as code actions on diagnostics that have missing
return values. It improves on the original goreturns functionality by:

- filling out empty return statements
- trying to match existing return values to the required return
  values and then filling in missing parameters

Fixes golang/go#37091

Change-Id: Ifaf9bf571c3bc3c61e672b0a2f725d8d734d432d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224960
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-01 19:27:44 +00:00
Rebecca Stambler
9d5940d493 internal/lsp: refactor and propagate errors from code actions
This change makes sure that all errors in code actions are returned,
instead of ignored. There is also a refactoring of the suggested fixes
for go.mod files so that we don't compute them unless there is already a
matching diagnostic.

Change-Id: I34afda0116f3cc7e47809d980a0171487787e55e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221221
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-31 20:20:46 +00:00
Rebecca Stambler
a7c0594f4e internal/lsp: avoid logging context cancellation
This change adds a helper function that checks if the context is
canceled, and if so, doesn't log the error. Tried to use it everywhere
in internal/lsp where it fits, which resulted in changing a few pieces
of error handling throughout.

Updates golang/go#37875

Change-Id: I59cbc6f893e3b70cf84524d9944ff7f4b4febd78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226371
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-31 01:46:33 +00:00
Rohan Challa
31583a0dbb internal/lsp/tests: remove ellipses from test names
This change makes working with tests easier by trimming the folder name
up to "testdata". This will remove any ellipses from the test folder name.

Fixes golang/go#38103

Change-Id: I33b931e527de63713b8fc370c50b1c382796b2b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226377
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-30 17:55:17 +00:00
Rohan Challa
eabff7e044 internal/lsp/analysis: add quickfix for "no new vars on left side"
This change adds a quick fix for type errors of the type "no new vars on left side of :=". It will replace the ":=" with an "=".

Updates golang/go#34644

Change-Id: I91af8eb82956104229c3b4f3d0fce60fdfdbb5ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 19:15:53 +00:00
Rohan Challa
f4fcf867e7 internal/lsp/analysis: add quickfix for "no result values expected"
This change adds a quick fix for type errors of the type "no result values expected". It will replace the return statment with an empty return statement.

Updates golang/go#34644

Change-Id: I3885748dfc69a2d19f8e7a2e81f36f6d0a20d25b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223666
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:57:59 +00:00
Rohan Challa
afab6edfad internal/lsp/source: remove unused parameters from functions
This change uses the new unusedparams analyzer to remove any unused parameters from functions inside of internal/lsp/source :)

Change-Id: I220100e832971b07cd80a701cd8b293fe708af3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225997
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:57:18 +00:00
Rohan Challa
8f81e2e6d4 internal/lsp/analysis: add quickfix for undeclared variable
This change adds a quick fix for diagnostics that have an error message of the form "undeclared name: %s". It provides a quick fix to add a new variable with that name.

Updates golang/go#34644

Change-Id: I6534ee79d1770d1a62bac169c3c7e52e2443f39e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222237
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:31:06 +00:00
Rohan Challa
42235f6384 internal/lsp: add support for type error analyzers
This change adds support within gopls for analyzers that work with type errors to provide suggested fixes.

Updates golang/go#34644

Change-Id: Ia8929173752fda6bd84a9edaabd310e758f25fe8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222761
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 17:27:16 +00:00
Rebecca Stambler
eca45d481d internal/lsp: refactor references/rename/implementations
As part of investigating golang/go#38100, I noticed a few things that I
wanted to clean up. Mostly, for renames, we were calling
qualifiedObjAtProtocolPos twice, so I factored out a shared helper
function. I also added an error return for builtins so that callers
don't have to check.

Change-Id: I28c75c801cbec1611736af931cfa72befd219201
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-27 17:09:18 +00:00
Rohan Challa
5d86d385bf internal/lsp/analysis: add simplify-slice pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

A slice expression of the form:
	s[a:len(s)]
will be simplified to:
	s[a:]

Updates golang/go#37221

Change-Id: Ibd4dedaadc9b129d5d39524f0c1ccc8a95bf7e0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223659
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-03-26 21:04:57 +00:00
Rohan Challa
e428a8eca3 internal/lsp/analysis: add simplify-composite-lit pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

An array, slice, or map composite literal of the form:
	[]T{T{}, T{}}
will be simplified to:
	[]T{{}, {}}

Updates golang/go#37221

Change-Id: I2dca46501983c8af3581c9319d973da5cf690388
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223660
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-03-26 20:50:12 +00:00
Rohan Challa
88b9c284fd internal/lsp/analysis: add pass for unused parameters
This change adds a pass that checks for unused parameters inside of a function. It is disabled by default.

Updates golang/go#36602

Change-Id: I9e8de3368f16f27e7816ec4ddb16935e1a05584e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222817
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:36:28 +00:00
Rohan Challa
94fe02cb5c internal/lsp/analysis: add simplify-range pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

A range of the form:
	for x, _ = range v {...}
will be simplified to:
	for x = range v {...}

A range of the form:
	for _ = range v {...}
will be simplified to:
	for range v {...}

Updates golang/go#37221

Change-Id: Ic6babbd0b8ab961ebb4d0d6df72df52d9acde6e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223661
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:18:45 +00:00
Rohan Challa
e46a7b92c0 internal/lsp: add support for sourceFixAll analyzers
This change adds support for analyzers that have suggested fixes of the kind Source.FixAll. This will allow these fixes to be applied on save if the user desires.

To auto apply these fixes on save, make sure your settings.json looks like:

"[go]": {
	"editor.codeActionsOnSave": {
		...
		"source.fixAll": true,
		...
	},
	...
}

Update golang/go#37221

Change-Id: I534e4f6c8c51ec2848cf2899aab68f587ba68423
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223658
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:01:16 +00:00
Rohan Challa
02a6ca6dc3 internal/lsp: change disabledAnalyses setting to be general
This change removes the disabledAnalyses setting and replaces it with a more general feature named "analyses". This will allow users to opt in as well as opt out of analyzers that they do not find useful. This also updates some documentation to show users what analyzers gopls is using and which are enabled by default.

Change-Id: Id3239b4c4c9667e834f262c889270d14fdba0f93
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223662
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 19:30:34 +00:00
Rebecca Stambler
52ff224b76 internal/lsp: avoid possible nil pointer in references/rename
Noticed this in https://github.com/fatih/vim-go/issues/2786. I don't
think that this will fix the problem in this issue, but we should avoid
nil pointers as much as possible. Also, remove a bit of extra whitespace
so that the style closer matches that of the rest of the project.

Change-Id: I94b924ea14e4a296382e3e68c52eeb43f6cd86a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225523
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-26 18:28:26 +00:00
Heschi Kreinick
e2e2519b1d lsp/source: create zero value ast.Exprs
In support of https://golang.org/cl/224960, add utility functions that
create zero value expressions for a given types.Type. There are probably
some bugs here but they pass the basic tests in that CL.

Known problems: it skips through type aliases, and can't handle
anonymous structs/interfaces. Hopefully it's pretty unusual to return
those.

Change-Id: I69a388d9ce9bd60bc230e4a3d2b30f581ec25698
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225177
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 17:41:01 +00:00
Rebecca Stambler
6fc5d0bc36 internal/lsp: print view-specific environment
For debugging purposes, we print the output of `go env` on start. Now,
we print a view-specific `go env`, which will helps us when debugging
multi-project workspaces. Additional information included: the folder of
the view, if the view has a valid build configuration, and the build
flags for the view.

Updates golang/go#37978

Change-Id: Iadffd46f8ac5410971558a304b8bbcb2f9bdbcc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-24 20:15:47 +00:00
Muir Manders
ef1313dc6d internal/lsp/source: prefer unexported and non-func candidates
A common annoying mis-completion is as follows:

    type foo struct {
      field int
    }

    func (f foo) Field() int { return f.field }

    func (f foo) logic() {
      if f.f<>
    }

Now at <> we prefer "field" over "Field()". Similarly:

    type foo struct {
    }

    func (f foo) DoSomething() { }

    func (f foo) doSomething() { }

    func (f foo) logic() {
      f.d<>
    }

Now at <> we prefer "doSomething()" over "DoSomething()". All else
being equally, you normally want private objects over public objects
when the private objects are available.

The same logic is applied to deep completions so we prefer "c.foo.bar"
over "c.Foo().bar".

Change-Id: Ic91cba7721ddb1f2a30338037693ddcce8c621f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223877
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-03-23 21:07:25 +00:00
Rebecca Stambler
18ea2c8f73 internal/lsp: fix nil pointer in renaming to "error"
Fixes golang/go#37963

Change-Id: I0a46cbcbd250729761b9a8323b942202bb1dbd75
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-23 16:43:54 +00:00
Rob Findley
9a0fabac01 internal/lsp: fix errors found by staticcheck
While experimenting with different static analysis on x/tools, I noticed
that there are many actionable diagnostics found by staticcheck. Fix the
ones that were not false positives.

Change-Id: I0b68cf1f636b57b557db879fad84fff9b7237a89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222248
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-19 19:20:54 +00:00
Ian Cottrell
d7fc2cf50e internal/telemetry: delete the event.TagOf method
It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.

Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:23:15 +00:00
Ian Cottrell
04208b9e8a internal/lsp: move the telemetry package
Move the lsp specific telemetry package that just declares the labels under the
debug package and call it tag, to make all the usages much more readable.

Change-Id: Ic89b3408dd3b8b3d914cc69d81f41b8919aaf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222850
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:49 +00:00
Muir Manders
11a475a590 internal/lsp/source: fix literal completions in variadic args
We now properly offer literal completions when completing the variadic
parameter. For example:

    func foo(...[]int) {}
    foo(<>) // now offers "[]int{}"

Updates golang/go#37656.

Change-Id: I91c47d455ae3f8051078c82a308f2b5ad4b3d4cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222199
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-18 05:47:22 +00:00