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>
* 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>
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>
