Our logic to detect whether suggested fixes delete unnecessary code did
not take into account suggested fixes that have a command.
Fixesgolang/go#48938
Change-Id: I70d2bebbf7059236525117cc1f454ef726162a96
Reviewed-on: https://go-review.googlesource.com/c/tools/+/355769
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This CL adds a new infertypeargs analyzer, which finds call exprs where
type arguments could be inferred, and suggests a quick fix to simplify
them.
Along the way, may two changes to the supporting frameworks:
- Initialized types.Info.Instances in go/packages
- Fail analysis tests run with suggested fixes if formatting the
resulting source fails.
Change-Id: Ib15e5bd7c26aa293c5fc18a4cff6bc047e9e31d2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351552
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>
This change handles the case of the user creating a go.work file
during the gopls session. It also defaults to go.work/gopls.mod being
used outside of experimental mode, so that you don't have to both set
a setting and have a file.
Change-Id: If118cd2fc95c1b5600a6c06217a3b61605b11e28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/342170
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>
I missed the TODO in undeclaredname to add support for functions, so
really this belongs in that analyzer. This removes a fair bit of code.
However, the type error analyzers don't really work with the go/analysis
testing framework because the suggested fixes are split from the
diagnostics, so I moved the tests into the gopls tests.
Change-Id: I861a7ad531d2732fe698ee0ac46f23ad53b16812
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351333
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>
All operations within a file should be in terms of token.Pos, until the
value must be returned.
Fixesgolang/go#48492
Change-Id: I2adfcad8b4729386700c08ba4618e033bf7c3db5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351629
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Trust: 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>
Add an analyzer that checks for empty interfaces in constraint position,
that could instead use the new predeclared "any" type.
Change-Id: I6c11f74c479c2cba64b3b12e61d70d157f94393b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351549
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>
This adds an analyzer that provides suggested fixes for undeclared name
errors on function calls, implementing a stub of the fuction (with an
empty body). As of now this doesn't try to guess returned types but
only parameters.
Generated functions are appended at the end of the file where these type
errors occur.
Updates golang/go#47558
Change-Id: Iaef45ada6b7b73de1fbe42e5f7e334512b65e6c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348829
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>
Change-Id: Idb7ed8ab7f736cac10be1e9d5592e1ffb3f048eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351090
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This change adds a command that can be used to see the difference in
API between two gopls versions. It prints out the changes in a way that
can be copy-pasted into the release notes.
Also, only run the copyright test with 1.18. I wanted to do this before
to use filepath.WalkDir, but now it also doesn't work with generic
syntax (it doesn't use packages.Load, so doesn't respect build tags).
Fixesgolang/go#46652
Change-Id: I3670e0289a8eeaca02f4dcd8f88f206796ed2462
Reviewed-on: https://go-review.googlesource.com/c/tools/+/327276
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>
As of CL 349949, this is not longer necessary.
Change-Id: Ie89a7771f5873c4acea54f4c2507b76ec9832706
Reviewed-on: https://go-review.googlesource.com/c/tools/+/350089
Trust: Robert Findley <rfindley@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
As per the current specification, the correct token name that represents methods is `method`. The current implementation does not produce the correct set of result and results in methods being incorrectly highlighted. Furthermore, the parameter names of interface methods are treated as `method` which is not true, so a fix has been applied to handle `ast.FuncType` being `parameter`.
Spec: https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_semanticTokens
Change-Id: Id5d2fafe9f948a4a175ae19b4c94cca8f48a4b79
GitHub-Last-Rev: 9a3b65ca4dd3bb0542969c043a2042001c7fcb63
GitHub-Pull-Request: golang/tools#340
Reviewed-on: https://go-review.googlesource.com/c/tools/+/349449
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
When resolving a position to a package we must consider all packages,
including intermediate test variants. This manifests, for example, when
jumping to definition in a package that is imported as a test variant
(see golang/go#47825).
For now, fix this by threading through an 'includeTestVariants' flag to
PackagesForFile. This isn't pretty, but should be a trivially safe
change: the only effect will be to increase the number of packages
considered in FindPackageFromPos. Since we are discussing future changes
to the API for querying packages from the snapshot, now did not seem
like a good time to undertake significant refactoring.
A regtest based on the original issue is included.
This CL is joint with rstambler@golang.org.
Fixesgolang/go#47825
Co-authored-by: Rebecca Stambler <rstambler@golang.org>
Change-Id: I4693ec69b50ed4acd569cff87883769c1edf332b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347563
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>
We use a case-insensitive comparison so this doesn't really
matter but it's a bit confusing that gopls/doc/settings.md and
internal/lsp/source/api_json.go were inconsistent here. I'm assuming
the latter also shows up as user-visible documentation somewhere,
probably in the VSCode plugin.
Change-Id: I14fa5b9d062266b6de0397aafb36e0ad84730752
Reviewed-on: https://go-review.googlesource.com/c/tools/+/344353
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>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There is a TODO to remove the "codelens" alias. This updates an
example in the documentation to use the new name.
Change-Id: Id807e99f5ba1da663eae9660fb65521596b0c488
Reviewed-on: https://go-review.googlesource.com/c/tools/+/344352
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>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Since golang.org/cl/343732 was merged the dev.typeparams branch is no
longer needed, just Go at tip.
Change-Id: Ifb2e5220bc2f544d86cfa84d43d12d93e4e7fddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343967
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>
It is unlikely that users want gopls operating on their node_modules
directories, so we should exclude them by default. If a user wants to
include them, they can override their directory filters setting.
This doesn't exclude *any* directory named "node_modules", so we still
need to implement golang/go#46438 to exclude node_modules completely.
Change-Id: I03c42208e62390dc35e44ac5176422ddf8dc53f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347297
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Because of how the compiler internally represents variable references,
it sometimes gets position information incorrectly for them. The
codelens test hits this issue because for
var x string
fmt.Println(x)
the diagnostic about "x" escaping to heap (which actually refers to
the implicit conversion to "interface{}" type) should be rightly
reported at the "x" identifier within the call arguments list.
However, due to the aforementioned compiler bug, historically we
reported the diagnostic at the "(" token instead.
In -G=3 mode, the compiler (correctly) report the diagnostic at "x"
instead; but with GOEXPERIMENT=unified, the compiler intentionally
matches the original -G=0 behavior and continues reporting at "("
instead.
This CL avoids the issue entirely by changing the line to
"fmt.Println(42)", which avoids the issue because the compiler always
correctly prints the diagnostic then at "42".
Change-Id: I22d4c756ae801489f3f16c440e4339bc9b115fb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343875
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When searching for references or renaming, we start from all packages
containing the current position. But as reported in golang/go#47564,
this fails if we're renaming an object in another package; we need to
start the search from the package containing the object definition.
This CL finds the missing packages by recursively searching all
locations we encounter. For now, this will cause us to consider the
object location, and may also help us behave correctly with respect to
build constraint variants in the future.
While at it, update the regtests to support renaming. This bug could
be exercised with marker tests, but it's good to have a regtest for
renaming anyway.
Fixesgolang/go#47564
Change-Id: I5517e2aeaaa744fcc6b6b96ffbb0b2625b498ed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340472
Trust: Robert Findley <rfindley@google.com>
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add the scope to the command-line-arguments package ID and path so that
multiple command-line-arguments packages can coexist in the workspace.
Fixesgolang/go#47584
Change-Id: Icbfe90d67627f384c54f352e46270ab2bf4240bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/341611
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>
I fixed a check in sandbox.go to check for size of config.Files
instead of nilness. The completion benchmarks run with an absolute
path workdir and were failing this check due to a non-nil but empty
config.Files.
I tweaked the benchmark output so it is compatible with benchstat. In
particular, the benchmark output now appears all on one line for an
imaginary benchmark named BenchmarkStatistics.
I also made a couple changes to the completion benchmarks:
- Don't modify the buffer before every completion. Type checking
completely dominates completion, so if it has to type check every
time then you aren't benchmarking the completion code at all.
- Don't try to exclude GC from the benchmark. I think amortized GC
time should be included in the benchmark timing. Plus, I'm not sure
that forcing a GC every 10 iterations was actually doing a good job
excluding GC from the benchmark.
Change-Id: I53718a5f6e25453146ccf5bb5fdfdfc65e244df3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323251
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
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>
Hook up the new fast SymbolMatcher as an option.
Benchmark ("test" in x/tools): 48ms->21ms (with the new matcher)
Benchmark ("test" in kubernetes): 857ms->199ms (with the new matcher)
Change-Id: Ic638eda1ed10572638f32879dd9b56467ae305ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338695
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>
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>
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>
Going from an import line to an import block with CRLF endings did not
previously work.
Fixesgolang/go#47200
Change-Id: I51334587ad51b828bd0828217ba39fb745d7e835
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334890
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>
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>
The documented Lua function for supporting imports with Neovim native
LSP support does work for imports, but also runs any other code action
available in the current range. For example, if the function was to run
with the user's cursor on an empty struct being initialized, the code
action for filling that struct would also run.
This change restricts the desired actions to only be organizeImports.
Fixesgolang/go#47181
Change-Id: Ie4eb5961cb4405091477b79175acc7052ff16d4c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334749
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Robert Findley <rfindley@google.com>
Instructions for Sublime Text + LSP + golsp,
including examples for:
minimal configuration
adjusting path
per-project settings
The per-project settings include breadcrumbs helpful
to someone working on Go itself.
Change-Id: I2fe41cd60d7327578bd5bd9999f2459e79e9be4c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/325189
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@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>
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>
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>
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>
The new file gopls/doc/semantictokens.md describes how gopls presently
decides which semantic tokens and modifiers to send to clients.
Change-Id: Ifc76e13954dde2ea58b6da446517f94be038a285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333010
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>
Change-Id: I261e1936b96e70ad8347655a7a8d0eb792348d44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330853
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Allow users to experiment with golang/go#45713 by adding experimental
support for the go.work file. We handle it like a special case, very
similar to the current gopls.mod file mechanism. The behavior is
undefined if both a gopls.mod and go.work file exist. Ultimately, we
will deprecate support for the gopls.mod file if the go.work file
proposal is accepted, so I don't think it's important to be careful
about handling both simultaneously.
Change-Id: Id822aeec953fc1d4acca343b742afea899dc70ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328334
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>
Change-Id: Ia7855890c72f9e376816a7c2d71f9715c7f2eb9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This change adds a shouldLoad field to knownMetadata so that we can be
more selective about reloading these.
If a package has invalid metadata, but its metadata hasn't changed, we
shouldn't attempt to reload it until the metadata changes.
Fixesgolang/go#40312
Change-Id: Icf5a13fd179421b8f70a5eab6a74b30aaf841f49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/298489
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>
Also improve the description of the failing expectations.
Updates golang/go#46773
Change-Id: I9465de8a5005bb7ee719a536f8550afc54bd6044
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328369
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>
Retrying CL 271477, this time with parts of CL 322650 incorporated.
This CL moves to a model where we don't automatically delete invalidated
metadata, but rather preserve it and mark it invalid. This way, we can
continue to use invalid metadata for all features even if there is an
issue with the user's workspace.
To keep track of the metadata's validity, we add an invalid flag to
track the status of the metadata. We still reload at the same rate--the
next CL changes the way we reload data.
We also add a configuration to opt-in (currently, this is off by
default).
In some cases, like switches between GOPATH and module modes, and when a
file is deleted, the metadata *must* be deleted outright.
Also, handle an empty GOMODCACHE in the directory filters (from a
previous CL).
Updates golang/go#42266
Change-Id: Idc778dc92cfcf1e4d14116c79754bcca0229e63d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324394
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>
Robert Findley
Hana
Rebecca Stambler
Francesco Renzi
Chitoku
Johannes Altmanninger
Dan Peterson
Matthew Dempsky
Muir Manders
Shoshin Nikita
danishprakash
David Chase
GGCristo
pjw
Jean de Klerk