Seems we've drifted a bit from go1.12 support, mostly due to error
wrapping.
Fix this, as well as some assorted other failures.
I haven't tested 1.12 interactively.
For golang/go#39146
Change-Id: Id347ead2a13e89b76d2ae0047750e6b6b49911eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250941
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If an analyzer doesn't return error, and doesn't have any suggested
fixs either gopls panics. Return an empty set of edits instead.
Change-Id: I1cd812fedcbd2ddc01229f48c0cc4467ee3f0105
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249998
Reviewed-by: Robert Findley <rfindley@google.com>
When falling back to messages for progress reporting, don't try to
implement cancellation via ShowMessageCommand dialogs. They are an
imperfect solution, as the dialog stays open even after the command
completed. Also, among the LSP clients that don't support workDone
reporting, I suspect many also don't support ShowMessageCommand (for
example, govim), so the audience for this feature is probably quite
small.
Just remove it, and instead show a (non-cancellable) message. If clients
want cancellation, workDone progress support is the way to provide it.
Also remove a redundant message on go-generate success, and attach logs
when tests fail. Without logs on failure, I find that the test command
is not very useful. I tested a bit with very verbose test output, and
both VS Code and coc.nvim handled it gracefully.
Finally, fix a bug causing benchmarks not to be run.
Change-Id: I05422bcefc857c25cd99e643e614a0bc33870586
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249702
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We did not adjust the range in extractFunction(). We only adjusted
the range in canExtractFunction().
Change-Id: Idc1eab775988ab61be6df8b4afd4b1a36a8bb0ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247405
Run-TryBot: Josh Baum <joshbaum@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
snapshot.View().Session().Cache().FileSet() has been driving me crazy
for a while. Add it to snapshot. Along the way, discover that the Cache
interface is now totally unused and delete it.
I also changed a bunch of View arguments to Snapshot while I was in the
area.
Change-Id: I1064d0020b1567c2ed28d2d55e0f4649eb94c060
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245324
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
FileHandle currently includes LSP-level information about Version and
Session. That's dangerous, because the cache operates in terms of
URIs and content only -- we explicitly want to share results across
sessions and versions if they happen to be the same.
Split the LSP information into separate types, VersionedFileHandle and
VersionedFileIdentity.
Change-Id: I158646b783375b58245468599301e2a29c657e71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245058
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
In the previous implementation, the initial verification in lsp/command
for whether extract function was relavant to the given range did not
contain much of the initial logic for extract function. This meant
that "canExtractFunction" produced many false positives (i.e. the
lightbulb would appear when it should not have in VSCode). This CL
moves more of the verification process from "extractFunction"
(lsp/source) to "canExtractFunction" (lsp/command).
Change-Id: If2683dc9ac3f4bfa8c3444418cf88edd8cbe73e6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245398
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The gc compiler will report its decisions about inlining, escapes, etc.
This can be turned on and off with a new optional code lens gc_details.
When enabled, the code lens will be displayed above the package
statement. The compiler's decisions are shown as information diagnostics.
(Other diagnostics have been errors and warnings.)
Change-Id: I7d1d5b5b5cf8acd7ff08f683e537ea618e269547
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243119
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We'd like to call canExtractVariable in extractVariable without
duplicating logic. The same needs to be done for canExtractFunction.
Change-Id: Ia99befabbafffcf13dd3bc12355f9ddb81a71002
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245135
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Just like ParseGoHandle, PackageHandle isn't very useful as part of the
public API. Remove it.
Having PackagesForFile take a URI rather than a FileHandle seems
reasonable, and made me wonder if that logic applies to other calls like
ParseGo. For now I'm going to stop here. I could also revert that part
of the change.
Change-Id: Idba8e9fdba0b0c48e841a698eb97e47fd5f23cf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244637
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
ParseGoHandles serve two purposes: they pin cache entries so that
redundant calculations are cached, and they allow users to obtain the
actual parsed AST. The former is an implementation detail, and the
latter turns out to just be an annoyance.
Parsed Go files are obtained from two places. By far the most common is
from a type checked package. But a type checked package must by
definition have already parsed all the files it contains, so the PGH
is already computed and cannot have failed. Type checked packages can
simply return the parsed file without requiring a separate Check
operation. We do want to pin the cache entries in this case, which I've
done by holding on to the PGH in cache.pkg.
There are some cases where we directly parse a file, such as for the
FoldingRange LSP call, which doesn't need type information. Those parses
can actually fail, so we do need an error check. But we don't need the
PGH; in all cases we are immediately using and discarding it.
So it turns out we don't actually need the PGH type at all, at least not
in the public API. Instead, we can pass around a concrete struct that
has the various pieces of data directly available.
This uncovered a bug in typeCheck: it should fail if it encounters any
real errors.
Change-Id: I203bf2dd79d5d65c01392d69c2cf4f7744fde7fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244021
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The logic for extracting a function is quite signficant, and the code
is expensive enough that we should only call it when requested by the
user. This means that we should support extracting through a command
rather than text edits in the code action.
To that end, we create a new struct for commands. Features like extract
variable and extract function can supply functions to determine if they
are relevant to the given range, and if so, to generate their text
edits. source.Analyzers now point to Commands, rather than
SuggestedFixFuncs. The "canExtractVariable" and "canExtractFunction"
functions still need improvements, but I think that can be done in a
follow-up.
Change-Id: I9ec894c5abdbb28505a0f84ad7c76aa50977827a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244598
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
