1
0
mirror of https://github.com/golang/go synced 2024-11-18 16:24:42 -07:00
Commit Graph

807 Commits

Author SHA1 Message Date
Rob Findley
df83f4e7c1 internal/lsp: fix builds and tests for go1.12+
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>
2020-08-27 14:30:46 +00:00
Rob Findley
17fd2f27a9 internal/lsp/regtest: add a workspace symbols benchmark
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>
2020-08-27 01:05:19 +00:00
Rebecca Stambler
b85e56c1dc internal/lsp/source: sort references and implementations results
We should make sure to return deterministic results for these requests.

Fixes golang/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>
2020-08-26 22:01:59 +00:00
Danish Dua
bc8aaaa29e internal/lsp: ignore period ('.') triggered completions in comments
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>
2020-08-26 04:07:57 +00:00
Rebecca Stambler
f57a28cb8c internal/lsp/source: use space character in markdown formatting
Use space characters instead of the HTML "&nbsp;" 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.

Fixes golang/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>
2020-08-26 03:44:07 +00:00
Danish Dua
e0bf2294bb internal/lsp/source: fix completion on final line of a document
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>
2020-08-25 22:56:35 +00:00
Danish Dua
b72e8bb66c internal/lsp: use prefix matcher with comment completion
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>
2020-08-25 21:01:48 +00:00
Pontus Leitzler
c024452afb internal/lsp: don't panic if there are no suggested fixes
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>
2020-08-23 20:58:32 +00:00
Rob Findley
1e23e48ab9 internal/lsp: improvements for command messages
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>
2020-08-21 20:07:30 +00:00
Danish Dua
3509cdc6e9 internal/lsp: add ast fields to comment completion for declarations
* 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>
2020-08-21 18:29:12 +00:00
Rob Findley
daa6538899 internal/lsp/source: fix panic in formatZeroValue for invalid type
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.

Fixes golang/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>
2020-08-21 17:11:49 +00:00
Muir Manders
74543c4034 internal/lsp/source: fix composite literal type name completion
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>
2020-08-21 15:12:09 +00:00
Muir Manders
9ac8e33b36 internal/lsp/source: improve completion of printf operands
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.

Fixes golang/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>
2020-08-21 13:58:45 +00:00
Heschi Kreinick
c8f3937451 internal/lsp/source: fix unexported references of non-workspace packages
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>
2020-08-20 18:02:10 +00:00
Danish Dua
b793a1359e internal/lsp: add outgoing calls call hierarchy
* 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>
2020-08-20 01:08:01 +00:00
Heschi Kreinick
d94536333c internal/lsp/cache: don't always type check in default mode
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.

Fixes golang/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>
2020-08-19 19:22:15 +00:00
Rebecca Stambler
d00afeaade internal/lsp/source: fix nil pointer in rename_check
Updates golang/vscode-go#534

Change-Id: I0a30ac7f52862a096b07c35665539cfed99d4828
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-17 02:38:11 +00:00
Muir Manders
90abf76919 internal/lsp/source: fix a couple issues completing append() args
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>
2020-08-15 16:56:00 +00:00
Muir Manders
6fbe43b70d internal/lsp/source: improve completion in append()
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.

Fixes golang/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>
2020-08-15 16:31:36 +00:00
Muir Manders
7c0525f229 internal/lsp/source: improve func literal completions
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.

Fixes golang/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>
2020-08-15 16:17:26 +00:00
Danish Dua
9882f1d182 internal/lsp: add initial workspace load notification
Add workspace package load (IWL) notification.

Closes golang/go#40632

Change-Id: I4d4c6fba1ece08bb0d6df52da2e6f08c959fd1ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248623
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-14 23:09:02 +00:00
Danish Dua
1365742343 internal/lsp: add incoming calls hierarchy to gopls
* 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>
2020-08-13 20:36:30 +00:00
Muir Manders
c42aa19e51 internal/lsp/source: improve unnamed type completion
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.

Fixes golang/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>
2020-08-12 23:05:10 +00:00
Brayden Cloud
c7ca52690a internal/lsp: add "run file benchmarks" code lens
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>
2020-08-12 18:37:58 +00:00
Rebecca Stambler
eb8585a966 internal/lsp: extend the mod handle functions to handle multiple files
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>
2020-08-12 18:23:14 +00:00
Peter Weinbergr
74512f09e4 internal/lsp: in gc_details change command to use a temporary file.
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>
2020-08-11 15:37:30 +00:00
Rebecca Stambler
fd80f4dbb3 internal/lsp: fix a few small staticcheck warnings
Address unused context / unused error warnings.

Change-Id: Ibd302d58a3d6db5aa274ed95fb6d5a337978779a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Danish Dua <danishdua@google.com>
2020-08-11 03:20:01 +00:00
Rebecca Stambler
48de4c84f0 internal/lsp: add a configuration to limit workspace scope to root URI
Some users may intentionally be opening subdirectories to avoid having
gopls load the whole module. Allow this via a configuration.

Fixes golang/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>
2020-08-11 01:57:04 +00:00
Josh Baum
2f2f27240c internal/lsp: support function calls in extract variable
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>
2020-08-10 22:41:34 +00:00
Josh Baum
72f2e0bf6d internal/lsp: limit code action false positives for extract to variable
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>
2020-08-10 22:02:59 +00:00
Heschi Kreinick
c1903db4db internal/memoize: switch from GC-driven to explicit deletion
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>
2020-08-10 19:02:17 +00:00
Pontus Leitzler
3e8281990c internal/lsp: build output to package during gc_details code lens
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>
2020-08-10 15:03:20 +00:00
Muir Manders
6f4f008689 internal/lsp/source: improve completion in switch cases
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>
2020-08-09 01:28:40 +00:00
Muir Manders
5bf02b21f1 internal/lsp/source: fix bug in deep completion score tracking
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>
2020-08-08 16:17:06 +00:00
Muir Manders
19738be007 internal/lsp/source: improve type switch case completion
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>
2020-08-07 23:56:57 +00:00
Rebecca Stambler
383b97c0b5 internal/lsp: watch directories in replace targets and update on changes
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>
2020-08-07 23:35:17 +00:00
Rebecca Stambler
c05a0f5be4 internal/lsp/cache: refactor go mod tidy error logic
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>
2020-08-07 22:43:23 +00:00
Danish Dua
9221131678 internal/lsp: release resources for call hierarchy file requests
* adds call to release resources for beginFileRequest in
  lsp/call_hierarchy.go
* fixes source_test

Change-Id: Id45f61ccd474a2df9f46be89fdb059cfe8584f0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247497
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-07 21:04:51 +00:00
Danish Dua
a5d4502270 internal: add call hierarchy cmd and lsp scaffolding
* 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>
2020-08-07 20:22:52 +00:00
Josh Baum
6756e73834 internal/lsp: fix bug in extract function when highlighting full line
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>
2020-08-07 18:02:56 +00:00
Muir Manders
990129eca5 internal/lsp/source: prefer funcs when completing go/defer
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>
2020-08-06 23:41:36 +00:00
Muir Manders
23e6869392 internal/lsp/source: don't prefer bool candidates in bool exprs
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.

Fixes golang/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>
2020-08-06 23:41:14 +00:00
Muir Manders
41a9f6dc66 internal/lsp/source: fix function completion ranking
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>
2020-08-06 22:03:01 +00:00
Josh Baum
110bd3ba6b internal/lsp: make function extraction smarter
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>
2020-08-06 19:37:29 +00:00
Rebecca Stambler
ee49e490f2 internal/lsp/source: fix bug introduced by CL 246757
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>
2020-08-06 18:32:31 +00:00
Muir Manders
90696ccdc6 internal/lsp/source: simplify variadic logic in completion
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>
2020-08-06 02:28:45 +00:00
Muir Manders
6fe4996ff4 internal/lsp/source: fix multi return value func completion
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>
2020-08-06 02:28:35 +00:00
Peter Weinbergr
0b898c9289 internal/lsp: add options to control which details gc_details shows
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>
2020-08-05 22:37:31 +00:00
Heschi Kreinick
f29cbc7105 internal/lsp: remove source.Cache
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>
2020-08-05 22:08:24 +00:00
Rebecca Stambler
d55f2eddcb internal/lsp/source: fix nil pointer in extract function
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>
2020-08-05 15:47:06 +00:00