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

5121 Commits

Author SHA1 Message Date
Heschi Kreinick
118ac038d7 internal/lsp: improve handling of non-Go folders
CL 244117 introduced a bug when modFile == os.DevNull: v.root is left
uninitialized, resulting in a view that appears to own all files. Fixing
that exposes a problem where opening a folder with no Go files and
GO111MODULE=on shows a popup. Skip the popup when no Go files are found.

Change-Id: I7f8b2d6fd2f954af64c3a65156ff44c649f3a5b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248620
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-17 19:03:02 +00:00
Rebecca Stambler
d00afeaade internal/lsp/source: fix nil pointer in rename_check
Updates golang/vscode-go#534

Change-Id: I0a30ac7f52862a096b07c35665539cfed99d4828
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-17 02:38:11 +00:00
Muir Manders
90abf76919 internal/lsp/source: fix a couple issues completing append() args
In this example:

     p := &[]int{}
     append([]int{}, *<>)

At <> we completed to "**p" instead of "*p...". There were two fixes:

1. builtinArgType() wasn't propagating the "modifiers", so we were
   forgetting about the preceding "*" pointer indirection and
   inserting it again with the completion. Fix by propagating
   modifiers.
2. The candidate formatting responsible for adding "..." had over
   simplified logic to determine if we are completing the variadic
   param. Now instead the candidate evaluation code marks the
   candidate as "variadic" so the formatting doesn't have to think at
   all.

Change-Id: Ib71ee8ecfafb915df331f1d2e55b76f76a530243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-15 16:56:00 +00:00
Muir Manders
6fbe43b70d internal/lsp/source: improve completion in append()
In the following example:

    var foo []someStruct
    foo = append(foo, <>)

we now downrank "foo" as a candidate at "<>". You very rarely append a
slice to itself, so having "foo" ranked highly was counterproductive.

Fixes golang/go#40535.

Change-Id: Ic01366aeded4ba2b6b64bfddd33415499b35a323
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-15 16:31:36 +00:00
Muir Manders
7c0525f229 internal/lsp/source: improve func literal completions
When a signature doesn't name its params, we make up param
names when generating a corresponding func literal:

    var f func(myType) // no param name
    f = fun<> // completes to "func(mt myType) {}"

Previously we would abbreviate named types and fall back to "_" for
builtins or repeated names. We would require placeholders to be
enabled when using "_" so the user could name the param easily. That
left users that don't use placeholders with no completion at all in
this case.

I made the following improvements:
- Generate a name for all params. For builtin types we use the first
  letter, e.g. "i" for "int", "s" for "[]string". If a name is
  repeated, we append incrementing numeric suffixes. For example,
  "func(int, int32)" becomes "func(i1 int, i2 int32").
- No longer require placeholders to be enabled in any case.
- Fix handling of alias types so the param name and type name are
  based on the alias, not the aliasee.

I also tweaked formatVarType to qualify packages using a
types.Qualifier. I needed it to respect a qualifier that doesn't
qualify anything so I could format e.g. "http.Response" as just
"Response" to come up with param names.

Fixes golang/go#38416.

Change-Id: I0ce8a0a4e2485dda41a0aa696d9fd48bea595869
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246262
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-15 16:17:26 +00:00
Danish Dua
9882f1d182 internal/lsp: add initial workspace load notification
Add workspace package load (IWL) notification.

Closes golang/go#40632

Change-Id: I4d4c6fba1ece08bb0d6df52da2e6f08c959fd1ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248623
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-14 23:09:02 +00:00
Rebecca Stambler
c4923e618c internal/lsp/cache: use a map to track package paths to reload
I could've sworn I'd already submitted this CL earlier. We've been
sending duplicate package paths to go/packages for no reason.

Updates golang/go#40690

Change-Id: I4c0d082a71e53df12991341b015e0ce8f504c318
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248403
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-14 17:20:26 +00:00
Rebecca Stambler
cd238244dd internal/lsp/cache: delete unused function
Change-Id: Ie7d488ffb21a2547ceb8bf49c4a746d3caa1c41b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248406
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-14 16:14:45 +00:00
Rebecca Stambler
0a73ddcff9 internal/lsp: check for context cancellation before showing messages
Noticed this as part of investigating golang/go#40567.

Change-Id: I977b1b0a6ceb139e35e087c16b6ab88d66af69a9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248400
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-13 23:17:17 +00:00
Danish Dua
1365742343 internal/lsp: add incoming calls hierarchy to gopls
* Adds incoming calls hierarchy to gopls. Returns function declarations/function literals/files enclosing the call/s to the function being inpected.
* Updates cmd to show ranges where calls to function in consideration are made by the caller.
* Added tests for incoming calls.

Example:
This example shows call hierarchy for PathEnclosingInterval in tools/go/ast/astutil.go
Show Call Hierarchy View: https://imgur.com/a/9VhspgA
Peek Call Hierarchy View: https://imgur.com/a/XlKubFk

Note:
* Function literals show up as <scope>.func() in call hierarchy since they don't have a name. Here scope is either the function enclosing the literal or a file for top level declarations
* Top level calls (calls not inside a function, ex: to initialize exported variables) show up as the file name
* Clicking on an item shows the the range where a call is made in the scope

Change-Id: I56426139e4e550dfabe43c9e9f1838efd1e43e38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247699
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-13 20:36:30 +00:00
Heschi Kreinick
d926bd178c internal/lsp/cache: always type check in default mode
Every package has a default type checking mode dictated by whether it's
in the workspace or not. Some features force full rather than exported
type checking, but AFAICT that ends up being more harm than good. For
example, let's say we want to Find References on fmt.Printf in the stdlib.
Before this CL, we'd force a new type check of the fmt package, then
find no references because nothing else would have been checked against
that new version.

While there may be some features that work better in the current regime,
I can't think of any, and we have no test coverage for them. So I'd
rather start with what makes sense, and if we want to change it maybe
let's write some tests.

Change-Id: Iea589efb4b4374fd2a54451c868b6e2bd5484e20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248380
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-13 18:10:07 +00:00
Heschi Kreinick
8ca64c5666 internal/imports: fix crash when adding stdlib imports
We failed to initialize the ProcessEnv when adding stdlib imports.
Somehow this went unnnoticed for a month or something. Initialize on
demand there too, and add a stronger check so there's a better error
message.

Fixes golang/go#40670.

Change-Id: I391c115f417390be4a0e18f92fbfbcbfbdcc052c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247797
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-13 17:47:04 +00:00
Heschi Kreinick
9176cd3008 internal/lsp/cache: trim ellipsis array literals
While looking at Kubernetes I noticed that golang.org/x/text packages
were some of the largest. The problem is the large code-generated
tables, which use ellipsis array literals. Teach gopls to trim the cases
that matter there.

While silly, this trims ~60MB off the live heap, so I think it might be
worth it.

Change-Id: I0cfd80bd5fbc8703ac628312982af9c6ed871758
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248180
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-12 23:16:40 +00:00
Muir Manders
c42aa19e51 internal/lsp/source: improve unnamed type completion
Tweak a few things so that unnamed, non-basic types are offered as
completions in certain cases:

    var _ []int = make(<>) // now properly suggests "[]int"

I also fixed type related keywords to not be offered if there is an
expected type:

    var _ *int = new(<>) // don't offer "func", etc.

There are still some cases that don't work properly. For example:

    var _ [][]int = make([]<>) // doesn't offer "[]int"

This would be harder to fix given the way things currently work.

Fixes golang/go#40275, golang/go#40274.

Change-Id: I2577d5863d4757845ad3ff7dbb125106b649a6b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246360
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-12 23:05:10 +00:00
Rob Findley
5ae4c3c160 internal/lsp/cmd: make -remote consistent across commands
The lsprpc package implements support for connecting to remotes on unix
domain sockets, as well as auto-starting the remote if it doesn't exist.

Unfortunately, commands other than 'serve' were instead assuming the tcp
network.

Fix this to share functionality.

Fixes golang/go#40732

Change-Id: I46c303ea91d087d7432e0e3236a68e58e1ea6344
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248181
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-12 19:50:22 +00:00
Rebecca Stambler
8586c7bd52 internal/lsp: fix bad go mod tidy quick fix title
Also, catch a potential nil pointer in the go.mod parsing code and a
typo.

Fixes golang/go#40659

Change-Id: Ic0adc8025a0d657cf713a101c333f28c15275f2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248037
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-08-12 18:39:12 +00:00
Brayden Cloud
c7ca52690a internal/lsp: add "run file benchmarks" code lens
This CL adds a code lens to run all benchmarks in a file. Additionally,
it updates the test command handler to better support both tests and
benchmarks.

Updates golang/go#36787

Change-Id: I6e90460f7d97607f96c263be0754537764bd0052
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246017
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-12 18:37:58 +00:00
Rebecca Stambler
3986990901 internal/lsp: lowercase drive letters on Windows to fix file watching
This is a work-around for
https://github.com/microsoft/vscode/issues/104387. We now always
lowercase the drive letter on Windows.

This CL also fixes a bug introduced by CL 245327, which caused URIs
to be used instead of paths in the GlobPattern.

We really need VS Code integration tests for this
(golang/vscode-go#404).

Updates golang/go#40661

Change-Id: I21be6d929288cfe41168cea34001fc2f41ac6c8b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247684
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-12 18:32:13 +00:00
Rebecca Stambler
eb8585a966 internal/lsp: extend the mod handle functions to handle multiple files
In the past, we assumed that we would only run these functions on the
view's go.mod file. As we expand the concept of a view to possibly
include multiple go.mod files, we need to allow these functions to work
on multiple go.mod files.

Change-Id: If9e7d131007e0977fc48ee2264365773da7d41f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248097
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-12 18:23:14 +00:00
Rebecca Stambler
48a8ffc5b2 internal/lsp/cache: flip noGopackagesDriver to hasGopackagesDriver
It's not very clear to use double negatives. Follow-up from CL 247817.

Change-Id: Ie162d8e71ce7229bffcb2c419a16f0a08a4b7b74
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-11 21:50:21 +00:00
Rebecca Stambler
d77521d074 internal/lsp: remove extra go env GOMOD logic for single file mode
Now that the view always looks for its module, we don't need to pass
in the module root path when creating a view. This allows us to remove
some extra logic.

Change-Id: I35ffb71ec762b2dec0b72c84195ce008f7b35872
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247897
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-11 17:27:22 +00:00
Peter Weinbergr
74512f09e4 internal/lsp: in gc_details change command to use a temporary file.
https://go-review.googlesource.com/c/tools/+/246419/2 fixed a problem
but introduced a new one, as go build treats -o directories differently
depending on whether or not a main package is being built.
(see  https://github.com/golang/go/issues/36784)

This change explicitly constructs a temporary file for go build
to use.

Change-Id: I096748e9af5014428dab8a5aad703f062fe88d50
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247899
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Pontus Leitzler <leitzler@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-11 15:37:30 +00:00
Rebecca Stambler
fd80f4dbb3 internal/lsp: fix a few small staticcheck warnings
Address unused context / unused error warnings.

Change-Id: Ibd302d58a3d6db5aa274ed95fb6d5a337978779a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Danish Dua <danishdua@google.com>
2020-08-11 03:20:01 +00:00
Rebecca Stambler
48de4c84f0 internal/lsp: add a configuration to limit workspace scope to root URI
Some users may intentionally be opening subdirectories to avoid having
gopls load the whole module. Allow this via a configuration.

Fixes golang/go#40567

Change-Id: I6167f62a74a1c0b7cf07c1cb247adda839ee41f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247617
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-11 01:57:04 +00:00
Rebecca Stambler
64be3c5c02 internal/lsp/cache: check the user's configuration for GOPACKAGESDRIVER
A user need not necessarily set GOPACKAGESDRIVER in their environment,
but they may still provide it through their configuration.

Change-Id: Ic48328e6a1596ff653a048b24256b8dc44c45b8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-11 01:10:27 +00:00
Rebecca Stambler
2d15c7a537 internal/lsp: handle results from setting options in initialize
gopls configuration can come in through initializeOptions or through
the workspace/configuration request. Make sure to handle error results
in both cases.

Change-Id: Iaf33d23aa33381ce86803f5e8d090deb369b7341
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227033
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-10 23:01:02 +00:00
Josh Baum
2f2f27240c internal/lsp: support function calls in extract variable
In the previous implementation, we could not extract call expressions
to variables.

Change-Id: I80ee82d7889247a618bd80f40abaa897d15ad20b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246761
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 22:41:34 +00:00
Josh Baum
72f2e0bf6d internal/lsp: limit code action false positives for extract to variable
Instead of only checking whether the selection is an AST expression in
canExtractVariable, we now also check what kind of AST expression
it is. This limits the frequency of situations where the lightbulb
appears (canExtractVariable succeeds), but nothing can be extracted
(extractVariable fails).

Change-Id: I1e63c982e482bb72df48b414bdb4e8037140afdb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247408
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 22:02:59 +00:00
Heschi Kreinick
c1903db4db internal/memoize: switch from GC-driven to explicit deletion
The GC-based cache has given us a number of problems. First, memory
leaks driven by reference cycles: the Go runtime cannot collect cycles
involving finalizers, which prevents us from writing natural code in
Bind callbacks. If we screw it up, we get a mysterious leak that takes a
long time to track down. Second, the behavior is generally mysterious;
it's hard to predict how long a value lasts, and harder to tell if a
value being live is a bug. Third, we think that it may be interacting
poorly with the GC, resulting in unnecessary memory usage.

The structure of the values we put in the cache is not actually that
complicated -- there are only 5 significant types: parse, typecheck,
analyze, parse mod, and analyze mod. Managing them manually should not
be conceptually difficult, and in fact we already do most of the work
in (*snapshot).clone.

In this CL the cache adds the concept of "generations", which function
as reference counts on cache entries. Entries are still global and
shared across generations, but will be explicitly deleted once no
generations refer to them. The idea is that each snapshot is a new
generation, and can inherit entries from the previous snapshot or leave
them behind to be deleted.

One obvious risk of this scheme is that we'll leave dangling references
to values without actually inheriting them across generations. To
prevent that, getting a value requires passing in the generation at
which it's being read, and an error will be returned if that generation
is dead.

Change-Id: I4b30891efd7be4e10f2b84f4c067b0dee43dcf9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242838
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-10 19:02:17 +00:00
Josh Baum
74a6bbb346 internal/lsp: enhance fillstruct and fillreturns to fill with variables
In the previous implementation, we always created a default
value for each type in the struct or return statement in fillstruct
and fillreturns, respectively. Now, we try to find a variable in scope
that matches the expected type. If we find multiple matches, we choose
the variable that is named most similarly to the type. If we do not
find a variable that matches, we maintain the previous functionality.

Change-Id: I3acb7e27476afaa71aaff9ffb69445913575e2b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245130
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 18:49:36 +00:00
Rebecca Stambler
e1f7ec57ef internal/lsp/cache: fix gopackagesdriver binary detection logic
GOPACKAGESDRIVER=off supersedes the existence of a gopackagesdriver
binary.

Change-Id: I0f2f78850c5cc1163f22ca858e5783b0899f4490
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247685
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-10 18:39:49 +00:00
Rob Findley
ace63f8701 gopls/integration/govim: use docker to build gopls in run_local.sh
Kokoro build runners have an old version of the Go command. Update the
run_local.sh script that they use to build in Docker.

For golang/go#40451

Change-Id: I89fc422e6824045ffdb610fe0f9a106c6cdb875e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247681
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 16:06:12 +00:00
Rob Findley
978e77c455 internal/lsp/progress: refactor progress reporting
Progress reporting has gotten complicated, and has had a couple bugs.
Factor out progress-related behavior to a new progressTracker type, and
use this to implement some unit tests.

Also rename some methods to remove stuttering, and reorganize the code
to be more logical.

Fixes golang/go#40527

Change-Id: I93d53a67982460e7171f892021e99f4523fe3e5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247407
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 15:18:52 +00:00
Rob Findley
30a0893a98 internal/lsp: use the token supplied by the client for progress
Our WorkDone reporting was generating a random token for each unit of
work, even if a token was supplied by the client.  Change this to use
the client token if it is non-empty, and skip the
workDoneProgress/create request.

After this change we can no longer rely on tokens being a string.
Update our progress tracking accordingly.

For golang/go#40527

Change-Id: I702f739c466efb613b69303aaf07005addd3b5e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247321
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 15:15:12 +00:00
André Martins
73b12cab25 go/analysis/passes/structtag: ignore warning if tag should be ignored
Having a private field with a json tag does not necessary means the
field needs to be exposed publicly. This change ignores the warning of
such fields that contain a json tag with the field tag "-".

The lines added in the unit tests will fail without these changes with
the error:
```
a/a.go:36:2: unexpected diagnostic: struct field b has json tag but is not exported
```

Change-Id: Ife987b99c264ae3b60a702e43a1f9c778f8c2d62
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245857
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 15:05:53 +00:00
Pontus Leitzler
3e8281990c internal/lsp: build output to package during gc_details code lens
The gc annotation details code lens did build binaries during
diagnostics to the module root. When trying to enable details for a
main package in a sub directory, it failed since the output binary name
conflicted with the sub directory name.

Instead, specify output to the package directory during build to avoid
conflicts.

Change-Id: Idc11e0c6a9ba15a66645aeef89bffb5abde76928
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246419
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-08-10 15:03:20 +00:00
Muir Manders
6f4f008689 internal/lsp/source: improve completion in switch cases
Now we downrank candidates that have already been used in other switch
cases. For example:

    switch time.Now().Weekday() {
    case time.Monday:
    case time.<> // downrank time.Monday
    }

It wasn't quite as simple as tracking the seen types.Objects.
Consider this example:

    type foo struct {
      i int
    }

    var a, b foo

    switch 123 {
    case a.i:
    case <>
    }

At <> we don't want to downrank "b.i" even though "i" is represented
as the same types.Object for "a.i" and "b.i". To accommodate this, we
track having seen ["a", "i"] together. We will downrank "a.i", but not
"b.i".

I applied only a minor 0.9 downranking when the candidate has already
been used in another case clause. It is hard to know what the user is
planning. For instance, in the preceding example the user could intend
to write "a.i + 1", so we mustn't downrank "a.i" too much.

Change-Id: I62debc5be3d5d310deb69d11770cf5f8bd9add1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247337
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-09 01:28:40 +00:00
Muir Manders
5bf02b21f1 internal/lsp/source: fix bug in deep completion score tracking
We keep track of the N highest seen scores so we can quickly skip deep
completions not in the top N. Our logic for maintaining the top N list
wasn't quite right, resulting in certain cases where we would let
non-high scoring candidates through. I don't think the bug impacted
correctness.

Change-Id: Ic105617523c82f0e71e4f95ef0ee216182a84252
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247418
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-08 16:17:06 +00:00
hasheddan
b8f4a4e40d internal/lsp/protocol/typescript: clean up type gen documentation
Updates wording in lsp protocol type generation documentation regarding
anonymous struct types for formal parameters.

Change-Id: Icd78545c0f8b3d78d41ab6eb95dac2bc381dcc1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247520
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-08 14:55:51 +00:00
Muir Manders
19738be007 internal/lsp/source: improve type switch case completion
Now we will filter out the types already used in other case
statements:

    switch ast.Node(nil).(type) {
    case *ast.Ident:
    case *ast.I<> // don't offer "Ident" since it has been used
    }

Note that the implementation was not able to use a map to track the
seen types.Types because we build up types.Type entries dynamically
when searching for completions (e.g. types.NewPointer() to make a
pointer type). We must use types.Identical() instead of direct pointer
equality.

Change-Id: I316638bb48bfd6802e2caea671f297d640291010
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247098
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-07 23:56:57 +00:00
Rebecca Stambler
383b97c0b5 internal/lsp: watch directories in replace targets and update on changes
This change adds the notion of a "workspace directory", which is
basically the set of directories that contains workspace packages. These
are mainly used for replace targets right now. It's a little trickier
than expected because the set of workspace directories can technically
change on any go.mod change.

At first, I wanted DidModifyFiles to report whether there was a change,
but I don't think it's actually that expensive to check on each call
and it complicates the code a bit. I can change it back if you think
it's worth doing.

The parse mod handle changes are because I needed an unlocked way of
parsing the mod file, but I imagine they'll conflict with CL 244769
anyway.

The next CL will be to "promote" replace targets to the level of
workspace packages, meaning we will be able to find references in them.

Change-Id: I5dd58fe29415473496ca6634a94a3134923228dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245327
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-07 23:35:17 +00:00
Rebecca Stambler
c05a0f5be4 internal/lsp/cache: refactor go mod tidy error logic
This CL is mostly a refactoring of the logic to compute errors for
`go mod tidy` diagnostics. It had been getting a little confusing, so
hopefully this makes things easier to read.

I made a few other small changes, such as slightly changing a few error
messages and showing diagnostics in the go.mod file for all missing
modules.

Change-Id: Ia8cf580731b997248591a2d64dff133accd9c5aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244610
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-07 22:43:23 +00:00
Danish Dua
9221131678 internal/lsp: release resources for call hierarchy file requests
* adds call to release resources for beginFileRequest in
  lsp/call_hierarchy.go
* fixes source_test

Change-Id: Id45f61ccd474a2df9f46be89fdb059cfe8584f0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247497
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-07 21:04:51 +00:00
Danish Dua
a5d4502270 internal: add call hierarchy cmd and lsp scaffolding
* adds gopls command line tool for call hierarchy
* adds lsp setup for call hierarchy
* adds handler for textDocument/prepareCallHierarchy to display selected
  identifier and get incoming/outgoing calls for it
* setup testing

Change-Id: I0a0904abdbe11273a56162b6e5be93b97ceb9c26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246521
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-07 20:22:52 +00:00
Rebecca Stambler
f15f0bfc61 internal/lsp/cache: check for a gopackagesdriver binary
We missed a possible case in checking for gopackagesdriver - a binary
named gopackagesdriver works the same way as setting GOPACKAGESDRIVER.

Change-Id: I676800d253950cb35d74211558bafab340310653
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247179
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-08-07 18:25:03 +00:00
Josh Baum
6756e73834 internal/lsp: fix bug in extract function when highlighting full line
We did not adjust the range in extractFunction(). We only adjusted
the range in canExtractFunction().

Change-Id: Idc1eab775988ab61be6df8b4afd4b1a36a8bb0ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247405
Run-TryBot: Josh Baum <joshbaum@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-07 18:02:56 +00:00
Josh Baum
1c672e2dff internal/lsp: prioritize non-"zero" values in fillreturns
In the previous implementation, we kept the first variable in the
return statement that matched the each given return type. Now, we
keep searching for a non-"zero" value, even if we have already found
a "zero" value.

Change-Id: Icf0987bab90239781452319979e7a30502807e36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246917
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-07 14:33:55 +00:00
Muir Manders
990129eca5 internal/lsp/source: prefer funcs when completing go/defer
Now we prefer functions when completing "go" and "defer" statements.
Previously we had no preference for the type of object. Further, we
will now also properly invoke functions.

    var f1 int
    var f2 func()
    go f<> // prefers "f2" and expands to "f2()"

Change-Id: I213551b74ba453c337ac89e825b5d495659e9d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246359
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-06 23:41:36 +00:00
Muir Manders
23e6869392 internal/lsp/source: don't prefer bool candidates in bool exprs
Consider this example:

    var foo, bar int
    if foo == 123 && b<> {
    }

Completing at "<>" previously preferred the unimported
"bytes.Contains()" function because it returns a bool. You often need
to compose a boolean expression from non-boolean candidates, so
preferring only bool candidates gives unhelpful results. Now we don't
infer any expected type for "&&" and "||", which allows the example to
prefer "bar" as the top candidate.

Fixes golang/go#37163.

Change-Id: Ic341da11dd626439cfb265d129288c5ca6008270
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246362
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-06 23:41:14 +00:00
Muir Manders
41a9f6dc66 internal/lsp/source: fix function completion ranking
I noticed an annoying completion ranking issue:

   // ranks "HandlerFunc" over "HandleFunc"
   http.HandleFunc<>

This was due to us downranking function calls to prefer fields/vars. I
tweaked the logic to only downrank methods (with a receiver).

Change-Id: Ia4040dc8a35f641be2d7d934bf555090831219ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246357
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-06 22:03:01 +00:00