This change adds support for package completion in all files at valid
positions. By parsing expressions from an invalid file, we can check if
the cursor is at a position where package completion would be a valid
suggestion. These are positions above any other statements or
declaration or at prefix of the keyword pacakge above these
statements/declarations. This also introduces imporved end of file
handling in completion.
Change-Id: I2a865d018f58c3a98b69fb4100d186b507d123bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251618
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
To match the actual gopls binary, use hooks.Options when creating the
regtest server.
Add a test for staticcheck diagnostics to leverage this.
For golang/go#39384
Change-Id: I52837c2b12bb586a2530343bdfae5172b08df49c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/252683
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Regtests are slow, and make `go test ./internal/lsp/...` slow. Also,
having them in the tools module means they can't use staticcheck,
go-diff, etc.
Move them to the gopls module. This means that they're annoying to work
with unless you open the gopls module, but hopefully that annoyance will
be gone soon when we support multi-module workspaces.
For golang/go#39384
Change-Id: Ib99c994ffdac56d4da13af981ad397a90a7523af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/252682
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix the comment end position for multi-line comments to account for the
closing "*/".
Fixesgolang/go#41057
Change-Id: I5dd3886a81d274514e78f47ac2e7194fd9cceb06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/252457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Refactor the checks for code lenses being enabled out of the source
package so that the mod code lenses can also make use of them.
Also, a few small changes to the titles of the `go mod tidy` and `go mod
vendor` code lenses.
Change-Id: I4e79ab08a4e7aea4d4d6de6fd652d0b77d30c811
Reviewed-on: https://go-review.googlesource.com/c/tools/+/252397
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This adds tests for golang/go#40690, as well as a case for a bug in
changing package names.
Updates golang/go#40690
Change-Id: I516404e77728024f8bee3da31a88bad1f1917a9b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251017
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This changes add package completions suggestions for new files. Package
suggestions are other packages used in the same directory, test
packages for those packages, the package 'main' and the directory name.
Fixesgolang/go#34008
Change-Id: I69922e0cb0787e82eebe505618c3c07aa48859e6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251160
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
In cases where the remote panics, make it clearer in the forwarder logs
that the remote has disconnected.
Change-Id: Ic7f9370f615198274696a18adaefe7dc62cac02e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250508
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This is better than parsing the default output.
Also, change the start progress message, since it ends up duplicating
the title in the message that the user sees.
Change-Id: I3540d30c7976c6be0722531b2e258341081e0b72
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251920
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Also, add a directory parameter to RunGoCommand. To make sure that the
parameters aren't misused, change args to a []string.
Updates golang/go#40340
Change-Id: Ib5ce606a401a18c29c904b570ec9339f067a3961
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Our metadata reloading model makes typing out import paths manually
very slow. We can avoid some of the slowness by not invalidating
metadata when a new import path is obviously invalid.
Updates golang/go#35877
Change-Id: Ifcf9ebaac0b146a2098ef8d411fa85fefa7ba6ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251086
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Danish Dua <danishdua@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The fuzzy matcher doc string says that it returns 0 on no match, but
this is in fact not true: it returns -1 on no match.
0 makes more sense, so fix the implementation rather than the docstring.
Change-Id: I997a6b5dcb1d7c25cc73b2c236d24647f9326c80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248417
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This code seems to be duplicate of L509-L522 in this CL. It doesn't
affect the results of completion in any way since we already return
early from where we can.
Change-Id: I30ee1c94e58860f86d773d46cb1e527b2e646ef4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251261
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is based on Paul Jolly's CL 228760, updated to use the new cache
API, support the symbolStyle configuration option, and lift up the
concept of symbol score for later improvement.
From that CL:
There are a number of issues with the current implementation:
* test variant packages are not handled correctly, meaning duplicate
symbols are returned
* fuzzy results are not ordered by score
We refactor the implementation of workspace symbol to use a
symbolCollector that carries context during the walk for symbols. As
part of resolving the test variant issue, we first determine a list of
packages to walk.
(*symbolCollector).collectPackages gathers the packages we are going to
inspect for symbols. This pre-step is required in order to filter out
any "duplicate" *types.Package. The duplicates arise for packages that
have test variants. For example, if package mod.com/p has test files,
then we will visit two packages that have the PkgPath() mod.com/p: the
first is the actual package mod.com/p, the second is a special version
that includes the non-XTest _test.go files. If we were to walk both of
of these packages, then we would get duplicate matching symbols and we
would waste effort. Therefore where test variants exist we walk those
(because they include any symbols defined in non-XTest _test.go files).
One further complication is that even after this filtering, packages
between views might not be "identical" because they can be built using
different build constraints (via the "env" config option). Therefore on
a per view basis we first build up a map of PkgPath() -> *types.Package
preferring the test variants if they exist. Then we merge the results
between views, de-duping by *types.Package.
Finally, when we come to walk these packages and start gathering
symbols, we ignore any files we have already seen (due to different
*types.Package for the same import path as a result of different build
constraints), keeping track of those symbols via symbolCollector.
Then we walk that list of packages in much the same way as before.
For golang/go#40548
Co-authored-by: Paul Jolly <paul@myitcv.io>
Change-Id: I8af5bdedbd4a6c3631a213d73a735aea556a13ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247818
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds completion within import blocks. Completions are suggested by directory depth of import so end user isn't shown a large list of possible imports at once. As an example, searching import for prefix "golang" would suggest "golang.org/" and then subdirectories under that (ex: "golang.org/x/"") on successive completion request and so on until a complete package path is selected.
Change-Id: I962d32f2b7eef2c6b2ce8dc8a326ea34c726aa36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250301
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add additional benchmarks following the pattern of symbol benchmarks.
One for initial workspace load, and another for completion.
Change-Id: Iba826b188cb81dffabb1b08287dc7b76250dc54c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250802
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The origin and meaning of error messages in IDEs is sometimes obscure.
For instance "unexpected config annotations\nSource: Go (extension)" in vscode has an ambiguous Source, and it's not clear what config annotations
are being referred to. This CL would change the first part of that
message to "unexpected gopls setting 'annotations', making clear that
the message is from gopls (and not the Go extension shim), and that the
suspect value is 'annotations' itself.
Change-Id: Ia79534859fe101e6fbbd660bf426e2c1ac619b7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251078
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a test for this case (empty test file in GOPATH mode)
to go/packages. It is skipped for now, as the go/packages work-around is
too brittle to justify adding. Instead, we suppress gopls-generated
error messages in pop-ups--users should only see error messages if they
come from go/packages.
Fixesgolang/go#40825
Change-Id: I0e2af53578d65739ebae582075498d997fc019d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250949
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
It's pretty easy to add an LSP benchmark using the regtests, provided we
run the benchmark ourselves from inside the runner. Do this for
workspace symbols to start, though we should add several of these.
Also fix some error messages when setting options.
Change-Id: Iab134018edec8837e90a0a926ec2e73addf95bb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250798
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We should make sure to return deterministic results for these requests.
Fixesgolang/go#40904
Change-Id: If10489e3eca0e1b6a5e449de851d332f2d91ceb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250737
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Period triggered completions don't provide any use in comments and in
worst case can be nuisance. LSP provides a completion context which
provides more info about what triggered a completion and hence we can
use this to ignore period triggererd completions. This will also provide
us options to deal with retriggered completions etc. better in the
future.
Change-Id: I8449aee0fe3cf5f9acf315865ac854d5c894d044
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250337
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Use space characters instead of the HTML " " character. VS Code and
other clients treat this character differently from a space, so it
results in different formatting. See
https://github.com/golang/go/issues/40947#issuecomment-680497904 for an
image with the difference.
Fixesgolang/go#40947
Change-Id: Ia78cb3ba82dcca8303d1b5a0a6d359a3eaca837c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Span treats an end of file as the beginning of the next line, which for a final line ending without a newline is incorrect and leads to completions being ignored. We adjust the ending in case range end is on a different line here.
Change-Id: Ic545dcb221493530b7e39d2be8eba57b69fb6597
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249706
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Since we used to manually set surrounding for comments (instead of using
setSurrounding with an ident), the matched was never initialized and
hence we had to manually do prefix matching. We now initialize matcher
from setSurroundingForComment too.
Change-Id: I8aa735933ebba2fe493182e4245de668997ef7af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249707
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fillstruct wasn't able to fill structs with fileds that held anonymous
structs or maps with anonymous structs/interfaces.
By adding *types.Struct and *types.Interface support in TypeExpr(),
fillstruct is now works for those as well.
Fixesgolang/go#40980
Change-Id: I2a1d6d4db237800197cf14e3ad5067a0e7ab701a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249999
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
Avoid logging to stderr as it will interfere with LSP communication.
Change-Id: Ic9ff4f3555eabb8b0b6503650df7de5ddb76ef98
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249997
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
packages.Load returns bad data for an empty xtest in GOPATH mode.
Updates golang/go#40825.
Change-Id: Ibab8dcbeafc22a71a2a4c3be0008b728c13c4c80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249703
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
* adds support for comment completion inside declarations
* improves scoring for completion results for comments
* adds comment completion support for non-exported symbols
* adds pruning for results that don't match text surrounding cursor
* tests for comment completion
Change-Id: Icb445a469cee3122fe032630bee037c7bdfe2e18
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249639
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
formatZeroValue is currently only used when formatting return values for
statement completion. Per golang/go#40956, it must be possible to hit
this codepath with an invalid type.
In this case, the empty string seems like a reasonable value. Perhaps we
could do better, but fix the panic for now.
Fixesgolang/go#40956
Change-Id: I45b559d41001c857cef34aea2a5ac4a9096fe950
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249818
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Fix completion in the following cases:
type foo struct{}
// now we offer "&foo" instead of "foo"
var _ *foo = fo<>{}
struct { f *foo }{
// now we offer "&foo" instead of "*foo"
f: fo<>{},
}
Composite literal type names are a bit special because they are part
of an arbitrary value expression rather than just a standalone type
name expression. In particular, they can be preceded by "&", which
affects how they relate to the surrounding context. The "&" doesn't
technically apply to the type name, but we must take it into account.
I made three changes to fix the behavior:
1. When we want to make a composite literal type name into a pointer,
we use "&" instead of "*".
2. Record if a composite literal type is already has a "&" so we don't
add it again.
3. Fix "var _ *foo = fo<>{}" to properly infer expected type of "*foo"
by not stopping at *ast.CompositeLit searching up AST path when the
position is in the type name (as opposed to within the curlies).
Change-Id: Iee828f259eb939646b68f5066614ea3a262585c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247525
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
There's a bunch of noise in regtest results related to the LSP forwarder
handshake.
lsprpc.StreamServer was already configurable to disable connection
logging in tests. Use this configuration to also disable handshake
logging.
Change-Id: I4b00e23d1f0bc6dc5fdd1f6f470f0b892c6791bc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249418
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change contains several improvements for progress reporting:
+ Consolidate the 'progressWriter' interface into the workDone
interface. Now all progress reporting should use workDone, and the
workDoneWriter struct is just an io.Writer adapter on top of
workDone.
+ Factor out the pattern of progress reporting, and use for all
asynchronous commands.
+ Make several commands that were previously synchronous async.
+ Add a test for cancellation when the WorkDone API is not supported.
+ Always report workdone progress using a detached context.
+ Update 'run tests' to use the -v option, and merge stderr and stdout,
to increase the amount of information reported.
+ Since $/progress reporting is now always run with a detached context,
the 'NoOutstandingWork' expectation should now behave correctly. Use
it in a few places.
A follow-up CL will improve the messages reported on command completion.
For golang/go#40634
Change-Id: I7401ae62f7ed22d76e558ccc046e981622a64b12
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248918
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We now rank printf operand candidates according to the corresponding
formatting verb. We follow what fmt allows for the most part, but I
omitted some things because they are uncommon and would result in many
false positives, or didn't seem worth it to support:
- We don't prefer fmt.Stringer or error types for "%x" or "%X".
- We don't prefer pointers for any verbs except "%p".
- We don't prefer recursive application of verbs (e.g. we won't prefer
[]string for "%s").
I decided against sharing code with the printf analyzer. It was
tangled somewhat with go/analysis, and I needed only a very small
subset of the format parsing.
I tweaked candidate type evaluation to accommodate the printf hints.
We now skip expected type of "interface{}" when matching candidate
types because it matches everything and would always supersede the
coarser object kind checks.
Fixesgolang/go#40485.
Change-Id: I6440702e33d5ec85d701f8be65453044b5dab746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246699
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
qualifiedObjsAtProtocolPos returned too early. Have it keep looking in
the rest of the candidate packages.
This changes the returned error slightly but AFAICT nobody cares.
Updates golang/go#40809.
Change-Id: Ic8199a484f0abcaa48cb6a3bcdd782195802d670
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249637
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add a new autocomplete function that completes based on import path
prefix rather than package name prefix.
Updates golang/go#35877.
Change-Id: Ib768080ee99debfff1c8c870d22dc7b7459deadd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249419
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Danish Dua <danishdua@google.com>
* Adds outgoing calls call hierarchy for function declarations to gopls. Returns all call ranges and call items for functions/literals being called.
* Adds tests for outgoing call.
* Updates cmd to account for call ranges and call items being in different files for outgoing calls.
* Updates prepare call hierarchy to return declaration as root instead of cursor position.
Example:
Example shows https://github.com/golang/tools/blob/master/internal/lsp/source/call_hierarchy.go
Show Call Hierarchy View: https://imgur.com/a/DA5vc6l
Peek Call Hierarchy View: https://imgur.com/a/fuiG0Be
Note:
* While incoming calls for a function defined in an interface return references to that function, outgoing calls don't return anything since we don't know what implementation to return outgoing calls for.* Outgoing calls to function literals show as variable name used to define the literal, compared to <scope>.func() for incoming calls.
Change-Id: Ib8afbd8617675d12952db0b80170ada5988e90ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248537
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When the client closes its connection, we get an EOF, but it's not
really an error. Suppress it so that the gopls binary doesn't exit with
an error.
In principle we should connect this to (lsp.Server).shutdown somehow,
but as far as I know a path for that doesn't exist, and this seems
pretty innocuous to me.
Fixesgolang/go#40832.
Change-Id: I9dd83d26bcf5c07e03188d3829e603005d80c8c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249417
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 248380 forced all type checking to be in the default workspace mode.
In that CL, I said I couldn't think of any features that would break. It
appears I didn't think very hard. Navigation features inside of
dependencies are something I use all the time and they broke.
Reintroduce the ability to get packages in a particular mode, and make
it convenient to get them in all relevant modes. Update some critical
features to do so, and add regression tests.
Fixesgolang/go#40809.
Change-Id: I96279f4ff994203694629ea872795246c410b206
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249120
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Some Commands require that the buffer is saved before running them (i.e.
generate, test and toggle details). If the buffer isn't saved gopls
sends a ShowMessage request to the client. Before this change it
did not return any error from the ExecuteCommand request itself (unless
ShowMessage failed).
A progress token can be provided by the client as a part of the
ExecuteCommand request, and that is, according to the LSP spec one way
to start a WorkDoneProgress. At this point the client expects
that gopls send progress updates.
With this change, ExecuteCommand now return an error if the buffer
isn't saved, so that the client know that it shouldn't expect any
progress updates with this specific token.
Change-Id: I8a7af20a0c1532fc4ad03efd4e04bfb1a6871644
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248766
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
go mod tidy lens only shows up for a dirty mod file. Tidying a dirty
mod file makes the lens disappear.
Closesgolang/go#40422
Change-Id: I3356ffe94deaf7a5cdfc159c16470894336edf9e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248717
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
CL 244117 introduced a bug when modFile == os.DevNull: v.root is left
uninitialized, resulting in a view that appears to own all files. Fixing
that exposes a problem where opening a folder with no Go files and
GO111MODULE=on shows a popup. Skip the popup when no Go files are found.
Change-Id: I7f8b2d6fd2f954af64c3a65156ff44c649f3a5b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248620
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In this example:
p := &[]int{}
append([]int{}, *<>)
At <> we completed to "**p" instead of "*p...". There were two fixes:
1. builtinArgType() wasn't propagating the "modifiers", so we were
forgetting about the preceding "*" pointer indirection and
inserting it again with the completion. Fix by propagating
modifiers.
2. The candidate formatting responsible for adding "..." had over
simplified logic to determine if we are completing the variadic
param. Now instead the candidate evaluation code marks the
candidate as "variadic" so the formatting doesn't have to think at
all.
Change-Id: Ib71ee8ecfafb915df331f1d2e55b76f76a530243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In the following example:
var foo []someStruct
foo = append(foo, <>)
we now downrank "foo" as a candidate at "<>". You very rarely append a
slice to itself, so having "foo" ranked highly was counterproductive.
Fixesgolang/go#40535.
Change-Id: Ic01366aeded4ba2b6b64bfddd33415499b35a323
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When a signature doesn't name its params, we make up param
names when generating a corresponding func literal:
var f func(myType) // no param name
f = fun<> // completes to "func(mt myType) {}"
Previously we would abbreviate named types and fall back to "_" for
builtins or repeated names. We would require placeholders to be
enabled when using "_" so the user could name the param easily. That
left users that don't use placeholders with no completion at all in
this case.
I made the following improvements:
- Generate a name for all params. For builtin types we use the first
letter, e.g. "i" for "int", "s" for "[]string". If a name is
repeated, we append incrementing numeric suffixes. For example,
"func(int, int32)" becomes "func(i1 int, i2 int32").
- No longer require placeholders to be enabled in any case.
- Fix handling of alias types so the param name and type name are
based on the alias, not the aliasee.
I also tweaked formatVarType to qualify packages using a
types.Qualifier. I needed it to respect a qualifier that doesn't
qualify anything so I could format e.g. "http.Response" as just
"Response" to come up with param names.
Fixesgolang/go#38416.
Change-Id: I0ce8a0a4e2485dda41a0aa696d9fd48bea595869
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246262
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
* Adds incoming calls hierarchy to gopls. Returns function declarations/function literals/files enclosing the call/s to the function being inpected.
* Updates cmd to show ranges where calls to function in consideration are made by the caller.
* Added tests for incoming calls.
Example:
This example shows call hierarchy for PathEnclosingInterval in tools/go/ast/astutil.go
Show Call Hierarchy View: https://imgur.com/a/9VhspgA
Peek Call Hierarchy View: https://imgur.com/a/XlKubFk
Note:
* Function literals show up as <scope>.func() in call hierarchy since they don't have a name. Here scope is either the function enclosing the literal or a file for top level declarations
* Top level calls (calls not inside a function, ex: to initialize exported variables) show up as the file name
* Clicking on an item shows the the range where a call is made in the scope
Change-Id: I56426139e4e550dfabe43c9e9f1838efd1e43e38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247699
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Every package has a default type checking mode dictated by whether it's
in the workspace or not. Some features force full rather than exported
type checking, but AFAICT that ends up being more harm than good. For
example, let's say we want to Find References on fmt.Printf in the stdlib.
Before this CL, we'd force a new type check of the fmt package, then
find no references because nothing else would have been checked against
that new version.
While there may be some features that work better in the current regime,
I can't think of any, and we have no test coverage for them. So I'd
rather start with what makes sense, and if we want to change it maybe
let's write some tests.
Change-Id: Iea589efb4b4374fd2a54451c868b6e2bd5484e20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248380
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We failed to initialize the ProcessEnv when adding stdlib imports.
Somehow this went unnnoticed for a month or something. Initialize on
demand there too, and add a stronger check so there's a better error
message.
Fixesgolang/go#40670.
Change-Id: I391c115f417390be4a0e18f92fbfbcbfbdcc052c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247797
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
While looking at Kubernetes I noticed that golang.org/x/text packages
were some of the largest. The problem is the large code-generated
tables, which use ellipsis array literals. Teach gopls to trim the cases
that matter there.
While silly, this trims ~60MB off the live heap, so I think it might be
worth it.
Change-Id: I0cfd80bd5fbc8703ac628312982af9c6ed871758
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248180
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Tweak a few things so that unnamed, non-basic types are offered as
completions in certain cases:
var _ []int = make(<>) // now properly suggests "[]int"
I also fixed type related keywords to not be offered if there is an
expected type:
var _ *int = new(<>) // don't offer "func", etc.
There are still some cases that don't work properly. For example:
var _ [][]int = make([]<>) // doesn't offer "[]int"
This would be harder to fix given the way things currently work.
Fixesgolang/go#40275, golang/go#40274.
Change-Id: I2577d5863d4757845ad3ff7dbb125106b649a6b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246360
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The lsprpc package implements support for connecting to remotes on unix
domain sockets, as well as auto-starting the remote if it doesn't exist.
Unfortunately, commands other than 'serve' were instead assuming the tcp
network.
Fix this to share functionality.
Fixesgolang/go#40732
Change-Id: I46c303ea91d087d7432e0e3236a68e58e1ea6344
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248181
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL adds a code lens to run all benchmarks in a file. Additionally,
it updates the test command handler to better support both tests and
benchmarks.
Updates golang/go#36787
Change-Id: I6e90460f7d97607f96c263be0754537764bd0052
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246017
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This is a work-around for
https://github.com/microsoft/vscode/issues/104387. We now always
lowercase the drive letter on Windows.
This CL also fixes a bug introduced by CL 245327, which caused URIs
to be used instead of paths in the GlobPattern.
We really need VS Code integration tests for this
(golang/vscode-go#404).
Updates golang/go#40661
Change-Id: I21be6d929288cfe41168cea34001fc2f41ac6c8b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247684
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In the past, we assumed that we would only run these functions on the
view's go.mod file. As we expand the concept of a view to possibly
include multiple go.mod files, we need to allow these functions to work
on multiple go.mod files.
Change-Id: If9e7d131007e0977fc48ee2264365773da7d41f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248097
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that the view always looks for its module, we don't need to pass
in the module root path when creating a view. This allows us to remove
some extra logic.
Change-Id: I35ffb71ec762b2dec0b72c84195ce008f7b35872
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247897
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
https://go-review.googlesource.com/c/tools/+/246419/2 fixed a problem
but introduced a new one, as go build treats -o directories differently
depending on whether or not a main package is being built.
(see https://github.com/golang/go/issues/36784)
This change explicitly constructs a temporary file for go build
to use.
Change-Id: I096748e9af5014428dab8a5aad703f062fe88d50
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247899
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Pontus Leitzler <leitzler@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some users may intentionally be opening subdirectories to avoid having
gopls load the whole module. Allow this via a configuration.
Fixesgolang/go#40567
Change-Id: I6167f62a74a1c0b7cf07c1cb247adda839ee41f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247617
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
A user need not necessarily set GOPACKAGESDRIVER in their environment,
but they may still provide it through their configuration.
Change-Id: Ic48328e6a1596ff653a048b24256b8dc44c45b8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls configuration can come in through initializeOptions or through
the workspace/configuration request. Make sure to handle error results
in both cases.
Change-Id: Iaf33d23aa33381ce86803f5e8d090deb369b7341
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227033
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In the previous implementation, we could not extract call expressions
to variables.
Change-Id: I80ee82d7889247a618bd80f40abaa897d15ad20b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246761
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Instead of only checking whether the selection is an AST expression in
canExtractVariable, we now also check what kind of AST expression
it is. This limits the frequency of situations where the lightbulb
appears (canExtractVariable succeeds), but nothing can be extracted
(extractVariable fails).
Change-Id: I1e63c982e482bb72df48b414bdb4e8037140afdb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247408
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The GC-based cache has given us a number of problems. First, memory
leaks driven by reference cycles: the Go runtime cannot collect cycles
involving finalizers, which prevents us from writing natural code in
Bind callbacks. If we screw it up, we get a mysterious leak that takes a
long time to track down. Second, the behavior is generally mysterious;
it's hard to predict how long a value lasts, and harder to tell if a
value being live is a bug. Third, we think that it may be interacting
poorly with the GC, resulting in unnecessary memory usage.
The structure of the values we put in the cache is not actually that
complicated -- there are only 5 significant types: parse, typecheck,
analyze, parse mod, and analyze mod. Managing them manually should not
be conceptually difficult, and in fact we already do most of the work
in (*snapshot).clone.
In this CL the cache adds the concept of "generations", which function
as reference counts on cache entries. Entries are still global and
shared across generations, but will be explicitly deleted once no
generations refer to them. The idea is that each snapshot is a new
generation, and can inherit entries from the previous snapshot or leave
them behind to be deleted.
One obvious risk of this scheme is that we'll leave dangling references
to values without actually inheriting them across generations. To
prevent that, getting a value requires passing in the generation at
which it's being read, and an error will be returned if that generation
is dead.
Change-Id: I4b30891efd7be4e10f2b84f4c067b0dee43dcf9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242838
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, we always created a default
value for each type in the struct or return statement in fillstruct
and fillreturns, respectively. Now, we try to find a variable in scope
that matches the expected type. If we find multiple matches, we choose
the variable that is named most similarly to the type. If we do not
find a variable that matches, we maintain the previous functionality.
Change-Id: I3acb7e27476afaa71aaff9ffb69445913575e2b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245130
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Progress reporting has gotten complicated, and has had a couple bugs.
Factor out progress-related behavior to a new progressTracker type, and
use this to implement some unit tests.
Also rename some methods to remove stuttering, and reorganize the code
to be more logical.
Fixesgolang/go#40527
Change-Id: I93d53a67982460e7171f892021e99f4523fe3e5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247407
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Our WorkDone reporting was generating a random token for each unit of
work, even if a token was supplied by the client. Change this to use
the client token if it is non-empty, and skip the
workDoneProgress/create request.
After this change we can no longer rely on tokens being a string.
Update our progress tracking accordingly.
For golang/go#40527
Change-Id: I702f739c466efb613b69303aaf07005addd3b5e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247321
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The gc annotation details code lens did build binaries during
diagnostics to the module root. When trying to enable details for a
main package in a sub directory, it failed since the output binary name
conflicted with the sub directory name.
Instead, specify output to the package directory during build to avoid
conflicts.
Change-Id: Idc11e0c6a9ba15a66645aeef89bffb5abde76928
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246419
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Now we downrank candidates that have already been used in other switch
cases. For example:
switch time.Now().Weekday() {
case time.Monday:
case time.<> // downrank time.Monday
}
It wasn't quite as simple as tracking the seen types.Objects.
Consider this example:
type foo struct {
i int
}
var a, b foo
switch 123 {
case a.i:
case <>
}
At <> we don't want to downrank "b.i" even though "i" is represented
as the same types.Object for "a.i" and "b.i". To accommodate this, we
track having seen ["a", "i"] together. We will downrank "a.i", but not
"b.i".
I applied only a minor 0.9 downranking when the candidate has already
been used in another case clause. It is hard to know what the user is
planning. For instance, in the preceding example the user could intend
to write "a.i + 1", so we mustn't downrank "a.i" too much.
Change-Id: I62debc5be3d5d310deb69d11770cf5f8bd9add1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247337
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We keep track of the N highest seen scores so we can quickly skip deep
completions not in the top N. Our logic for maintaining the top N list
wasn't quite right, resulting in certain cases where we would let
non-high scoring candidates through. I don't think the bug impacted
correctness.
Change-Id: Ic105617523c82f0e71e4f95ef0ee216182a84252
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247418
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now we will filter out the types already used in other case
statements:
switch ast.Node(nil).(type) {
case *ast.Ident:
case *ast.I<> // don't offer "Ident" since it has been used
}
Note that the implementation was not able to use a map to track the
seen types.Types because we build up types.Type entries dynamically
when searching for completions (e.g. types.NewPointer() to make a
pointer type). We must use types.Identical() instead of direct pointer
equality.
Change-Id: I316638bb48bfd6802e2caea671f297d640291010
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247098
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds the notion of a "workspace directory", which is
basically the set of directories that contains workspace packages. These
are mainly used for replace targets right now. It's a little trickier
than expected because the set of workspace directories can technically
change on any go.mod change.
At first, I wanted DidModifyFiles to report whether there was a change,
but I don't think it's actually that expensive to check on each call
and it complicates the code a bit. I can change it back if you think
it's worth doing.
The parse mod handle changes are because I needed an unlocked way of
parsing the mod file, but I imagine they'll conflict with CL 244769
anyway.
The next CL will be to "promote" replace targets to the level of
workspace packages, meaning we will be able to find references in them.
Change-Id: I5dd58fe29415473496ca6634a94a3134923228dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245327
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL is mostly a refactoring of the logic to compute errors for
`go mod tidy` diagnostics. It had been getting a little confusing, so
hopefully this makes things easier to read.
I made a few other small changes, such as slightly changing a few error
messages and showing diagnostics in the go.mod file for all missing
modules.
Change-Id: Ia8cf580731b997248591a2d64dff133accd9c5aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244610
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
* adds gopls command line tool for call hierarchy
* adds lsp setup for call hierarchy
* adds handler for textDocument/prepareCallHierarchy to display selected
identifier and get incoming/outgoing calls for it
* setup testing
Change-Id: I0a0904abdbe11273a56162b6e5be93b97ceb9c26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246521
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We missed a possible case in checking for gopackagesdriver - a binary
named gopackagesdriver works the same way as setting GOPACKAGESDRIVER.
Change-Id: I676800d253950cb35d74211558bafab340310653
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247179
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
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>
In the previous implementation, we kept the first variable in the
return statement that matched the each given return type. Now, we
keep searching for a non-"zero" value, even if we have already found
a "zero" value.
Change-Id: Icf0987bab90239781452319979e7a30502807e36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246917
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@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 current implementation, the return value of wasFirstChanges() is
reversed and did not record the URI of the changed file. In addition,
the snapshot was not stored in the snapshotByURI.
So I fixed when didChange() is executed, the URI of the edited file is
registered and wasFirstChanges() returns true if the URI is not
registered. Also the snapshot is now stored in snapshotByURI.
Fixesgolang/go#40531
Change-Id: I0adbf7459593d70660beb3b37900ffc88f707917
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247118
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
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>
In the previous implementation, fillreturns only altered return
statements that contained too few values. Now, fillreturns also examines
return statements with too many return values. In these situations,
we remove any value that is a "zero value" and does not match a type
in the return signature.
Change-Id: I7548307234ff4b16534b72a8aead95a322eb535a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246520
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
In the previous implementation, a test would pass if the given
command could not be applied to the given range.
Change-Id: I2e63972472cbd146cb5f27a3e27c878387222cb6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246517
Reviewed-by: Rebecca Stambler <rstambler@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>
parseGoHandles have lifetimes separate from the packages they belong to.
For example, a package may be invalidated by a change to one of its
files, but we still want to retain the parse results for all the rest.
Track them explicitly.
Change-Id: I03a4ffe283bf2b252d2d838bdb2cf332cd981075
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245059
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
Previously, we were only invalidating workspace packages when the go.mod
changed, but we really need to be invalidating all known packages.
Fixesgolang/go#40456
Change-Id: I9ad353a26ab40c74c7760ed7a1c5de517640cfab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245779
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change is generated whenever you run go test . --golden to generate
updated golden files. Commiting it so it doesn't show up in other CLs.
The module names are now sorted and hence this change shouldn't happen again.
Change-Id: I5d6c44d9a4f3ca24e336afad301136af7123eef7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245997
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Danish Dua <danishdua@google.com>
Packages that have gc_details enabled is stored internally in a
map[span.URI]struct{}, but when toggling on/off via ExecuteCommand, the
scheme is lost. The mismatch lead to no diagnostics from gc_details.
This fix uses span.URIFromPath() to parse the file path, so that the
internal keys are found when diagnostics are calculated.
Change-Id: I53b4e869625251c366b12b570eb03212319d2770
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245858
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@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>
We can prevent crashing on non-file URIs by checking the URIs of the
workspace folders, as well as the root URI.
Updates golang/go#40272
Change-Id: Ieddc6d6053fbb3d61e4c26fc8831c092328f6f33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244602
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
The FileIdentity struct mixes information about the file itself
(filename, hash) with information about the LSP references to that file
(session ID, version). When we create a cache key using it, we only want
the former, as returned by the String method. Otherwise we split the
cache whenever those irrelevant fields are different.
We also use FileIdentity as an element of diagnosticsKey, but I believe
that use is appropriate.
Change-Id: I094e00d2700e05778da635effbb69d0ebcb6726e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244020
Reviewed-by: Robert Findley <rfindley@google.com>
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 memoize cache buys us little for files: the cache value is not
really a function of the inputs, but rather the filesystem state. It's
pretty much just as easy to manage them explicitly, and it's a start at
simplifying our caching strategy.
We do lose one small feature: if we try to read the same file
concurrently, reads will not be deduplicated. I suspect that doesn't
matter.
Change-Id: I75e219467fb7a512d9cfdf87443d012c85f03df9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243197
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
1. Change code.ts so it generates []json.RawMessage in place of []interface{}
for Command-related arguments. As usual, vscode introduces a lot of
whitespace-only changes.
2. Generate code based on the July 28 version of vscode-languageserver.
The changes are mostly related to SemanticToken, and didn't require
any changes to gopls, other than in the generated code.
Change-Id: I673e29e2fbc097409683dfe7af911d8f66e25c5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245134
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Code actions that apply convenience fixes were filtered by the start
line so it wasn't possible to narrow the scope to a specific range.
This change allows clients to send a specific range (or cursor position)
to filter all fixes where the range doesn't intersect with the provided
range. It also widens the diagnostic returned by fillstruct analysis.
The idea is to provide a way to narrow the scope without breaking
clients that do want to ask for code actions using the entire line.
Updates golang/go#40438
Change-Id: Ifd984a092a4a3bf0b3a2a5426d3e65023ba4eebc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244519
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I'm not sure how the regtest didn't catch this - is it possible that
it could unmarshal a single string a slice of string? Either way, I'd
like to get the fix in quicker - I'll try to add more regtests for this
later.
Also, validate the upgrade results more thoroughly.
Change-Id: I812a3fecd9f0642a1408c0a9c0376bb98d50b397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245065
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I finally spent the time to understand why branch changes were causing
unexpected errors. There may be other bugs, but this is the first I
spotted. For batched invalidations, we were overriding the value of
invalidateMetadata for each file, so the results depended on the order
of files in the didChangeWatchedFiles notification.
Change-Id: Id3ca7a758af0115c46dcd74ede590a0be3f8307d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244606
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
I noticed this wasn't working correctly when debugging issues with file
changes on disk. I think I misunderstood how to correctly match on
zero or more path segments. Confirmed that this is working as expected
with VS Code, but we really need regression tests for this in VS Code
(it seems to be based directly on VS Code's API). Filed
golang/vscode-go#404.
Change-Id: Ib906f73a97317dfa9bd30099877c90d4072651cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244605
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When a package consists of files that only fail to parse, we fail to
associate parse errors with the package, and therefore return no
diagnostics. To address this, we need to associate the errors with the
package. This involves adding a *token.File to the parseGoData so that
error messages and positions can be properly extracted, even when there
is no associated AST.
Fixesgolang/go#39763
Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@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>
Some modules may need to be added to the go.mod without being associated
with an import statement. In such cases, we show the missing module
diagnostic on the whole go.mod file. This isn't ideal, but mapping to
the full require statement isn't that simple, and this is an easy enough
starting point. The code in mod_tidy.go is becoming more unwieldy - I
think I will clean it up in a follow-up.
Fixesgolang/go#39784
Change-Id: Ib32ec1fd74c455ce42ba778ea6cba0a475cf245a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243218
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
Using the new run options to configure a testing environment suitable
for long-running tests, a trivial stress test is added to type
arbitrarily in a known problematic repository
(github.com/pilosa/pilosa).
Change-Id: I2c8237843111f17ff5a096515cb4704c62513ed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244441
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A bunch of options are added to enable long-running performance-oriented
tests in existing directories. They will be used in a later CL to
implement a simple stress test, as an example of what is possible.
Change-Id: I531b201b415362ea135978238b3d64b903226359
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The Sandbox constructor was getting out of control, and this allows
binding regtest options directly to the sandbox configuration struct.
Change-Id: I18da4ddf43c86b3b652516c3ddca7726cd35e3b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244439
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This removes some unncessary mangling of strings, and allows the editor
to fully own its configuration.
Change-Id: I0fde206824964124182c9138ee06fb670461d486
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244360
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change is a follow-up from CL 242477. We now also surface errors
in indirect dependencies as diagnostics in the go.mod file. Any errors
we cannot match are surfaced as diagnostics on the entire go.mod file.
This isn't the most user-friendly way, but it seems simplest.
Change-Id: Ife92c68c09b74a0b53de29d95b6c4c2a9c78d3b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change expands the scope of a workspace to the whole module, if the
user is in module mode. This means that diagnostics will appear and will
be updated for the whole module, even if the user only opens a
subdirectory. Similarly, references and other such queries will always
return consistent results, no matter which directory the user opens.
A new "root" field is added to the view. This is either the view's
folder or its module root. Almost all uses of view.folder have been
changed to view.root.
Updates golang/go#32394
Change-Id: I46f401f7c44b1b8429505aa032e0c15e88c4e2ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I noticed a race in these logs:
https://build.golang.org/log/ec2915e97319de219284ed022338f2ebc549aff6.
We need to copy the environment and build flags for each config, since
we're treating the config as single use.
Change-Id: I9e717e688def088cb60f2b23b71d731e2b20b259
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244118
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 242579 changed the mechanism of reporting diagnostics so that we now
combine reports from multiple sources. Previously, we overwrote existing
diagnostics if new ones were reported. This was fine because
source.Diagnostics generated all of the diagnostics for Go files, and
mod.Diagnostics generated all of the diagnostics for go.mod files.
Now, we combine diagnostics from both sources -- mod.Diagnostics can
generate reports for Go files. We may get duplicate reports for packages
with test variants, so now, we check for those and dedupe.
Change-Id: I42e98079b4eead380058dd029a3a0c72a1796ebb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243778
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>
This change ensures that, when the initial workspace load fails, we
re-run it if the go.mod file changes. Previously, if a user opened a
workspace with a corrupt go.mod file, we never recovered.
To reinitialize the workspace on-demand, we use the initializeOnce field
as an indicator of whether or not we should reinitialize. Every call to
awaitInitialized (which is called by all functions that need the IWL),
passes through the initialization code. If a retry isn't necessary,
this is a no-op, but if it is, we will call the initialization logic.
Only the first attempt uses a detached context; subsequent attempts can
be canceled by their contexts.
To indicate that we should reinitialize, we call maybeReinitialize.
Right now, we only call this when the go.mod file changes. In the
future, we may need it in other cases.
Fixesgolang/go#38232
Change-Id: I77eefebb0ac38fbd0fe2c7da09c864eba45b075f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242159
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>
Convenience code actions operates at line-by-line, but a line can
contain more than one fix.
To be able to tell them apart each response now return the actual
range as reported by the analyzer.
Fixesgolang/go#40328
Change-Id: I9ffe4737653d2a5a6dafc3574e74975a71c71937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243877
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We had previously been treating file changes with no content in the same
way as the deletion of content. Now, we distinguish between the two.
Fixesgolang/go#38424.
Change-Id: I44b338006af0c13a8a3f842844521789ea378470
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243577
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
We still don't have a good way to make sure that we don't break 1.12
support, but this is an easy enough fix to start with.
Updates golang/go#39146
Change-Id: I14fe997fa1f3d60320d77e664208e25d97ae6f4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243578
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This is unnecessary. It also saves a lot of time for the regtests.
Change-Id: Ifc12d4c8e0c215f336e958316d415bc7f30e5344
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243538
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
The gopls daemon had different default logging behavior than the sidecar
gopls: by default, the daemon was started with -logfile=auto.
Additionally, because most logs are reflected back to the forwarder, the
actual daemon logs have very little (if any) information.
This means that if you simply start gopls with -remote=auto, you'll get
a single logfile named /tmp/gopls-<pid>.log, which is mostly empty. This
is not a delightful experience.
Fix this via several improvements:
+ Log lifecycle events in the Daemon, to give the log a purpose.
+ Give the daemon a new default log location:
/tmp/gopls-daemon-<pid>.log.
+ Don't pass -logfile=auto to the daemon by default.
Fixesgolang/go#40105
Change-Id: I5e91ea98b4968c512bce76a596bbae441f461a66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is an alternative to CL 241739, preserving the exiting variadic
options pattern and just adding a helper method to put the options up
front.
All things considered, I think this is better. CL 241739 was too
complicated, and being able to reuse "curried" runners isn't actually
important.
Updates golang/go#39384
Change-Id: I7ecd80d310cac879520b2d1feebcf5bd5e96e89b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243057
Run-TryBot: Robert Findley <rfindley@google.com>
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>
This change moves the "generic" go generate code lens code into
progress.go. Fix up a few generate-specific things in the progress
writers.
The remaining generate code isn't much, and is moved into command.go.
Change-Id: I2b7b6279da2442c0b92758b9b3e259f25787fabc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242919
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This configuration deals with the fact that VS Code has the same
shortcut for go-to-definition and clicking a link. Many users don't like
the behavior of this shortcut triggering both requests, so we allow
users to choose between each behavior.
Fixesgolang/go#39065
Change-Id: I760c99c1fade4d157515d3b0fd647daf0c8314eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
importPrefix in format.go computes the end of the package statement
including a trailing newline. This causes a panic if the user has
not typed the newline. The code now checks for that case.
Fixesgolang/go#40208
Change-Id: I4c5118a5de78027e6c5e6ed91dfddca81e38f7e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242638
Reviewed-by: Heschi Kreinick <heschi@google.com>
The current behavior is inconsistent across different configurations.
The signature is the top line for FullDocumentation, but the last line
for synopsis documentation.
Change-Id: I2fe1e1c7bfa38ae46fda06b0ecd4c7712b0e74af
GitHub-Last-Rev: d3b768558f21f518d1296b6fc776cf3974f441d9
GitHub-Pull-Request: golang/tools#241
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242104
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change reworks CL 240118 to apply gofumpt directly as a formatter,
not an analyzer. Depending on how gofumpt changes, we may be able to use
it as an analyzer in the future, but for now it's just easier to add it
as a formatting hook.
Fixesgolang/go#39805
Change-Id: I227fde4b1916d8a82557f30dfca88e155136dff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I wanted to add a regtest for this, but then immediately got stumped on
how to do it well. The best I've got is to add a helper that wraps
the client's call to executeCommand, but it's used in so many places that
an easier fix was to put it in executeCommand itself.
Fixesgolang/go#40101
Change-Id: Iafb804fb77e57e6ac01e3de1d115058b21ac1954
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241984
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Not sure how this hasn't come up earlier. We aren't noticing file
changes on-disk for go.mod/go.sum files.
Fixesgolang/vscode-go#297
Change-Id: I4532a252f330404515efec244a9c266a765e6bcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242160
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This test exposes a lot of fundamental issues with `go list` overlays.
Now that we have the regression test framework, we can copy it over and
make any flakes completely reproducible by waiting for each change to
complete.
The issue here ended up being that initial workspace load returns
workspace packages with no associated files due to golang/go#39986.
Working around this allows invalidation to proceed as usual.
In the process of debugging this, I also noticed that packages can get
loaded as command-line-arguments even when we can get the correct
package path for them. It's pretty complicated to fix this, so
restructured the code a tiny bit and left a TODO. I'd like to come back
to this at some point, but it's not pressing since I was able to fix
this bug.
Fixesgolang/go#39646.
Updates golang/go#39986.
Change-Id: Id6b08a5e92d28eddc731feb0ef3fd3b3fc69e64b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change addresses an underlying issue with the go.mod code, which is
that it was modifying go.mod files without cloning them. This could've
resulted in some ugly race conditions.
We also handle the fact that new dependencies weren't being added
cleanly to files that already had unused dependencies.
Fixesgolang/go#39041
Change-Id: I96ee0052d8d29a25e24f0bda9688e780a0fa7442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241443
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Replacing the text in the comment line-by-line prevents issues to do
with CRLF/LF line endings.
No test, because txtar expects LF line endings, and I didn't think it
was worth more invasive changes to handle this.
Fixesgolang/go#39364.
Change-Id: Ia26b311a851396e4dde1954ebfc1b40c0a3c04fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The default regtest timeout was increased to 60s a while ago to try to
avoid flakes.
We think we've figured out the flakes, so drop it back down. Start with
20s, since TestGOMODCACHE (which had overridden its timeout to 5s)
occasionally fails on freebsd:
https://build.golang.org/log/6c607c7fd3e886cf23127c958ae17637440fbc63
Change-Id: I10e6927985b8fb1d149070e204bd8ca120309c7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240060
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.
This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...
Also give regtest sandboxes names derived from their test name.
Updates golang/go#39384
Updates golang/go#38490
Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Danish Dua
Rob Findley
Rebecca Stambler
Heschi Kreinick
Peter Weinbergr
Pontus Leitzler
Muir Manders
Brayden Cloud
Josh Baum
hasheddan
tennashi
smasher164
Hyoyoung CHANG