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

207 Commits

Author SHA1 Message Date
Rebecca Stambler
3057e18543 internal/lsp: move error range computations into cache package
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>
2019-10-21 22:12:58 +00:00
Heschi Kreinick
d2fffb4b84 internal/imports: cache things outside the mod cache
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>
2019-10-21 20:45:41 +00:00
Heschi Kreinick
eb46839a96 internal/imports: return non-stdlib candidates from GetAllCandidates
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>
2019-10-21 20:45:15 +00:00
Rebecca Stambler
747b8b11d4 internal/lsp: refactor error handling code in type-checking
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>
2019-10-21 18:57:46 +00:00
Rebecca Stambler
a1005cf9b2 internal/lsp: fix race in checkPackageHandles
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>
2019-10-18 19:24:35 +00:00
Rebecca Stambler
0bbdbb2ef6 internal/lsp: associate package with its snapshot
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>
2019-10-18 19:08:19 +00:00
Muir Manders
920acffc3e internal/lsp: fix bad *ast.ArrayTypes for completion
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>
2019-10-17 20:53:01 +00:00
Rebecca Stambler
de666e9706 internal/lsp: add a test to make sure we handle bad imports
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>
2019-10-17 20:52:05 +00:00
Rebecca Stambler
f6a1a6ff8e internal/lsp: check if the go/packages context has been canceled
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>
2019-10-16 22:21:47 +00:00
Rebecca Stambler
f0068bd333 internal/lsp: remove misleading check span
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>
2019-10-16 19:48:01 +00:00
Rebecca Stambler
9c6d90b5a7 internal/lsp: use the analyzer's pointer instead of name
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>
2019-10-15 21:12:01 +00:00
Rebecca Stambler
f936694f27 internal/lsp: add analyses to the snapshot
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>
2019-10-15 15:04:14 +00:00
Rebecca Stambler
b53505e708 internal/lsp: cache analysis using memoize package
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>
2019-10-14 17:19:42 +00:00
Muir Manders
59731289e9 internal/lsp: include context in go/packages config
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.

Fixes golang/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>
2019-10-11 19:56:53 +00:00
Rebecca Stambler
638914d249 internal/lsp: return an error from {Narrowest,Widest}CheckPackageHandle
This will prevent us from panicking in cases with errors.

Fixes golang/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>
2019-10-11 16:51:06 +00:00
Ian Cottrell
5889748991 internal/lsp: use options hooks to install diff driver
Change-Id: I2f94c2a68d0036a47ccac3fce07cf9f3b784d443
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-10-11 16:38:31 +00:00
Rebecca Stambler
9901302c21 internal/lsp: use dependencies in cache keys
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>
2019-10-04 02:16:33 +00:00
Rebecca Stambler
f0a16743a2 internal/lsp: address staticcheck warnings
Change-Id: I67c689d31b61425b1eec8b4719e906ce26777759
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198441
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-04 02:16:23 +00:00
Rebecca Stambler
329c8d646e internal/lsp: delete unused code
Change-Id: Ifda9ebcd3fdbb7457b9b46380e238a6fe0081015
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198258
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-10-01 18:41:21 +00:00
Rebecca Stambler
57610eddc9 internal/lsp: rework snapshots and cache FileHandles per-snapshot
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>
2019-10-01 16:26:22 +00:00
Rebecca Stambler
ea99b82c7b internal/lsp: move the missing imports handling into the metadata
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>
2019-09-25 23:05:17 +00:00
Rebecca Stambler
125cfdbd7b internal/lsp: fix merge conflict and race condition
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>
2019-09-25 16:33:38 +00:00
Rebecca Stambler
eb7cb10de1 internal/lsp/cache: move to a model of caching in a snapshot
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>
2019-09-25 02:06:38 +00:00
Rebecca Stambler
3af8461759 internal/lsp: associate code action diagnostics with suggested fixes
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>
2019-09-24 23:33:35 +00:00
Rebecca Stambler
035fdb12d3 internal/lsp: remove unnecessary "justOpened" field from goFile
Change-Id: I00a43ba8e2bc3ddc1f270113d8e1a49c801376a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196986
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-24 02:17:48 +00:00
Rebecca Stambler
0f9bb8f614 internal/lsp: only cache type information for active packages
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>
2019-09-23 23:01:26 +00:00
Rebecca Stambler
fe7d98e288 internal/lsp: cache multiple packages depending on parse modes
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>
2019-09-23 21:39:02 +00:00
Ian Cottrell
9cb49539a2 internal/lsp: make the analyzers configurable per view
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>
2019-09-23 16:54:13 +00:00
Rebecca Stambler
1081e67f6b internal/lsp: support running go mod tidy as a code action
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>
2019-09-20 13:08:46 +00:00
Rebecca Stambler
db1d4edb46 internal/lsp: make sure that deps are only checked in trimmed mode
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>
2019-09-19 22:30:14 +00:00
Muir Manders
f68e2b6f23 internal/lsp: fix infinite recursion while fixing AST
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.

Fixes golang/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>
2019-09-19 16:27:28 +00:00
Rebecca Stambler
2c18af7e64 internal/lsp: ensure that an AST cannot be nil without an error
Fixes golang/go#34366

Change-Id: I2f5269cd0b270aef5accc69adeced8be4523cb65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196142
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-18 18:10:22 +00:00
Rebecca Stambler
1cc9451822 internal/lsp: distinguish parse errors from actual errors
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>
2019-09-17 21:21:32 +00:00
Rebecca Stambler
3b4f30a44f internal/lsp: remove helpers for getting packages
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>
2019-09-17 16:23:42 +00:00
Ian Cottrell
11bbd741f5 internal/memoize: changes to only one handle per key
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>
2019-09-17 14:40:27 +00:00
Rebecca Stambler
2dc213d980 internal/lsp: remove cachedFileToMapper function
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>
2019-09-17 03:27:47 +00:00
Rebecca Stambler
fff8d94173 internal/lsp: use ParseGoHandles for the builtin package
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>
2019-09-16 23:04:25 +00:00
Rebecca Stambler
5edc6aefed internal/lsp: reduce usage of column mapper
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>
2019-09-16 22:00:07 +00:00
Muir Manders
b31ee645dd internal/lsp: avoid unnecessary type checking
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>
2019-09-16 20:14:40 +00:00
Rebecca Stambler
c7d52e45e2 internal/lsp: use the view options, not the session options
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>
2019-09-11 22:59:40 +00:00
Rebecca Stambler
b13fa046aa internal/lsp: merge session and view options into one
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>
2019-09-11 19:36:49 +00:00
Ainar Garipov
feee8acb39 all: fix more typos
Change-Id: I978ad5e1800ebfceb78aaced438331a8341715d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194697
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-11 15:13:14 +00:00
Rebecca Stambler
3e6e7c4239 internal/lsp: add ID to the package cache key
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>
2019-09-10 22:12:34 +00:00
Muir Manders
5e3480f0e0 internal/lsp: start handling watched file deletes
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>
2019-09-10 20:08:16 +00:00
Muir Manders
3cd124fa3e internal/lsp: fix completion for nested *ast.BadStmt
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>
2019-09-10 16:55:22 +00:00
Ian Cottrell
3d22a3cfff internal/lsp: only build a view when we have its configuration
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>
2019-09-10 14:40:41 +00:00
Rebecca Stambler
238129aa63 internal/lsp: derive ASTs from type information
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>
2019-09-10 13:53:09 +00:00
Michael Matloob
75be6cdcda internal/lsp: enable suggested fixes by default
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>
2019-09-09 19:40:07 +00:00
Muir Manders
dc339cc7c5 internal/lsp: improve completions in go and defer statements
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>
2019-09-09 19:09:43 +00:00
Rebecca Stambler
cdebb59945 internal/lsp: remove the GetToken and GetAST functions
Change-Id: Iddbdde5f47a31da9baab6539cd2b5bd858e7f811
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194057
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-09 18:10:35 +00:00