1
0
mirror of https://github.com/golang/go synced 2024-10-01 12:38:31 -06:00
Commit Graph

1099 Commits

Author SHA1 Message Date
Rebecca Stambler
189207f339 internal/lsp: use URIs instead of FileIdentity in errors
This change modifies the source.Error type to have a URI instead of a
FileIdentity associated with an error.

Change-Id: I8056bdc881dd3ec43af02cca1366815c0bc6dfcd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214586
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 19:14:11 +00:00
Muir Manders
d31a08c2ed internal/lsp/source: improve completion support for args to builtins
We now understand what "kind" of type is expected when using various
builtins. For example, when completing "close(<>)" we prefer channels,
and when completing "delete(<>)" we prefer maps.

I also added some code to infer the expected type for the second
argument to "delete()" and for the args to "copy()":

delete(map[someType]int{}, <>) // expect "someType"

copy([]int{}, <>) // expect "[]int"
copy(<>, []int{}) // expect "[]int"

And I marked "new()" as expected a type name, and it infers the type
name properly:

var _ *int = new(<>) // expected type at "<>" is "int"

Fixes golang/go#36326.

Change-Id: I4295c8753f8341d47010a0553fd2d0c2586f2efa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212957
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 05:24:53 +00:00
Rebecca Stambler
3b9e235283 internal/lsp: fix context cancellation
Change-Id: Ib932722b0e66910b8cd23031e756f16099ee82dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 01:26:48 +00:00
Rebecca Stambler
294c8f8251 internal/lsp: use latest file versions in diagnostics
This change makes sure that diagnostics are sent with the most recently
seen version for a file, instead of a cached version.

Fixes golang/go#36476

Change-Id: Ibac2757099fdfc71989987cf5851a678f589aadf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214422
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 00:45:52 +00:00
Rebecca Stambler
5a294e27f3 internal/lsp: allow subdirectories of module roots when checking errors
Previously, we would surface a warning message if a user had a missing
dependency in a subdirectory of their module root. This is not
necessary, so do a better job checking for that case.

Change-Id: Ib6fcdcbf6ac191478b9bb1f5f8a55d154fd30b5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214420
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 00:24:04 +00:00
Rohan Challa
544dc8ea2d internal/lsp: change go1.14 check to be more lenient
This change adds the -e flag to ensure that we get the release tags when we are checking if the go version is at least 1.14. This also adjusts the check to be more lenient when it comes to processing the output of the version check.

This also fixes another issue where if the version is not 1.14 we were publishing empty diagnostics for an empty uri. This arose because we did not check if there was a valid go.mod file before we published the reports.

Fixes golang/go#36488

Change-Id: I5a8057c153f49167811e2bf8e4ade52bf6171d6b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214290
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-13 22:38:16 +00:00
Rebecca Stambler
533f309ed4 internal/lsp: reload workspace package metadata on demand
This change removes functions from the snapshot that return package IDs.
We prefer PackageHandles, since getting PackageHandles in a granular
fashion is not effective and causes us to spawn many `go list`
processes. By only ever returning PackageHandles, we can batch metadata
reloads for workspace packages. This enables us to add a check to
confirm that the snapshot is in a good state before returning important
data, like reverse dependencies and workspace package handles.

Change-Id: Icffc8d8e0449864f207c15aa211e84cb158c163f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214383
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:10:07 +00:00
Rebecca Stambler
8f45075ebc internal/lsp: merge completion options into source.Options
This change flattens the completion options type into UserOptions and
DebuggingOptions, which will enable us to generate documentation for
these options more effectively. This results in some modifications in
the tests.

Additionally, the fuzzyMatching and caseSensitive boolean flags are
merged into one setting, matcher, which can be used to specify the type
of matcher that is used for completion. Other requests (notably
workspaceSymbols) may need to use a matcher in the future.

Change-Id: I185875e50351be4090c7a2b3340d40286dc9f4a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212635
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:09:44 +00:00
Rebecca Stambler
6316571e2c internal/lsp: don't require type-checking for formatting
We had previously required a type-checked package for formatting
requests, in order to determine if the package contained any parse
errors. We can get this information directly from the ParseGoHandle, so
there is no need to check the package. This will prevent `go list`
errors from making their way into formatting requests, for which a `go
list` really is not needed.

Updates golang/go#36511

Change-Id: I297f8880367e800d2343f468535efa80d2937071
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214421
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 19:43:37 +00:00
Rebecca Stambler
86d412b4c6 internal/lsp: fix support for watching changed files
This is the beginning of the CLs to refactor the file watching code with
the normal text synchronization code. This hasn't yet been tested other
than with some minimal local testing, so follow-up CLs will be needed.

Updates golang/go#31553

Change-Id: Id081ecc59dd2903057164171bd95f0dc07baa5f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214277
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 19:19:55 +00:00
Rebecca Stambler
4a54ec1d38 internal/lsp: remove view.FindPosInPackage and view.FindMapperInPackage
There is no reason for these functions to live on the view. They make
more sense as unexported functions in internal/lsp/source.

Initially, I had to propagate contexts through a lot of functions in
internal/lsp/source, but instead I removed the unused contexts forom
snapshot.GetFile.

Change-Id: I8323419d0356feb2010091fe8d3ed35e511f801a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214384
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 18:51:11 +00:00
Rohan Challa
30cae5f2fb internal/lsp: surface diagnostics for invalid go.mod files
This change will surface errors that come from the mod package. It will handle incorrect usages, invalid directives, and other errors that occur when parsing go.mod files.

Updates golang/go#31999

Change-Id: Icd817c02a4b656b2a71914ee60be4dbe2bea062d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213779
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-13 15:48:38 +00:00
Muir Manders
eac381796e internal/lsp: fix completion ordering workaround
Our loop to make all candidates use the same "filterText" wasn't
including the final candidate. This was causing strange ordering of
candidates in VSCode when the "worst" candidate happened to match the
prefix exactly (causing VSCode to reorder it to the top).

Fixes golang/go#36519.

Change-Id: Ie2119ca94d13f337edd241fbde862706bdeee191
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214184
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-13 04:08:37 +00:00
Rebecca Stambler
a7a6caa82a internal/lsp: initialize the view on creation
This change moves the initialization of the view into the view's
creation, instead of forcing the tests to call WorkspacePackageIDs to
initialize.

Change-Id: Iccea820ec268b1851d58821481d92c7a3d4772c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214279
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:31:25 +00:00
Rebecca Stambler
3e6f5d44f4 internal/lsp: don't invalidate workspace when a mod file is opened
Opening a mod file is not sufficient cause to invalidate in the
workspace, so don't.

Change-Id: I2b7703008528e4469be312165deb17fe6856fd74
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:20:43 +00:00
Rebecca Stambler
945ed6e034 internal/lsp: fix link handling for links without schemes
Unfortunately, this can't be tested until golang/go#35880 is resolved,
because only the third-party xurls library detects links without
schemes.

Change-Id: I620581d6ce99c79ae89f3f22ea8ee634c3850a8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214280
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:19:44 +00:00
Rebecca Stambler
e8a26f4160 internal/lsp: prevent initial workspace load from being canceled
In one of the many iterations on CL 212102, the contexts propagated
through the initial workspace load were allowed to be canceled. This
should not be allowed because the initial workspace load has to be
completed.

Change-Id: I6c6273b4e58fb9041af518f329f4766ed5f1f81b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213641
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:08:40 +00:00
Rohan Challa
428f1ab0ca internal/lsp/mod: test go.mod is unchanged when tempModfile=true
This change adds a test to ensure that your go.mod file remains unchanged when the tempModfile flag is activated. Specifically, it adds a test to ensure that a go directive does not get added to a user's go.mod file when there was not one included before.

Updates golang/go#36247

Change-Id: If8db5508ace5b7222112408255ffa66e4d38797f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214260
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-10 14:27:00 +00:00
Rebecca Stambler
e2f26524b7 internal/imports: create listener map after clearing for new scan
The module resolver needs to recreate the listener map if it's cleared
for a new scan.

Change-Id: If5e945d4f2059f2a79aef3129f963a2c50e90229
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 04:28:03 +00:00
Heschi Kreinick
563860d11d internal/imports: fix use of uninitialized resolvers
Resolvers are lazy initialized. That worked fine until the addition of
the scan semaphore -- it's not a good idea to create that lazily, since
you can't synchronize on a channel that doesn't exist.

Specifically, this caused a gopls hang when completion finished without
needing to use the resolver. In that case, we'd call ClearForNewScan/Mod
on an uninitialized resolver, and then hang receiving from a nil
channel.

Instead, eagerly initialize where convenient, and particularly the scan
semaphore.

Change-Id: I3adb1ae76b751650995e50f50346e06fd9c9f88d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214258
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-10 00:40:31 +00:00
Rebecca Stambler
0a1579a33b internal/lsp: don't run full workspace diagnostics on mod file change
Minimize the issues at master by not running workspace-level diagnostics
on mod file changes. Once the initial workspace load stabilizes we will
be able to go back to that approach.

Also, a couple of minor changes along the way while debugging.

Change-Id: Ib3510e15171326a1b89f08ef0031a3ef7d9ac4ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-10 00:12:05 +00:00
Heschi Kreinick
5d34a75004 internal/imports: don't block completions on walks
Filesystem walks of large GOPATHs/module caches can take seconds,
especially on systems with slow filesystems like MacOS and WSL. We don't
want to block completion requests on walks finishing. At the same time,
cancelling a walk midway through results in an unusable cache, where we
don't know which parts have been scanned so far.

The best option is to run the walks in a separate goroutine. Then we can
detach and let them finish. On the other side, we need to be able to
reattach for the next completion request.

Introduce a new method on caches, ScanAndListen, which first processes
all the items in the cache, then notifies of any new items. This allows
us to reattach to an existing scan without missing anything.

The background scan introduces concurrency to the resolvers where there
wasn't any before. We can't use mutexes, because there's no way to stop
Lock() when a context expires. Use a 1-element semaphore channel to
accomplish the same effect.

Along the way: Only rescan GOPATH if the resolver has been cleared. None
of this makes sense for GOPATH without that. Fix a bug where we were
scanning the main module twice in module mode. Stop loading exports in
module tests, it slows them down a ton.

Change-Id: I978efae733ccba0c0cdc8e8fe6892bf5f15feac8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213217
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-09 22:24:46 +00:00
Rebecca Stambler
a9a43c4726 internal/lsp: store workspace package IDs with package paths
A test variant for a package can only be reloaded by running go/packages
on the non-test variants import path with the -test flag. We need to
cache this import path in order to be able to reload a test package
on-demand.

Also, always ignore test main packages by detecting them in the
metadata.

Fixes golang/go#36473

Change-Id: I6e5a61c8e73689212303ae09fa3aed4a2ee8762a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213660
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 22:04:34 +00:00
Rebecca Stambler
8a1a3df092 internal/lsp: allow diagnostics to be computed per-file or per-package
This change splits source.Diagnostics into source.FileDiagnostics and
source.PackageDiagnostics. There isn't much difference in functionality,
but this just simplifies the process of getting diagnostics if you only
have a PackageHandle.

Change-Id: I62c0de8778065a4c46cc5672e2834ce5c63fe012
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213644
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:49:23 +00:00
Rebecca Stambler
700ee2612c internal/lsp: propagate errors from find references
We were ignoring errors in a few cases and returning incorrect errors in
the case of the command-line interface (no identifier found when there
were just no references). I think it is better for references to return
an error than incomplete results.

Change-Id: Id90bca58ebdd9f6a910853cb4ac5b6ab6bec57f8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:23:09 +00:00
Rebecca Stambler
dfcf57064e internal/lsp: stop requiring file kind when fetching a file
This change consolidates the FileKind into only the FileHandle.
Previously, it had been set in multiple places, which required users to
pass in a FileKind when fetching a file. This resulted in confusion,
particularly in places when users did not have access to the file kind.

Change-Id: I9e07d7320c46a21d453ffe108d1431a615706a71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213459
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:19:36 +00:00
Rebecca Stambler
487733262f internal/lsp: don't send diagnostics for old file versions
This isn't a strictly necessary change, but it simplifies things for
open files when the initial workspace load diagnostics come in too late.

Updates golang/go#36452

Change-Id: I85a9c201876b7c63a97d8dca020fc396b3c57706
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213820
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:16:13 +00:00
Rebecca Stambler
675cf513f4 internal/lsp: warn user in case of a multi-file ad-hoc package
This change surfaces a warning to the user if they might be coding in a
multi-file ad-hoc package.

Updates golang/go#36416

Change-Id: Ifaa15a0777ea97e62c1477fb33911636b13e073e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213458
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 20:44:29 +00:00
Rohan Challa
f270e23f6a internal/lsp: add warning diagnostics for unused dependencies in go.mod files
This is the first step to surfacing potential fixes and suggestions to a user's go.mod file. Specifically, it will show a warning if you have a dependency that is not used, or if a dependency is declared as indirect when it should be direct and vice versa.

This CL adds functionality for version of Go that are >= 1.14.

Updates golang/go#31999

Change-Id: Id60fa0ee201dcd843f62e2659dda8e795bd671db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211937
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-09 20:05:13 +00:00
Muir Manders
89082a3841 internal/lsp/source: fix some invalid literal candidates
We were marking all literal candidates as addressable so we were
getting invalid candidates like "&int()". Fix it to only mark literal
struct, array, slice and map types as addressable.

I also fixed the unnamed literal candidate to pass the dereferenced
expected type. For example, if the expected type was "*[]int" we were
passing a literal type of "*[]int" which wasn't working anymore. Now
we pass "[]int" and take its address as "&[]int{}".

Change-Id: I5d0ee074d3cc91c39dd881630583e31be5a05579
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212677
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-08 20:36:44 +00:00
Heschi Kreinick
316d2f2484 internal/lsp/source: only add names to imports when necessary
As usual, I forgot to clear out the import spec's name when it matches
the import path.

Change-Id: I4ddd49b70e0db95fcd30d2968b098327fac39a92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213222
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: zikaeroh <zikaeroh@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-08 19:54:15 +00:00
Rohan Challa
acbc0e3ba1 internal/lsp: modify how we check for go 1.14 for modfiles
When the tempModfile flag was enabled, there would be some go
commands that would run without the -modfile flag. This change adds
the config's buildFlags to the invokeGo command in go/packages and
updates how we are checking if the go version is 1.14 within the
modfileFlagExists function.

Fixes golang/go#36247

Change-Id: I436666c3fcad33e85ba00aec5f227fe4a0e638b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-08 19:23:01 +00:00
Rebecca Stambler
11e9d9cc00 internal/lsp/source: set default InsertTextFormat to PlainText
CL 212519 unintentionally changed the default for this option.

Change-Id: I42df51ba68e78af1493d0461ad4efff3dd55da9d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-07 18:40:32 +00:00
Muir Manders
a222fb47e2 internal/lsp/source: don't downrank builtin constant completions
We downrank untyped constant candidates so that we prefer candidates
whose type matches exactly. However, this was causing builtin
constants like "true" to be outranked by candidates that fuzzily match
"true". Fix by not downranking builtin constants.

Fixes golang/go#36363.

Change-Id: I14801688c96efdbb7ff9fee69f66028530df984c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-07 18:15:58 +00:00
Rebecca Stambler
7201abb308 internal/lsp: parallelize initial workspace load
The initial workspace load was happening when a view was created, in serial.
It should really just be kicked off in a separate goroutine once we create a
new view. Implementing this change required some other significant changes,
particularly the additional work being done by the WorkspacePackageIDs
method.

Some other changes had to be made while debugging. In particular, the
modification to the circular dependencies test was a consequence of
golang/go#36265.

Change-Id: I97586c9574f6c4106172d7983e4c6fad412e6aa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212102
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-07 18:15:18 +00:00
Rebecca Stambler
7be0a674c9 internal/lsp: fix minor bug in the PackageHandle function
Test variants and test mains can result in multiple packages being
loaded for a single ID. Handle this case in the PackageHandle function
instead of returning an error.

Change-Id: Ic0024c5bded162a3e78a9cdcb9566449f3683e35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-06 19:01:16 +00:00
Heschi Kreinick
774c71fcf1 internal/lsp/cache: fix type error reporting in cgo
When dealing with cgo files, we need to pass a Mapper for the authored
file. I got most of the sites in CL 208501, but missed this one.

I was nervous about starting to propagate errors, but all the tests
passed, so shrug.

Change-Id: I7505e670e9c01d719c72c3f99d01c8153c3d2ff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212862
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-03 22:14:40 +00:00
Heschi Kreinick
7bda30096d internal/imports: actually skip things in scan
An important part of letting the callback choose what to load is...not
loading the stuff it doesn't want.

Change-Id: I4048d7aed756b6ebc26fb6f8e384f44c64281f90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213129
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-03 21:11:27 +00:00
Rebecca Stambler
6de373a276 internal/lsp: fix minor issues in diagnostic caching
There were a few cases that we were missing when caching diagnostics.
The first was caching empty diagnostics from the initial workspace load,
so we were resending old empty diagnostics. The second missing case was
resending diagnostics for the same version of a file.

Fixes golang/go#36340.

Change-Id: I5c55b47a60fe94b10e76be99714cb983f9b84b54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213122
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-02 20:01:21 +00:00
Daisuke Suzuki
27f5d1b104 internal/lsp/cmd: fix documentation
Change-Id: I60d6337b07b2cad4d20d877d61b11ebe33d26cf6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212582
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-01-02 17:13:01 +00:00
Rebecca Stambler
2aa90c603a internal/lsp: miscellaneous cleanup
CL 212102 contains a few cleanup-type fixes that are unrelated to the
actual content of that CL. Pull them out to make the diffs simpler.

Change-Id: I6b7e34320f2889d05179c8aeb8cb7f8f4c505a3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212917
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-30 22:03:29 +00:00
Heschi Kreinick
ba16e80ae2 internal/imports: filter out self-import completions
Fixes golang/go#36321.

Change-Id: Ic6cdad4b611e5a16e086743f53f85bcb71070a21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212897
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:44:38 +00:00
Heschi Kreinick
1e586a538e internal/imports: clean up dead code
Now that we're storing module information per-directory, we don't need
to pre-compute the need for a replace statement. And we never have the
package name in the place it used to be set.

Change-Id: I3b0845dc49f52f8c449840410dbb786fe903d29d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212861
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:44 +00:00
Heschi Kreinick
9a28a1fa70 internal/lsp/source: scan loaded packages first for completions
Packages that have already been loaded by gopls are more likely to be
used, and have full type information. Check them for completion
candidates before scanning the disk.

Also, minor bug fixes: add a missing mutex, and use a lower-than-usual
score for typed unimported completions.

Change-Id: I46388802913f9a89342fb47290f704b471154ec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:36 +00:00
Heschi Kreinick
2f3125dfbf internal/imports: filter candidates on directory name
When finding completion candidates, we can use the same tricks
goimports uses to ignore directories that look irrelevant.

Change-Id: I114a3d4e487aed7f59fc48b2f86d42129baf5183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212859
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:29 +00:00
Heschi Kreinick
872f4f411e internal/imports: filter roots with callback
Now that we have all these callbacks, it's strange to have a list of
root types to exclude on the side. Merge that into the callback.

Change-Id: I8dc88e095362a8d2e180196ad9b81e17d4d34949
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212858
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:40 +00:00
Heschi Kreinick
7ec15289dd internal/imports: optimize scan implementations
In scan implementations, stop after cancellation, and swallow the
context's error for convenience.

In the module implementation specifically, try to avoid scanning if the
cache is enough to satisfy the user. When we do have to scan, prioritize
module dependencies before the whole cache.

Change-Id: I23dc98df016f9fca4f31c7ded3d11bc257c29b94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212857
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:33 +00:00
Heschi Kreinick
c2a8f45ada internal/imports,lsp: use callbacks for completion functions
We only need to return a relatively small number of completions to the
user. There's no point continuing once we have those, so switch the
completion functions to be callback-based, and cancel once we've got
what we want.

Change-Id: Ied199fb1f41346819c7237dfed8251fa3ac73ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212634
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:27 +00:00
Heschi Kreinick
3f7dfa39cf internal/lsp: sort by label after score
I want to stop sorting unimported completions. We still want to show
users something reasonable, so use label as a tiebreaker for score in
the higher level completion function.

To maintain the current sorting, we need to adjust scores by search
depth (height?) for lexical completions. A few tests are really ties,
and need sorting in the test case.

Change-Id: Ie2d09fdcbebf6fda4ab33a2f16c579d12b0f26ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212633
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:21 +00:00
Heschi Kreinick
0a57c09236 internal/lsp/source: always use default goimports options
No point in constructing the defaults in three places.

Change-Id: I2b0776910a933a7250245bd82dc27e63c34df18a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212632
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:29 +00:00
Heschi Kreinick
50c778fb86 internal/imports: redesign scan API
We have multiple use cases for scanning: goimports, import completion,
and unimported completions. All three need slightly different features,
and the latter have very different performance considerations. Scanning
everything all at once and returning it was not good enough for them.

Instead, design the API as a series of callbacks for each
directory/package: first we discover its existence, then we load its
package name, then we load its exports. At each step the caller can
choose whether to proceed with the package. Import completion can stop
before loading exports, goimports can apply its directory name
heuristics, and in the future we'll be able to stop the scan short once
we've found all the results we want for completions.

I don't intend any significant changes here but there may be some little
ones around the edges.

Change-Id: I39c3aa08cc0e4793c280242c342770f62e101364
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212631
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:22 +00:00
Heschi Kreinick
ac3e9e73d1 internal/imports: remove go/packages support
We don't use the go/packages resolver in goimports, and as we develop
gopls it only becomes harder to use there. Give up.

Change-Id: Ic8b566c6dd730b23b0c81d7d34a41f16fe0be7e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212630
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:16 +00:00
Heschi Kreinick
fd66c7521c internal/lsp/source: don't get unnecessary unimported completions
Unimported completions are always low-priority. If the user already
has 100 completion options, the unimported ones are probably not useful.
There's no point in calculating any of them.

Also, only do unimported completions for package members when they're
enabled. Oops.

Change-Id: I7535a22ad56bed869dceb6cd0ffdfc6390cf8eb5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212629
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:09 +00:00
Rebecca Stambler
234df48a20 internal/lsp: load metadata for a single package ID, when needed
The metadata for the workspace packages may not be available when we
need it, so we should allow loading a single package ID. This can be
improved in follow-up CLs by consolidating the individual IDs into one
call to packages.Load. Some adjustments from CL 212102 were split out
into this CL.

Change-Id: I173a79a3cb136530bc99d093f1c2be189eac8ce2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212628
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-30 21:09:35 +00:00
Dan Kortschak
6c68fec0bc all: remove many cases of space-space
Change-Id: I49eb8410d4143c67dfccf027f8b2794e66963415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212580
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-30 19:07:42 +00:00
Dan Kortschak
9fb4d21460 tools/internal/lsp/cmd: fix documentation
Change-Id: Ie21babc8879977fb1a3ef02737dbd24904722c26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212579
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-30 18:10:14 +00:00
Rebecca Stambler
7b8e75db28 internal/lsp: create links for golang/go#1234-style strings
This change treats text of the format golang/go#1234 as a link to the Go
issue tracker. This will improve the readability of TODOs that include a
link to an issue, since it doesn't have to be an actual link.

Change-Id: I27606ceb9cbb15bc6bfb1d7aa660d3f4fdd08739
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-27 05:39:25 +00:00
Rebecca Stambler
99d11d0e63 internal/lsp: refactor and document options
This change cleans up the structure of the Options struct in order to
clearly delineate which options should be configurable for the user.
Follow-up work is needed to refactor the completion options to fit into
this structure, as well as to make sure that the name of the field is
the name of the setting. This will make it easier to generate markdown
documentation from the code.

Also, remove options that are no longer in-use and mark them as
deprecated.

Change-Id: Ib34ae25789e21b76077a564601e487fbebfc5f48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-27 05:17:27 +00:00
Koichi Shiraishi
819aba5d6d internal/lsp/cmd: remove unnecessary message from help
remove "goplsgoplsserve" from before the main commands output.

Change-Id: Ib41376d4ad4b601650730f053682868408617c4d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212577
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-27 05:00:04 +00:00
Rebecca Stambler
6b505debf4 gopls: use mvdan.cc/xurls for textDocument/documentLink
Our current implementation isn't robust, and it doesn't seem worth it to
invest significant effort in improving it when this library exists.

Also, make the protocol part of the default URL regex non-optional, as
the alternative is that any string of the format "foo.bar" will appear
to be a link.

Updates golang/go#33505

Change-Id: Ia430a1c193eded394f8af12050bdd4dc2a9ccc94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212517
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-26 21:20:25 +00:00
Muir Manders
dd894d0a8a internal/lsp: trim address operator from completion filterText
VSCode doesn't like (read: ignores) candidates whose filterText begins
with "&", so trim it off.

I also tweaked "addressed" candidates to include the "&" prefix in the
item label as well so the user can see what they will get.

Change-Id: I85840d036e379a202b72e28c5257807a069ae45d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212406
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-24 05:57:32 +00:00
Muir Manders
3721262b3e internal/lsp: support taking address for completion candidates
We now support taking the address of objects to make better completion
candidates. For example:

i := 123
var p *int = <> // now you get a candidate for "&i"

This required that we track addressability better, particularly when
searching for deep candidates. Now each candidate knows if it is
addressable, and the deep search propagates addressability to child
candidates appropriately.

The basic propagation logic is:

- In-scope *types.Var candidates are addressable. This handles your
  basic "foo" variable whose address if "&foo".

- Surrounding selector is addressable based on type checker info. This
  knows "foo.bar.<>" is addressable but "foo.bar().<>" isn't

- When evaluating deep completions, fields after a function call lose
  addressability, but fields after a pointer regain addressability. For
  example, "foo.bar()" isn't addressable, but "foo.bar().baz" is
  addressable if "bar()" returns a pointer.

Fixes golang/go#36132.

Change-Id: I6a8659eb8c203262aedf86844ac39a2d1e81ecc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212399
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 23:54:10 +00:00
Muir Manders
918115ff85 internal/lsp: refactor find-implementation handling
This is the start of a significant refactoring to implementations,
references, and rename (i.e. everything that searches across
packages). The main goal of the refactoring is to push the package
variant logic from internal/lsp into internal/source so that all users
of source benefit, not just internal/lsp. It also makes it easier to
write tests for various cases because the source tests invoke the
source package directly (so previously did not include all the package
variants).

Currently source.Identifer() handles lots of disparate use cases.
Things like definition and hover don't care about package variants but
do care about other random bits of info that may not apply to
implementations or references. So, I'm splitting implementations out
from source.Identifier. As I work through references and rename
hopefully things will end up separated into smaller chunks.

I also improved implementation deduping to happen earlier. I thought I
could dedupe using obj.Pos(), but mirror objects in package variants
have different positions (suggesting they aren't reusing the
same *ast.File). Instead I used token.Position to dedupe.

Change-Id: I81c2b3ec33bf12640accb852be9ecdea4aa24d69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211718
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 22:59:19 +00:00
Muir Manders
4d2fe2ba67 internal/lsp/source: move some data onto "candidate" struct
Make the score and import info be fields on "candidate" since they are
properties of the candidate. There shouldn't be a functional change
here; I'm just consolidating things in preparation for an additional
piece of candidate metadata.

Change-Id: I4c7c8ef40e8e5db7b52691cca21490ba13c17642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212398
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 22:26:30 +00:00
Muir Manders
7bd96bd597 internal/lsp/source: improve completion in value spec
If the enclosing value spec specifies a type on the LHS, we now prefer
completions of that type on the RHS. For example:

i := 123
var foo int = // prefer "i" since we know we want an int

I also added a special case to lexical() to know that we can't offer
objects defined on the LHS as completions on the RHS. For example:

var foo int = // don't offer "foo" as completion

Change-Id: I8e24245a2bc86a29887360e7f642a4cbb87fa6ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212401
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 21:16:02 +00:00
Rohan Challa
c7341709c6 internal/lsp: cleanup temporary go.mod file on shutdown
This commit will delete the temporary go.mod file that is attached
to a view when the session is shutdown.

Updates golang/go#31999

Change-Id: I0f6b0663b0a814ce45d9b12468669f415f8c36aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212479
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-23 19:58:47 +00:00
awh6al
8c5978f193 internal/lsp: make golint happy
This PR fixes internal/lsp/fuzzy/matcher.go:230:4: should replace skipPenalty += 1 with skipPenalty++ .

Change-Id: I7d5b6c20e25503ea266e26783e68ad92fd8b36e0
GitHub-Last-Rev: 36fd2c25232f593f905051692e84d8c634fb7a46
GitHub-Pull-Request: golang/tools#194
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212400
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-23 18:17:04 +00:00
Ian Cottrell
f13409bbeb internal/telemetry: clean up test data
the current implementation likes to sort maps, so we make sure
tag lists are in sorted order already so that a stable encoder
produces the same result

Change-Id: Ia7ce05f35edb636817c354d9df02de753a48fe1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210216
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:47:30 +00:00
Ian Cottrell
c7346ecdc6 internal/telemetry: remove an extraneous comment
There was a fragment of a sentence that must have been from a previous version
(as it talks about a return value for a function that does not have one).

Change-Id: I9d154fe10711344f93e1d49b68a811dbc9772710
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212241
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:47:11 +00:00
Ian Cottrell
814139985e internal/telemetry: obey the onlyErrors flag in the log writer
has no impact because there are no use cases that don't set it to true right now

Change-Id: I2bc485226078c710bdc36397b96755cdce82d9cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212242
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:46:55 +00:00
Ian Cottrell
2f6d8bf0ad internal/lsp: change tests to use the main exporter
Now the tests are at a high enough level, we can
switch them to using the full exporter by bulding
a fake http client to bind it to. This lets us
use the true public interface and also excercise
more of the functionality in the tests.
With this we are now ready to replace the entire
implementation safely.

Change-Id: Ifdbf6230de3ec7c7c5381c840b135cb7a0bc1e55
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209161
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 19:55:05 +00:00
Rohan Challa
75f8c4427c internal/lsp: change -modfile flag to tempModfile
Remove double negative issues and rename the disableTempModfile flag
to be tempModfile.

Updates golang/go#31999

Change-Id: Id62aa3707fef6758a1026c864a962f0bed36bc2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212240
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-20 19:10:06 +00:00
Rebecca Stambler
5e752206af internal/lsp: don't clear file contents on save
CL 212037 introduced a bug with saving overlays. Since VS Code sends nil
contents for a file on save, we were deleting overlay contents on save.
This resulted in very strange behavior.

Also rename overlay.data to overlay.text to match variable names.

Fixes golang/go#36224

Change-Id: I7f2d12e369aa7f6daa2c9f36c33468ec6bf61930
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212199
Reviewed-by: Michael Matloob <matloob@golang.org>
2019-12-19 23:08:27 +00:00
Rebecca Stambler
145a1e401f internal/lsp/cache: detach context before invalidation
In one of my previous refactoring changes, I lost the fact that the
context should be detached before invalidating a file's contents. If
this function is canceled, we will be in a bad state.

Also, small change to return ctx.Err() instead of a custom error message
from (*packageHandle).check.

Change-Id: I19e513e09e438feee105fdd89cb7364a0c3c5e7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212104
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 21:23:07 +00:00
Rebecca Stambler
041a08a54a internal/lsp/cache: remove errors from dependencies
Now that `go list` errors are sufficient for us to determine a circular
dependency, we don't need to cache errors on dependency packages.

Change-Id: I0633aeb356f93d21afed3371d61d7eae7de255ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-19 21:17:59 +00:00
Rebecca Stambler
85a3356613 internal/lsp/cache: consolidate function to update overlays
This change merges the small helper functions that modified overlays
into a single function and removes the openFiles sync.Map in the view.

Change-Id: Id94c7d86228c9628b7373fab0030ad0c8018dda5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212037
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 21:06:08 +00:00
Heschi Kreinick
62b9674312 internal/imports: consider direct mod deps more relevant
As a followup to CL 204203, prefer direct dependencies over indirect.
This should improve results for common names like "log" and "errors".

Updates golang/go#36077.

Change-Id: I3f8cfa070832c2035aec60c4e583ee1c0abf5085
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212021
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-19 20:57:09 +00:00
Rebecca Stambler
2208e1677e internal/lsp: eliminate source.File type and move GetFile to snapshot
This change eliminates the extra step of calling GetFile on the view and
getting the FileHandle from the snapshot. It also eliminiates the
redundant source.File type. Follow up changes will clean up the file
kind handling, since it still exists on the fileBase type.

Change-Id: I635ab8632821b36e062be5151eaab425a5698f60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 20:51:25 +00:00
Muir Manders
56b0b28a00 internal/lsp: put verbose go/packages output behind verboseOutput flag
On startup gopls runs go/packages over the entire workspace. The log
message in question outputs each package found along with all the
package's filenames. Obviously in a large project this produces an
incredible amount of output. Fix by putting the log message behind the
"verboseOutput" flag when invoking go/packages in the "dir/..." mode.
I also added the go/packages "query" string to the
once-per-go-packages-call log message so it is more useful.

Change-Id: I651419e34a855325056bca6720eda8671f7d5fa8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210739
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-19 19:20:50 +00:00
Rohan Challa
bdebc07e97 internal/lsp: fix highlighting of inexact matches when highlighting from the right
There is an issue when highlighting a single character identifier if the cursor is on the right of it. This problem
is a result of an assumption made in astutil.PathEnclosingInterval relating to the arguments passed in. Specifically,
if start==end, the 1-char interval following start is used instead. As a result, we might not get an exact match
so we should check the 1-char interval to the left of the passed in position to see if that is an exact match.

Updates golang/go#34496

Change-Id: If689fdf695df6ec1bc1935088e50d3de055bd5db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-19 19:05:06 +00:00
Peter Weinberger
a98e7fcfeb internal/lps/protocol: bring code.ts up to date with latest changes.
This CL makes sure that code.ts will generate the latest version
of tsprotocol.go. It has a more succinct way of deciding which fields
need to be pointers.

Change-Id: I6854cb2f096d3707bc3b828a9601f9384638f475
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212140
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-19 18:45:41 +00:00
Emmanuel T Odeke
2ad5dca7a5 telemetry/log: sample reference for benchmarking harness
Serves as a reference harness to get benchmarking started.
Steps to run benchmark:

$ go test -run=^$ -bench=. -count=10

After you grab the output, put the various comparisons
into files before.txt and after.txt then edit those
result names to have a common name e.g.
    s/BenchmarkLoggingNoExporter/BenchmarkIt/g
    s/BenchmarkNoTracingNoMetricsNoLogging/BenchmarkIt/g
    s/BenchmarkLoggingStdlib/BenchmarkIt/g

Now run benchstat:

* All compared
$ benchstat no_log.txt tellog.txt stdlog.txt
name \ time/op    no_log.txt  tellog.txt   stdlog.txt
It-8              289ns ± 2%  2780ns ± 3%  5100ns ± 3%

name \ alloc/op   no_log.txt  tellog.txt   stdlog.txt
It-8              80.0B ± 0%  728.0B ± 0%  568.0B ± 0%

name \ allocs/op  no_log.txt  tellog.txt   stdlog.txt
It-8               5.00 ± 0%   32.00 ± 0%   28.00 ± 0%

* No logging vs telemetry log
$ benchstat no_log.txt tellog.txt
name  old time/op    new time/op    delta
It-8     289ns ± 2%    2780ns ± 3%  +862.31%  (p=0.000 n=10+9)

name  old alloc/op   new alloc/op   delta
It-8     80.0B ± 0%    728.0B ± 0%  +810.00%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      5.00 ± 0%     32.00 ± 0%  +540.00%  (p=0.000 n=10+10)

* No logging vs Standard library "log"
$ benchstat no_log.txt stdlog.txt
name  old time/op    new time/op    delta
It-8     289ns ± 2%    5100ns ± 3%  +1665.16%  (p=0.000 n=10+9)

name  old alloc/op   new alloc/op   delta
It-8     80.0B ± 0%    568.0B ± 0%   +610.00%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      5.00 ± 0%     28.00 ± 0%   +460.00%  (p=0.000 n=10+10)

* telemetry log vs Standard library "log"
$ benchstat tellog.txt stdlog.txt
name  old time/op    new time/op    delta
It-8    2.78µs ± 3%    5.10µs ± 3%  +83.43%  (p=0.000 n=9+9)

name  old alloc/op   new alloc/op   delta
It-8      728B ± 0%      568B ± 0%  -21.98%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      32.0 ± 0%      28.0 ± 0%  -12.50%  (p=0.000 n=10+10)

Change-Id: I53b15e9da315615278c576f3a60108435417a9f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212078
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-12-19 18:19:13 +00:00
Rohan Challa
3aa5a36464 internal/lsp: add gopls setting to disable use of -modfile flag behavior
This CL adds a "disableTempModfile" boolean that can be turned on or off.
While we are adding support for go.mod files in gopls, this flag will allow
users to opt out of using the -modfile flag that is enabled in Go 1.14. This
flag might be removed at a future point, the decision still needs to be made.

Updates golang/go#31999

Change-Id: Ic90229333e988fcc6d461ab1ee47bce1114bd965
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212139
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-19 18:11:24 +00:00
Rohan Challa
4f29fbcc08 go/packages: change import cycle errorkind from UnknownError to ListError
The import cycle not allowed error should be returned as a ListError not
an UnknownError.

Fixes golang/go#35964

Change-Id: Ibc575f92d926ff715c0da67a4fceda05badcc652
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212138
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-19 18:10:03 +00:00
Muir Manders
979b82bfef internal/lsp/cache: fix excessive recursion in (*snapshot).clone()
It wasn't infinite, but gopls would sit at 100% cpu for ~25 seconds
whenever I made a change to a package imported by essentially
everything in my project.

Change-Id: Ifa253a4de06897260e0791888284527258e8de48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212000
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 04:18:53 +00:00
Heschi Kreinick
84f0c7cf60 internal/lsp/cache: don't forget files just because they change
The situation in golang/go#35638 was as follows:

didOpen main.go creates a snapshot that knows main.go is in package
"mod.com".
didChange main.go creates a snapshot. When a file changes, we discard
its contents by leaving the file handle out of the "files" map.
didOpen const.go creates a snapshot, and attempts to invalidate the
metadata for packages in the same directory.

The way we detect packages in the same directory is by iterating through
the files in the snapshot. But we threw away the only file in "mod.com"
in step 2 when its contents changed. If a diagnostics run happened to
get in between the two steps, it would re-load main.go and the bug would
go away. If not, step 3 would find no files and fail to invalidate
"mod.com".

The best way to fix this is to insert the new file handle eagerly during
cloning. That way there's no confusion.

Fixes golang/go#35638.

Change-Id: I340bd28a96ad7b4cc912032065f3c2732c380bb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211578
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-18 22:55:20 +00:00
Rebecca Stambler
ca0407e66b internal/lsp: return snapshots from text modifications
Eliminate the file watcher, since it led to a lot of confusion and
difficulty reasoning about the flow of a file action. This change splits
a file invalidation into the two logical steps - 1) things that affect
the overlay, and 2) things that affect the view. It is based on top of
CL 211757, so the diffs will look better once that CL is merged.

Change-Id: I277475569b61f3c80feaa6b6fe457b4bace82e35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-18 22:53:40 +00:00
Rebecca Stambler
41c101f395 internal/lsp/source: fix completion budget type
Change-Id: I43d7b067e8bedc6371c32a2655afb9dc87f71f66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211759
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
2019-12-18 21:55:16 +00:00
Rebecca Stambler
61483d104a internal/lsp: show comments for grouped variable declarations
Fixes golang/go#36139

Change-Id: I8aaf1a5d65b8d66ea7bb350439cc960c7505307d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-18 20:39:36 +00:00
Rebecca Stambler
d270ebf96e internal/lsp/cache: move overlay and debug handling into separate files
This change has no code modifications. Just move the handling for
overlays and debugging into separate files to make them easier to find.
Also, add some missing copyrights.

Change-Id: I7256f704c017457fa3418818d03f89f061af6fc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-18 19:17:43 +00:00
Heschi Kreinick
bc4a8d3946 internal/lsp/cache: don't invalidate dependents' metadata
Rerun of CL 210458. Only invalidate metadata for packages directly
involving the changed file.

Change-Id: Id28647851254a9bdcb3dbe7a762194bb025da913
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211779
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-18 18:41:16 +00:00
Billie Cleek
6f9e13bbec internal/lsp/debug: print the debug port
Print the debug port to stderr when the debug server listens on a
dynamic port so that clients can determine which port to use when
launching a browser to the debug view.

Change-Id: I92f5e334df5cefdf54f7242ac2328b026852c70e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211798
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-12-18 04:04:34 +00:00
Rohan Challa
62a9628863 internal/lsp: use the -modfile flag to update a different go.mod file
In the upcoming Go 1.14 release, there is an introduction of the -modfile
flag which allows a user to run a go command but choose where to direct the
go.mod file updates. The information about this can be found here: golang/go#34506.

This change starts setting up the infrastructure to handle the seperate modfile
rather than keep changing a user's go.mod file. To support versions of Go that are
not 1.14, we run a modified "go list" command that checks the release tags to see
if 1.14 is contained.

Updates golang/go#31999

Change-Id: Icb71b6402ec4fa07e5f6f1a63954c25520e860b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211538
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-17 22:15:16 +00:00
Rebecca Stambler
38570b7665 internal/lsp: fix links for interface methods
This change handles fixes the link behavior for interface methods.
For example, before this change, hovering on "Name"  would point to an
incorrect link: https://pkg.go.dev/go/types?tab=doc#object.Name.
The correct link is
https://pkg.go.dev/go/types?tab=doc#TypeName.Name.

var typ *types.Named
typ.Obj().Name()

Change-Id: If28bff6069239d487e5d86cacafc20800493ebe8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211585
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-17 19:37:44 +00:00
Muir Manders
640059499b internal/lsp: kill some dead code
Clear out a few things I noticed aren't really used anymore.

Change-Id: I03f955015632197b56230ae0443cfb3871b54db2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211717
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-17 19:30:50 +00:00
Rebecca Stambler
f2a421a9e5 internal/lsp: change completionBudget to milliseconds
Change-Id: Ic12f6325bc8307056481edff61da06cfc4f232b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211677
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
2019-12-17 18:37:38 +00:00
Bryan C. Mills
01c78d57fd internal/imports: set the Dir field on the build.Context (instead of WorkingDir) if present
Updates golang/go#36168

Change-Id: Ibdb277176a28da72d85a4c7c8ed7c903c278125e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211599
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-12-17 14:41:53 +00:00
Rebecca Stambler
42268a1f04 internal/lsp: fix link anchors for struct fields
This change fixes the link anchors for fields within a struct or
composite literal by getting the enclosing types.Type.

Fixes golang/go#36138

Change-Id: I534a900fad6fa6fa1b1acaa5a63ca264c5d34c39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211582
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-17 02:48:38 +00:00
Rebecca Stambler
c39ce2148d internal/lsp: expose option to disable timeouts for completion
This is useful for integration testing.

Fixes golang/go#36142

Change-Id: I175510df19f384a0a027267337925ebae15ef827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211584
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-17 01:14:48 +00:00
Heschi Kreinick
621d4eef75 imports: pass more of the environment
goimports now wants various module flags, but I forgot to set them up in
the many places we create environments. Do so.

Change-Id: Ic3817caeb8fc4d564b49006ef6ca6842b2498eaf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211581
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-17 00:36:01 +00:00