...since we do our own parsing.
Change-Id: Id762cca408692b9535b8bb36017d6719180e5bb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/449498
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This change will make it possible to do semantic token regtests.
Change-Id: I9963c60f61af30f973a2ee4cd32aaa5545bdc4ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448296
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Coverage.go computes the test coverage from running all the gopls tests.
This CL accounts for the changed source tree (internal/lsp is gone)
and new actions returned by go test -json ('pause' and 'cont').
Change-Id: I970b3ec107746ce02e3dcdcad9f8c19cffad8d11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448295
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This test is flaky, likely due to some race in the
experimentalWorkspaceModule logic. Since we're about to delete support
for that experimental feature, simply skip the test to stop the flakes.
Leave the test as an artifact, as it will be deleted as part of the
clean up of experimentalWorkspaceModule. No need to delete it before
then.
Updates golang/go#55331Fixesgolang/go#55923
Change-Id: Ic17485e42e335459df462af00a2088812ecfb5f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448116
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Findley <rfindley@google.com>
When awaiting diagnostics, we should almost always wrap expectations in
a OnceMet(precondition, ...), so that tests do not hang indefinitely if
the diagnostic pass completes and the expectations are still not met.
Before this change, the user must be careful to pass in the correct
precondition (or combination of preconditions), else they may be
susceptible to races.
This change adds an AllOf combinator, and uses it to define a new
DoneDiagnosingChanges expectation that waits for all anticipated
diagnostic passes to complete. This should fix the race in
TestUnknownRevision.
We should apply a similar transformation throughout the regression test
suites. To make this easier, add a shorter AfterChange helper that
implements the common pattern.
Fixesgolang/go#55070
Change-Id: Ie0e3c4701fba7b1d10de6b43d776562d198ffac9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448115
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
While checking for changes to go.mod files in the importsState, there is
no reason for special handling based on workspace mode: we can simply
hash all active modfiles. This moves us towards an improved API for the
workspace: it should simply be responsible for tracking active modfiles.
This also incidentally avoids the panic reported in golang/go#55837.
Fixesgolang/go#55837
Change-Id: I8cb345d1689be12382683186afe3f9addb19d467
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447956
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Update selected dependencies following the v0.10.0 release, excluding
sergi/go-diff and x/vuln.
Gofumpt@v0.4.0 requires go1.18, so link it selectively following the
pattern of staticcheck. While at it, clean up some things related to the
wiring of staticcheck and gofumpt support. Notably, in VS Code error
messages do not support formatting such as newlines or tabs.
Add a test for the conditional Gofumpt support.
For golang/go#56211
Change-Id: Id09fdcc30ad83c0ace11b0dea9a5556a6461d552
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446736
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Remove the internal guard preventing the loopclosure analyzer from
checking parallel subtests.
Also, improve the accuracy of the parallel subtest check:
- only consider statements after the final labeled statement in the
subtest body
- verify that the *testing.T value for which T.Parallel() is invoked
matches the argument to the subtest literal
Fixesgolang/go#55972
Change-Id: Ia2d9e08dfa88b5e31a9151872025272560d4b5e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447256
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
This CL includes some clarifications while trying to
understand the performance of the initial workspace load
and analysis. No significant behavior changes.
Server.diagnose:
- Factor the four copies of the logic for dealing
with diagnostics and errors.
- Make the ActivePackages blocking step explicit.
Previously mod.Diagnostics would do this implicitly,
making it look more expensive than it is.
Server.addFolders:
- eliminate TODO. The logic is not in fact fishy.
- use informative names and comments for WaitGroups.
- use a channel in place of a non-counting WaitGroup.
Also, give pkg a String method.
Change-Id: Ia3eff4e784fc04796b636a4635abdfe8ca4e7b5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445897
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Rename (*connection).AddFile to openFile, which is more accurate, and
fix a deadlock resulting from holding a Client lock while issuing a
Server request.
Fixesgolang/go#56450
Change-Id: Ie6f34613e1e10e3274c3e6728b12f77e3a523b89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446856
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Fix a panic during completion on variables of type *error. As a
predeclared type, the error type has nil package. Fix the crash
resulting from this oversight, as well as a related crash in the tests
analyzer, from which the new completion code was adapted.
Fixesgolang/go#56505
Change-Id: I0707924d0666b238821fd14b6fc34639cc7a9c53
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446815
Auto-Submit: Robert Findley <rfindley@google.com>
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>
Memory profiles show heavy allocation for the stack
and the function closure of FindDeclAndField.
This change moves both outside the loop, reducing
this function's fraction of allocation from 6.7%
before to 5.0% after, and reducing running time
by 3-7% on these benchmarks.
before
BenchmarkStructCompletion/completion-8 100 10432280 ns/op
BenchmarkImportCompletion/completion-8 1350 921785 ns/op
BenchmarkSliceCompletion/completion-8 100 10876852 ns/op
BenchmarkFuncDeepCompletion/completion-8 142 7136768 ns/op
BenchmarkCompletionFollowingEdit/completion-8 63 21267031 ns/op
After
BenchmarkStructCompletion/completion-8 100 10030458 ns/op
BenchmarkImportCompletion/completion-8 1311 918306 ns/op
BenchmarkSliceCompletion/completion-8 100 10179937 ns/op
BenchmarkFuncDeepCompletion/completion-8 150 6986303 ns/op
BenchmarkCompletionFollowingEdit/completion-8 63 20575987 ns/op
Change-Id: Ia459e41ecf20851ff4544f76ad7b415a24606cd1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446185
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Serve had a misleading name and signature: it did not actually block
on serving the connection, and never returned a non-nil error.
Updates golang/go#56281.
Change-Id: Ia6df0ba20066811b0551df3b3267dff2fffd7881
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443678
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Also make (*Connection).Close safe to call from Bind.
A jsonrpc2_v2.Server has no good way to report an error from Bind.
If the Server saves the error to return from its own Wait method, that
might not ever actually happen: Wait waits for in-flight connections
to complete, but if some existing connection stays up then Wait will
not return.
If the Server goes ahead with establishing the connection and installs
its own Handler, that Handler needs to decide whether to serve the
error from Bind or something more opaque, and at that point Bind may
as well return a handler that makes that choice more precisely.
If the Server merely logs the error and closes the Connection, then
the Bind method itself may as well do that directly too.
It seems to me that the only winning move is not to play. Only Bind is
in a position to decide how to handle its errors appropriately, so it
should not return them to the Server.
Updates golang/go#56281.
Change-Id: I07dc43ddf31253ce23da21a92d2b6c0f8d4b3afe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443677
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Also implement and use the Shutdown method, which was mentioned in a
doc comment in CL 292169 but not actually present at that time.
With proper synchronization, we don't need heuristics to determine
whether an error is due to a connection or listener being closed.
We know whether we have called Close (and why), and we can assume
that if we have called Close then that is probably the reason for
any returned error.
Fixesgolang/go#56281.
Change-Id: I5e0ed7db0f736ca8df8cd8cf556b674e7a863a69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443675
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
When the default filter glob changed to **/node_modules,
the performance of BenchmarkWorkspaceSymbols was observed
to degrade by an astonishing 20%.
(CPU profiles of the benchmark reported that the Disallow
functions percentage had increased only slightly, but these
measures are misleading since the benchmark has a very
CPU-intensive set-up step, so all the percentages are
quotients of this figure, masking their relative importance
to the small region during which the benchmark timer is
running.)
This change removes the unnecessary ^.* prefix from the
generated regular expression. Really the regexp package
ought to do this.
Also, minor cleanups and tweaks to the surrounding code.
Change-Id: I806aad810ce2e7bbfb2c9b04009d8db752a3b10d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446177
Run-TryBot: 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>
CL 445581 inadvertently removed suppression of the Go version error if
the Go version was undetected. Add it back, with a test.
Fixesgolang/go#56465
Change-Id: I352369096280c8d3423a7345123ec9309359fb58
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446175
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
VSCode Go Nightly uses `allExperiments` setting which triggers
calling this option. It doesn't make sense to add the settings
that are scheduled to be deleted.
Change-Id: I443d7b1722feafee04b6c63a06ff514a396c5d50
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446095
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Improve the Go version deprecation message to be a warning at 1.13-15,
and provide better instructions for making it go away. Clarify support
in the gopls README.
Change-Id: I6b08e0bd698f5c085eee7a851a130c53affb8ab5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445581
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: 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>
Previously, the disaster logic in the new diff implementation
would "encrypt" the before/after files using a monoalphabetic
substitution, which has been insecure since the 9th century.
Instead, save plain text, in file with mode 0600, and invite
the user to audit the file before sharing it with us.
Also, separate the two files using a NUL byte, not a newline,
which is highly ambiguous.
Also, in the JSON diff stats writer:
- print a warning if we can't create the log file.
(The previous code was subtle--it stored a nil *os.File in
an io.Writer, which caused Writes to fail with an error,
in effect, silently.)
- Don't hold the mutex around the write operation.
- Fix minor off-by-one error (re: 15)
- Crash if JSON encoding fails; it "can't happen".
Change-Id: I9b6a4145451afd77594f0ef9868143634a9c4561
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445580
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>
In func f(x int) int {return x;} the second x used to be marked as a
variable, but now is marked as a parameter, visually tying it to its
definition.
Fixesgolang/go#56257
Change-Id: I8aa506b1ddff5ed9a3d2716d48c64521bdea0fd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445095
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This is the first in a series of CLs implementing the new stub
generator. The code is intended to reproduce exactly the current
state of the generated code.
This CL has the final file layout, but primarily consists
of the parsing of the specification.
The LSP maintainers now provide a .json file describing the messages and
types used in the protocol. The new code in this CL, written in Go,
parses this file and generates Go definitions.
The tests need to be run by hand because the metaModel.json file is not
available to the presubmit tests.
Related golang/go#52969
Change-Id: Id2fc58c973a92c39ba98c936f2af03b1c40ada44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443055
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
When a module contains multiple vulnerabilities, previously gopls published
one diagnostic message for each vulnerability. This change modifies this
behavior to publish only one vuln diagnostic for each module. This will
make the PROBLEMS panel more concise and readable. However, the information
about each vulnerability finding is useful, so we supplement this diagnostics
by sending a hover message in the module require line. An added benefit
of this approach is that, unlike the Diagnostics, VS Code supports rich
text rendering for Hover messages. So we can use markdown to add links and
necessary highlighting.
Before this change, go.mod require hover messages (e.g. go mod why result)
were associated only with the module path part, excluding the version string
part. But for vulnerability information hover message, I think it is better
to be applied to the entire module require line (both module path & version)
because they are information about the specific module/version. Currently
LSP hover returns only one hover, so we cannot use this different range only
to the vulnerability information hover. Thus, one side effect of this change
is that the module info hover message will be also shown to the version
part of each require statement.
Change-Id: Iccacd19fdebadc4768abcad8a218bbae14f9d7e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444798
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>
The test had a missing import of "fmt" that is somehow
ignored by the current analysis implementation (but was
flagged as an error by my pending redesign).
Add the import, and update the go.sum hashes.
Change-Id: I6dd91b2863a7cbd0f16018151c942867bddc92e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444795
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
DepsBy{Pkg,Imp}Path are two different ways to look up the PackageID
of a direct dependency. (We need both.)
MissingDeps is now represented as a blank value in DepsByImpPath.
Also try to make sense of MissingDependencies.
(The deleted pkg.types==nil condition was provably false.)
Change-Id: I6a04c69d3d97b3d78b77d4934592735bb941d05f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444538
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Because we normally suppress internal options, the documentation for
this setting is managed manually, and was stale. Update it, and bring it
in-line with the actual setting docstring.
Change-Id: Id3ceaa7303df4ee2a6bf07c54d087451169962cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444539
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>
We need to search intermediate test variants to find all imports of a
renamed package, but were missing them because we only considered
"active packages". Fix this by building up the set of renamed packages
based on metadata alone: it is unnecessary and potentially too costly to
request all (typechecked) known packages, when we only need access to
the metadata graph to determine which packages must be renamed.
In doing so, it becomes possible that we produce duplicate edits by
renaming through a test variant. Avoid this by keeping track of import
path changes that we have already processed.
While at it, add a few more checks for package renaming:
- check for valid identifiers
- disallow renaming x_test packages
- disallow renaming to x_test packages
Also refactor package renaming slightly to pass around an edit map. This
fixes a bug where nested import paths were not renamed in the original
renaming package.
Fix some testing bugs that were exercised by new tests.
For golang/go#41567
Change-Id: I18ab442b33a3ee5bf527f078dcaa81d47f0220c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443637
Reviewed-by: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
An import path is not the same as a package path.
For example, the declaration:
import "example.com/foo"
may load a package whose PkgPath() is "vendor/example.com/foo".
There was a comment hinting at this in load.go.
This change introduces the concept of ImportPath as a
distinct type from PackagePath, and documents clearly
throughout the relevant APIs which one is expected.
Notes:
- Metadata.PkgPath is consistently set from the PackagePath,
not sometimes (when a root) from PackagePath and sometimes (when
indirectly loaded) from an arbitrary ImportPath that resolves to it.
I expect this means some packages will now (correctly)
appear to be called "vendor/example.com/foo"
in the user interface where previously the vendor prefix was omitted.
- Metadata.Deps is gone.
- Metadata.Imports is a new map from ImportPath to ID.
- Metadata.MissingDeps is now a set of ImportPath.
- the package loading key is now based on a hash of
unique imports. (Duplicates would cancel out.)
- The ImporterFunc no longer needs the guesswork of
resolveImportPath, since 'go list' already told us
the correct answer.
- Package.GetImport is renamed DirectDep(packagePath)
to avoid the suggestion that it accepts an ImportPath.
- Package.ResolveImportPath is the analogous method
for import paths. Most clients seem to want this.
Change-Id: I4999709120fff4663ba8669358fe149f1626bb8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443636
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This reverts commit 9b5e55b1a7.
Reason for revert: Should have been fixed by https://go.dev/cl/443100
Change-Id: I44eeff6605697b745c3729ac1ec49b9c09114f71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443340
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Since we removed the 'recover' in analysis, which swept crash
bugs under the rug, we've seen a large number of crashes on
the builders in which an analyzer crashes while trying
to use the result of another analyzer (e.g. the inspector)
on the same package. Logging shows that the "same" package
is in fact a stale version of the same package.
The root cause is that the key for the analysis cache
is the analyzer name paired with the "recipe" for the
package. However, analysis is not content with an equivalent
package: identity matters for ast.Nodes, types.Objects, etc.
This change attemps to fix the problem by, in effect, using
the identity (not the recipe) of the package as part of the key.
It is materialized by adding a store of promises to the pkg,
and then using the analysis name as the key within this store.
Change-Id: I057807d2781492a685f27c815250c3445e7475f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443100
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Unlike other calls of qualified functions, the type of certain functions
in the unsafe package (namely Slice, Add) is not constant, and cannot be
determined by the types.Func being called. Update Finder.expr to
pre-emptively handle calls to functions in the unsafe package, similarly
to how it handles other builtins.
Fixesgolang/go#56227
Change-Id: I7af51c1ecacbdf35e39c8e7b8273ffe6f953427f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443096
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We now understand the longstanding race causing panics in gopls analysis
(the cache key for analysis handles is not correct, and can lead to
cache hits for analysis of stale packages).
It will take a little while to redesign our analysis caching, the
simplest solution being to hang analysis results off of the actual
package they are analyzing. In the meantime, suppress the panic as
before, to eliminate flakes.
For golang/go#55201
For golang/go#56035
Change-Id: I96600f186f9f31b67dae10c0a95a14fa73630571
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442794
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change moves the *pkg out of the actionHandle into
the actionResult. (In future we may not need to load the
package before creating the handle.) The actionResult
retains only the exported type information, to avoid keeping
the syntax trees (etc) live.
Also, perform object fact filtering once after the analysis pass,
instead of every time it is imported by another analysis.
Also, filter out unexported constants.
Also, log internal errors even when we don't bug.Reportf,
since errors merely returned by analysis are usually ignored,
making it harder to understand the analysis crashes.
Change-Id: I7257e8da5dc269090bff2817409948e46dcf7176
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442556
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
A stray print statement was accidentally left in CL 441877. Remove it.
Change-Id: I44e4408059f30d35ad8c84b070aea3e197762d1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442782
gopls-CI: kokoro <noreply+kokoro@google.com>
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>
Add support in gopls for working on "standalone main files", which are
Go source files that should be treated as standalone packages.
Standalone files are identified by a specific build tag, which may be
configured via the new standaloneTags setting. For example, it is common
to use the directive "//go:build ignore" to colocate standalone files
with other package files.
Specifically,
- add a new loadScope interface for use in snapshot.load, to add a bit
of type safety
- add a new standaloneTags setting to allow configuring the set of build
constraints that define standalone main files
- add an isStandaloneFile function that detects standalone files based
on build constraints
- implement the loading of standalone files, by querying go/packages for
the standalone file path
- rewrite getOrLoadIDsForURI, which had inconsistent behavior with
respect to error handling and the experimentalUseInvalidMetadata
setting
- update the WorkspaceSymbols handler to properly format
command-line-arguments packages
- add regression tests for LSP behavior with standalone files, and for
dynamic configuration of standalone files
Fixesgolang/go#49657
Change-Id: I7b79257a984a87b67e476c32dec3c122f9bbc636
Reviewed-on: https://go-review.googlesource.com/c/tools/+/441877
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change eliminates these redundant helper functions
- cache.rangeFromPositions
- source.LineToRange
- source.ByteOffsetsToRange
and makes various other simplifications and documentation
improvements.
Change-Id: Ic820ab560d71b88bde00b005e3a919334a5d1856
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442015
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Make sure we aren't sending multiple code actions
that do the same thing. This also adds a upgrade
to latest code action.
Change-Id: Ic9cecd0a9410648673d4afe63da5a940960a4afc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436776
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: Suzy Mueller <suzmue@golang.org>
The LSP protocol doesn't require edits to be sorted,
but some clients such as govim were relying on it,
as we learned when I recently removed the sort.
This change restores the sort behavior.
See also govim/govim#1171
Change-Id: I589b6cfde933ea6907859bf40acd33c1a7fcde02
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442256
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Most invalidation happens in snapshot.clone, but to be safe we were also
invalidating package data when new metadata is set via packages.Load. At
the time, I wasn't sure why this was necessary.
Now I understand: with experimentalUseInvalidMetadata it is possible
that we re-compute invalidated data using stale metadata in between the
initial clone and the subsequent reload. I noticed that it was also
possible to have stale analysis results after the Load results arrive.
Fix this by invalidating analysis results on Load, in addition to
packages. Factor out invalidation into a new helper method.
Since we believe this may fix analyzer panics, re-enable strict handling
of analysis panics during tests.
For golang/go#56035
For golang/go#54762
For golang/go#42857
Change-Id: I8c28e0700f8c16c58df4ecf60f6127b1c05d6dc5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420538
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
When finding references or implementations, we must include objects
defined in intermediate test variants. However, as we have seen we
should be careful to avoid accidentally type-checking intermediate test
variants in ParseFull, which is not their workspace parse mode.
Therefore eliminate the problematic TypecheckAll type-check mode in
favor of special handling in this one case where it is necessary.
Along the way:
- Simplify the mapping of protocol position->offset. This should not
require getting a package, or even parsing a file. For isolation,
just use the NewColumnMapper constructor, even though it may
technically result in building a token.File multiple times.
- Update package renaming logic to use TypecheckWorkspace, since we
only need to rename within the workspace.
- Add regtest support for Implementations requests.
Fixesgolang/go#43144
Change-Id: I41f684ad766f5af805abbd7c5ee0a010ff9b9b8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438755
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
We've been using this setting in experimental for some time without
issue, so switch it to default.
For golang/go#52967
Change-Id: Ib4d786e689d4b0f009195cc86d7dd5d8269cf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427534
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Unaffecting vulnerabilities that appear should be shown as
informational diagnostics. These do not have current version.
Change-Id: I5dc8d111fd9de8388195627c8f050a2660426abb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/441875
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: Suzy Mueller <suzmue@golang.org>
VS Code suppresses notifications if we send too many, but we don't want
users to miss warnings about deprecated settings. Merge them all into a
single message body.
Also fix a race in a test that added in the preceding CL: the old go
warnings may race with the initial workspace load.
For golang/vscode-go#2487
Change-Id: I69d61a17e0e6e888fa04fa1edce864e28a8d650e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/440180
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Alan Donovan
pjw
Robert Findley
Hana (Hyang-Ah) Kim
Bryan C. Mills
Suzy Mueller