This change stops the memory leak from happening. It does not stop the
large number of allocations done by analysis, however, so these still
need to be prevented through correct cancellation.
Change-Id: I1cf7fbf6f85ed413af1f2ea55f91862a6d7f2db7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204558
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change adds command line support for signatureHelp.
If the location provided corresponds to a function, that
function signature is displayed. In case that function is
documented the related comment is shown as well.
Example:
$ gopls signature ~/tmp/foo/main.go:7:5
$
$ Next(n int) []byte
$
$ Next returns a slice containing the next n bytes from
$ the buffer, advancing the buffer as if the bytes had been
$ returned by Read.
Note that linebreaks shown in the comment are just to adhere
commit message guidelines. The command prints documentation
comments on one line.
Updates golang/go#32875
Change-Id: Ib0dcc3267c594f95d80b74f289c1235c2c0c5f64
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204057
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds support for the LSP implemention call, based
on the guru code for getting implementations. The guru code
did much more than we need, so some of the code has been
dropped, and other parts of it are ignored (for now).
Fixesgolang/go#32973
Change-Id: I1a24450e17d5364f25c4b4120be5320b13ac822b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203918
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This adds support for calling suggestedfix from the gopls command line, e.g.
$ gopls suggestedfix ~/tmp/foo/main.go
Optional arguments are:
-w, which writes the changes back to the original file; and
-d, which prints a unified diff to stdout
With no arguments, the changed files are printed to stdout.
Wasn't sure if the command should be `suggestedfix` or just `fix` or `quickfix`?
Also this applies all changes to a file, does not allow for selective fixes.
Updates golang/go#32875
Change-Id: I8b75f9824be82974f6edb7c03383b4d56116943c
GitHub-Last-Rev: 070fcda33ac3494bfe8f19c2cd78c089c713ed98
GitHub-Pull-Request: golang/tools#174
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202480
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When proposing packages to import, we can propose more relevant packages
first. Introduce that concept to the pkg struct, and sort by it when
returning candidates.
In all cases we prefer stdlib packages first. Then, in module mode, we
prefer packages that are in the module's dependencies over those that
aren't. We could go further and prefer direct deps over indirect too,
but I didn't have the code for that handy.
I also changed the alphabetical sort from import path to package name,
because that's what the user sees first in the UI.
Updates golang/go#31906
Change-Id: Ia981ee9ffe3202e2a68eef3a36f65e81849a4ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204203
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
go/parser has switched from reporting no position for the end of a
broken file to reporting an invalid position. This broke on of our tests
that contains broken code. Change the test case as a result.
Change-Id: I4feb7790539994e593c56d5ae84929364c1eec1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204202
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When our expected type is a named type from another package, we now always
search that other package for completion candidates, even if it is not currently
imported.
Consider the example:
-- foo.go --
import "context"
func doSomething(ctx context.Context) {}
-- bar.go--
doSomething(<>)
"bar.go" doesn't import "context" yet, so normally you need to first import
"context" through whatever means before you get completion items from "context".
Now we notice that the expected type's package hasn't been imported yet and give
deep completions from "context".
Another use case is with literal completions. Consider:
-- foo.go --
import "bytes"
func doSomething(buf *bytes.Buffer) {}
-- bar.go--
doSomething(<>)
Now you will get a literal completion for "&bytes.Buffer{}" in "bar.go" even
though it hasn't imported "bytes" yet.
I had to pipe the import info around a bunch of places so the import is added
automatically for deep completions and literal completions.
Change-Id: Ie86af2aa64ee235038957c1eecf042f7ec2b329b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201207
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Closing over the checkPackageHandle creates a cycle that forces the
checkPackageHandle not to be garbage collected until the value is
created. If a value is never created, the handle will not be collected.
Change-Id: I0f94557da917330ebe307a0e843b16ca7382e210
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204079
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change encompasses the refactorings needed to correctly implement
CL 204079. The goal of this CL is to make the actual relevant diffs more
clear.
Change-Id: I38acfd436e2380be790910e01b6e37d8280e9100
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
CL 203820 removes an assumption in go/build that srcDir is in the main
module, since in general it need not be. That requires the use of some
other mechanism for callers to communicate the correct location of the
main module.
Fortunately, we already have a WorkingDir field on the ProcessEnv
struct that does exactly that. We can simply propagate it through if
the corresponding field is present on go/build.Context.
Updates golang/go#34860
Change-Id: Idabf9ae06d8383a72772d5a589fae1d10f206c01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203857
Reviewed-by: Heschi Kreinick <heschi@google.com>
This adds support for calling import from the gopls command line,
e.g.
$ gopls imports -w ~/tmp/foo/main.go
Optional arguments are:
-w, which writes the changes back to the original file; and
-d, which prints a unified diff to stdout
With no arguments, the changed file is printed to stdout.
Updates golang/go#32875
Change-Id: I12f980d977fe12c16e51b024c9dd28c33ba6c002
GitHub-Last-Rev: c3fdd90e25204e7a12a94e9dfde389b7674e7e6d
GitHub-Pull-Request: golang/tools#176
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202624
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change eliminates the need for the importer struct. We should no
longer need the "seen" map for cycle detection. This is because
go/packages will not return import maps with cycles, and we fail in the
Import function if we see an import we do not recognize.
Change-Id: I06922c74e07eb47ce63b56fa2ac2099e7fc8bd8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This adds (or makes exported) a convenience function for reporting diagnostics with a
node directly (which is what folks usually want).
Change-Id: Ieb7ef2703f99d3a24ba7e48a779be62a7761cd0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180237
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In cases like:
type myInt int
const (
a = 1
b myInt = 2
)
var foo myInt = <>
We now prefer "b" over "a" since b's type matches the expected type
exactly.
Change-Id: I675934761cc17f6b303b63b4715b31dd1af7cea1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202737
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now a budget of 0 disables mean unlimited and tests no longer set the
budget. Hopefully the deep completion tests will stop flaking.
Updates golang/go#34617
Change-Id: Icdff5e78dcf1cc3d3fcbf0326716b39b00f0a8c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203338
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, address minor comments that I ignored from Heschi because I am
obnoxious.
Change-Id: I99dcac38578585af2cdd951dd2b9755732ef945f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203281
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we are using the memoize package to cache analysis results, we
can use that cache for suggested fixes.
Change-Id: I42905a6fe575f49d38979d53d58ea8ec59210ae0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There is no need to cache the pass on the actionHandle, as it does not
need to be reused and does not live outside the exec function.
Change-Id: I1737271383776b35718df3475b4f888232d57ae4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, goimports half-supported vendor mode -- it searched the
module cache on some code paths and the vendor dir in others. That
seemed to work okay, probably because people happened to have a populated
module cache. In 1.14, it's much more likely that people will work
solely from the vendor directory.
In this CL we bite the bullet and fully support vendor mode. 1.14 makes
this particularly challenging by disabling list -m ... in vendor mode, and
by enabling it automatically under some circumstances. We need to mirror
that behavior, which means knowing whether we're running with 1.14, and
figuring out whether vendoring should be enabled given that. We collect
the information we need with a list -m -f query just on the main module.
If vendor mode is enabled, we throw away all the modules and replace
them with a single pseudo-module rooted at /vendor. Everything basically
works at that point.
Fixesgolang/go#34826
Change-Id: Ia4030344d822d5a4a3bbc010912ab98bf2f5f95b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203017
Reviewed-by: Bryan C. Mills <bcmills@google.com>
At tip, $GOROOT/src/go.mod is 1.14, $GOROOT/src/vendor exists, and so
vendor mode is automatically enabled. That causes golang/go#34826.
This will be fixed by my upcoming vendoring CL, but for now skip.
Updates golang/go#34826
Change-Id: I5fff51fff54cf83e6369ae76bf3b19cfb7b5ac15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203257
Run-TryBot: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We now expect a type name when in the key or value of a *ast.MapType.
I also added an extra filter to expect a comparable type for the key.
Change-Id: I647cf4d791b2c0960ad3b12702b91b9bc168599b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197439
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We don't need to worry about a package's errors unless it is the
top-level package. Also fix some fallback logic in the type error range
computation.
Change-Id: Ib26b5e25bd70193ea24ec4a197811eedf69b0e2c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202622
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
When VSCode applies its own fuzzy matching/filtering/ranking logic to
completion candidates, it can end up reordering and even omitting some
of our candidates. It is mainly a problem with deep completions (i.e.
VSCode downranks or completely hides deep completion candidates that
should be ranked at the top).
We now trick VSCode into not reordering our candidates by setting each
candidate's "filterText" to the completion prefix. This makes every
candidate look like an identically perfect match, so VSCode just
maintains the order specified by "sortText".
Note that we don't do this trick if server side fuzzy matching and
deep completions are disabled. In this case unimpeded client side
candidate filtering is necessary.
Change-Id: I677047bca12b9ce05a953016d0d89182f1fe44d6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202717
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This change attaches start timestamps to timeseries and end
timestamps to the points in each timeseries. Int64Data,
Float64Data, HistogramInt64Data, and HistogramFloat64Data
have also had an EndTime field added to keep track of the last
time the metric was updated.
What works:
* Start and end timestamps will now be attached to timeseries.
What does not work yet:
* MetricDescriptors will not have a unit attached.
* No labels will be attached to timeseries.
* Distributions will not have SumOfSquaredDeviation attached.
Updates golang/go#33819
Change-Id: I692e1676bb1e31de26c1f799b96428fc9a55d6c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203060
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This change effectively reverts CL 202039. This CL was a mistake, as it
creates a cycle. Snapshots hold CheckPackageHandles, which in turn hold
pkgs.
Change-Id: I944304cb365f0ef98b5e54ea38edea6cece40453
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202740
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
If there are no imports that need organizing, don't send the "Organize
Imports" code action.
Change-Id: Id01521edd1524fb3f7372fd787d6c90418740cf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202825
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
*ast.ArrayTypes are type expressions like "[]foo" or "[2]int". They
show up as standalone types (e.g. "var foo []int") and as part of
composite literals (e.g. "[]int{}"). I made the following
improvements:
- Always expect a type name for array types.
- Add a "type modifier" for array types so completions can be smart
when we know the expected type. For example:
var foo []int
foo = []i<>
we know we want a type name, but we also know the expected type is
"[]int". When evaluating type names such as "int" we turn the type
into a slice type "[]int" to match against the expected type.
- Tweak the AST fixing to add a phantom selector "_" after a naked
"[]" so you can complete directly after the right bracket.
I split out the type name related type inference bits into a separate
typeNameInference struct. It had become confusing and complicated,
especially now that you can have an expected type and expect a type
name at the same time.
Change-Id: I00878532187ee5366ab8d681346532e36fa58e5f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change modifies the invalidContent function to take a file change
type. This allows us to eliminate the separate invalidateMetadata
function. The logic of watching changed files is then further pushed
into the caching layer.
Updates golang/go#34218
Change-Id: Id31b3931c45ec408b6e7b4a362e00f9091ba4f70
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201221
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Originally the fuzzy matcher required a match in the final candidate
segment. For example, to match the candidate "foo.bar", the input had
to have at least one character that matched "bar". I previously
removed this requirement as it is too restrictive for deep completions
to be useful.
However, there was still some lingering final-segment favoritism in
the matching algorithm. In particular, there were penalties for not
matching the final segment's first character and for not matching the
final segment's word initial characters. However, these penalties only
made sense when we also required a final segment match. Consider this
example:
User input: "U"
Candidate "ErrUnexpectedEOF" - with only a single segment, we got big
penalties for not matching the leading "E" (since it is the final
segment).
Candidate "ErrUnexpectedEOF.Error" - "ErrUnexpectedEOF" is no longer
the final segment, so we didn't get penalties. And we didn't get
penalties for the final segment "Error" because we finished matching
after the first "U". As a result, this candidate slips through with a
higher score.
Fix by simplifying the skip penalty. Now we only penalize for skipping
the first character of the first or final segment (and the penalty is
lower). For deep completions, the first and final segment are both
"important" segments, so I think it makes sense to focus on both of
them. We don't want to penalize all segment starts because that makes
it harder to match deeper candidates where you often "ignore"
intermediate segments.
I had to adjust a few scores in the tests, but I don't think the
impact will be too big other than fixing the bug.
Fixesgolang/go#35062.
Change-Id: Id17a5c80bf0f80ce252fe990ccfbd51c1bac1c72
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202638
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove the input type option. Now everything behaves as "symbol".
We don't use the "text" or "filename" input types, and I don't foresee
us using them. Removing them simplifies the code a bit, but simplifies
the tests a lot. It was tedious to make changes to the matcher logic
because you had to fret over test failure details that didn't actually
matter because we didn't use that functionality.
Change-Id: I651debde9e63ee283d7bc3ad718d22f4b9a127c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202637
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For *ast.Ident completion requests, this checks the parent node to
see if the token begins a statement and then based on the path adds
possible keyword completion candidates. The test lists some cases where
this approach cannot provide completion candidates.
The biggest thing missing is keywords for file level declarations
Updates golang/go#34009
Change-Id: I9d9c0c1eb88e362613feca66d0eea6b88705b9b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196664
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change allows us to hanel cancel messages as they go into the queue, and
cancel messages that are ahead of them in the queue but not being processed yet.
This should reduce the amount of redundant work that we do when we are handling
a cancel storm.
Change-Id: Id1a58991407d75b68d65bacf96350a4dd69d4d2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200766
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
To avoid proposing unusable completions, such as those in modules that
need a replace statement to be usable, we need to know what module a
directory is in. That involves walking up the directory tree to find a
go.mod file, which is expensive to do over and over. Really, we just
need to check if the directory we're in has a go.mod file, then use the
parent dir's results.
Add module information to the cache and use it when figuring out what
module a dir is in.
Change-Id: Ia74ba9b37d73fca5e6786a94c73c8fd71b591645
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202541
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A continuation of CL 202298, only for analysis errors.
Change-Id: I957d52cef31938ef66be73463e92695a5b56869c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202540
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Since a user's module cache is generally going to be much bigger than
their main module, one would expect that caching just information about
the module cache would be sufficient. It turns out that's not correct.
When we discover something in the module cache, we have to make sure
that a different version of it isn't already in scope. Doing that can
require information about the main module or replace targets, so that
needs to be cached too.
Concretely, when I'm working in x/tools, if a scan discovers a version
of x/tools in the module cache, it should usually ignore that version.
But that might not be true in more complicated cases, especially those
involving nested modules whose boundaries change.
So, cache everything except GOROOT. Since the new data is mutable,
we store it separately from the module cache data so that it can be
discarded easily between runs.
Change-Id: I47364f6c0270fee03af8898fec6c85d1b9c8d780
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202045
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Scan most sources, including GOPATH, the module cache, the main module,
and replace targets as appropriate. Use the cached stdlib instead of
scanning GOROOT.
We heavily cache the contents of the module cache, so performance is
decent. But we have to look at all the modules not in the module cache
too to get the right versions of modules (see
(*ModuleResolver).canonicalize), which currently isn't cached at all,
even just for a single run. That ends up being pretty expensive.
The implementation changes are relatively small; add package name
loading to scan(), cache that result, and allow callers to control what
directories are scanned so that it can skip GOROOT.
I also cleared out most of the stdlib from the unimported completion
test and added a simple external completion to it for safety's sake.
Change-Id: Id50fd4703b1126be35a000fe90719e19c3ab84bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199178
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function was removed in CL 202298, but used in CL 200597.
Analysis diagnostics should be converted to the source.Error type in the
analysis runner as a complete fix, but this is fine for now.
Change-Id: Ie5f3f566719073d7df6ab4646f855c9f9ce22ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL adds support for "related information", which allows
associating additional source positions and messages with a
diagnostic.
Change-Id: Ifc0634f68c9f3724b6508dc6331c62c819a24f78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200597
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a source.Error type which is used to collect the error
information that comes out of the loading, parsing, and type checking
stages. We also add specific sources per-error, rather than having them
all be labeled as "LSP".
This change will enable follow-ups that do a better job of extracting
error ranges.
Change-Id: I3fbb5e42d66aa2c5bb1b2f41d1eadfc45f3a749b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We were caching the package before setting the handle, so a
checkPackageHandle might be in use before it was fully created.
Change-Id: Ic98e7c351cbed5e4caa87098e95ad04d4f54f8df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202040
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A package really should always be associated with its snapshot rather
than its view. This eliminates some extra parameters in a few utility
functions.
Change-Id: I60f9b7286e0072d3268602f6bd32052a3d2e5559
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202039
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Before this commit, when running imports.Process concurrently, the program
panics with a fatal error due to concurrent map iterations and map writes.
This CL fixes this by adding a copy of the map to the packageInfo structure.
Fixed#34895
Change-Id: If009e6108813f86495c7e20e69739186b8b236d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200865
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Currently array and slice literals don't work very well for
completion. When go/parser is not expecting a type, it often turns
array types (e.g. "[]int") into *ast.BadExpr, which messes up
completion because we can't figure out the prefix from *ast.BadExpr,
and *ast.BadExprs don't get type checked.
This change addresses the first problem of not being able to figure
out the prefix. If we see an *ast.BadExpr, we now blindly try to
reparse it as a composite literal by adding on "{}". If we end up with
an *ast.CompositeLit with an *ast.ArrayType "Type", we swap
the *ast.BadExpr for the *ast.ArrayType. This approach is dumb but
simple, and fixes lexical completions in array types.
Change-Id: Ifa42e646bcbf2a30170d73e6dd11982384d40b43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197437
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There was a regression where gopls would not type-check any package with
a bad import. This change fixes the regression and adds a test to make
sure it doesn't happen again.
Change-Id: I3acf0917d46e9444c20135559f057f0ecd20e15b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change adds support for converting HistogramFloat64Data and
HistogramInt64Data to *wire.Metric. Timestamps are not attached
as they are not yet available.
What works:
* convertMetric will now convert HistogramInt64Data and
HistogramFloat64Data to *wire.Metric.
What does not work yet:
* StartTime and EndTime will not be attached to timeseries and
points.
* MetricDescriptors will not have a unit attached.
* No labels will be attached to timeseries.
* Distributions will not have SumOfSquaredDeviation attached.
Updates golang/go#33819
Change-Id: Iee52ab751542ee1ade07ef32120de853b41fd27b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200538
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This is specifically necessary to test CL 197879.
Change-Id: I2b4bbdd322d52097fc1444242d3e26a3d8ea75e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201520
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We now continue deep completion search across function calls. The
function must take no arguments and return a single argument. For
example, when completing "fo<>" you might get candidates such as
"foo.bar().baz()".
Previously we would stop searching for deep completions when we hit a
function call. For example, we would stop at "foo.bar()", never
finding "foo.bar().baz()". At the time I was worried about the search
scope growing too large, but now that we dynamically limit the search
scope there isn't much left to worry about.
Change-Id: I48772c154400662876682503c1f58ef6e3dca688
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201222
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Previously we unconditionally qualified literal candidate types with
their package. For example:
var buf *bytes.Buffer
buf = &bytes.Bu<>
would complete to:
buf = &bytes.bytes.Buffer{}
Now we don't qualify the type if the cursor position is in the
selector of an *ast.SelectorExpr. We only generate literal candidates
for type names, so if we are in a selector then we can assume it is a
package qualified type (as opposed to an object field).
We also handle the insertion of "&" for literal pointers better. If you are in
the selector of an *ast.SelectorExpr, we prepend the "&" to the beginning of the
expression rather than the selector. For example, you will end up with
"&bytes.Buffer{}" instead of "bytes.&Buffer{}".
Updates golang/go#34872.
Change-Id: I812aa809cd4e649a429853386789f80033412814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now we offer completion candidates for labels when completing "break",
"continue", and "goto" statements. We are reasonably smart about
filtering unusable labels, except we don't filter "goto" candidates
that jump across variable definitions.
Fixesgolang/go#33987.
Change-Id: If296a7579845aba5d86c7050ab195c35d4b147ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Recently been noticing errors where we don't have full metadata for a
given package. It seems to me that, since we added the context to the
packages.Config, there have been cases where the context is canceled on
the first load, and then we type-check with incomplete data. I'm still
not sure if allowing go/packages to be canceled is the correct approach.
Change-Id: I6767ce763538bd579458c8f8db07f15c9eec7b4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: Michael Matloob <matloob@golang.org>
This span resulted in misleading information, since it would appear any
time you called the Check function. This function often returns cached
results, so it's not particularly useful.
Change-Id: I26be652a99b906fb2e8703ff9af86fbe2ef5fc5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201219
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The -rpc.trace flag in gopls enables logging in "lsp inspector format".
Currently sent responses isn't parsed by the lsp inspector so it fails to
parse gopls logs.
The lsp inspector regexp matcher requires the duration to be prefixed with
"took" instead of "in", e.g. "took <d>ms.".
This change updates gopls to match the log parser in lsp inspector,
see 9aff7a6939/lsp-inspector/src/logParser/rawLogParser.ts (L88)
Change-Id: I3696faf34ba4f0b3d4e205693eaf378941f2f68f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Analyzer names are not guaranteed to be unique.
Change-Id: I4d4cc9fa746cbfbea9926f2cae0eb5dfc41027a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201217
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change makes sure that actionHandles are cached within the
snapshot, to minimize the cost of re-computing action handles on each
run of go/analysis.
Change-Id: If6191c5224285eb207aebb997787ea551a47fbaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change moves to the approach of caching the analysis using the
memoize package. This means that we will do less work, as we no longer
need to recompute results that are unchanged. The cache key for an
analysis is simply the key of the CheckPackageHandle, along with the
name of the analyzer.
Change-Id: I0e589ccf088ff1de5670401b7207ffa77a254ceb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Some notifications, like exit, have no other arguments, a case the code
was not handling correctly.
Change-Id: Ib5ff536352ba6ad0d09cd80c9d5f0a987abbe500
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200298
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Modifies the various Subscriber signatures to take in
an "at" time.Time field, which captures the time that
a measurement was made. This field will then be used
when exporting points as their EndTime.
Fixes#34490.
Change-Id: I9e87b65e374876ae3d4df0abedb64bb7dcb221b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199577
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Include the context when invoking go/packages.Load so it is
cancelable. Otherwise if the user is manually typing an import or
otherwise messing around with imports, a big queue of potentially very
slow go/packages.Load calls will build up.
Fixesgolang/go#34414.
Change-Id: I80732086b478b908c720739708dd773e81fe2b81
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This will prevent us from panicking in cases with errors.
Fixesgolang/go#34824
Change-Id: I02c20655f6926ec00c1591a905ff5a107cc44192
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200300
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We don't want to support sub line edits or non line context in the unified diff
printing.
Change-Id: I4267b11b50f12d817dac0fbbae7e1db44ca3dad0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199739
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
also improve the actual verification of the unified diff
Change-Id: I9c23c24e1fc8571cce2a7879463659ec7069fe99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199738
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Test is broken by recent cmd/go vendoring changes. Skipping the test
case while we figure out what to do.
Updates golang/go#34826
Change-Id: I31a6f18e40420970251e25836edcf32c56c10ef5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200299
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Histogram data still needs to be converted and
timestamps are not attached as they are not yet available.
What works:
* convertMetric will now convert Int64Data and Float64Data.
What does not work yet:
* Histogram64Int and Histogram64Float will still not be converted.
* StartTime and EndTime will not be attached to timeseries and points.
* MetricDescriptors will not have a unit attached.
* no labels will be attached to timeseries.
Updates golang/go#33819
Change-Id: I65f9af716ba6282e809d0a9d10777d70475e4c83
GitHub-Last-Rev: 10820a9971e1f4c0529fadc567b2533256c2e961
GitHub-Pull-Request: golang/tools#170
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199857
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
We already accepted it as a valid input, but for output we still produced
last_line,last_column+1
Also add package docs and improve error messages
Change-Id: I017aa326e8392134e67e8fabec47e750c8055454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199641
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We used to return an error from textDocument/highlight if the cursor
wasn't over an identifier. Logging these errors is not useful, as the
cursor is often not on an identifier.
Change-Id: Ibb43908149315c72923a22bdca567aa2b3ee68d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This should be a faster but equivalent implementation.
Change-Id: I7bc756644c601b953ba7715e093bfa10ca5ea97b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Make sure everything is documented and move things to sensible files now all the
cross package shuffling is done
Change-Id: I884053a207d6741cda066afa5da91b00f1dfd31c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198877
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The only exposed symbol is now the ComputeEdits function, all other functionality
is now moved up to the diff layer and using edits instead of operations.
Change-Id: I149e4f3276592e1a7c2c52e6eaffc826cc22a9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198518
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
It is now in the diff package written in terms of edits, instead of the myers package.
This also means that the unified handling is no longer pluggable because it does
not need to be.
Change-Id: I7141b023e95ed0c1d21cbc81c7420c117fc5ef1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198517
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This now checks at the diff.TextEdit layer rather than myers.Op
Change-Id: I706657a6c5705f0ad7109aa81f4dce174dda5f2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This makes it so the diff package is depended on by all implementations, rather
than the diff package having to depend on the default myers implementation.
Change-Id: I04b9caee6ff1017fa8e5476a7434e4b0e17753c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198379
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change includes dependencies in the cache keys for
CheckPackageHandles. This should fix the issue with propagating results
to reverse dependencies.
Updates golang/go#34410
Change-Id: I025b7f0d6b0dcaa89c3461ebd94eadd35de4955f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We only need one implementation of this, it must cope with all inputs, and it
has no freedom in it's results, so it does not need to be pluggable.
Change-Id: I6fec0c339eb288649a670fc3e2cb00c726467e20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198377
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Looks like I dropped this line accidentally after resolving a merge
conflict. I restored the original code checking the "deep" option in
addition to my intended drive-by fix of also checking the "fuzzy"
option. In either case the client should not cache completion
candidates.
Change-Id: I586daa28e3e4e4cc64665ba507245be4e91b08f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198490
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In cases like "fmt.Pr<>int()" we previously would replace "Print" with
the new completion, yielding for example "fmt.Println()". Now we no
longer overwrite, yielding "fmt.Println()int()". There are some cases
where overwriting the suffix is what the user wants, but it is hard to
tell, so for now stick with the more expected behavior of not
overwriting.
Fixesgolang/go#34011.
Change-Id: I8c3ccd8948245c27b52408ad508d8e01dc163ef4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196119
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change does not complete the work to handle snapshots correctly,
but it does implement the behavior of re-building the snapshot on each
file invalidation.
It also moves to the approach of caching the FileHandles on the snapshot,
rather than in the goFile object, which is now not necessary.
Finally, this change shifts the logic of metadata invalidation into the
content invalidation step, so there is less logic to decide if we should
re-load a package or not.
Change-Id: I18387c385fb070da4db1302bf97035ce6328b5c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197799
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The log code left out the case where a notification had no parameters at all.
Change-Id: I1d832edb7b7e85422ef6baba1e05286e69dd0cde
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197724
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This allows us to run a single failing test easily when we need.
It also improves the ability to determine which test fails.
Change-Id: I400212d8c4d8c1f97059260add635ce2015990a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The loops are common to all the testing layers, so lift them.
This prepares for more test improvements, without any funcitonal changes.
Change-Id: Ib750c8a7bb4c424a185cb0bd841674a69db1385b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This makes it much easier to keep them up to date.
It is also less fragile against accidental changes.
Change-Id: If119f8527c0896d210650859960e77f3e0fa5a99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197505
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 192137 deep fuzzy matching was enabled by default. We also have
options independent options "deepCompletion" and "fuzzyMatching" to
control this. When fuzzy matching is disabled, case insensitive prefix
matching is used.
Provide an option, "caseSensitiveCompletion", which allows for case
sensitive prefix matching when fuzzy matching is disabled.
Change-Id: I17c8fa310b2ef79e36cc2f7303e98870690b5903
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change uses the missing imports detection to return diagnostics and
warning messages to the user.
Updates golang/go#34484
Change-Id: If7bb7e702b8bdbd7e1ad5e26f93acc50d16209b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now we always expect type names inside of *ast.FieldList. This expands
the previous func signature logic to also work for *ast.StructType
and *ast.InterfaceType. For example, we will now prefer type names in
cases like:
type myStruct struct { i i<> }
Also, fix a check for anonymous fields to make sure the field is
actually embedded. This fixes cases like this to properly have no
completions:
type myStruct struct { i<> i }
where this will still give type name completions:
type myStruct struct { i<> }
I introduced a new error type source.ErrIsDefinition so source_test.go
could avoid erroring out on tests that make sure definition
identifiers have no completions.
Fixesgolang/go#34412.
Change-Id: Ib56cb52af639f2e2b132274d1f04f8074c0d9353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196560
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix objects defined in the function signature to only be completable
inside the function body. For example:
func (dog Dog) bark(d<>) { // Don't complete <> to "dog".
d<> // Do complete <> to "dog".
}
Change-Id: Ic9a2dc2ce6771212780f2d6af2221a67d203f35f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196559
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This prevents piping back to the file, a common pattern.
Multi file forms should use the unified diff.
Change-Id: I1ea140c59de24feb74a64b0cb41890536f23cd3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197157
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Race condition: I deleted the code that acquired the mutex when checking
if a file's imports have changed.
Merge conflict: I submitted a change without rebasing and re-running TryBots.
Change-Id: Iae351f2fff893cfd94db79d9f79578d9827a8c21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197297
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
source.DiagnosticSeverity and source.CompletionItemKind are duplicated
and not worth maintaining.
Change-Id: I8d6c8621a227855309c0977da59d8c9fa53617bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Normally you don't want literal candidates for basic types (e.g.
"int(0)") since you can type the literal value without the type name.
One exception is if you are creating a named basic type that
implements an interface. For example:
http.Handle("/", http.FileServer(<>))
will now give "http.Dir()" as a candidate since http.Dir is a named
string type that implements the required interface http.FileSystem.
Change-Id: Id2470c45e469ea25cd0f9849cfdad19ac0e784bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195838
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Suggested fixes was totally broken (invalid command) and many others were not in
correct sorted order.
There were lots of golden entries that were no longer used.
The regeneration script itself was broken
The definition tests are skipped, so the entries were not regenerated.
Files must have been hand edited, we probably need to document how to generate
them somewhere.
Change-Id: I1c021aeadd81f08f4572c2124f0c61912a3cd89e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196987
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I was messing around with the new diagnostic tags feature and
had to update to get it to work.
Change-Id: I4294513b460ec4806d23af20bf908cee8673f7c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
This change moves from caching package information the file object to
caching in a map that gets invalidated when content changes.
This simplifies cache invalidation and reduces the number of fields
guarded by the (*goFile).mu lock.
Change-Id: I33fef6e0b18badb97e49052d9d6e3c15047c4b63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196984
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Instead of relying on the diagnostics cached on the package, use the
diagnostics sent by the code action when computing suggested fixes.
Change-Id: I77f7fd468b34b824c6c5000a51edbe0f8cc6f637
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197097
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our completion tests check for a lot of different behaviors. It may be
easier to develop if we have separate tests for things like deep
completion and completion snippets.
Change-Id: I7f4b0c0e52670f2a6c00247199933fd1ffa0096f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196021
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Currently, we cache source.CheckPackageHandles for each file and package
that we are aware of, as well as dependencies. This is not necessary,
since the active packages pin their imports in memory.
Change-Id: Ia0101f4d4a2d36d5baeb890af3d7c8baec297847
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196982
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This allows them to be run from the gopls module as well to test
the code with the hooks installed.
Change-Id: I3079a04ffe3bd221ccc2523e746cbed384e05e2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196321
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change shifts our approach to make sure that a top-level package
only ever imports "trimmed" packages.
Change-Id: I63c35791ef6efad7dac248a9ff877835f46df9ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196523
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This might not be necessary after we fix handling for line directives,
but it's always better to avoid the panic here.
Updates golang/go#34433
Change-Id: Ica4fb571dff6753fb15bf8d397c55f713284aa27
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196662
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will allow view configuration to modify the set of analyzers being applied, and also allow the main gopls to inject new analyzers
Change-Id: Ic2a76118c3e29b059e19b31bd1fb54b1d9e15e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196320
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit adds support for calling rename from the gopls command
line, e.g.
$ gopls rename -w ~/tmp/foo/main.go:8:6
$ gopls rename -w ~/tmp/foo/main.go:#53
Optional arguments are:
- -w, which writes the changes back to the original file; and
- -d, which prints a unified diff to stdout
With no arguments, the changed files are printed to stdout.
It:
- adds internal/lsp/cmd/rename.go, which implements the command;
- adds "rename" to the list of commands in internal/lsp/cmd/cmd.go;
- removes the dummy test from internal/lsp/cmd/cmd_test.go; and
- adds internal/lsp/cmd/rename_test.go, which uses the existing
"golden" data to implement its tests.
Updates #32875
Change-Id: I5cab5a40b4aa26357b26b0caf4ed54dbd2284d0f
GitHub-Last-Rev: fe853d325ef91f8f911987790fcba7a5a777b6ce
GitHub-Pull-Request: golang/tools#157
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This changes adds basic support for running `go mod tidy` as a code
action when a user opens a go.mod file. When we have a command
available like `go mod tidy -check`, we will be able to return edits as
part of the codeAction. For now, we execute the command directly.
This change also required a few modifications to our handling of file
kinds so that we could distinguish between a Go file and a go.mod file.
Change-Id: I343079b8886724b67f90a314e45639545a34f21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196322
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change encodes an invariant that, a dependency package will only
ever be parsed with trimmed ASTs.
Updates golang/go#34410
Change-Id: I2ceab3672c0bae0b98cec2a8e60b92a0c01a900f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In it previous implementation, tool.Main was meant to be called from
an applications main(). If it encountered an error it would print the
error to standard error and exit with a non-zero status (2).
It is also called recursively and in various test functions.
Exiting on an error makes testing difficult, unnecessarily.
This change breaks the functionality into to parts: an outer
tool.MustMain() that is intended to be called by main() functions and
an inner Main that is used by MustMain() and by test functions.
None of the existing test functions use Main()'s error value, but the
failure case tests for the command line invocation of rename (#194878)
require it.
Fixes#34291
Change-Id: Id0d80fc4670d56c87398b86b1ad9fdf7a676c95b
GitHub-Last-Rev: cd64995c91c94b997754c8d8b1004afc488bf8b7
GitHub-Pull-Request: golang/tools#159
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195338
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were recursing infinitely in cases like this:
switch true {
case true:
go foo.F<>
}
There were three things that came together to cause this:
1. We recently starting recursively fixing broken go/defer statements.
2. In this case we were failing to swap in the correct ast.Node in for
the *ast.BadStmt because we were only looking
for *ast.BlockStmt (and *ast.CaseStmt has no block).
3. After 2), we weren't returning an error so the fix() code thought
it should recurse.
Fix 2) by using reflection to swap AST nodes in a generic way. Perhaps
a bit overkill in this case, but I happened to have already written
this for an upcoming change, so I just pulled it in to fix this bug.
Fix 3) by returning an error if we fail to swap the AST nodes.
Fixesgolang/go#34353.
Change-Id: I17ff1afd52ae165c0ba9de5820dcec4cb7d756cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now we will offer function literal completions when we know the
expected type is a function. For example:
sort.Slice(someSlice, <>)
will offer the completion "func(...) {}" which if selected will
insert:
func(i, j int) bool {<>}
I opted to use an abbreviated label "func(...) {}" because function
signatures can be quite long/verbose with all the type names in there.
The only interesting challenge is how to handle signatures that don't
name the parameters. For example,
func HandleFunc(pattern string, handler func(ResponseWriter, *Request)) {
does not name the "ResponseWriter" and "Request" parameters. I went
with a minimal effort approach where we try abbreviating the type
names, so the literal completion item for "handler" would look like:
func(<rw> ResponseWriter, <r> *Request) {<>}
where <> denote placeholders. The user can tab through quickly if they
like the abbreviations, otherwise they can rename them.
For unnamed types or if the abbreviation would duplicate a previous
abbreviation, we fall back to "_" as the parameter name. The user will
have to rename the parameter before they can use it.
One side effect of this is that we cannot support function literal
completions with unnamed parameters unless the user has enabled
snippet placeholders.
Change-Id: Ic0ec81e85cd8de79bff73314e80e722f10f8c84c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193699
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This was causing microsoft/vscode-go#2749, which was a result of users
having folders named "go", and VSCode-Go returning the "go"
configuration settings (which are all of its settings, not those of
gopls).
Change-Id: Ifbde3e32ad2de79265ed6adea53588c730ecd716
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When running in GOROOT/src, `go list -m all` shows the std package (or
cmd package) as the main module. This confuses goimports into adding
std/ or cmd/ at the beginning of import paths. Skip canonicalization for
paths under GOROOT.
Fixesgolang/go#31814
Change-Id: Iff5cc7e2a2053e4cc87c1a579a4c47d856cd0a2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195063
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add support for literal completion candidates such as "[]int{}" or
"make([]int, 0)". We support both named and unnamed types. I used the
existing type matching logic, so, for example, if the expected type is
an interface, we will suggest literal candidates that implement the
interface.
The literal candidates have a lower score than normal matching
candidates, so they shouldn't be disruptive in cases where you don't
want a literal candidate.
This commit adds support for slice, array, struct, map, and channel
literal candidates since they are pretty similar. Functions will be
supported in a subsequent commit.
I also added support for setting a snippet's final tab stop. This is
useful if you want the cursor to end up somewhere other than the
character after the snippet.
Change-Id: Id3b74260fff4d61703989b422267021b00cec005
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193698
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The sortslice analyzer warns the user when they're
using a non-slice parameter to sort.Slice. It can
also suggest fixes for 3 different cases of misuse:
1. The parameter is a pointer to a slice.
2. The parameter is an array
3. The parameter is a function with no arguments
that returns a slice.
Change-Id: If6327b19ab3c476cf17ca3c5fd75bc43bb8683e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195898
Run-TryBot: Johan Brandhorst <johan.brandhorst@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If we encounter `go list` errors when loading a user's package, we
should try to see if they've encountered any of our common error cases.
They are: 1) a user has GO111MODULE=off, but is outside of their GOPATH,
and 2) a user is in module mode but doesn't have a go.mod file.
Fortunately, go/packages does a great job handling edge cases so gopls
will work well for most of them. The main issue will be unresolved
imports. These show up in DepErrors in `go list`, so go/packages doesn't
propagate them through to the list of errors. This will require changes
to go/packages.
Updates golang/go#31668
Change-Id: Ibd5253b33b38caffeaad54a403c74c0b861fcc14
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Parse errors need to be treated separately from actual errors when
parsing a file. Parse errors are treated more like values, whereas
actual errors should not be propagated to the user. This enables us to
delete some of the special handling for context.Canceled errors.
Change-Id: I93a02f22b3f54beccbd6bcf26f04bb8da0202c25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195997
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Also, we were repeating the same code for diagnostics in 4 places.
Extract into a function.
Change-Id: I18d00e512a67679b915347a61126970cf02a7a4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196019
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We had too many options for functions to use to get type information for
a package. Now we stick with having one option to get the check package
handles, and then the caller can refine the results as needed.
Change-Id: I81f69a670e1539854ee23b6f364159a7de9b782f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is to remove the confusion around having only handles that have had Get
called pin the value into memory.
Instead now there is a single handle per key, and it is the handle that is
weakly held not the value.
Change-Id: I9e813a0dfe2adf4cb651af9b5cfc8878fa71c041
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186839
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function incorrectly used cached packages to get ASTs and type
information that should have been directly found from the origin
package. Shift to using pkg.FindFile instead.
Change-Id: I9f73209bb1a1343f53b430150e78ffd180e14a44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Tweak a couple things to improve how we reduce our search scope based
on remaining time budget:
- Check our budget on the first candidate rather than waiting for the
1000th candidate. If type checking is slow you can be out of budget
before you even begin.
- Reduce our budget check interval from 1000 candidates to 100
candidates. This just helps us adjust our search scope faster.
The first tweak required me to raise the completion budget for tests
because 100ms is not always enough. I moved the budget into the
completion options so that tests can raise it.
Change-Id: I1aa7909d7baf9c998bc830c960dc579eb33db12a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195419
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Revert my previous change to include fuzzy matches with a score of
zero. Zero scorers have some characters that match, but they are
pretty poor overall. Pulling in all the extra junk candidates was
slowing things down in certain cases.
Change-Id: I560f46903281f21b0628c9b14848cddf1e3c0a38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195418
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change allows to remove some of the special handling for the
builtin package.
Change-Id: I105fcefd8812af2d42ff42edca954824c98db429
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195758
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A mapper is always uniquely tied to a file at a specific version, so
just build it when we get a new *ast.File. We build the mapper using the
*token.File associated with the particular *ast.File, which is why there
is one per ParseGoHandle instead of FileHandle.
Change-Id: Ida40981ef91f6133cdd07e9793337fcd67510fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Previously we would type check all package handles as we fetched them.
This meant if you only cared about a file's primary package you would
still have to wait for all its packages to be type checked. For
example, when completing in foo.go, you would wait for [foo.go] and
[foo.go, foo_test.go] to be checked (the latter being the test
variant).
Now we don't type check packages as we put together the package
handles.
Change-Id: Ibca0c6b34cf4f0a07bcdeb62959d60158f4ccf17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I wasn't sure if we should just manually construct the URI here or use
the URL unescaping function. Let me know which you think is best.
Updates golang/go#34270
Change-Id: Idb48fc2650d39f3e54cac141a70f356c31e303ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195618
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Changes go.ts to check that the commit hash of the vscode is the one that it
is expecting. README.md now contains more explanation.
Change-Id: Ia5a947c6d5d026c2b7d9ab18877c320e8a7f45d2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195438
Reviewed-by: Ian Cottrell <iancottrell@google.com>
protocol.Position doesn't implement String(), so don't use "%s"
formatting directive. We could implement String(), but it is generated
code, so its a bit iffy.
Change-Id: If21b5fee66c6e1bd59f19b520c7d36f0e648cb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195042
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
These versions of go.ts and requests.ts generate tsprotocol.go,
tsserver.go, and tsclient.go. README.md now gives the version of the
vscode source used, showing that the typescript code and tsprotocol.go
are matched to the same git commit of vscode.
Many of the diffs are just whitespace from vscode's formatting.
Fixesgolang/go#34225
Change-Id: Ib66dad9476b452f332a4c0e990faf2c6060a588e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195297
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We turned them off for the release, but let's keep them on at master so
that we can get more feedback.
Change-Id: Iaadf27d59ef925ba8ee2bc02acbb9c77c2935ce2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195059
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This was causing tests to report that they required a binary
called "off".
Change-Id: I592109029c86af20d48d2ee9989d4a25fdb11b3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194978
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Previous changes to the config mechanism made the config options
per-view, not per-session. We should now make sure to obey config
changes per-view. This does not fix the configuration handling for
"watchChangedFile" however. This should be done in a future CL.
Change-Id: I73f6236386c36d2587fdb9c0601670833a4366c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This fixes the issue of config options not being applied.
Also, handle config errors and deprecation by showing a message to the
user.
Change-Id: I850d5303a7a1e301c97324060a87929710ee6700
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194682
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change temporarily disables fuzzy matching and deep completion by
default, leaving them opt-in for the tagged versions. These will be
re-enabled after the tag on master to continue iterating on them.
Also, update the hard-coded version string.
Change-Id: I0aa688ce067abfe8ae8ebe52a25c8514ec2c7e48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194777
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, handle *ast.StarExpr in the identifier code. This fixes a specific
case with deep completions and documentation.
Change-Id: I630ae4e8f1c123ba1fdea85e6862ae93396e2cd4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194564
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Go 1.13 introduced a module in GOROOT/src. That triggered goimports to
think that it was an invalid module, usable only as a replace target,
because its declared module path "std" didn't match its apparent path
"src". The stdlib is always in scope, so skip the needs-replace check
for GOROOT.
Fixesgolang/go#34199
Change-Id: I1199378b940cfedc04e9a4e943c46b9ffdf18446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194570
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our original caching plan was to use only the file ParseGoHandles as
cache keys to define a given package. However, because of package test
variants, we cannot rely on files alone. A package may have the exact
same set of files, but be a test variant. Add the ID to the key to avoid
clobbering entries in the cache.
Also, remove the unused metadata ID cache.
Change-Id: I4b33de1f83f6c769d23441e98a2a7324ff0fa1b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194571
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In api/*.txt, interface declarations are represented with lines like:
pkg container/heap, type Interface interface { Len, Less, Pop, Push, Swap }
or, when they have no exported methods:
pkg go/ast, type Expr interface, unexported methods
The latter form confuses mkstdlib into thinking that it's a method
because of the extra comma, and then it skips the interface entirely.
Running this program is a matter of seconds once per release, so rather
than trying to fix the optimization, just remove it. The parsing logic
doesn't care about the extra lines.
And the corresponding change to the copy in lsp/testdata/unimported.
Updates golang/go#34199
Change-Id: Ic34b8a47537608401e4ef6683617d797f9f50f8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194568
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Now when a file is deleted we force the file's packages to refresh
go/packages metadata, and kick off diagnostics.
I made a couple other changes to watched file handling:
- Kick off diagnostics in a goroutine to match how DidChange works.
This will allow us to work through big sets of file changes faster,
and will save duplicated work once type checking can be canceled.
- Don't assume a watched file is only part of one view.
Two interesting cases we don't handle yet:
- If the deleted file was the only file in the package, we don't
currently update diagnostics for dependent packages. This requires
rejiggering how diagnostics are invoked a bit.
- If the deleted file is still open in the editor and then later
closed, we don't trigger metadata/diagnostics refresh on DidClose.
Updates golang/go#31553
Change-Id: I65768614c24d9800ffea149ccdbdbd3cb7b2f3d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193121
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
And the corresponding change to the copy in lsp/testdata/unimported.
Change-Id: I604ae6d5217356e19bb18f7cbe69a8dd71e5f23e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194567
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Now when trying to fix *ast.BadStmt, we parse the manually extracted
expression using parser.ParseFile instead of parser.ParseExpr.
ParseFile will yield *ast.BadStmt nodes for any bad statements nested
in our first bad statement, allowing us to fix them recursively.
To turn our expression into a "valid" file we can pass to
parser.ParseFile, I wrapped it thusly:
package fake
func _() {
<our expression>
}
Change-Id: I0d4fd4ebce6450021da8e03caa11d0ae5152ea8d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194342
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We now wait to build views until we have the options for that view,
and pass the options in to the view constructor.
The environment and build flags are now part of the view options.
Change-Id: I303c8ba1eefd01b66962ba9cadb4847d3d2e1d3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In the case of documentation items for completion items, we should make
sure to use the ASTs and type information for the originating package.
To do this while avoiding race conditions, we have to do this by
breadth-first searching the top-level package and its dependencies.
Change-Id: Id657be969ca3e400bb2bbd769a82d88e91865764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
I moved the "usePlaceholders" config field on to CompletionOptions.
This way the completion code generates a single snippet with a little
conditional logic based on the "WantPlaceholders" option instead of
juggling the generation of two almost identical "plain" and
"placeholder" snippets at the same time. It also reduces the work done
generating completion candidates a little.
I also made a minor tweak to the snippet builder where empty
placeholders are now always represented as e.g "${1:}" instead of
"${1}" or "${1:}", depending on if you passed a callback to
WritePlaceholder() or not.
Change-Id: Ib84cc0cd729a11b9e13ad3ac4b6fd2d82460acd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193697
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add a new @completePartial note that does not require you to specify
the full list of completions. This gets rid of a lot of noise when you
just want to test the relative order of some completion candidates but
don't care about all the other candidates in scope.
I changed one existing test to use @completePartial as an example.
Change-Id: I56005405477e562803f094c0cac05ef2b854ad1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192657
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Remove the wantSuggestedFixes flag, and run the flagged code
by default.
Add test cases for suggested fixes.
Generate a suggested fix to the assign analysis that suggests removing redundant assignments.
Fix the propagation of suggested fixes (using rstambler's code).
Change-Id: I342c8e0b75790518f228b00ebd2979d24338be3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193265
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 193726 accidentally turned off deep completions and fuzzy matching by default.
Re-enabling them now.
Change-Id: Ia120766b3a72243efe9c398c0efd6d1a101d0e7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194020
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Improve the existing fix-the-AST code to better identify the
expression following the "go" or "defer" keywords:
- Don't slurp the expression start outside the loop since the
expression might only have a single token.
- Set expression end to the position after the final token, not the
position of the final token.
- Track curly brace nesting to properly capture an entire "func() {}"
expression.
- Fix parent node detection to work when BadStmt isn't first statement
of block.
- Add special case to detect dangling period, e.g. "defer fmt.". We
insert phantom "_" selectors like go/parser does to prevent the
dangling "." from messing up the AST.
- Use reflect in offsetPositions so it updates positions in all node
types. This code shouldn't be called often, so I don't think
performance is a concern.
I also tweaked the function snippet code so it properly expands
"defer" and "go" expressions to function calls. It thought it didn't
have to expand since there was already a *ast.CallExpr, but the
CallExpr was faked by us and the source doesn't actually contain the
"()" calling parens.
Note that this does not work for nested go/defer statements. For
example, completions won't work properly in cases like this:
go func() {
defer fmt.<>
}
I think we can fix this as well with some more work.
Change-Id: I8f9753fda76909b0e3a83489cdea69ad04ee237a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193997
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This replaces the definition of ApplyEdits to be more like that in
go vet -fix, so that we can apply the results of suggested fixes.
Change-Id: Ib5724139464954e3790bc51ed1edc3ce4b2115ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193959
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The latest version of the LSP protocol introduces a number of changes.
It is now possible to indicate partial results and progress. request.ts
had to construct some new types (at the end of tsclient.go and tsserver,go)
to avoid using a struct for a formal parameter type. Also,
instead of using the same type for many RPCs, most RPCs now have their own
types.
Change-Id: I095a3e872f42a9f851c01ca4e3c6ac6e32446042
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194177
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Downranking builtins causes weird interplay with other completion
candidates due to fuzzy matching. For example:
notNil := 123
var foo *int = nil<>
ranks "notNil" before "nil" in the builtin list, which is counter
productive.
Change it to not downrank builtins. In my testing with this change,
builtins never were ranked above lexical items with similar names. I
think this is because the "natural" order of completion items puts
builtins last, and we stable sort items by score, so their relative
order is preserved.
Change-Id: Ifbad02be205e3cb26c1d4ce500b77690e7ac5b04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193897
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The next in the sequence of CLs to convert to using protocol positions.
Change-Id: Ib3421bfc73af1b546b60c328ca66528cb9031e19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193719
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This cl is the first in a set that change the configuration behaviour.
This one should have no behaviour differences, but makes a lot of preparatory changes.
The same options are set to the same values in the same places.
The options are now stored on the Session instead of the Server
The View supports options, but does not have any yet.
Change-Id: Ie966cceca6878861686a1766d63bb8a78021259b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193726
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The existing code uses maps to associate requests with responses. This
change adds locking to avoid simultaneous and illegal reads and writes.
Change-Id: I7bfb21cad6b37ac25e4f6946cb660d82f23c2b80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193058
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Folding ranges need to be computed to present folding ranges that make
sense when lineFoldingOnly is true. This change computes the folding
ranges to include the lines that are contained within the start and end
parenthesis/braces when applicable.
Folding ranges are not returned when the contained nodes begin or end on
the same lines as the parenthesis/brace. This is to avoid misleading
folding ranges like the following in unformatted code:
if true {
fmt.Println("true") } else {
fmt.Println("false")
}
---folding "if true {}"--->
if true {
fmt.Println("false")
}
Change-Id: I2931d02837ad5f2dd96cc93da5ede59afd6bcdce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192678
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In shouldRunGopackages we would reset a goFile's metadata and pkgs in
advance of re-running go/packages. However, if we did not end up
running go/packages for whatever reason (read: we got canceled), the
goFile gets stuck in the unfortunate state of not belonging to any
packages because "pkgs" is empty. I think this leads to "no
CheckPackageHandle" errors, at least in relation to GetCachedPackage()
calls.
Fix by deferring the reset of goFile's metadata and pkgs until after
the go/packages call has succeeded.
Change-Id: I95aace85c026e1232b42cadee9e7772951c817d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193601
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change propagates the file handle through the type-checking
process, ensuring that the same handle is used throughout. It also
removes the ordering constraint that f.mu needs to be acquired before
f.handleMu. To make this more correct, we should associate a cached
package only with a FileHandle, but this relies on correct cache
invalidation, so that will be addressed in future changes.
Updates golang/go#34052
Change-Id: I6645046bfd15c882619a7f01f9b48c838de42a30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193718
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We had previously been returning the metadata map in a few of the
loading functions. We don't actually need the map; we only need the
actual metadata. The race was caused by the return of the f.meta field
in a few functions, unprotected by the f.mu lock. This was likely a
result of the f.mu lock being added after the fact.
Fixesgolang/go#33978
Change-Id: Ice5778d9d6dea23304237baf321b55d4fee6599c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193717
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If someone puts something silly in their module cache, ignore it instead
of crashing.
Fixesgolang/go#34027.
Change-Id: I114e10010bd6bc483f865a628dc2b331c3a34a11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193268
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
There are issues with contexts being propagated through the calls to
type-checking, and I think that a lot of these were related to us using
the importer's context. Instead, we should propagate the context from
the store as much as possible - only using the importer's context when
absolutely necessary (in the call to Import). This change propagates the
correct context where possible.
Updates golang/go#34103
Change-Id: I4bdc37d014ee1f775b720c9e7ad8abffffcf6ba3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193480
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This feature has been in an experimental state for a long enough time
that I think we can enable it by default at master.
Change-Id: I9bbb8b41377719f0e97f608f6e5163a883a176b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The memoize store passes a detached context to the value getter
function. This is important since if the value getter experiences a
context cancellation it will end up caching context.Canceled, which
you never want. When type checking, we were ignoring the detached
context and using the "real" request context. This would cause the
context.Canceled error to get cached and continue popping up in
various situations.
Fix by swapping the importer's context to the detached context. It is
a little messy since the importer stores the context as a field. I
added a defer to restore the original context since it doesn't seem
correct to let the detached context escape the memoize function.
Updates golang/go#33678
Change-Id: I20dd466b0072ac2e856adbe993364f77e93ab054
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192719
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
If the client registers with foldingRange.lineFoldingOnly = true, only
return folding ranges that span multiple lines. Do this as they are
computed, so that if other filtering is applied later, we do not include
ranges that would go unused by the client anyway.
Change-Id: I27ea24428d25f180e26892de0f6d16c211225bf7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192477
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
