Commit Graph

2975 Commits

Author SHA1 Message Date
Robert Griesemer 40cfafff02 x/tools/internal/lsp/source: disable some tests for std lib changes
CL 425716 changes parser behavior for incorrect code (it is more
lenient) which breaks a few lsp tests. Disable them for now so we
can submit CL 425716.

For golang/go#54511.
For golang/go#54822.

Change-Id: I00fa67e29628137f3e4e44c38e92094ea581a61b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427654
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-01 22:03:33 +00:00
Brian Pursley f16be35d92 internal/lsp/source: add functions to type hover output
Add functions to the output when hovering over a type.

Fixes golang/go#54008

Change-Id: Ia0a7b5a878c3d63c4bbc549f003c45592db1c135
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420714
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-01 17:52:14 +00:00
Robert Findley dfc8d49339 internal/lsp/testdata: fix diagnostics checksum
Fix an incorrect diagnostics checksum causing builder failures.

Change-Id: Ief4f9edd6acbf8d42eaf1109ec6ddc0085f20b05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427536
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Dylan Le <dungtuanle@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2022-09-01 17:30:47 +00:00
Alan Donovan 6c10975b72 internal/lsp/cache: honor RunDespiteErrors=false
This change prevents analysis from running on a package containing
any kind of error unless the analyzer has indicated that it is robust.
This is presumed to be the cause of several panics in production.

And a test.

Updates golang/go#54762

Change-Id: I9327e18ef8d7773c943ea45fc786991188358131
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426803
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-01 15:27:30 +00:00
Robert Griesemer 49ab44de90 x/tools/internal/lsp: re-enable a test with adjusted error message
This is a follow-up on CL 425497. Must be submitted after CL 425007.

For golang/go#54511.

Change-Id: Ice9c1c62b911efa8efa20ea295bf313aabe8c5bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425594
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-09-01 03:24:42 +00:00
Robert Findley 550e1f5a55 internal/lsp/tests: use a shorter module path for test data
While moving internal/lsp to gopls/internal/lsp, we discovered that
we're bumping up against a command line length limit on windows. Use an
arbitrary shorter module path to avoid this, for now.

Updates golang/go#54800
Updates golang/go#54509

Change-Id: I7be07da29a769c1ce7c31cbbd374ca47b0944132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426801
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-31 21:10:26 +00:00
Robert Findley 4ccc73cbb5 internal/lsp/tests: simplify comparison of markdown at go1.18
In several places throughout the x/tools codebase, the internal/lsp/diff
package is used to provide clearer test output when comparing large,
multi-line strings. In some places, this is implemented via the
tests.Diff helper function, but in others it is not (presumably due to
import cycles).

Factor out this pattern into a diff.Pretty function, and add commentary.
Also remove the *testing.T argument, as diffs should never fail; opt to
panic instead. Also add a test.

Using this function, simplify the logic to comparing our 1.18 markdown
output with 1.19 golden content, by normalizing differences between the
two.

This is necessary as markdown content will change as a result of moving
internal/lsp to gopls/internal/lsp.

For golang/go#54509

Change-Id: Ie1fea1091bbbeb49e00c4efa7e02707cafa415cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426776
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-08-31 21:08:24 +00:00
Alan Donovan 3eb8a8f04e internal/lsp/cache: delete misleading Package.IllTyped method
This method does not in fact report whether the package is ill-typed.
It actually reports whether any of three fields of pkg are nil.
They never are.
This does rather explain why type-error analyzers are executed
on (genuinely) ill-typed packages, despite the !IllTyped()
condition. And why analyzers that aren't prepared to handle
ill-typed code (!RunDespiteErrors) sometimes crash in production.

Updates golang/go#54762

Change-Id: I95421584bec68fb849b5ed52cc4e6d9b6bb679be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426802
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-08-31 20:07:46 +00:00
Alan Donovan cb91d6c88f internal/lsp/cache: clarify error control flow of analysis
This change consolidates the logic of the memoized function
for analysis (previously split across execAll and runAnalysis)
to make the error control flow clearer. The new function,
actionImpl, follows the same conventions as in other memoized
functions.

Errors from await (e.g. cancellation) are now separate from
errors from analysis itself. Only the latter are now ignored
in the diagnostics loop over the roots.

Also, where previously the recursion would ignore failed
dependencies (proceeding to call runAnalysis even after
seeing an &actionData{err!=nil}), now the recursion aborts
if one of the prerequisites fails, as it should.
The only edges currently enabled are "horizontal" (inputs,
not facts). This would explain the crash in golang/go#54798,
in which the result of the nspector pass was not available
to the structtag pass.

Also:
- merge the loops over deps in execAll and runAnalysis.
- remove unnecessary checks for context cancellation in
  loops of pure computation.
- don't suppress errors from awaitPromise.
- add commentary.
- turn two "can't happen" errors into panics.
- remove unnecessary logging of analysis failures.

Fixes golang/go#54798

Change-Id: Iefb42d2d074a31f2f717dc94c38aed7f1dab1c80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426800
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-31 18:33:44 +00:00
Alan Donovan 41c3a9b12b internal/lsp/analysis/fillreturns: be defensive w.r.t. type errors
In the presence of type errors, TypeOf may return nil, and this appears
to have contributed to a crash in the fillreturns analysis. I haven't
been able to find or deduce a test case, but this change makes the logic
more defensive.

Also remove a stale pre-go1.17 test that used to trigger a panic
(the one fixed here? unclear) to assert that panics were recoverable.

Updates golang/go#54655

Change-Id: Ic9ca9a307eede50a2d4db6424822a155dd43e635
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426019
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-08-31 14:39:29 +00:00
Robert Findley ddbeb75429 internal/lsp: run internal/lsp/reset_golden.sh
Run reset_golden.sh, so that our golden files are stable. This will be
useful later, when we migrate internal/lsp to gopls/internal/lsp, and
golden files must be updated to account for changing offsets.

For golang/go#54509

Change-Id: I2e9a8d3493d64d632b9f0f0e0360d633803f9d92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426797
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-31 13:23:13 +00:00
Alan Donovan 248c34b88a internal/lsp: support regular expressions in Diagnostics tests
The diagnostics emitted by the Go toolchain vary from one
release to the next, and the gopls tests run against multiple
versions. Therefore our tests need a way to express multiple
possibilities in their expectations. This change interprets
the patterns in @diag test annotations as regular expressions,
allowing some flexibility in recognizing both old and new
messages. It's not a panacea but it is one step to reducing
the considerable friction of making changes to the compiler
or go/types in the main tree.

Details:
- Factor the three implementations of the Tests.Diagnostics
  abstract method so that they all use DiffDiagnostics
  to report differences, substantially rewriting one of them.
- Move the "no_diagnostics" hack, of which there were three
  copies, not all consistent, into DiffDiagnostics.
- Eliminate the named type for each tests.Data field;
  a type alias is all that's needed.
- Add Diagnostics.String method.
- Add various TODOs for further improvements.
- Add various apparently missing Fatal statements within
  the tests.

Change-Id: Id38ad72a851b551dd4eb1d8c021bcb8adbb2213f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425956
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
2022-08-29 18:21:50 +00:00
Alan Donovan 431f4eff4f internal/lsp/tests: re-enable ARM tests
This is a simple revert of CL 425497, which disabled the tests.

The problem was due to a misencoded FMOVD instruction causing
some 0.0 constants to compile to nonzero; fixed by CL 425188.

Updates golang/go#54655
Updates golang/go#425188

Change-Id: I2231b57b7b78ac1ae2e8b60cda62898ea7122cda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426017
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
2022-08-29 18:03:26 +00:00
Suzy Mueller 7f2330708b internal/lsp: limit diagnostics for upgrades to codelens go.mod file
The Check for upgrades codelens was only looking for upgrades for
the current module, but was applying diagnostics to all go.mod
files in the workspace. This change makes sure to only apply the
diagnostics in the same selected go.mod.

Fixes golang/go#54556

Change-Id: I1eacbc8af2e9dcfe1e1a67516f047bcb94099872
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425195
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-08-26 17:39:52 +00:00
Alan Donovan 7c5e03569b internal/lsp: fix suppressed panic in analyzer
This change ensures that the End position provided to span.NewRange
in suggestedAnalysisFixes is valid even when the diagnostic has
only a valid start position. This seems to be the cause of
some panics observed in the ARM builders in the attached issue.

Also, reduce the scope of the recover operation to just the analyzer's
run method: we don't want to hide further bugs (or discard stack traces)
in the setup or postprocessing logic.

Also:
- split a single assertion in span.NewRange into two.
- Add information to various error messages to help identify causes.
- Add TODO comments about inconsistent treatment of token.File
  in span.FileSpan, and temporarily remove bug.Errorf that
  is obviously reachable from valid inputs.
- Add TODO to fix another panic in an analyzer that is covered
  by our tests but was hitherto suppressed.
- Add TODO to use bug.Errorf after recover to prevent recurrences.
  We can't do that until the previous panic is fixed.

Updates https://github.com/golang/go/issues/54655

Change-Id: I0576d03fcfffe0c8df157cf6c6520c9d402f8803
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425356
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-08-26 15:28:25 +00:00
Alan Donovan 2f38e1deaa internal/lsp/tests: disable failing test on ARM
This is a stopgap until I can diagnost the problem, but in the meantime
we need to fix the builders.

Updates https://github.com/golang/go/issues/54655

Change-Id: I6260828e8c07e3121c45f99166a26d51aa9805a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425575
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
2022-08-25 21:25:49 +00:00
Alan Donovan d35bb19708 internal/lsp/tests: improve assertion error message
Change-Id: I487faada9f1041434dde981d5aded195f6b40054
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425574
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
2022-08-25 19:06:41 +00:00
Robert Griesemer 7111c2e56d x/tools/internal/lsp: disable a test so we can change the parser error
This will allow us to submit CL 425007 after which we can re-enable
this code and adjust the error accordingly.

For golang/go#54511.

Change-Id: I2861a8f372bce214824d7cbdffad6abf7ca4a58e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425497
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
2022-08-25 18:47:38 +00:00
pjw 587a15310b internal/lsp: hover to render go 1.19 doc comments
Go 1.19 introduced new formatting for doc comments, and a new package
for processing them. This change uses the new package when gopls is
compiled with go 1.19 or later.

The difficulty is with the hover tests, which have to work both when
gopls is compiled with earlier versions of go, and with go 1.19.
Fortunately the changes in formatting the test cases are easily checked.

Fixes golang/go#54260

Change-Id: I9e8e7f0cf3392afa0865b5d3f4e5fcdd88dfe75f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421502
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-19 18:26:38 +00:00
Robert Findley e55fb40e67 internal/lsp/cache: clear shouldLoad IDs on load
CL 417576 externalized shouldLoad tracking into a map, which was used to
trigger a reload and cleared once reload completes. Unfortunately, it
overlooked the fact that we may also reload the entire workspace (via
reinitialization). In this case, we should clear newly loaded IDs from
the shouldLoad map, so that they are not subsequently loaded again.

Fixes golang/go#54473

Change-Id: I26f49552cae502644142dc4a4e946294db37f6f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/424074
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-16 15:07:15 +00:00
Alan Donovan b3851a823f internal/lsp/cache: tweaks to metadata graph
- ignore error from buildPackageHandle, with rationale.
- remove unnessary optimization in call to Clone(updates={}):
  Clone does the same check internally.
- more comments.

Change-Id: I4551cf560aea722d972fb6da404aed71a79f4037
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416217
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-08-16 01:59:44 +00:00
Robert Findley 987de349f4 internal/lsp/completion: don't use Type.String for checking identity
Completion is very performance sensitive, and building a string to check
for *testing.F has a significant cost.

StructCompletion-8         20.7ms ±14%  16.8ms ± 1%  -18.59%  (p=0.000 n=10+10)
ImportCompletion-8         1.36ms ± 5%  1.05ms ±18%  -22.55%  (p=0.000 n=9+10)
SliceCompletion-8          23.5ms ± 2%  19.3ms ±18%  -17.85%  (p=0.000 n=7+10)
FuncDeepCompletion-8       17.6ms ± 2%  15.5ms ± 2%  -11.82%  (p=0.000 n=8+8)
CompletionFollowingEdit-8  81.2ms ± 8%  74.2ms ± 5%   -8.60%  (p=0.000 n=9+9)

For golang/go#53992
For golang/go#53798

Change-Id: Ia138cbadce142a424caabe8259bda05bcc536055
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422906
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-08-15 17:37:01 +00:00
Robert Findley 5a26068387 internal/lsp/source: remove utm_source from pkgsite links
This query parameter is not needed.

Change-Id: Id45d7be0b1cbe5d383bcc6768ef20df26de3e7b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422901
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
2022-08-15 16:22:50 +00:00
Robert Findley bebd890374 go/analysis: remove stray print statement in the timeformat analyzer
A debugging print statement was left in the analyzer, which breaks
gopls' communication over stdin/stdout.

Fix this, and add tests.

Fixes golang/vscode-go#2406

Change-Id: I1b785fa09e66eae2f1b1e03806e5b59d2015e75e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422902
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Tim King <taking@google.com>
2022-08-12 17:40:51 +00:00
Suzy Mueller 37a81b68a0 internal/lsp: add unnecessary tags for unused vars and imports
Diagnostic Tags add another hint for the client for how to
display diagnostics. The Unnecessary tags allows clients to
fade out the diagnostic instead of adding a squiggle. This adds
this tag to unused import errors as well as unused vars.

For golang/vscode-go#2285

Change-Id: I0c93b28a6a2ef4eed314dcf30a37c27dd65940ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415499
Reviewed-by: Robert Findley <rfindley@google.com>
2022-08-11 16:19:14 +00:00
Robert Findley 3807419523 internal/lsp/cache: validate the range for critical errors in go files
This avoids the panic reported in golang/go#54395.

Fixes golang/go#54395

Change-Id: Ief35985a503d3cc13971499dc6f4e9c1d1d63ea3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422894
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-08-11 13:46:09 +00:00
Robert Findley 0ad49fdeb9 internal/imports: update stdlib index for 1.19
Updates golang/go#38706

Change-Id: I361c8862b4dcd5215ea0574715908f552f937e6c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422654
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-10 17:41:25 +00:00
Robert Findley 3950865150 gopls/internal/regtest/bench: add a -gopls_version flag
To facilitate testing specific gopls versions, add a -gopls_version flag
that installs and uses a specified gopls version.

Also fix a bug in the DidChange benchmark that causes it to be
inaccurate when the benchmark function is invoked multiple times (by the
benchmark runner or via -count): because the server environment is
shared across invocations, we can't use the benchmark iteration to
derive the expected number of changes: we must use our count of actual
DidChange notifications from the server.

Change-Id: I1ad733cb3e695d382e81dfb7e31346162d9b7635
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422368
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nooras Saba‎ <saba@golang.org>
2022-08-10 15:08:24 +00:00
Dylan Le 6fa767d87c internal/lsp: update documentation for directoryFilters setting and default value
Add `**` usage to directoryFilters documentation. Change directoryFilters default value to `-**/node_modules`

For golang/go#46438

Change-Id: I3ea14ad8a20893d19df4cf8d584a7c7f9b213aab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422356
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-08-09 20:31:19 +00:00
Robert Findley 96d05aa120 gopls/internal/regtest: fix TestFailingDiagnosticClearingOnEdit
This test was asserting immediately on the non-presence of published
diagnostics, and therefore was racing with the diagnostics calculation
pass. Following https://go.dev/cl/420539 this became a flake, because
once gopls has calculated diagnostics for the open event, it will
actually publish empty diagnostics for the opened file.

Update the test to use a OnceMet so that we only evaluate the
expectation once we've finished the diagnostics pass. As a result, the
test deterministically failed, which was fixed by using the
EmptyOrNoDiagnostics expectation.

Fixes golang/go#54271

Change-Id: Id3f220ce44c7996132699a724b6c627f034e367f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422136
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-09 15:36:31 +00:00
Robert Findley 92d58ea4e7 internal/lsp/cache: register a file watcher for explicit GOWORK values
When the go.work file is set by the GOWORK environment variable, we must
create an additional file watching pattern.

Fixes golang/go#53631

Change-Id: I2d78c5a9ee8a71551d5274db7eb4e6c623d8db74
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421501
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-08-08 18:11:08 +00:00
Robert Findley 98aef77998 internal/lsp/cache: track explicit go.work files outside the workspace
In order to correctly process changes to the go.work file, the workspace
must know about GOWORK settings configured in the users environment.
Compute this when initializing the view, and thread this through to the
workspace.

At this point, workspace information is spread around in a few places.
Add some TODOs to clean this up.

Also remove some module data that was not used in
TestBrokenWorkspace_DuplicateModules.

Updates golang/go#53631

Change-Id: Ie0577d702c8a229304387bc7fe53a8befb544acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421500
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-08 18:11:01 +00:00
Robert Findley fff6d6d39f internal/lsp: update the broken workspace message to mention go.work
For users of Go >= 1.18, update the error message presented to users
with broken workspaces to suggest using a go.work. For older Go
versions, suggest updating to 1.18+.

Also:
- add support for changing workspace folders from the fake editor
- update the test to confirm that changing workspace folders resolves
  the error message
- inline validBuildConfiguration, and make a few TODOs for how the code
  could be further cleaned up

Fixes golang/go#53882

Change-Id: Ia03dcfec59569b1a3ac941dc40d079b9c2593825
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421499
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Dylan Le <dungtuanle@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-08-08 18:10:56 +00:00
Robert Findley 06d96ee8fc gopls/internal/regtest/bench: add a test for completion following edits
For golang/go#53992

Change-Id: Ia1f1e27663992707eef9226273b152117ee977ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420220
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2022-08-05 17:04:18 +00:00
Hana (Hyang-Ah) Kim 81c7dc4e4e internal/lsp: polish vulncheck progress messages
gopls.run_vulncheck_exp runs `gopls vulncheck`
(fork of govulncheck) and pipes its stderr (logs)
as progress messages. The default log format
includes timestamp and that is too long for progress
message. Tell gopls vulncheck to omit timestamp
in the log message.

Use "govulncheck" as the progress message prefix,
instead of the long "Checking vulnerabilities".

Change-Id: I92fe9958b20d0260711a42af9b5f9f399e267587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420998
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 20:05:03 +00:00
Hana (Hyang-Ah) Kim af2a0a8167 internal/lsp: use exec.CommandContext when running vulncheck
That, hopefully, handles process termination upon context
cancellation. (not 100% guaranteed)

Change-Id: I79441afb794c37e9e55f710f124871a82aa4effe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420997
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-04 20:04:51 +00:00
Hana (Hyang-Ah) Kim 3519aa25b8 internal/lsp/cmd: remove unused Env from pkgLoadConfig
Instead rely on the process env vars the `gopls vulncheck`
command runs with.

Change-Id: I313a035d9bb7dbbdf2199474e0864cdb591e15ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420996
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-04 20:04:16 +00:00
Hana (Hyang-Ah) Kim 6c27717f2a internal/lsp/mod/code_lens: add "run govulncheck" codelens
And, make gopls.run_vulncheck_exp show an information/error
message popup after a successful run. This is temporary.
We plan to publish the results as diagnostics and quick-fix.

Finally, changed the stdlib vulnerability info id in
testdata to GO-0000-0001 which looks more like a vulnerability
ID than STD.

Changed TestRunVulncheckExp to include tests on codelens
and use the command included in the codelens, instead of
directly calling the gopls.run_vulncheck_exp command.

Change-Id: Iaf91e4e61b2dfc1e050b887946a69efd3e3785b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420995
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 20:03:56 +00:00
Hana (Hyang-Ah) Kim b5fd08821a internal/lsp/command: replace VulncheckArgs Dir with URI
commandHandler.run expects a valid file for forURI and
forURI is necessary to fully populate commandDeps.
Our use of directory URI does not work.

We plan to use this custom command triggered through
codelenses on documents (e.g. go.mod), so we use
the document's URI.

Change-Id: I4de6d488dec5a4b71716499e7fc5e8328ecf3e49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420994
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-08-04 18:58:55 +00:00
Robert Findley 99fd76f9c0 internal/lsp/cache: delete KnownMetadata.PkgFilesChanged
It is no longer needed, now that we don't consider invalid packages to
be workspace packages.

Change-Id: I6155a2609ab07046b9507dc04717eea7b974f1b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421257
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-08-04 18:51:46 +00:00
Robert Findley 01c9ff0536 internal/lsp/cache: invalid packages should not be workspace packages
To support the experimentalUseInvalidMetadata mode, we keep around
invalid metadata. This can help gopls features work when, for example,
the go.mod file is broken. It is debatable whether this feature is worth
supporting (see golang/go#54180), but in the meantime there is a very
negative side-effect when module paths are changed in the go.mod file:
we keep around a bunch of workspace packages with a stale module path.
As a result we can be left with a lots of extra type-checked packages
in memory, and many inaccurate diagnostics.

Fix this by skipping packages with invalid metadata when computing
workspace packages. While we may want to use invalid metadata when
finding the best package for a file, it does not make sense to re-
type-check and diagnose all those stale packages.

Fixes golang/go#43186

Change-Id: Id73b47ea138ec80a9de63b03dae41d4e509b8d5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420956
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 18:51:19 +00:00
pjw bd68922a85 internal/lsp: new options to disable certain kinds of semantic tokens
Semantic tokens, as defined by the LSP, have no way of marking parts of
strings or numbers, for instance, to emphasize escape character. But
if gopls returns no semantic tokens for strings, then the editor
will use its coloring for strings, which may be more useful (and
similarly for components of numbers).

This change introduces boolean flags noSemanticString and
noSemanticNumber that can be set to true to suppress the semantic
token and let the editor's formatting shine through.

Fixes: Fixes golang/go#45753

Change-Id: Ibae880a08fb9a67daa73aa172375a1c949431e11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421256
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
2022-08-04 18:30:35 +00:00
Hana (Hyang-Ah) Kim bceee4b059 internal/lsp/command: let RunVulncheckExp call gopls vulncheck
By making gopls.run_vulncheck_exp (RunVulncheckExp implements)
call `gopls vulncheck`, we achieve

  - gopls.run_vulncheck_exp can run asynchronously and be cancellable
  - log information can be forwarded as progress messages
  - isolate any failures during vulncheck execution

In this CL, we also changed not to include test files in the analysis
(match the default of govulncheck). We will add an option in the future.

TODO:
 - prevent concurrent gopls.run_vulncheck_exp
 - convert the gopls vulncheck output to diagnostics and publish it
 - remove timestamps from the `gopls vulncheck` log messages
   for simplify progress messages
 - add test to check vulnerability in third-party dependencies

Change-Id: I21592e03794cd9e9d96ed3989973a2ab7d75c538
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420717
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-04 18:21:12 +00:00
Robert Findley 3e0a5031e3 internal/lsp: use directoryFilters in import scanning
Based on dle8's CL 414454, wire in directoryFilters into the goimports
ModuleResolver scan. A few changes/fixes/additions from that CL:

- Fix a bug in filter validation that was causing -**/a not to validate.
- Fix a bug in regex building where -a was not treated as an excluded
  prefix (it was matching 'a' anywhere).
- Use absolute paths in the SkipPathInScan, so that we can evaluate
  directory filters relative to the workspace folder.
- Add several regression tests.
- Consolidate directoryFilters regression tests under a new
  directoryfilters_test.go file.
- Add several TODOs.

Fixes golang/go#46438

Change-Id: I55cd3d6410905216cc8cfbdf528f301d67c2bbac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420959
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Le <dungtuanle@google.com>
2022-08-04 15:50:11 +00:00
Robert Findley 87f47bbfb4 gopls/internal/regtest/bench: refactor and improve benchmarks
Significantly refactor the gopls benchmarks to turn them into proper
benchmarks, eliminate the need for passing flags, and allow running them
on external gopls processes so that they may be used to test older gopls
versions.

Doing this required decoupling the benchmarks themselves from the
regtest.Runner. Instead, they just create their own regtest.Env to use
for scripting operations. In order to facilitate this, I tried to
redefine Env as a convenience wrapper around other primitives.

By using a separate environment setup for benchmarks, I was able to
eliminate a lot of regtest.Options that existed only to prevent the
regtest runner from adding instrumentation that would affect
benchmarking. This also helped clean up Runner.Run somewhat, though it
is still too complicated.

Also eliminate the unused AnyDiagnosticAtCurrentVersion, and make a few
other TODOs about future cleanup.

For golang/go#53992
For golang/go#53538

Change-Id: Idbf923178d4256900c3c05bc8999c0c9839a3c07
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419988
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 14:58:59 +00:00
Erik Dubbelboer 371fc67d3b go/tools: add check for time formats with 2006-02-01
yyyy-dd-mm is a time format that isn't really used anywhere [1]. It is
much more likely that the user intended to use yyyy-mm-dd instead and
made a mistake. This happens quite often [2] because of the unusual way
to handle time formatting and parsing in Go. Since the mistake is Go
specific and happens so often a vet check will be useful.

1. https://stackoverflow.com/questions/2254014/are-there-locales-or-common-programs-that-use-yyyy-dd-mm-as-the-date-format
2. https://github.com/search?l=&p=1&q=%222006-02-01%22+language%3AGo&type=Code

Updates golang/go#48801

Change-Id: I20960c93710766f20a7df90873bff960dea41b28
GitHub-Last-Rev: 496b9917b5eda0525cb75d04a3487d174ccf8fea
GitHub-Pull-Request: golang/tools#342
Reviewed-on: https://go-review.googlesource.com/c/tools/+/354010
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-08-03 20:00:23 +00:00
Robert Findley ddb90ecd31 internal/lsp/cache: fix data races to view.options
Use the concurrency-safe view.Options method.

Fixes golang/go#54214

Change-Id: If75a544ae477ee7361540c3933a18e3366d8ffd7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420954
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dylan Le <dungtuanle@google.com>
2022-08-03 12:09:24 +00:00
Robert Findley 0d04f65da9 internal/lsp: re-send diagnostics on file events
Fix golang/go#50267 by ensuring that diagnostics are re-sent following
didOpen or didClose events. Additionally, introduce a new hidden
'chattyDiagnostics' option that causes diagnostics to be resent on
*every* file change event. This latter option is for LSP clients that
get confused when diagnostics are not re-sent for later file versions.
For now, be conservative and only force diagnostic publication on
didOpen and didClose.

Update tests whose 'NoDiagnostics' assertions were broken by the new
behavior.

Fixes golang/go#50267

Change-Id: I6332d66a1851e0d8261599d37020a03b4c598f7d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420539
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-02 18:52:36 +00:00
Robert Findley d025cced83 internal/lsp/source: don't crash requesting gc_details for an empty file
Requesting gc_details for an empty file never worked, but in the past it
would inadvertently get handled by our forgiving position helpers. With
https://go.dev/cl/409935, it became a crasher.

Fix the crash by adding handling when the package name position is
invalid.

Also move gc_details related codelens to their own file.

Fixes golang/go#54199

Change-Id: I7339b3903abd1315f1fea3dd9929d81855a8264a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420715
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-02 18:19:01 +00:00
Robert Findley 10cb4353f9 internal/lsp/regtest: improvements for shared execution modes
Following up on CL 417587, make several improvements to the regtest
runner related to shared execution modes:

- guard lazily-allocated resources with sync.Once rather than a common
  mutex.
- for simplicity, always set Runner.goplsPath
- start the separate process server synchronously, to ensure that it is
  running before test execution
- cancel the separate process server when the test runner exits
- remove the testing.T argument from server constructors
- close the regtest runner in a deferred function

Tested manually via -enable_gopls_subprocess_tests.

Change-Id: Ide3972a94c129bcce554c10dd167df01c3040d31
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419954
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-02 18:16:22 +00:00