We can avoid allocating strings when performing workspace symbol search
by having the fuzzy match operate directly on chunks.
When operating on a single string, this slows down the matcher slightly
(perhaps 10%) due to copying bytes rather than accessing the string
directly. We could work around this using unsafe, but this could also be
mitigated by generics.
Benchmark ("test" in x/tools): 48ms->46ms
Benchmark ("test" in kubernetes): 868ms->857ms
Change-Id: Icf0f15aaa5cc3c875cf157a7b90db801045d9ed4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338694
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Whenever possible we should avoid doing string operations when computing
workspace symbols. This CL lays the groundwork for optimizations of this
sort by changing the signature of matcherFunc to accept chunks. It is
done in a naive way though, so this doesn't yet improve performance.
Benchmark ("test" in x/tools): 40ms->48ms
Benchmark ("test" in kubernetes): 799ms->868ms
Change-Id: I171c654b914e9764cfb16f14d65ef1aed797df73
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338693
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Coupling workspace symbols to package checking means that they do not
function when the workspace is contracted, and also forces us to do
duplicate work traversing file declarations.
This CL changes the workspace symbol implementation to precompute
symbols based only on syntactic information, allowing them to function
in degraded workspace mode, improving their performance, and laying the
groundwork for more significant performance improvement later on.
There is some loss of precision where we can't determine the kind of a
symbol from syntactic information alone, but this is minor: we fall back
on 'Class' if we can't determine whether a type definition is a basic
type, struct, or interface.
Benchmark ("test" in x/tools): 56ms->40ms
Benchmark ("test" in kuberneted): 874ms->799ms
Change-Id: Ic48df29b387bf029dd374d7d09720746bc27ae5e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338692
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Before this CL, we preferred less qualified symbols in the "dynamic"
symbol style by executing multiple matches from right to left. This is
significantly inefficient for only a marginal benefit over using the
match indexes produces by the fuzzy matcher.
Instead, change the signature of the matcherFunc to expose the matched
index to compute the dynamic symbol.
Benchmark ("test" in x/tools): 144ms->56ms
Benchmark ("test" in kubernetes): 2.6s->874ms
Change-Id: I0bf017feee436bc0d8b14bdda1e64fd227669dd7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338691
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
ParseHeader mode is used to parse only the package and import declarations.
However, change of go:embed directive should also invalidate metadata.
So, we must use ParseFull mode to get all file comments to compare
old and new go:embed directives.
Fixesgolang/go#47436
Change-Id: If7cdb6741e895315bb6a6de2f207b404e15b269a
GitHub-Last-Rev: 64d606cead064ed5996eb7d55c3664940e7a1deb
GitHub-Pull-Request: golang/tools#333
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339469
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Fix connection error not being returned resulting in errors like `gopls: remote disconnected: <nil>`.
Change-Id: I67fa1143b2fa0fd44c946040fc1bad51b1636183
GitHub-Last-Rev: 9ebae5c3b85b7bb73eb67bb1ee8028f6f504a83d
GitHub-Pull-Request: golang/tools#334
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339709
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
No significant changes.
The only change is to CompletionItemLabelDetails, which is new in 3.17.
Change-Id: I172f0ff72f5c27c0907d7ad733f19a6320c5f510
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339089
Run-TryBot: Peter Weinberger <pjw@google.com>
Trust: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Template tokenization was handling quoting incorrectly.
(The previous code would misunderstand {{"{{"}}.)
While the user is typing an action in a template file the template
parser was returning a correct but useless error, so gopls had no
information about the file. The new code improves this by tokenizing
and parsing an adjusted version of the file. That is, it can sometimes
tell when the user has started typing an action so the {{, }} delimiters
are unbalanced. It replaces the new {{ by blanks, thereby suppressing
a lot of useless error messages from gopls.
Something like this seems to be a prerequisite for provdiding completions
based on the contents of the rest of the file.
Change-Id: I6b6396a4d9e599d671e778b303e6628642585a90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/337351
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
Recent changes to the go/ast APIs for type parameters have broken the
internal/typeparams package when built with -tags=typeparams.
Fix this by adjusting the internal/typeparams API. Also update a few
tests accordingly.
Bump the build constraint used by the internal/typeparams package to
go1.18, as we are no longer compatible with the 1.17 typeparams API. It
is no long possible to opt in to type parameter specific functionality
1.17, which is fine as the dev.typeparams branch has moved to 1.18.
Even after these fixes, not all x/tools tests pass with go1.18. Some
completion tests are failing due to finding 'any' in types.Universe.
Change-Id: I5f92870aaf7853e531e3a154987f98520a52d70c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339349
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Edits from the code action "extract variable" wasn't ordered.
The reason this wasn't picked up by the tests (and in fact, the test
failed when edits were in order) is that multiple edits to the same file
was sent as multiple "DocumentChanges" with a single edit each.
By updating ApplyFix() to consolidate edits to the same file into a
single DocumentChange with multiple edits, we avoid the undefined(?)
behaviour of how clients should handle multiple DocumentChanges to the
same file & version.
Fixesgolang/go#47486
Change-Id: Ie4a4718ceb40693b84c014f66c8fc0d221843d88
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338869
Trust: Pontus Leitzler <leitzler@gmail.com>
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Check that a position is in range before using it.
Fixesgolang/vscode-go#1656
Change-Id: I1598ebab76a1775afd8f63b9849049b31fb74a8b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339169
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
This logic was duplicated as part of the invalid metadata CL.
I believe the loop on L1873 is the intended behavior.
Change-Id: I93ceb3da4045f0536be7dc9da7cecc4323e14a92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339112
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Previously, if the position was before the "." in a selector,
pathEnclosingObjNode would move the position to right after the
".". This created confusing behavior where editors would highlight
the identifier before the ".", but references and go to definition
would be applied on the identified after the selector. This change
removes the shifting of the position if found on a selector.
Fixesgolang/go#47408
Change-Id: If2504e7d5af2fae24b97c5c1e88b9cbe67aaaaf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338789
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In my testing, the gopls degraded memory mode (currently set via
"memoryMode": "DegradeClosed") did not save as much memory as expected
due to still type checking all packages in the workspace (even if in
ParseExported mode). It is also annoying to get incomplete results from
references and renaming.
I think we can (and should) fix both problems: don't even consider
packages that aren't 'reachable' via open files, but fully type check
the reverse transitive closure of the packages you're working on. This
CL does exactly that, by swapping out the concept of 'workspace
packages' with 'active packages'.
In testing, this decreased my memory footprint while working on std by
3-4x when compared to normal mode, and 2x when compared to the previous
implementation of DegradeClosed.
It still needs more testing before we move this option out of
experimental.
For golang/go#46902
Change-Id: I1e319d0b1607d344d27e797ce32de057d7a583f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/336410
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When enriching identifier info with full syntax, it's cleaner to find
the enclosing decl. Use the full decl in hover if we were unable to find
a node in the original type-checked package.
Update the regtest to exercise hovering in a non-workspace package.
Updates golang/go#46158
Change-Id: Ic1772a38fb1758fb57a09da9483a8853cc5498f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333690
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Currently the `SignatureHelp` function provides signature help even when the
requested range lies within a string literal. Let's suppress this behavior and
return an error when someone requests signature help from within a string
literal.
Fixesgolang/go#43397
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Change-Id: Ib03e87258622f4294bf9385bf5f0a8effe0050ee
GitHub-Last-Rev: 0c9549ae68b0263a3cac274da133e9ab4b4c7bf5
GitHub-Pull-Request: golang/tools#332
Reviewed-on: https://go-review.googlesource.com/c/tools/+/337170
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
We currently support evaluating int literals on hover
if it's a const declaration but not if it's a var. This
change adds support for the same for var.
Fixesgolang/go#45802
Change-Id: I3c4f6024b4b58fed38a5111253aa9e2ac30249fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330309
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
"Extract method" allows users to take a code fragment and move it
to a separate method. This is available if the enclosing function
is a method.
Change-Id: Ib824f6b79b13ca73532223283a050946c90a47e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330070
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix the imports text edits by computing it on first principles. This
fixes at least a couple bugs:
- Incorrect handling of positions between \r and \n with windows line
endings.
- Incorrect computation of edits when the imports source prefix is
synthetically terminated with just \n, but the actual source has
\r\n.
This is an unsatisfying solution, necessary because of the interaction
of token.File with file termination (token.File does not capture this
information).
Efforts to fix token.File proved complicated, and this is causing
problems for our users, so I think we should do this for now.
For golang/vscode-go#1489
Change-Id: I235caf3960c7201af93800d65546fbab5c6e3f4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319129
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL adds various improvements for matching nested fields and
methods:
- Limit the symbols we produce to not show unqualified fields/methods,
and not show partial package paths.
- Handle embedded selectors, by trimming the package path.
- Improve the internal API used by symbolizers to operate on named
chunks.
Fixesgolang/go#46997
Change-Id: I86cbe998adbb8e52549c937e330896134c375ed7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334531
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
I'm not sure how this can happen, but it seems possible that a bad
expression might somehow have an invalid position.
Fixesgolang/go#47231
Change-Id: I0794bdfb66f668fc375e9fe561c9f239c8b92492
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334892
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
There is insufficient type information to compute semantic tokens in
packages that don't compile. Particularly affected are test files
and files being actively edited in new packages.
Further, existing code could
panic on poorly formed imports; this has been fixed.
Computing semantic tokens for identifiers
having neither use or definition information has been improved.
(Each of the many cases in the new function unkIdent() occurs
in existing code or test files.)
Change-Id: Id1b5db0622b17076de1ed23a950a20cd03c3750a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333869
Run-TryBot: Peter Weinberger <pjw@google.com>
Trust: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
If didModifyFiles fails we were never closing the diagnoseDone channel.
This was potentially leaking goroutines in the regtests.
Noticed in
https://storage.googleapis.com/go-build-log/ab4085ce/freebsd-amd64-12_2_149b05c3.log
Change-Id: I906b643d415c915d7be1951e5d8d21bf9016acee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334250
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The debounce API becomes both more testable and more elegant when using
channels rather than callbacks to signal events, as suggested by bcmills
in CL 333349. Adopt these suggestions.
Fixesgolang/go#45085
Change-Id: Ic1843f582d514af8aa109c24f5e3311536e54a60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334252
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Taking the address of the variables defined by range in a for loop is not
safe since they are reused. Get the address from the original slice.
Change-Id: If7fbf3fdbfeeaf329f36e416642582002895bbce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330649
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When we open a file in a package, independent of whether it is in the
workspace, we type check in ParseFull mode. However, several other
code paths don't find this better parse mode.
We need a better abstraction, but for now improve a couple code paths
specifically for the purpose of fixing Hover content.
Updates golang/go#46158
Updates golang/go#46902
Change-Id: I34c0432fdba406d569ea963ab4366336068767a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333689
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Using the -cpuprofile testing flag for profiling didChange handling
causes the profile to capture the IWL. Add a new -didchange_cpuprof
flag that instruments just the change handling.
Also fix a check for empty workspace files that was preventing
This inaccurate check was preventing the didChange benchmark from
working.
Change-Id: Ie9f5402960ddccda5d6b9b36ae3c111aa5b51bb8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333939
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
*ast.AssignStmt doesn't have an associated comment group. So, we should
try to find and return a comment just before the identifier.
Fixesgolang/go#42134
Change-Id: Ie40717a4973ccfdbd99c3df891c2cfffbb21742d
GitHub-Last-Rev: da75fde2dbf3613f3325dbc5930dfc84ea813b90
GitHub-Pull-Request: golang/tools#323
Reviewed-on: https://go-review.googlesource.com/c/tools/+/327229
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
CL 309276 added logic to retry TestDebouncer if its execution was
determined to be invalid.
Unfortunately it also reduced the delay period, which increases the
likelihood of a flake on any individual execution. This appears to have
more than offset any robustness resulting from the retries.
This CL does a few things to try to improve the test:
- Remove t.Parallel: we want goroutines to be scheduled quickly.
- Increase the debouncing delay.
- Improve the logic for determining if a test was invalid.
- Guard the valid variable with a mutex, since this was actually racy.
For golang/go#45085
Change-Id: Ib96c9a215d58606d3341f90774706945fcf9b06c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333349
Trust: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Refactor the lsprpc package to move the logic for 'automatic' server
discovery into an AutoDialer abstraction, which both implements the v2
jsonrpc2 Dialer interface, and provides a dialNet method that can be
used for the existing v1 APIs.
Along the way, simplify the evaluation of remote arguments to eliminate
the overly abstract RemoteOption.
Change-Id: Ic3def17ccc237007a7eb2cc41a12cf058fca9be3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332490
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This reverts commit 5b540d349b.
Reason for revert: left in logging statements
Change-Id: I9ef5cd79e9ae8c94098fceca3a356fa3377c16e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333711
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
Error messages associated with bad parses and when working at the end
of the file are not useful to users, so they are no longer generated.
The logic around recognizing function calls has been improved, and
a panic sometimes caused by bad imports has been fixed.
The logic is imprecise/incorrect in some cases; there will be
another CL.
Change-Id: I72be3a0a003569fe06d458989e3dbbb46b7a22c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332909
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 333289 introduced a panic, which was subsequently suppressed in test
error output due to the deferred t.Fatal (an interesting gotcha that I
honestly wasn't aware of).
Fix both the panic, and the suppression of regtest panics.
Also fix the regtest editor shutdown to run on a detached context, so
that shutdown doesn't fail for tests that have timed out.
For golang/go#46773
Change-Id: I080a713ae4cd4651476d8b4aab1d2291754a4f5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333510
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If a context was canceled during load, only report it as a critical
error if the load actually failed.
Along the way, simplify evaulation of the critical error to use a switch
statement.
Also await IWL in the second Env used in shared regtests. Presumably it
is this Env that is being shutdown prior to IWL, triggering the
panicking code-path from golang/go#47030. I wasn't able to reproduce,
but all panics are occurring in regtest/misc, and this seems highly
plausible.
Fixesgolang/go#47030
Change-Id: I4df65697f644cff275ab1babb783868fd9e10c2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332589
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Gopls doesn't send empty diagnostics for open files in some cases
(likely a race to context cancellation). This is probably a bug itself,
but for now don't let this cause TestResolveImportCycle to fail.
Added a TODO to investigate further.
Fixesgolang/go#46773
Change-Id: I197d9b09885951b47b3f90a0480ae75679d2a1a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333289
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TestDebouncer inherently depends on the timing of events. Specifically,
it can fail if the pause between two subsequent events is more than the
debouncing delay.
When this has proved flaky in the past I just increased the debouncing
delay. However, tests are still occasionally flaking. Rather than just
increase the delay arbitrarily, attempt to make the test more robust by
retrying up to three times if the base assumption (that goroutines are
scheduled within a reasonable amount of time) is not met.
Fixesgolang/go#45085
Change-Id: Ifa32e695d64ae4bcfe9600a0413bf6358dff9b7a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309276
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The view workspace was written concurrently, meaning that changes to a
go.mod or go.sum could race to updating the view workspace, leaving it
in an incorrect state.
Move control over writing this workspace onto the view, and hold the
snapshotMu while writing.
Change-Id: I47f58769cc77860e9c9674c8f6bf5fd0793e7937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309289
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This option has been enabled by default for a while now: remove it from
'experimental' to 'advanced'.
Change-Id: Id8116bf7b8976204a61fe1bbf9dc0b8bd69c68d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309271
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Ideally we could at some point break the snapshot->view->session->cache
reverse traversal, but for now at least don't copy this pattern around
everywhere.
Change-Id: Ib144e6d322016f5b9563f21c56a0691c1a8ec97d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309270
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Gopls's serial processing of didChangeWatchedFile notifications has
historically been a pain point for clients that don't batch file events,
when branch switching or running go generate.
It's "not that tricky" for us to debounce and batch up watched file
notifications on our end, so this CL introduces this functionality as a
new experimental setting. Truth be told it ended up being harder than
expected, due to (1) our requirement for regtests to be able to
determine when diagnostics have completed, and (2) our reliance on
jsonrpc2 for sequencing changes to the server.
To address (1), I factored out the actual processing of change
notifications into a separate method (thus increasing our surprisingly
long chain of method calls). To address (2), I guarded the processing of
file changes with a mutex.
I also guarded some places where views and snapshots were accessed in
potentially racy ways. Our interaction with session.views was rather
complicated, so I had to switch session.viewsMu to a RWMutex.
Add this to the experimental regtest mode, and more generally enable all
experimental features in this mode, rather than just the experimental
workspace module.
Fixesgolang/go#41691
Change-Id: Ifccdbdf86263dbe2e37ffe9f7bbf2a2cd74218b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309269
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When setting metadata, we must invalidate any packages that have been
computed with old metadata.
Also make setMetadata atomic, by locking around it in Load. This is just
writing memory after a Load, so should be fast and infrequent. It is
critical that our various maps related to metadata are coherent, so it
makes sense to err on the side of coarser locking, IMO.
Fix a race in TestUpgradeCodeLens.
Change-Id: Iee8003b7e52b9f21681bdba08a009824f4ac6268
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331809
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Currently, flags are not being printed when users run `gopls help` because they
never get parsed in Main. This CL fixes that issue.
Updates golang/go#35855
Change-Id: Ic9882d0d2410a0f045aa0ecaa87b36c23eb569fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330854
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
There are no changes, other than the date and git checksum.
Change-Id: Ic7e93829f5e4cd95b7c2b5023c9c0905a07655ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332690
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Add a middleware to enrich LSP messages with Go environment on the
forwarder.
Change-Id: Ia294b0c2fa1276b09b75542436e5f4179f81f302
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331771
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will be used for starting the debug server along the serving path.
Change-Id: I19d13753ad70c179a53f348ed574aaea19c0d301
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331770
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The number of concerns satisfied by the lsprpc package is proving to be
a barrier to both factoring out the debug and event package, and use of
service discovery.
The new Binder API admits a nice abstraction that can help make the
lsprpc package more modular, or perhaps even unnecessary: a binder
middleware that can instrument all aspects of the connection lifecycle.
In this CL, this pattern is used to decouple the server handshake from
the actual forwarding setup. Later CLs will implement additional
functionality using this pattern.
The TestEnv helper is refactored to be more scriptable.
Change-Id: I6060bc4bba57e4ee7e161a5d6edbc40c6fccbaa8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Lower the timeout for the go command to 10 minutes.
Also, if the first workspace load attempt fails because it times out,
treat the timeout as a critical error, since we explicitly don't cancel
the first workspace load.
Fixesgolang/go#46859
Change-Id: Iccd26509177e4c47ca4b2c8ab4111df9be0f934e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330969
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Rob Findley
Shoshin Nikita
Nicholas Asimov
pjw
Pontus Leitzler
Rebecca Stambler
Suzy Mueller
Karthik Nayak
danishprakash
Jean de Klerk