Commit Graph

6799 Commits

Author SHA1 Message Date
Hana (Hyang-Ah) Kim a8b9ed3e93 gopls/internal/lsp/source: remove Govulncheck from Hooks
Now lsp packages are moved to the gopls module where the vulncheck
implementation exists. We no longer need to inject govulncheck
through the hook.

Change-Id: Ia627f1abe4c626d254d3e72b778535d6cb1ab41e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429938
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-09-12 13:57:24 +00:00
Hana (Hyang-Ah) Kim 678efe0184 gopls/internal/lsp/cmd: fix vulncheck error handling
Fix 1 - print usage info only when appropriate:

tool.CommandLineError is a special error that makes a command
exits with full usage documentation. When Govulncheck hook runs and
fails, it's unlikely the failure was from command line misuse
and the extra usage doc will distract users from the root cause.
Let's stop that by returning a plain error.

Fix 2 - stop printing package loading errors twice:

Package loading error was printed by the logger in gopls/internal/vulncheck.cmd
and once more by the gopls command line handler. Print it once.

For testing, added back the replace statement to go.mod.
We will update go.mod back after this commit is merged.

Change-Id: Ifaf569a31bbbc84d7b84e1b6d90a8fa0b27ac758
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429515
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429937
2022-09-12 13:21:00 +00:00
Tim King e71c338beb go/ssa/ssautil: initialize info when there is syntax
Only initialize *types.Info when there are []*ast.Files available.

Previously, when packages x and z, where x imports y and y imports z,
are loaded with packages.LoadSyntax and built with ssautil.Packages,
then package y will have *types.Info but no files.
If y has any globals initialized from a func variable, the bodies
of the function literals did not have type information and failed
to build.

Fixes golang/go#53604

Change-Id: I3cce608d6a127ac44b65947b68cf0d748fc2dfc6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422639
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-09-11 21:08:46 +00:00
Tim King 0eebaabce7 go/analysis: allow for identical SuggestedFixes
Stop treating identical SuggestedFixes as overlapping text edits.
Packages can be loaded in multiple ways due to testing, e.g.
"p" and "p [p.test]". Currently SuggestedFixes from both are
considered overlapping and so are not applied.

Updates applyFixes() to report errors in more situations.

Fixes golang/go#54740

Change-Id: I73acb3b73d88535144cfae5161faabb0615a1774
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426734
Reviewed-by: Abirdcfly Abirdcfly <fp544037857@gmail.com>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-09-09 18:15:02 +00:00
Robert Findley eeaf4eb24c internal/lsp/fake: add rename file support for testing
This CL implements the fake.Editor.RenameFile method, which mimic's the
behavior of VS Code when renaming files. Specifically:
- open buffers affected by the renaming are closed, and then reopened
  with new URIs
- files are moved on disk
- didChangeWatchedFile notifications are sent

Along the way, add missing comments and fix one place where the editor
mutex was held while sending notifications (in Editor.CreateBuffer).
Generally, the editor should not hold any mutex while making a remote
call.

For golang/go#41567

Change-Id: I2abfa846e923de566a21c096502a68f125e7e671
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427903
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-09 15:30:36 +00:00
cuiweixie 4754f75dad go/analysis/passes/copylock: modify match pattern to satisfy change sync.Once.done to atomic.Bool
Change-Id: I387c946736d76613c83d2363cb85cbc038056e0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/428986
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: xie cui <523516579@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-09-08 19:07:57 +00:00
Robert Findley a63075185d x/tools: update go.mod following moving LSP code to x/tools/gopls
For golang/go#54509

Change-Id: I1c3cb834e7216bde11ebd86654ae4dbce02d655e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429216
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-09-07 22:05:45 +00:00
Robert Griesemer 308e02cdee x/tools/go/ssa: disable slice-to-array test
CL 428938 implements slice to array conversions in the type checkers,
per accepted proposal issue golang/go#46505. The disabled test was
expected to fail, but it won't fail anymore for Go 1.20 and higher.
Disable it for now (and track it in golang/go#54822) so we can make
progress with CL 428938.

For golang/go#46505.
For golang/go#54822.

Change-Id: I87fc78dccaa2bf1517cf875fec97855e360d0f25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429295
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
2022-09-07 21:28:53 +00:00
Robert Findley 3ee171058f gopls/doc: update stale documentation and improve link names
Following up on comments from CL 428595 and CL 426796, improve links to
'here', and update a stale comment on gopls' code location.

Updates golang/go#54509

Change-Id: Ie0e04b01b6e7193294fb9c39a809cee1a5b981c5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429215
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-09-07 18:17:13 +00:00
cui fliter 5f27e05096 all: remove redundant type conversion
Change-Id: Iffb04ebd6b4afbe1a13b96b7e7469ec3b3e3380f
GitHub-Last-Rev: 6682724eb7cce8e3194650d28cb5eebd78e14700
GitHub-Pull-Request: golang/tools#396
Reviewed-on: https://go-review.googlesource.com/c/tools/+/428980
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-07 17:33:19 +00:00
Robert Findley b15dac2b88 gopls: migrate internal/lsp to gopls/internal/lsp
This CL was created using the following commands:

    ./gopls/internal/migrate.sh
    git add .
    git codereview gofmt

For golang/go#54509

Change-Id: Iceeec602748a5e6f609c3ceda8d19157e5c94009
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426796
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-09-07 16:44:44 +00:00
Timon Wong dd1bab2d98 go/analysis/passes/printf: fix panic parsing argument index
Fixes golang/go#54828

Change-Id: I516dc83230f6bc96b0ff21f3bbae702f1511e5b0
GitHub-Last-Rev: b26f46a2df820d4862b7c81200c86551b84719fa
GitHub-Pull-Request: golang/tools#395
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426875
Auto-Submit: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Tim King <taking@google.com>
2022-09-07 16:01:16 +00:00
Abirdcfly bac5507b3e go/analysis/internal/checker: make applyFixes work from the first character
Make sure modifying the first character of the file takes effect.

Fixes golang/go#54774

Change-Id: Ib77231b9bd15f35fe50b2c2d6c7ea260c9c3cba5
GitHub-Last-Rev: b58bbdf4c247b22947223b157a1d36ecc856c652
GitHub-Pull-Request: golang/tools#393
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426654
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Tim King <taking@google.com>
2022-09-07 15:31:31 +00:00
Robert Findley c1dd25e80b gopls/internal/migrate.sh: a script to migrate internal/lsp to gopls/
Add a script that does the migration of the internal/lsp directory to
gopls/internal/lsp. This is done in a separate CL so that in-progress
CLs can rebase on top of *this CL*, run this script, and then rebase to
tip.

For golang/go#54509

Change-Id: I6f529c1e4ba29b4d88dc26278d54a055f1ef212e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426795
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-09-07 14:10:36 +00:00
Peter Weinberger 83d76192b2 gopls : add a mention of staticcheck to settings documentation
Fixes golang/go#52874

Change-Id: I04664154d68e31f48234c13aefe8470b09f0413e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/428595
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-06 14:55:05 +00:00
Dylan Le d815cba582 internal/lsp: update LSP protocol for WorkspaceEdit
Change DocumentChanges field type from []TextDocumentEdit to []DocumentChanges. DocumentChanges is a union type of TextDocumentEdit and RenameFile used for renaming package feature. It distinguishes a renaming directory ops from a text document edit ops.

For golang/go#41567

Change-Id: I25d106b34821effc53b712800f7175248e59ee25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427538
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-02 19:58:35 +00:00
Robert Griesemer 6a585a2bf9 x/tools/internal/typeparams: use regexp to match wanted result
CL 410955 changes printing of unions which breaks two typeparams tests.
Use regexp matching and adjust the wanted results accordingly.

For golang/go#53279.
For golang/go#54822.

Change-Id: I7060df47d36ce3069570237dafff024aaad637a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/428055
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@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: Robert Griesemer <gri@google.com>
2022-09-02 19:06:05 +00:00
Abirdcfly be9eab19b4 go/analysis/passes/inspect: fix example return
Change-Id: I9e248c45078d36ded9c6008e2b7e2c8bf8586df1
GitHub-Last-Rev: 33900677c6118060af36c6ec16db5b937912ae61
GitHub-Pull-Request: golang/tools#390
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425204
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2022-09-02 18:39:41 +00:00
Robert Griesemer 5ba85415fa x/tools/internal/lsp/source: disable some tests for std lib changes
CL 410955 changes printing of unions which breaks a few lsp tests.
Disable them for now so we can submit CL 410955.

For golang/go#53279.
For golang/go#54822.

Change-Id: I54ff99a4f5530181a39557b6b62e776af082c28d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/428054
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
2022-09-02 18:17:25 +00:00
Hana (Hyang-Ah) Kim eb8352e307 gopls/internal/govulncheck: sync x/vuln@62b0186
VulnDB OSV schema was changed recently
  https://go-review.googlesource.com/c/vulndb/+/424895
to fix the misinterpretation of 'affected.package.name',
and the database entries were repopulated with the new schema.
We need to update the client library to pick up the change.
We also need to update the fake vulndb entries used in tests.

gopls/regtest/misc/testdata/vulndb was copied from
  golang.org/x/vuln/cmd/govulncheck/testdata/vulndb @ 62b0186
(the version updated in cl/424895)

Also reverse golang.org/cl/425183 which includes the position
information in the SummarizeCallStack result. Like in govulncheck -v,
the position info is already available in the callstack, thus
this is unnecessary for us. Since x/vuln is currently frozen
until the preview release, revert it from gopls/internal/vulncheck.

Ran go mod tidy -compat=1.16; otherwise, the transitive dependency
on github.com/client9/misspell from golang.org/x/vuln breaks go1.16
build.

Updated copy.sh script to copy x/vuln/internal/semver package
(golang/go#54401) and add the build tags back to all go files.
Gopls's builder builds&tests packages with old go versions,
so we still need go1.18 build tag.

Fixes golang/go#54818

Change-Id: I37770d698082378656a7988d3412a4ca2196ca7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427542
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
2022-09-02 15:10:33 +00:00
Robert Findley 655abda1c0 gopls/internal/regtest: TestEditGoDirectiveWorkspace should fail eagerly
TestEditGoDirectiveWorkspace did an unconditional wait on expected
diagnostics. Turn this into OnceMet conditions that fail as soon as the
relevant diagnostic pass is complete.

Fixes golang/go#54825

Change-Id: I2654682108561dd60546a589ebd1c6aa2ebaff1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427543
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-09-02 14:20:31 +00:00
Robert Griesemer 33c1ddd5a8 tools/gopls/internal/regtest/diagnostics: handle new error message
Pending CL 426477 changes go/types error messages containing the phrase:

	type parameters require

to:

	type parameter requires

Adjust diagnostics test accordingly (note that regular expressions are
currently not supported).

For golang/go#54511.

Change-Id: I561cb940a41cb6cc949c44e0d4b8f009336a46cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427736
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
2022-09-02 00:49:50 +00:00
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 fe1a27b55b gopls/doc: make doc generation work regardless of the current directory
As a nice side effect, make it easier to migrate internal/lsp/ to
gopls/internal/lsp.

For golang/go#54509

Change-Id: Ib541c08426f1f1d1e2a42b2d1cab47eab96dc092
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426775
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-31 13:23:24 +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
Abirdcfly 717a671622 go/analysis/passes/printf: remove unused hasBasicType
Change-Id: Ic1be5931a620e3fd15b58a7acc34d0013f011a20
GitHub-Last-Rev: 32b11c95b8c0715aa13a741913759198ec208942
GitHub-Pull-Request: golang/tools#391
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425834
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: David Chase <drchase@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
2022-08-26 17:59:00 +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
Matthew Dempsky db6a62ca56 go/internal/gcimporter: call Interface.Complete in unified importer
Port of CL 425360 and CL 425365 from stdlib importer to x/tools.

Fixes golang/go#54653.

Change-Id: Ib475f715ae70400e3ebfb91d6b7755d8e1ddee37
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425362
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-24 21:20:11 +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
Russ Cox a3cac11881 godoc/redirect: delete golang.org-specific code
This logic is now in x/website.
Delete from here to avoid confusion.

Change-Id: Iec8efaa3490fa471a4ebd7e1fb34b4927a39062d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/301309
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
2022-08-16 14:47:05 +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 938e162bcf gopls/internal/regtest: unskip TestDeleteModule_Interdependent
Reloading has been significantly refactored recently. Unskip this test
to see if it flakes:
 - If it does not flake, that is a useful signal.
 - If it does flake, that is also a useful signal.

Notably, following CL 419500 we allow network when reloading the
workspace, and so do not need to apply quick-fixes in order to download
the new module from the proxy.

For golang/go#46375
For golang/go#53878

Change-Id: Idde7195730c32bdb434a26b28aac82649dd1b5ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422910
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-15 20:02:31 +00:00
Rob Findley e8507bed92 gopls/internal/regtest/bench: replace -gopls_version with -gopls_commit
The go command can't always resolve arbitrary commits as versions, and
some commits have an x/tools replace directive that must be honored, so
use 'git clone' with an arbitrary commit ref, instead of 'go install'
with a Go module version, to install gopls.

Also rename BenchmarkIWL to BenchmarkInitialWorkspaceLoad.

For golang/go#53538

Change-Id: Ic3a08e4c023e0292f6595cc5b2ab59954d073546
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422908
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-15 19:50:52 +00:00
Robert Findley 8c830569a8 gopls/internal/regtest: unskip TestSumUpdateFixesDiagnostics
Metadata reloading has been significantly refactored recently. Unskip
this test to see if it still flakes.

For golang/go#51352
For golang/go#53878

Change-Id: I9f2ae1835bbace1b5095c2d45db082c4e709437b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/423974
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
2022-08-15 18:38:24 +00:00