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>
Now we prefer functions when completing "go" and "defer" statements.
Previously we had no preference for the type of object. Further, we
will now also properly invoke functions.
var f1 int
var f2 func()
go f<> // prefers "f2" and expands to "f2()"
Change-Id: I213551b74ba453c337ac89e825b5d495659e9d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246359
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Consider this example:
var foo, bar int
if foo == 123 && b<> {
}
Completing at "<>" previously preferred the unimported
"bytes.Contains()" function because it returns a bool. You often need
to compose a boolean expression from non-boolean candidates, so
preferring only bool candidates gives unhelpful results. Now we don't
infer any expected type for "&&" and "||", which allows the example to
prefer "bar" as the top candidate.
Fixesgolang/go#37163.
Change-Id: Ic341da11dd626439cfb265d129288c5ca6008270
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246362
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I noticed an annoying completion ranking issue:
// ranks "HandlerFunc" over "HandleFunc"
http.HandleFunc<>
This was due to us downranking function calls to prefer fields/vars. I
tweaked the logic to only downrank methods (with a receiver).
Change-Id: Ia4040dc8a35f641be2d7d934bf555090831219ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246357
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In the previous implementation, the extracted function would
sometimes include superfluous parameters and return values. It
might also unnecessarily initialize variables. This CL introduces
3 rules to limit this behavior. (1) a variable is not passed as a
parameter to the extracted function if its first use within the
function is its own redefinition. (2) a variable is not returned
from the extracted function if its first use after the function is its
own redefinition. (3) when possible, we redefine variables in the call
expression to the extracted function.
Change-Id: Ideb5a7eff8a1bf462c83271a2f043116ff5d8b76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244770
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 246757 resulted in an infinite loop because the value of "o" is
never updated.
Change-Id: I79cf265349838de19089c4468128c565a9a3cda3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247182
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Baum <joshbaum@google.com>
In the example:
append([]T{}, <>)
We used to track the "objType" as "[]T", and "variadicType" separately
as "T". However, most things are more interested in "T" vs "[]T", so
they had to fiddle around with swapping types. Now instead we track
objType as T, and add a "variadic=true" flag indicating that "[]T" is
also an acceptable type.
Change-Id: I8ee3ef840917378c8406368cb5c660a377498dfd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246698
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix a minor completion ranking issue:
foo := func(int, int) {}
foo(123, <>)
Previously we were preferring "foo()" at "<>" even though it can't be
used. We mistakenly thought we were completing the first param because
the *ast.CallExpr appears to only have a single param.
Change-Id: Iedbbb1870a4b9eb5d5be4ed266b8bb3e313b496b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246697
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The gc_details command, which shows the gc compiler's decisions, can
produce thousands of diagnostics for a package. New gopls options
'noBounds', 'noEscape', 'noInline', 'noNilcheck' will suppress diagnostics
of less interest to the user. These are in a new 'annotations' section
parallel to 'codelens' or 'analyses'.
Change-Id: Ica75de25b14f38b67ddfa9f997f581674f45221d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246477
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@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>
Ran into this while debugging another issue.
Change-Id: I154493418c7676a24457a4e11431ad4f0311c07a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Baum <joshbaum@google.com>
This CL addresses completion prefix not being overwritten for completion
in comments for exported variables/functions/types etc. Instead of
setting the surrounding range as cursor position, we expand out from
cursor instead to replace the word we're currently on.
Fixesgolang/go#39262
Change-Id: I90c28562e3ef285ce6848598f8d7bd7545d5c957
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246237
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, the suggested fix tests did not properly handle the case
in which one fix contained at least two edits. We also prevent
the server from panicing when we cannot extract the selection.
Change-Id: I38f7b6d871b2f2741349a3fd94fd95b396f5fd33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246458
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were returning the AST node for the identifier that the rename was
called from, not the actual declaration, so the doc comments weren't
getting updated.
Fixesgolang/go#40463
Change-Id: Id8ba0b0aeb8f42d2aaa561e7a964edcca5202916
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This CL removes duplicate code in lineFoldingRange function under
lsp/source/folding_range.go and generally improves code quality.
Fixes bug with composite literal folding where gopls was folding literals
with braces on the same line as end token (paranthesis/braces).
Change-Id: I742f285d866d72a243129c0aef0935fe2a1ad0dd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245757
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
The builtin package was the one special case where we parsed Go outside
the context of a Snapshot. Move it up.
Change-Id: I1f4bb536adb40019e0ea9c5c89f38b15737abb8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245057
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
To manually collect cache entries, we need to know when a snapshot is
idle. Add a reference count in the form of a WaitGroup and keep track of
its uses. The pattern is that any time a snapshot is returned, it comes
with a release function that decrements the ref count.
Almost all uses of a snapshot originate in a user-facing request,
handled in beginFileRequest. There it's mostly an exercise in passing
Snapshots around instead of Views.
In the other places I took the path of least resistance. For file
modifications I tried to minimize the amount of code that needed to deal
with snapshots. For diagnostics I just acquired the snapshot at the
diagnostics call.
Change-Id: Id48a2df3acdd97f27d905e2c2be23072f28f196b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241837
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Continuing the massacre, remove ParseModHandle, and Mod*Handle, from the
source API.
Notably, having the snapshot available means we can simplify the go
command invocation paths a lot.
Change-Id: Ief4ef41e42f93d653f719a230004861e5e1ef70b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244769
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There were a few merge conflict-related issues in the GC optimization
details CL. Also fixed a few things I noticed after the fact, like
separating out a new mutex.
Staticcheck caught a few things, and I also fixed a bug I noticed
in the cache package.
Change-Id: I3fc519373253418586dca08fdec3114b30a247ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245399
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@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>
I still keep seeing this crash too, even after CL 244841.
Fixesgolang/go#40464
Change-Id: Ic587045e65f34c24bd6df452e24517fd90e36bbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245440
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
The logic to resolve the enclosing type for an identifier is somewhat
tricky. Add a unit test to exercise a few edge cases.
This would probably be easier to read and write using a hybrid approach
that extracts markers from the source.
This test uncovered a bug, that on the SelectorExpr branch we were
accidentally returning a nil *Named types.Type, rather than a nil
types.Type.
Change-Id: I43e096f51999b2a6e109c09d3805ad70a4780398
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244841
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
Due to the runtime's inability to collect cycles involving finalizers,
we can't close over handles in memoize.Functions without causing memory
leaks. Up until now we've dealt with that by closing over all the bits
of the snapshot that we want, but it distorts the design of all the code
used in the Functions.
We can solve the problem another way: instead of closing over the
snapshot/view, we can force the caller to pass it in. This is somewhat
scary: there is no requirement that the argument matches the data that
we're working with. But the reality is that this is not a new problem:
the Function used to calculate a cache value is not necessarily the one
that the caller expects. As long as the cache key fully identifies all
the inputs to the Function, the output should be correct. And since the
caller used the snapshot/view to calculate that cache key, it should
always be safe to pass in that snapshot/view. If it's not, then we
already had a bug.
The Arg type in memoize is clumsy, but I thought it would be nice to
have at least a little bit of type safety. I'm open to suggestions.
Change-Id: I23f546638b0c66a4698620a986949087211f4762
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244019
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The PackageHandle interface is fairly redundant with the Package
interface. As much as convenient, move users to Package and
weaken/remove methods from PackageHandle.
I would like to get rid of CompiledGoFiles too but
NarrowestPackageHandle is a little annoying. I think this is
unambiguously a step forward so I figured we can get it in and keep
iterating.
Change-Id: I6c5a3f462b1f19cbca6a267fedc36ce54613b6fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244018
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The importPrefix logic is complicated by Windows line endings, since
go/ast isn't aware of different line endings in comment text. I made a
few changes to the way that import prefixes are computed to handle this.
Specifically, for comments, we try to make sure the range ends on a full
line as much as possible, because that addresses the line ending issue.
Fixesgolang/go#40355
Change-Id: I84c1cfa0d0bae532e52ed181e8a5383157feef24
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244897
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, users could not extract code that contained a return
statement. Now, users can extract code with return statements, as long
as the statements are nested within an if, case, or other control
flow statement.
Updates golang/go#37170
Change-Id: I2df52d0241517472decabce3666a32392ff257bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243650
Run-TryBot: Josh Baum <joshbaum@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>
Our logic to generate documentation links did not account for embedded
fields and methods. The types.Info.ObjectOf an embedded field returns
the *types.Var created for the field, not its types.TypeName, so we have
to navigate back to the actual definition of the field. This requires
traversing through all of the named types in the top-level type.
Fixesgolang/go#40294
Change-Id: Ia6573aebe66b7f60e2d6861a381cd7b07e7d7eaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244178
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
There was a bug in the hover for type switch variables. For example:
var x interface{}
switch y := x.(type) {
case string:
case int:
}
Hovering over y would previously show "var y string", because y's object
would be mapped to the first types.Object in the type switch. Now we
show the hover for y as "var y interface{}", since it's not yet in the
cases.
Change-Id: Ia9bd0afc4ddbb9d33bbd0c78fa32ffa75836a326
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This CL is a follow-up from CL 241983. I didn't realize that the
undeclaredname analysis was also using the go/printer.Fprint trick,
which we decided was both incorrect and inefficient. This CL does
approximately the same things as CL 241983, with a few changes to make
the approach more general.
source.Analyzer now has a field to indicate if its suggested fix needs
to be computed separately, and that is used to determine which
code actions get commands. We also make helper functions to map
analyses to their commands.
I figured out a neater way to test suggested fixes in this CL, so I
reversed the move to source_test back to lsp_test (which was the right
place all along).
Change-Id: I505bf4790481d887edda8b82897e541ec73fb427
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242366
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL adds benchmarking by using the "-bench" flag for the test
command.
Change-Id: Idf714de1c6c3350d26a6874e7b4278927d0336fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243159
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This change adds support for `go mod tidy` on save when users opt into
import organization on save. Previously, we supported this with the
go mod tidy command, but there's no need to do this when we already
have a ModTidyHandle available.
Change-Id: Ibb47b6a7611fd823dac2a3b4f3a43b9fbb3289b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change attempts to parse diagnostics out of `go list` error
messages so that we can present them in a better way to the user. This
approach is definitely tailored to the unknown revision error described
in golang/go#38232, but we can modify it to handle other cases as well.
Fixesgolang/go#38232
Change-Id: I0b0a8c39a189a127dc36894a25614535c804a3f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, we only updated the opened file's overlay, but not the
snapshot. This meant that the snapshot was still operating with stale
data. Invalidating the snapshot creates a new snapshot with the correct
set of overlays.
The test is skipped because it will flake until we have a better caching
strategy for `go mod tidy` results.
Updates golang/go#40269
Change-Id: Ia8d1ae75127a1d18d8877923e7a5b25b7bd965ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Our approach to commands and their arguments has been ad-hoc until this
point. This CL creates a standard way of defining and passing the
arguments to different commands. The arguments to a command are now
json.RawMessages, so that we don't have to double encode. This also
allows us to check the expected number of arguments without defining
a struct for every command.
Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This change moves the suggested fixes logic for fillstruct out of the
analysis and into internal/lsp/source. This logic is then used as part
of a new fillstruct command. This command is returned along with the
code action results, to be executed only when the user accepts the code
action.
This led to a number of changes to testing. The suggested fix tests in
internal/lsp doesn't support executing commands, so we skip them. The
suggested fix tests in internal/lsp/source are changed to call
fillstruct directly. A new regtest is added to check the command
execution, which led to a few regtest changes.
Also, remove the `go mod tidy` code action, as it's made redundant by
the existence of the suggested fixes coming from internal/lsp/mod.
Change-Id: I35ca0aff1ace8f0097fe7cb57232997facb516a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241983
Reviewed-by: Heschi Kreinick <heschi@google.com>
Extract function is a code action, similar to extract variable. After
highlighting a selection, if valid, the lightbulb appears to trigger
extraction. The current implementation does not allow users to
extract selections with a return statement.
Updates golang/go#37170
Change-Id: I5fc3b19cf7dbca4407ecf0cc37017661223614d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241957
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I had previously suggested that users set LinkTarget to "" to avoid
links in the hover text. However, this work-around isn't perfect because
it also disables the documentLink behavior in other cases.
Change-Id: I3df948e2a2e4d2312998de65ccea8dfb404768ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243239
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Currently, diagnostics for modules that are missing from the go.mod
appear in the Go files in which those modules are imported. As a result,
the diagnostics were previously calculated as part of the Go file
diagnostic calculations. This is convoluted and required passing around
an extra map.
This CL puts that logic in the ModTidyHandle where it belongs.
The diagnostics for the Go files are combined from the multiple sources.
Also, added a skipped test for golang/go#39784, since this CL was
originally intended to be a fix for that issue...
Change-Id: Ic0f9aa235dcd56ea131a2339de9801346f715415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
getFile still returns a FileHandle, even if the file doesn't exist on
disk. Work-around this by checking if the file exists before adding
the file handle to the map.
Fixesgolang/go#38498
Change-Id: Ie02679068b37bf4f3d19966c6cfcf2361086b1de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242924
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I didn't properly understand Heschi's comment, and missed a case where
we failed to return early.
Change-Id: I692c26678b2e659c6b748085bb4b16090e04472d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change refactors lens funcs to use only a FileHandle - each
code lens should manually compute a ParseGoHandle if it's needed. The
issue was that, if the code lens needed to also get a package, the
already parsed *ast.File was not necessarily the file used in
type-checking that package. I noticed that the code lens wasn't always
coming up.
Change-Id: Ic5a2c2b3d1010555acaa64ef8c6aa2484b8fbc9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242920
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
