1
0
mirror of https://github.com/golang/go synced 2024-09-30 16:18:35 -06:00
Commit Graph

4626 Commits

Author SHA1 Message Date
Rohan Challa
099440627f internal/lsp: add goreturns like functionality as quickfix
This change ports the functionality of https://github.com/sqs/goreturns
to be used as code actions on diagnostics that have missing
return values. It improves on the original goreturns functionality by:

- filling out empty return statements
- trying to match existing return values to the required return
  values and then filling in missing parameters

Fixes golang/go#37091

Change-Id: Ifaf9bf571c3bc3c61e672b0a2f725d8d734d432d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224960
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-01 19:27:44 +00:00
Rebecca Stambler
9d5940d493 internal/lsp: refactor and propagate errors from code actions
This change makes sure that all errors in code actions are returned,
instead of ignored. There is also a refactoring of the suggested fixes
for go.mod files so that we don't compute them unless there is already a
matching diagnostic.

Change-Id: I34afda0116f3cc7e47809d980a0171487787e55e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221221
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-31 20:20:46 +00:00
Bryan C. Mills
ac2e956812 go.mod: upgrade goldmark dependency to fix js/wasm build
For golang/go#38183
Updates yuin/goldmark#120

Change-Id: I913c3409fe0c774209ce046a85699ef845e7c7b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226560
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-03-31 19:25:49 +00:00
Rebecca Stambler
a30bf2db82 gopls/integration/govim: revert back to using the latest govim
A new version of govim with the fix for the original issue has been
released.

Change-Id: Iaea2eff7387f352cae1657949c5e364a66337f68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226557
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 02:57:13 +00:00
Rebecca Stambler
e18c1c42de internal/lsp: add a reg test to test "go mod init"
This change adds a RunGoCommand function to the workspace, which will
allow us to test how gopls responds when a certain go command is
executed. Add a test that checks behavior after "go mod init" runs.

Change-Id: I679249c1654b136d44187397b4196b8a10b5615e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226478
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 02:48:52 +00:00
Rebecca Stambler
a7c0594f4e internal/lsp: avoid logging context cancellation
This change adds a helper function that checks if the context is
canceled, and if so, doesn't log the error. Tried to use it everywhere
in internal/lsp where it fits, which resulted in changing a few pieces
of error handling throughout.

Updates golang/go#37875

Change-Id: I59cbc6f893e3b70cf84524d9944ff7f4b4febd78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226371
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-31 01:46:33 +00:00
Ian Cottrell
a42d6a358d internal/telemetry: give Event a custom implementation of TagMap
This cuts the allocation cost of preparing an event for export from 4 down to 1.

name                old time/op    new time/op    delta
/LogIgnore-8          5.97µs ± 6%    4.18µs ± 4%  -29.87%  (p=0.000 n=20+19)
/TraceIgnore-8        13.6µs ± 4%    10.8µs ± 6%  -20.92%  (p=0.000 n=18+19)
/StatsIgnore-8        11.3µs ± 6%     7.9µs ± 6%  -30.29%  (p=0.000 n=20+20)
/Log-8                47.8µs ± 6%    43.9µs ± 6%   -8.15%  (p=0.000 n=20+19)
/Trace-8              60.5µs ±12%    54.7µs ± 4%   -9.58%  (p=0.000 n=20+19)
/Stats-8              13.8µs ± 3%    10.6µs ± 5%  -23.57%  (p=0.000 n=20+19)

name                old alloc/op   new alloc/op   delta
/LogIgnore-8          5.12kB ± 0%    3.58kB ± 0%  -30.00%  (p=0.000 n=20+20)
/TraceIgnore-8        14.6kB ± 0%    11.5kB ± 0%  -21.05%  (p=0.000 n=20+20)
/StatsIgnore-8        10.2kB ± 0%     7.2kB ± 0%  -30.00%  (p=0.000 n=20+20)
/Log-8                24.0kB ± 0%    20.9kB ± 0%  -12.81%  (p=0.000 n=18+20)
/Trace-8              31.0kB ± 0%    27.9kB ± 0%   -9.92%  (p=0.000 n=20+20)
/Stats-8              10.2kB ± 0%     7.2kB ± 0%  -30.00%  (p=0.000 n=20+20)

name                old allocs/op  new allocs/op  delta
/LogIgnore-8            64.0 ± 0%      16.0 ± 0%  -75.00%  (p=0.000 n=20+20)
/TraceIgnore-8           160 ± 0%        64 ± 0%  -60.00%  (p=0.000 n=20+20)
/StatsIgnore-8           128 ± 0%        32 ± 0%  -75.00%  (p=0.000 n=20+20)
/Log-8                   382 ± 0%       286 ± 0%  -25.13%  (p=0.000 n=20+20)
/Trace-8                 480 ± 0%       384 ± 0%  -20.00%  (p=0.000 n=20+20)
/Stats-8                 128 ± 0%        32 ± 0%  -75.00%  (p=0.000 n=20+20)

Change-Id: I76ff4ac9658b766daf3929e7cfc0d2cfcab036ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226359
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:51:03 +00:00
Ian Cottrell
3b4a2bfdc9 internal/telemetry: removing the IsEmpty method of TagMap
It is not being used, and implementing it efficiently is infeasible without
reducing the efficiency of cases that matter more.

Change-Id: Ie53250f0e63ad08a418724c60a74a52edbdfdb29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226358
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:50:56 +00:00
Ian Cottrell
4edcf52965 internal/telemetry: add a noop exporter benchmark
This has the exporter registered but doing nothing, to measure the basic cost of
having any exporter without any specific exporter costs.
Specifically at the moment this measures the cost of filling in the time on the
event and building the TagMap that is passed down.

Change-Id: Iaae5659e3de9b871dc281c509fa2ee9c3e1d049a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226357
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:50:42 +00:00
Ian Cottrell
80f63e2b9b internal/lsp: rewrite the stats using the newer telemetry features
This allows us to reduce the handler interface and delete the telemetry handler.
It is also safer and faster, and can be easily disabled along with the rest of
the telemetry system.

Change-Id: Ia4961d7f2e374f7dc22360d6a4020a065bfeae6f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225957
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:50:33 +00:00
Ian Cottrell
657a652153 internal/telemetry: add a synchronization to telemetry
Some things that used to be safe due to the global
lock now need ther own synchronization primitives.

Fixes golang/go#38102

Change-Id: I03c692682d57620d96fe84b7dc74efae0d3f8f09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226317
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-30 19:15:27 +00:00
Rebecca Stambler
f8bfb4ee30 internal/gocommand: fix environment on Windows
Seems like os.Environ() is necessary after all. My bad for not testing
my earlier change on Windows. I'm not able to reproduce the behavior
in my test, so it's not really testing the crash correctly.

Fixes golang/go#38062

Change-Id: Ied45bbf572023a9dcd5d020db49bf3e95c824602
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226370
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-30 18:31:14 +00:00
Rohan Challa
31583a0dbb internal/lsp/tests: remove ellipses from test names
This change makes working with tests easier by trimming the folder name
up to "testdata". This will remove any ellipses from the test folder name.

Fixes golang/go#38103

Change-Id: I33b931e527de63713b8fc370c50b1c382796b2b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226377
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-30 17:55:17 +00:00
Rebecca Stambler
9c79f685b7 internal/lsp: fix view rebuilding when go mod init runs
https://github.com/microsoft/vscode-go/issues/3076#issuecomment-605062933
inspired me to write a regression test for this case. Turns out we
weren't handling it correctly after all...

This change makes sure that we only rebuild the view once a new go.mod
file is saved, not just created. It also preserves the snapshot ID
number when the view is recreated so that diagnostic caching continues
to work as expected.

Change-Id: I63bd559c3bd33b91828171cd7ddb3d099c31cddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226017
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-30 17:42:33 +00:00
Rob Findley
046453f0ab gopls/integration/govim: fix govim tests without a go.mod file
govim tests without a go.mod file were accidentally picking up the
go.mod file from the build context at /workspace/go.mod.

To fix this, run govim tests in a new, dedicated artifacts volume
mounted at /artifacts.

Change-Id: Ie224bbb730741d047be26a6d057a023ddc8a3453
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226157
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-30 17:32:57 +00:00
Rob Pike
fa3cc9eebc x/tools/present: point to instagram sted twitter for Renee French
Her near-official site is now on Instagram; she doesn't use
Twitter much. Might as well give a useful link.

She doesn't use the acute accent either, so drop that as well.

Change-Id: I84d6fe34277c19763004c8d18d1f0b745f3d3adb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226285
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-30 04:01:39 +00:00
Ian Cottrell
fd4102a86c internal/telemetry: minor improvement to span events
This changes span events to return a cheaper end function if there is
no exporter and also to deliver the end event to the same exporter
that the start even was delivered to rather than which was active
when the end event is triggered.

Change-Id: I831283da20e8cc991a0cf7490952ae194d125428
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-29 02:58:19 +00:00
Ian Cottrell
36db529775 internal/telemetry: add fast non variadic event functions
This adds variants of the main event functions that take specific
numbers of tags, rather than a tag slice. This reduces the allocation
cost when using those functions with no exporter to zero.

name                old time/op    new time/op    delta
/Baseline-8            158ns ± 7%     154ns ± 1%      ~
/StdLog-8             6.90µs ± 1%    6.83µs ± 1%      ~
/LogNoExporter-8      1.78µs ± 5%    1.20µs ± 3%   -32.37%
/TraceNoExporter-8    3.11µs ± 3%    2.48µs ± 2%   -20.39%
/StatsNoExporter-8    3.18µs ± 5%    1.87µs ± 3%   -41.16%
/Log-8                46.8µs ± 2%    44.8µs ± 1%    -4.33%
/Trace-8              55.1µs ± 5%    54.3µs ± 3%      ~
/Stats-8              15.8µs ± 3%    13.4µs ± 1%   -15.00%

name                old alloc/op   new alloc/op   delta
/Baseline-8            0.00B          0.00B           ~
/StdLog-8               552B ± 0%      552B ± 0%      ~
/LogNoExporter-8      1.02kB ± 0%    0.00kB       -100.00%
/TraceNoExporter-8    1.54kB ± 0%    0.51kB ± 0%   -66.67%
/StatsNoExporter-8    2.05kB ± 0%    0.00kB       -100.00%
/Log-8                26.0kB ± 0%    24.0kB ± 0%    -7.87%
/Trace-8              28.7kB ± 0%    27.1kB ± 0%      ~
/Stats-8              13.3kB ± 0%    10.2kB ± 0%   -23.08%

name                old allocs/op  new allocs/op  delta
/Baseline-8             0.00           0.00           ~
/StdLog-8               30.0 ± 0%      30.0 ± 0%      ~
/LogNoExporter-8        16.0 ± 0%       0.0       -100.00%
/TraceNoExporter-8      32.0 ± 0%      16.0 ± 0%   -50.00%
/StatsNoExporter-8      32.0 ± 0%       0.0       -100.00%
/Log-8                   430 ± 0%       382 ± 0%   -11.16%
/Trace-8                 496 ± 0%       464 ± 0%    -6.45%
/Stats-8                 192 ± 0%       128 ± 0%   -33.33%

Change-Id: I629c0d506ab07de6f12b0acbecfc7407f59a4285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225580
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-29 02:57:12 +00:00
Ian Cottrell
f61d083d3b internal/telemetry: use tags instead of special event fields.
Change-Id: I0e6a26c62bd1f6eaa07c38a06152cb7a0f2eedc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225579
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-29 02:56:35 +00:00
Ian Cottrell
f8bbb56955 internal/telemetry: change tag to use type specific storage
Because packing values into interface{} causes allocations
this gives us a major improvement on allocation count, but
tag is bigger so a significant loss on bytes allocated.
It is a minor performance win overall, especially in the
null exporter case when it really matters the most.

name                old time/op    new time/op    delta
/Baseline-8            147ns ± 3%     146ns ± 3%     ~     (p=0.349 n=5+5)
/StdLog-8             6.83µs ± 1%    6.78µs ± 2%     ~     (p=0.548 n=5+5)
/LogNoExporter-8      1.15µs ± 1%    1.05µs ± 1%   -8.63%  (p=0.008 n=5+5)
/TraceNoExporter-8    1.01µs ± 4%    0.99µs ± 1%     ~     (p=0.087 n=5+5)
/StatsNoExporter-8    1.89µs ± 0%    1.95µs ± 3%   +3.27%  (p=0.016 n=4+5)
/Log-8                23.7µs ± 1%    24.4µs ± 6%   +3.14%  (p=0.032 n=5+5)
/Trace-8              42.2µs ± 1%    42.2µs ± 1%     ~     (p=0.841 n=5+5)
/Stats-8              7.37µs ± 3%    7.10µs ± 1%   -3.74%  (p=0.008 n=5+5)

name                old alloc/op   new alloc/op   delta
/Baseline-8            0.00B          0.00B          ~     (all equal)
/StdLog-8               552B ± 0%      552B ± 0%     ~     (all equal)
/LogNoExporter-8        680B ± 0%     1024B ± 0%  +50.59%  (p=0.008 n=5+5)
/TraceNoExporter-8      512B ± 0%      512B ± 0%     ~     (all equal)
/StatsNoExporter-8    1.26kB ± 0%    2.05kB ± 0%  +62.03%  (p=0.008 n=5+5)
/Log-8                5.20kB ± 0%    6.06kB ± 0%  +16.46%  (p=0.008 n=5+5)
/Trace-8              11.8kB ± 0%    11.8kB ± 0%     ~     (p=0.167 n=5+5)
/Stats-8              2.29kB ± 0%    3.07kB ± 0%  +34.27%  (p=0.008 n=5+5)

name                old allocs/op  new allocs/op  delta
/Baseline-8             0.00           0.00          ~     (all equal)
/StdLog-8               30.0 ± 0%      30.0 ± 0%     ~     (all equal)
/LogNoExporter-8        30.0 ± 0%      16.0 ± 0%  -46.67%  (p=0.008 n=5+5)
/TraceNoExporter-8      16.0 ± 0%      16.0 ± 0%     ~     (all equal)
/StatsNoExporter-8      62.0 ± 0%      32.0 ± 0%  -48.39%  (p=0.008 n=5+5)
/Log-8                   172 ± 0%       158 ± 0%   -8.14%  (p=0.008 n=5+5)
/Trace-8                 336 ± 0%       336 ± 0%     ~     (all equal)
/Stats-8                94.0 ± 0%      64.0 ± 0%  -31.91%  (p=0.008 n=5+5)

Change-Id: I81d2443c9a32d25a5b60fffa60759caa33566da2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225381
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-29 02:53:08 +00:00
Rebecca Stambler
3db5fc6bac internal/gocommand: don't delete command's environment
My fix in CL 225817 introduced another bug :)

Fixes golang/go#38101

Change-Id: I3de1a051d3e86474c6aa0fb0e427a590438e2172
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-28 03:18:15 +00:00
Heschi Kreinick
82bb89366a internal/lsp/cache: validate workspace path case
On case-insensitive file systems, the editor may send us a path that
works but doesn't match the actual file's case. Then when we run go
list, we'll get mismatching paths and everything will break.

We can't reliably fix this problem: tracking what case the editor
expects is too difficult to be worth it. Instead, check the workspace
path and bail if it's mismatched.

Possibly we should also check files on DidOpen or such, but we can start
with this.

Updates golang/go#36904.

Change-Id: I7635c8136bf9400db4143a0f2fde25c39b5abc44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225239
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-03-27 19:55:53 +00:00
Rebecca Stambler
8faa90c243 internal/gocommand: only set working dir if it's not empty
This was causing crashes by setting the working directory to be empty.

Fixes golang/go#38062
Fixes golang/go#38101

Change-Id: I6d679ee1d5dcab914df3d565d83aa6de0dd74cb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-27 19:39:47 +00:00
Rohan Challa
eabff7e044 internal/lsp/analysis: add quickfix for "no new vars on left side"
This change adds a quick fix for type errors of the type "no new vars on left side of :=". It will replace the ":=" with an "=".

Updates golang/go#34644

Change-Id: I91af8eb82956104229c3b4f3d0fce60fdfdbb5ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 19:15:53 +00:00
Michael Matloob
d190e260e5 go/packages: add a workaround for different import stacks between go versions
Go 1.15+, as of CL 224660, puts the importing package on the top of the stack
(because it makes more sense in the errors). Look there by default and fall
back to second from top position if top of stack is the package itself.

Updates golang/go#36173

Change-Id: I1681089b4a18af9e535661668329ad32b1ba1936
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225677
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 19:06:04 +00:00
Rohan Challa
c12078ef08 go/analysis/passes/unreachable: add suggested-fix to remove dead code
This change adds suggested fixes to the unreachable analysis pass by giving the user a fix to remove the code.

Change-Id: If0add84e6977f12a1cef6e92120a1b4571b95a11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223664
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:58:09 +00:00
Rohan Challa
f4fcf867e7 internal/lsp/analysis: add quickfix for "no result values expected"
This change adds a quick fix for type errors of the type "no result values expected". It will replace the return statment with an empty return statement.

Updates golang/go#34644

Change-Id: I3885748dfc69a2d19f8e7a2e81f36f6d0a20d25b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223666
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:57:59 +00:00
Rohan Challa
afab6edfad internal/lsp/source: remove unused parameters from functions
This change uses the new unusedparams analyzer to remove any unused parameters from functions inside of internal/lsp/source :)

Change-Id: I220100e832971b07cd80a701cd8b293fe708af3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225997
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:57:18 +00:00
Rob Findley
17a19b5fe7 internal/lsp/cmd: add a flag to disable telemetry
govim integration tests (and probably some real user sessions) are
broken because telemetry metrics are not threadsafe, resulting in an
index out of range panic.

Fix this by adding a flag (labeled temporary) to disable telemetry
export.

Also temporarily update govim to master to pick up some fixes, and run
only the -short tests to avoid timeouts.

Updates golang/go#38042

Change-Id: I584e5d200c2f732bd4024002ee6253d09623b29f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226057
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-03-27 18:51:17 +00:00
Rohan Challa
8f81e2e6d4 internal/lsp/analysis: add quickfix for undeclared variable
This change adds a quick fix for diagnostics that have an error message of the form "undeclared name: %s". It provides a quick fix to add a new variable with that name.

Updates golang/go#34644

Change-Id: I6534ee79d1770d1a62bac169c3c7e52e2443f39e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222237
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 18:31:06 +00:00
Rohan Challa
42235f6384 internal/lsp: add support for type error analyzers
This change adds support within gopls for analyzers that work with type errors to provide suggested fixes.

Updates golang/go#34644

Change-Id: Ia8929173752fda6bd84a9edaabd310e758f25fe8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222761
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 17:27:16 +00:00
Rebecca Stambler
eca45d481d internal/lsp: refactor references/rename/implementations
As part of investigating golang/go#38100, I noticed a few things that I
wanted to clean up. Mostly, for renames, we were calling
qualifiedObjAtProtocolPos twice, so I factored out a shared helper
function. I also added an error return for builtins so that callers
don't have to check.

Change-Id: I28c75c801cbec1611736af931cfa72befd219201
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-27 17:09:18 +00:00
Rohan Challa
8db92c5f61 internal/lsp/hooks: enable staticcheck hooks
Forgot to enable the staticcheck hooks when I created a struct for analyzers inside of the lsp (https://go-review.googlesource.com/c/tools/+/223662).

Change-Id: Ice7798089100107113741caed3ddc41d172830b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225837
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-27 16:43:12 +00:00
Rohan Challa
5d86d385bf internal/lsp/analysis: add simplify-slice pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

A slice expression of the form:
	s[a:len(s)]
will be simplified to:
	s[a:]

Updates golang/go#37221

Change-Id: Ibd4dedaadc9b129d5d39524f0c1ccc8a95bf7e0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223659
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-03-26 21:04:57 +00:00
Rohan Challa
e428a8eca3 internal/lsp/analysis: add simplify-composite-lit pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

An array, slice, or map composite literal of the form:
	[]T{T{}, T{}}
will be simplified to:
	[]T{{}, {}}

Updates golang/go#37221

Change-Id: I2dca46501983c8af3581c9319d973da5cf690388
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223660
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-03-26 20:50:12 +00:00
Rohan Challa
88b9c284fd internal/lsp/analysis: add pass for unused parameters
This change adds a pass that checks for unused parameters inside of a function. It is disabled by default.

Updates golang/go#36602

Change-Id: I9e8de3368f16f27e7816ec4ddb16935e1a05584e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222817
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:36:28 +00:00
Rohan Challa
94fe02cb5c internal/lsp/analysis: add simplify-range pass from "gofmt -s"
This is one of the simplifications that "gofmt -s" applies.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command

A range of the form:
	for x, _ = range v {...}
will be simplified to:
	for x = range v {...}

A range of the form:
	for _ = range v {...}
will be simplified to:
	for range v {...}

Updates golang/go#37221

Change-Id: Ic6babbd0b8ab961ebb4d0d6df72df52d9acde6e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223661
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:18:45 +00:00
Rohan Challa
e46a7b92c0 internal/lsp: add support for sourceFixAll analyzers
This change adds support for analyzers that have suggested fixes of the kind Source.FixAll. This will allow these fixes to be applied on save if the user desires.

To auto apply these fixes on save, make sure your settings.json looks like:

"[go]": {
	"editor.codeActionsOnSave": {
		...
		"source.fixAll": true,
		...
	},
	...
}

Update golang/go#37221

Change-Id: I534e4f6c8c51ec2848cf2899aab68f587ba68423
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223658
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:01:16 +00:00
Daisuke Suzuki
d58d27dcbb tools/gopls: add cmd support for workspace_symbol
This change adds command line support for workspace/symbol.
Symbols are formatted as '{span} {name} {type}'.

$ gopls workspace_symbol -matcher fuzzy 'wsymbols'
$
$ workspacesymbol/a/a.go:5:7-31 WorkspaceSymbolConstantA Constant
$ workspacesymbol/b/b.go:5:6-28 WorkspaceSymbolStructB Struct

Optional arguments are:
-matcher, which specifies the type of matcher: fuzzy, caseSensitive, or caseInsensitive.
The default is caseInsensitive.

Updates golang/go#32875

Change-Id: Ieef443b13710f9c973210e58f66ab7679f258b30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224677
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 19:59:04 +00:00
Rohan Challa
b1df990128 go/analysis/analysistest: expand testing to handle suggested fixes
This change expands go/analysis to add the ability to verify the suggested fixes returned by an analyzer.

Change-Id: Ic38e1a24342a5c24356f8b83d196da012d8e8e01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224959
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-26 19:47:25 +00:00
Rohan Challa
02a6ca6dc3 internal/lsp: change disabledAnalyses setting to be general
This change removes the disabledAnalyses setting and replaces it with a more general feature named "analyses". This will allow users to opt in as well as opt out of analyzers that they do not find useful. This also updates some documentation to show users what analyzers gopls is using and which are enabled by default.

Change-Id: Id3239b4c4c9667e834f262c889270d14fdba0f93
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223662
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 19:30:34 +00:00
Rebecca Stambler
52ff224b76 internal/lsp: avoid possible nil pointer in references/rename
Noticed this in https://github.com/fatih/vim-go/issues/2786. I don't
think that this will fix the problem in this issue, but we should avoid
nil pointers as much as possible. Also, remove a bit of extra whitespace
so that the style closer matches that of the rest of the project.

Change-Id: I94b924ea14e4a296382e3e68c52eeb43f6cd86a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225523
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-26 18:28:26 +00:00
Ian Cottrell
c9942794f0 internal/telemetry: render trace tags using typed keys
Type switch on the key and use it to get the value and decide how
to render it.
Use the same render for all debug tag printing.

Change-Id: Ia305fded7dcf05b57c5805f48bb5c22fa7def71f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:46:26 +00:00
Ian Cottrell
1386b938c6 internal/telemetry: convert attributes using the key type
This uses the strongly typed key to get the value rather than
type switching on the value itself.

Change-Id: I8f72d1d9cac0191b0565c14d8a1108459ee6df36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225379
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:46:10 +00:00
Ian Cottrell
1249273038 internal/telemetry: make metrics take a strongly typed key
Now that keys are solidly typed, we can use them for the metrics.
This prevents accidentally using the wrong type of key, and
allows us to use the typed accesorrs rather than the raw value.

Change-Id: I553bd8e12128d3f00a3e926dbd3bfd420cd3f135
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225378
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:58 +00:00
Ian Cottrell
063d392fe0 internal/telemetry: normalize the event reciever names to all use ev
Change-Id: Ie622d8d5c4f1c7c272400e549981cd182fe0ab67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225377
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:47 +00:00
Ian Cottrell
8f75b710dd internal/telemetry: improve the telemetry benchmark
This changes the benchmarks to be less heavy weight.
It allows the baseline to have no allocations and be
very lightweight. This makes it easier to see
realistic numbers for the costs of the various
operations, making it easier to track and optimize.

Change-Id: I07699881d6976c3ab3432373f057b563752509b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225337
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:41 +00:00
Heschi Kreinick
e2e2519b1d lsp/source: create zero value ast.Exprs
In support of https://golang.org/cl/224960, add utility functions that
create zero value expressions for a given types.Type. There are probably
some bugs here but they pass the basic tests in that CL.

Known problems: it skips through type aliases, and can't handle
anonymous structs/interfaces. Hopefully it's pretty unusual to return
those.

Change-Id: I69a388d9ce9bd60bc230e4a3d2b30f581ec25698
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225177
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 17:41:01 +00:00
Rebecca Stambler
f53864d0db internal/lsp: remove command-line-arguments as a workspace package
If a package starts out as command-line-arguments, and then becomes
"valid" (i.e., gets a package declaration), we shouldn't continue to try
to diagnose "command-line-arguments". We should remove
"command-line-arguments" from workspace packages any time its metadata
is invalidated (assuming it may get added back if a file= query produces
it again).

Include the relevant regression test.

Fixes golang/go#37978

Change-Id: I7fc51edeb58007b4b4a163336cbeb752a53da322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-25 20:31:30 +00:00
Rob Findley
4c1ea0558e gopls/integration/govim: improvements/fixes for run_local.sh
With the update to go1.14, run_local.sh runs into the ubuntu mlock
issue. Fix this by setting memlock=unlimited.

Also make it possible to run docker with sudo, and run -short tests.

Change-Id: I8c9e44cd16f0a2a0d77bf28133ad4f111058b5cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225520
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-25 20:25:52 +00:00