Commit Graph

6716 Commits

Author SHA1 Message Date
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 d08f5dc9fb gopls/internal/regtest: unskip all of TestModFileModification
I believe the races described in the issue have been fixed: we should
invalidate mod tidy results on any metadata change. If this invalidation
doesn't work due to a race, we want to know about it.

Update the test to wait for file-related events to complete before
removing files, in an attempt to avoid windows file-locking issues.

For golang/go#40269
For golang/go#53878

Change-Id: I91f0cb4969851010b34904a0b78ab9bd2808f92e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420718
Run-TryBot: Robert Findley <rfindley@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-03 18:07:40 +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
Robert Findley 4d0b383458 internal/lsp/regtest: minor cleanup for magic regtest envvar
For some reason, we were not reusing the constant runTestAsGoplsEnvVar
in the regtest Main function. Fix this, add a bit more commentary, and
check the envvar before doing anything else. Also fix the threading of
hooks to the created gopls server.

Tested manually via -enable_gopls_subprocess_tests.

Change-Id: Ieb5329aa5850e845f4d9e3868703bfa16387bce3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420716
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-02 16:05:48 +00:00
Robert Findley 310ea71b71 gopls/internal/regtest: add a test that ignoring a file resolves errors
Add a test that when a user adds a "//go:build ignore" to a file, gopls
correctly invalidates diagnostics.

This used to be broken, but was fixed by CL 417576.

Fixes golang/go#54147

Change-Id: I554fcfc0a56b72f657e19b3c0ae53a66d1a99a76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420537
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-08-02 14:13:40 +00:00
Robert Findley 21861e6be5 gopls/internal/regtest/bench: put feature benchmarks in their own file
Purely for consistent organization. No functional changes.

Change-Id: Id26d55d6496523827c154f8a2a17b3660f6081eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419982
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-08-01 21:40:07 +00:00
David Chase c7f11917cb go/internal/gcimporter: set underlying types in proper order; flatten imports
These are copied from go/internal, for processing
export data from unified IR compilation.

The underlying types order problem appeared in google-internal
testing. If the run-later functions are run in the wrong order,
type definitions won't resolve properly.

Flatten imports makes the unified IR export/import match the
behavior of export data from older compilers, though it's not
clear that this behavior was intended, it is now expected.

See https://go.dev/cl/419996 and https://go.dev/cl/419596

Change-Id: I4197fe9e93ee07eb7f24597ba9157ce083a1d086
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420534
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-01 20:48:56 +00:00
Dylan Le bd3f524777 internal/lsp: rename all the package names in the renamed package
This CL contains a partial implementation of package renaming. Currently gopls is only able to rename references to the renaming package within the renaming package itself.

prepareRename is still expected to return an error for renaming a package.

For golang/go#41567

Change-Id: I3683a0a7128bba7620ef30db528f45b753e6c08f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420219
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dylan Le <dungtuanle@google.com>
2022-08-01 17:28:00 +00:00
Robert Findley 9f65685098 internal/lsp/source: enable the new diff with allExperiments
When enabling all experiments (done by VS Code nightly), switch to the
checked version of the new diff implementation.

Also remove some experimental settings that are now on by default.

Updates golang/go#52967

Change-Id: Id272c4a646006a739e49d48f0f09b2f8b0982bab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419981
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-01 13:49:32 +00:00
Dylan Le 9580c84d57 internal/lsp: Check if user's editor support rename operation
Change-Id: Iadda768e93eda1d53fa00a5ff8a28013a575ef57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419774
Run-TryBot: Dylan Le <dungtuanle@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-29 20:19:23 +00:00
Robert Findley f560bc877f internal/lsp/cache: don't set context cancellation as a critical err
Fix a tricky race in gopls/internal/regtest/diagnostics.Test_Issue38211:
When reloading the workspace, we can encounter context cancellation if
the snapshot is cancelled, and can write this cancellation as a critical
error *before* the context is cloned, leading to a state where there is
a critical error that won't go away.

This should resolve test flakes reported in golang/go#44098.

For golang/go#44098

Change-Id: I41c0f49b2fe999131f4c31166e69b2cde85470b7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419502
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-07-29 15:25:28 +00:00
Robert Findley 8ea5687987 internal/lsp/regtest: remove arbitrary timeout for closing the editor
Following up on some post-submit feedback in CL 417583.

Fixes golang/go#53819

Change-Id: Iebb1e6496ab1d6fde8961d8617d0b63e19c7033b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419503
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-28 20:06:00 +00:00
Robert Findley d01bb2ff91 internal/lsp/source: document the handling of GOPRIVATE for linkTarget
Document that modules matching GOPRIVATE will not be linked.

Updates golang/vscode-go#2362

Change-Id: I7e2447bb50a2cd0d7d394f8589ccd4498f889048
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419979
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-07-28 19:37:47 +00:00
Robert Findley 98bfcd1bee internal/memoize: fix race in Store.Promise
When releasing a promise, there was a theoretical race whereby a
promise's refcount could be incremented before Store.promisesMu was
acquired and the promise deleted.

We could fix this by double-checking after acquiring Store.promisesMu,
but it seemed simpler to just guard Promise.refcount with
Store.promisesMu, and skip using atomics. We already lock promisesMu
when acquiring the promise, and so locking when releasing should not
significantly affect our level of contention.

Additionally, make it a panic to call the returned release function more
than once, and document this behavior.

Change-Id: I1135b558b1f13f2b063dcaad129a432c22da0b28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419504
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-28 15:35:11 +00:00
Robert Findley e02e98a037 internal/lsp/cache: allow network whenever reloading the workspace
Per the explanation at golang/go#54069, allow network access whenever
loading the workspace. Enable one test that exercises this behavior.
More tests will be added in subsequent CLs.

Updates golang/go#47540
Updates golang/go#53676
Updates golang/go#54069

Change-Id: I9c3bb19d36702bc6b8051bee6b7cddaec5b97c0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419500
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-27 19:28:51 +00:00
Robert Findley b52794acc2 internal/lsp/cache: simplify snapshot.Clone reinitialization logic
The logic to detect whether the snapshot should be reinitialized is
complicated, and had at least a couple latent bugs:
 - It depends on workspace.Clone to learn whether relevant mod/work
   files have been saved.
 - It naively checks for changes in vendor directories, and
   reinitializes if there was an initialization failure.
 - It copies the initializeOnce field. Pragmatically this may not matter
   (due to context cancellation, for example), but as a matter of best
   practices we should not copy this field. It could lead to missing an
   initialization on the new snapshot.
 - Writing initializeOnce was technically racy.

Furthermore, due to vendored file handling being interleaved with other
changes, it was possible that we detect that the snapshot should be
reinitialied only after we have processed other changes, and that
processing of other changes depended on knowing whether the snapshot
will be reinitialized.

Simplify this by
- Replacing 'initializeOnce' with an 'initialized' bool, which is
  guarded with snapshot.mu. We were relying on the view initialization
  semaphore to prevent races anyway, so there is no need for a sync.Once
  to prevent multiple in-flight initializations.
- Removing the 'changed' result from workspace.Clone: it shouldn't
  actualy matter for snapshot.Clone (only reinit matters), and in any
  case can be derived by comparing the new workspace with the old.
- Detect changes to vendored files before processing the rest of the
  changes.

Change-Id: I3624a46d4be9c054a6f719f20549599986b64cbd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419499
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-27 19:27:52 +00:00
Robert Findley f1bb5ca08f internal/lsp/cache: report a critical error when go.work is invalid
When a go.work file fails to validate, the workspace is left in an
invalid state: we will detect that the workspace is defined by the
go.work, but will not actually parse any active modules. This should be
a critical error.

Fix this by adding allowing the workspace to surface critical errors via
a new cache.workspace.criticalError method.

Additionally:
 - only build the workspace mod file in workspace.build if the mode is
   fileSystemWorkspace (in all other modes the modfile is already
   determined)
 - rename workspace.invalidate to workspace.Clone, to be consistent with
   other data structures
 - rename CriticalError.DiagList to CriticalError.Diagnostics
 - add several TODOs for observations while reading the code
 - create a new file for regtests related to broken workspaces
 - make the regtest sandbox panic when duplicate paths are present in
   the sandbox file set (an error I made while writing the test)

Updates golang/go#53933

Change-Id: If8625ab190129bc9c57e784314bc9cc92644c955
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417593
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-07-27 18:59:40 +00:00
Robert Findley b3b5c13b29 internal/lsp/cache: invalidate packages with missing deps when files are
added

Add logic to invalidate any packages with missing dependencies when a
file is added.

Also fix a latent bug overwriting 'anyFileOpenenedOrClosed' for each
loop iteration.

Fixes golang/go#54073

Change-Id: I000ceb354885bd4863a1dfdda09e4d5f0e5481f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419501
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-07-27 16:29:04 +00:00
Robert Findley 39a4e36475 internal/lsp/regtest: only run /default tests with -short
Due to shared caching, the "default" tests can run faster than other
execution modes and still retain most of the test coverage. Update the
test runner to only run the singleton tests if testing.Short() is true,
independent of GOOS.

On the other hand, we lost noticeable coverage when disabling the
Forwarded testing mode. Now that the regtests are lighter weight in
general, reenable it on the longtests builders.

While at it, clean up tests that used the server-side Options hook to
instead use Settings, since clients would configure their server via
Settings and Options can't work with a shared daemon.

Updates golang/go#39384

Change-Id: I33e8b746188d795e88841727e6b7116cd4851dc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418774
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-26 21:20:53 +00:00
Robert Findley f157068c1b internal/lsp/regtest: allow sharing memoized results across regtests
Each regtest does a significant amount of extra work re-doing things
like parsing and type-checking the runtime package. We can share this
work across regtests by using a shared cache, significantly speeding
them up at the cost of potentially hiding bugs related to timing.

Sharing this work still retains most of the benefit of the regtests, so
implement this in the default mode (formerly called "singleton" and now
renamed to "default"). In a subsequent CL, modes will be cleaned up so
that "default" is the only mode that runs with -short.

Making this change actually revealed a caching bug: our cached package
stores error messages extracted from go/packages errors, but does not
include these errors in the cache key. Fix this by hashing all metadata
errors into the package cache key.

Updates golang/go#39384

Change-Id: I37ab9604149d34c9a79fc02b0e1bc23fcb17c454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417587
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-07-26 21:20:42 +00:00
Hana (Hyang-Ah) Kim 8ccb25c9a3 internal/lsp: treat struct tags as string type
For golang/go#54066

Change-Id: Ia4f0bf0b4d76743a7f4fafc375859db7184753fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419498
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-07-26 20:35:51 +00:00
Suzy Mueller 6c8a6c4093 internal/lsp: suppress parameter hint when argument matches parameter
Suppress the parameter hint when it would present redundant
information.

Fixes golang/go#2361

Change-Id: I4340a903046f212f8a035eab847da665e2692f1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419497
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-07-26 18:43:08 +00:00
Suzy Mueller c83f42da70 internal/lsp: update inlay hints documentation to include go snippets
Update the markdown documentation for inlay hints and fix a couple
of typos in the examples.

Change-Id: I114502a81999bc5e4f25384ab619888f3e31a731
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419496
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-26 18:27:49 +00:00
Robert Findley 8b47d4e187 all: update dependencies
Update all x/tools dependencies.

Change-Id: I0a81f9821a9267bef9057f294efc3ac1c13b59ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419495
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-26 14:37:02 +00:00
Robert Findley 76004542dc gopls: update dependencies
Update all dependencies, except sergi/go-diff.

Also tidy x/tools with -compat=1.16, as it had recently been broken at
1.16.

Change-Id: I2e6c9bf48c6bedb2dff0fa418bf588dd07918866
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419494
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-07-26 14:36:43 +00:00
Dylan Le 2a6393fe54 internal/lsp: Refactor to share logic with rename
If a package is being operated on, getPackage returns that package information; otherwise nil.

Change-Id: I881056510b8d6862c274a7532fdfbc840c938468
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418791
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-07-25 19:36:27 +00:00
Abirdcfly 4375b29f44 cmd/auth/cookieauth: delete unreachable os.Exit
Change-Id: I5c60eeb8667423544b2bc8b9cf5f51279b0a941d
GitHub-Last-Rev: cb0eca5ff34755a0185729e5c81bf08eb92fab39
GitHub-Pull-Request: golang/tools#388
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418854
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-07-25 18:11:01 +00:00
Hana (Hyang-Ah) Kim 005c07ac5a gopls/internal/vulncheck: adjust logging
Report # of findings

Change-Id: Ib10d18a23280a8644b9c08a8d51d6e288ae761db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415496
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-07-25 16:55:29 +00:00
Suzy Mueller 04bd087817 internal/lsp: enable fillstruct for generics
This enables some fill struct code actions for instances of structs with
type parameters.

This additionally adds a filtering mechanism to the suggested fixes in
order to account for multiple suggested fixes in the same location.

Change-Id: I98866b462b026f4c5a4897bc278f704381623f25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418415
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
2022-07-25 16:40:20 +00:00
Robert Findley 6ec939a616 internal/span: fix incorrect bounds check in ToOffset
token.File.LineStart panics if line < 1, but we were checking line < 0.
Surprising that this was not hit more often: it looks like we
pre-validate input except for parsed positions coming from the Go
command. It is possible that an older version of the Go command returned
invalid positions.

Also report a bug for one error condition in ToOffset: the position
returned by LineStart should always be valid.

Fixes golang/go#54006

Change-Id: I5965af9c62976b3e00b023512df334a8de943a3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419109
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-25 14:31:12 +00:00
Robert Findley 178fdf98da gopls/internal/regtest: unskip Test_Issue38211
This test was originally skipped due to deadline exceeded errors. In the
time since, we've made performance improvements, fixed races, and
altered the regtests to remove arbitrary deadlines. Unskip it to see if
it still flakes.

For golang/go#44098
For golang/go#53878

Change-Id: I06530f2bc9c6883f415dc9147cfcbf260abb2a00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418898
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-07-25 14:16:17 +00:00
Robert Findley 1cfe623ebf gopls/internal/regtest: unskip TestQuickFixEmptyFiles
This test ran slowly sometimes. Now that we have improved performance,
elimininated arbitrary timeouts, and improved cacheability of
computed results when running with -short, I suspect this test should no
longer flake.

If it does, we can reduce its cost in other ways rather than turning it
off entirely.

Updates golang/go#48773
Updates golang/go#53878

Change-Id: I878e78117df5a1a25f4ac5f72e02f28fc078ec73
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419106
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-25 14:15:48 +00:00
Peter Weinberger 3d474c8905 internal/lsp/diff: new diff implementation to replace go-diff
The new implementation is based on Myers' paper, and is in the
package diff/lcs.

There is a new option newDiff, that can be set to 'old', 'new',
or 'both'. The default is 'both', although that may not be
the right choice for a release. See gopls/hooks/diff.go.
'both' runs both the old and new diff algorithm and saves some
statistics in a file in os.Tempdir(),

When (or if) the new code becomes the default, this logging (and
some internal checking) will be removed.

The new implementation has internal checking, which currently
panics. The code in gopls/hooks/diff.go tries to save an encrypted
(for privacy) version of the failing input.

The package diff/myers has not been replaced, but it could be.

Fixes golang/go#52966

Change-Id: Id38d76ed383c4330d9373580561765b5a2412587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/396855
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
2022-07-24 16:55:18 +00:00
Robert Findley a2a24778ba gopls/internal/regtest: externalize shouldLoad tracking
The fundamental bug causing TestChangePackageName to fail has been
fixed, yet unskipping it revealed a new bug: tracking whether or not a
package should be loaded requires that we actually store that package in
s.meta. In cases where we drop metadata, we also lose the information
that a package path needs to be reloaded.

Fix this by significantly reworking the tracking of pending loads, to
simplify the code and separate the reloading logic from the logic of
tracking metadata. As a nice side-effect, this eliminates the needless
work necessary to mark/unmark packages as needing loading, since this is
no longer tracked by the immutable metadata graph.

Additionally, eliminate the "shouldLoad" guard inside of snapshot.load.
We should never ask for loads that we do not want, and the shouldLoad
guard either masks bugs or leads to bugs. For example, we would
repeatedly call load from reloadOrphanedFiles for files that are part of
a package that needs loading, because we skip loading the file scope.
Lift the responsibility for determining if we should load to the callers
of load.

Along the way, make a few additional minor improvements:
 - simplify the code where possible
 - leave TODOs for likely bugs or things that should be simplified in
   the future
 - reduce the overly granular locking in getOrLoadIDsForURI, which could
   lead to strange races
 - remove a stale comment for a test that is no longer flaky.

Updates golang/go#53878

Change-Id: I6d9084806f1fdebc43002c7cc75dc1b94f8514b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417576
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-22 21:31:20 +00:00
Jamal Carvalho 7b605f471d gopls/internal/vulncheck: pass go version to vulncheck config
For golang/go#53869

Change-Id: I8c34adaf81415dafb724ca67fa731912832c3ee5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418538
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-07-22 16:51:37 +00:00
Jamal Carvalho 126ef8f864 gopls/internal/govulncheck: sync x/vuln@b9a3ad9
For golang/go#53869

Change-Id: I8cf795b792380596be306b2437e26faf990cff8b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418537
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-22 16:51:01 +00:00
Jamal Carvalho a732e45cc7 gopls: update golang.org/x/vuln
For golang/go#53869.

Change-Id: If173f0e7e3a06c11cd358ff9917c73b50d2dcb28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418536
Run-TryBot: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-07-22 16:50:46 +00:00
Russ Cox 980cbfeacb A+C: delete AUTHORS and CONTRIBUTORS
In 2009, Google's open-source lawyers asked us to create the AUTHORS
file to define "The Go Authors", and the CONTRIBUTORS file was in
keeping with open source best practices of the time.

Re-reviewing our repos now in 2022, the open-source lawyers are
comfortable with source control history taking the place of the
AUTHORS file, and most open source projects no longer maintain
CONTRIBUTORS files.

To ease maintenance, remove AUTHORS and CONTRIBUTORS from all repos.

For golang/go#53961.

Change-Id: Icaaaf04cc7884b479c7fd1231c53c8bf1f349623
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419105
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-22 15:53:04 +00:00
Suzy Mueller ec1f92440b internal/lsp: add check for nil results to fillreturns
Avoid panicking when allocating an array for a nil results list
by returning early.

Change-Id: I26953b5cef7832bad3006bd316d59978a5d94cbd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418416
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-20 19:56:16 +00:00
aarzilli 79f3242e4b godoc: support go1.19 doc comment syntax
Call go/doc.(*Package).HTML on go1.19 instead of the deprecated
go/doc.ToHTML.

Change-Id: Ie604d9ace7adc179f7c2e345f17a2e0c0365d1a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/411381
Run-TryBot: Alessandro Arzilli <alessandro.arzilli@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
2022-07-19 22:18:38 +00:00
Zvonimir Pavlinovic 2957e9da5d go/callgraph/vta: use types.IsInterface instead of our own isInterface
Change-Id: I9e5a81e4f59f32e3bfc6baf2348ee3e4db411aae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417674
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-18 23:12:37 +00:00
Zvonimir Pavlinovic 2eaea86599 go/callgraph/vta: do not include interface types during propagation
Some interface types could be propagated around type graph. This does
not affect precision as the results are ultimately intersected with the
initial call graph. However, this types of propagation are not
necessary. On very large projects, this can save few seconds.

Change-Id: I73d41c082a52734f50669af19fee940725aed662
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417514
Reviewed-by: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-18 22:51:18 +00:00
Dylan Le dc45e742f0 internal/lsp: Update FilterDisallow to support matching directories at arbitrary depth.
In FilterDisallow, change filter to regex form to match with file paths. Add a unit regtest for FilterDisallow.

For golang/go#46438

Change-Id: I7de1986c1cb1b65844828fa618b72b1e6b76b5b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414317
Run-TryBot: Dylan Le <dungtuanle@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-18 17:53:36 +00:00
Robert Findley ce6ce76626 internal/lsp/regtest: increase the time allowed for shutdown
Now that we await ongoing work during shutdown, we are seeing regtest
flakes simply due to outstanding go command invocations.

Allow more time for cleanup. If this is insufficient, we can be more
aggressive about terminating go command processes when context is
cancelled.

For golang/go#53820

Change-Id: I3df3c5510dae34cb14a6efeb02c2963a71e64f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417583
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dylan Le <dungtuanle@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-07-15 16:56:31 +00:00
Robert Findley 32129bf2c9 go/internal/gcimporter: adjust importer to match compiler importer
This is a port of CL 288632 to x/tools/go/internal/gcimporter. This
logic was ostensibly unported to avoid breaking build compatibility with
go1.12. Now that gopls no longer support 1.12, It is safe to make this
change.

Fixes golang/go#53803

Change-Id: Ic9b4d7a60511076a83d8fa72cf7c4a3bdcab3fce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417580
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-07-15 15:17:57 +00:00
David Chase 22d149443a internal/gcimporter: add support for reading unified IR export data
This does not include writing export data in the unified IR format.

Most of this change is an import of code from the Go implementation,
with minor tweaks and gaskets added.

This does not (yet) address the registry issue mentioned in golang/go#52163.

Updates golang/go#52163.

Change-Id: I98030e6c9ff35c6ff678b8a7ce9b653b18e65e17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/412821
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-15 15:11:26 +00:00
Alan Donovan c3af7c2fa9 internal/lsp/cache: delete workspacePackageHandles (dead code)
This should have been in CL 417116.

Also:
- (related to CL 417415), rename packageHandle.check to await
  to indicate its blocking nature.
- rename typeCheck to typeCheckImpl, following the pattern.
- move "prefetch" parallel loop into typeCheckImpl.
- add some comments.

Change-Id: Iea2c8e1f1f74fb65afd0759b493509147d87a4bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417581
Run-TryBot: Alan Donovan <adonovan@google.com>
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>
2022-07-15 14:53:26 +00:00