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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
This is the Go blog house style for the rest of the text.
We just missed the author list, presumably because it
was less code (only two entries still has no comma).
Change-Id: I8bb1ab582d6e300d521ac471736ee8ab6e0f61ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225517
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There is no reason for it to be public now, it is no longer possible to build an
event
without using the helpers.
This also allows us to delay setting the time until after the nil exporter
check, for
a significant time saving in the no exporter case.
name old time/op new time/op delta
/Baseline-8 588ns ± 4% 575ns ± 4% -2.24% (p=0.000 n=18+18)
/StdLog-8 9.61µs ± 3% 9.58µs ± 3% ~ (p=0.384 n=18+18)
/LogNoExporter-8 3.94µs ± 2% 2.26µs ± 2% -42.50% (p=0.000 n=17+18)
/TraceNoExporter-8 2.77µs ± 4% 1.11µs ± 2% -59.82% (p=0.000 n=18+18)
/StatsNoExporter-8 3.83µs ± 5% 2.15µs ± 3% -43.70% (p=0.000 n=18+17)
/Log-8 23.3µs ± 6% 23.0µs ± 1% ~ (p=0.245 n=18+17)
/Trace-8 26.4µs ± 3% 26.5µs ± 4% ~ (p=0.269 n=18+17)
/Stats-8 5.36µs ± 2% 5.45µs ± 3% +1.68% (p=0.000 n=17+18)
Change-Id: Ibde0e20eaf99d03f786cd1436f05eab7b2a17b20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This separates the concerns of tag collections that have to be iterated
and tag collections that need lookup by key.
Also make it so that events just carry a plain slice of tags.
We pass a TagMap down through the exporters and allow it to be
extended on the way.
We no longer need the event.Query method (or the event type)
We now exclusivley use Key as the identity, and no longer have a
common core implementation but just implement it directly in each
type.
This removes some confusion that was causing the same key through
different paths to end up with a different identity.
Change-Id: I61e47adcb397f4ca83dd90342b021dd8e9571ed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
In an effort to be idiomatic, I made the regtest func signature
func(context.Context, testing.T, *Env), despite the fact that Env
already has a Context and a T.
This just ended up causing more confusion, as it's not clear which
Context or T to use. Remove this and just use the fields on Env.
Change-Id: I54da1fdfe6ce17a67601b2af8640d4d2ea676e8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225157
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since the fix is only available at master, make the test 1.15-specific.
Change-Id: Ie17c8806479d3d53076c2351da8bcfcfbfc8257f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Expressing regtests in terms of textual coordinates is hard to read: the
reader ends up counting lines and characters to understand the text edit
or assertion.
To address, this, add two new functions for fake.Editor: RegexpSearch
and RegexpReplace, as well as a symmetric RegexpSearch function for
workspace files and wrappers for regtext.Env.
This allows expressing edits as well as buffer locations in terms of
easily scannable regexps.
An alternative solution to this problem is to integrate markers ala
packagestest. I tried this, but it ended up being cumbersome to
implement and less usable than regexps, due to the static nature of
markers: after the buffer has been edited all markers must be
updated.
Updates golang/go#36879
Change-Id: Iad087cf0d529737034197beef7b729816a159c69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224757
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
For debugging purposes, we print the output of `go env` on start. Now,
we print a view-specific `go env`, which will helps us when debugging
multi-project workspaces. Additional information included: the folder of
the view, if the view has a valid build configuration, and the build
flags for the view.
Updates golang/go#37978
Change-Id: Iadffd46f8ac5410971558a304b8bbcb2f9bdbcc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This is helpful for users to diagnose issues, but it annoys people who
have workspaces with both non-Go and Go projects. I'm going to spend
some more time thinking about this, particularly about which error
messages we should retry the initial workspace load for.
Updates golang/go#32394
Change-Id: I7d02b385da232914fe7342837dc3a9c4185399fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225237
Reviewed-by: Heschi Kreinick <heschi@google.com>
I'm not sure if this is entirely the correct approach, but at the very
least we should never pass "command-line-arguments" to packages.Load. We
still need to cache packages with the path "command-line-arguments" to
avoid an excessive number of calls to packages.Load.
Updates golang/go#37978
Change-Id: I1b5263a3dab95aacd797f03f742069fd0c53619c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change updates the metrics tutorial to include example code for
exporting traces from go tools.
Change-Id: Ie1d3c373ed4308ef0160f6389b74c642b348bed6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225061
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change updates the metrics tutorial to be compatible with the new event system.
Change-Id: I8b75f6b02d241bc9dede01cfac167a982f1df67c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225058
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
go vet should warn against using %v verb in Error() methods just like it
does for String().
Fixesgolang/go#33884
Change-Id: I14e30aae316dc84adc62e2b2a0144cc157bb2698
GitHub-Last-Rev: 12240ac78d9c8afb12b014eff8f81ea0bf42471f
GitHub-Pull-Request: golang/tools#214
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223239
Reviewed-by: Michael Matloob <matloob@golang.org>
A common annoying mis-completion is as follows:
type foo struct {
field int
}
func (f foo) Field() int { return f.field }
func (f foo) logic() {
if f.f<>
}
Now at <> we prefer "field" over "Field()". Similarly:
type foo struct {
}
func (f foo) DoSomething() { }
func (f foo) doSomething() { }
func (f foo) logic() {
f.d<>
}
Now at <> we prefer "doSomething()" over "DoSomething()". All else
being equally, you normally want private objects over public objects
when the private objects are available.
The same logic is applied to deep completions so we prefer "c.foo.bar"
over "c.Foo().bar".
Change-Id: Ic91cba7721ddb1f2a30338037693ddcce8c621f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223877
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We can re-enable this when we have a version of Go that includes the fix
for golang/go#37971. Then we can continue playing whack-a-mole with go
list corner cases :)
Fixesgolang/go#37958
Change-Id: I2417731bf5c01fbde867fef2ef03b5025ae779c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224538
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Consider this input:
This is preformatted:
line 1
line 2
line 3 after blank line
Before, this would split into two different <pre> sections
in Markdown mode. Now it matches legacy mode and doesn't.
Fixesgolang/go#37972.
Change-Id: I6bf156c76ef9c63b6c3ffe6cce431d9589def867
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224958
Reviewed-by: Andrew Bonventre <andybons@golang.org>
See testdata/README for test format.
This will make it easier to fix problems and not regress.
Markdown tests in followup CL (with bug fixes).
Change-Id: I5dedca26d3c29fd428066ffb38c6605343784a19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224957
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This allows us to hide the implementation details of how tags are stored on a
context from the normal interface, to allow us to explore more efficient
mechanisms.
The current storage is not intended as the most efficient choice, this cl is
about isolating the API so we can experiment with benchmarks in the future.
Change-Id: Ib101416bccd8ecdee269cee636b1564d51e1da8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222854
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The next version of VS Code Go has to be released before this can be
released.
Fixesgolang/go#37966
Change-Id: I6b97f2a3ff9c8400ae85b815d8bf83a9b12b1069
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This change updates the metric exporting tutorial to use oragent to spin
up OpenCensus, Prometheus, and Zipkin all at once using docker-compose
rather than manually setting each one up. This allows developers to set
up an environment for testing metrics and traces with minimal effort.
While oragent also spins up Zipkin for traces, the tutorial does not yet
have a section outlining how to export traces from Go tools. A section
for traces will added in a later CL.
Change-Id: I07f49977f7ab49995853ff8ee8eb6ccdf6ef1642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224258
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
definition.Run requires a nil check of opts before applying to avoid
panic, and test must be run with markdown enabled.
When running 'go test' without -run flag, connection.initialize() was
not called and there was no problem because the default value of
Options.PreferredContentFormat was Markdown.
In addition, currently using the same connection despite different
options, therefore make to use a different connection for different
options.
Change-Id: I6ef773c14cbdcae621da9295d4c618c3aff0beee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While experimenting with different static analysis on x/tools, I noticed
that there are many actionable diagnostics found by staticcheck. Fix the
ones that were not false positives.
Change-Id: I0b68cf1f636b57b557db879fad84fff9b7237a89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222248
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This way, renaming an article does not make it reappear
in an RSS reader as a new article.
Change-Id: I4e92707f9ea4983bf8506cde826d5e5f48a3b36f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223751
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This changes the way keys work, there is still a single internal key
implementation for performance reasons, but the public interface is a set of key
implementations that have type safe Of and Get methods.
This also hides the implemenation of Tag so that we can modify the storage form
and find a more efficient storage if needed.
Change-Id: I6a39cc75c2824c6a92e52d59f16e82e876f16e9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223137
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.
Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Move the lsp specific telemetry package that just declares the labels under the
debug package and call it tag, to make all the usages much more readable.
Change-Id: Ic89b3408dd3b8b3d914cc69d81f41b8919aaf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222850
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Rather than building cusom events and driving the exporter, the test now
registers a custom exporter and then uses the normal higher level methods to
deliver events to it.
This means we are testing the actual information that would arise in a user
program, and also means we no longer have to expose internal features just for
tests.
Metrics are not fully converted yet.
Change-Id: I63248a480fb1b3e6274dce0c2e1d66904d055978
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222849
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This allows early exporters to adjust the event for later ones.
This is used to lookup key values from the context if needed.
Also add a Query type event which is intended to perform all event
modifications but nothing else, and is used to lookup values from
the context. This cleans up a weirdness where the current lookup
presumes there will be an exporter with a matching mechanism.
Change-Id: I835d1e0b2511553c30f94b7becfe7b7b5462c111
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When searching for deep completions, we can end up enumerating struct
types' fields a lot. Optimize fieldSelections to reduce work:
- Wait until we see an embedded field before we create the "seen"
map.
- Use a callback style to iterate over the struct's fields rather than
returning a slice of fields.
- Change "seen" checking strategy back to track struct types rather
than each individual field.
Struct with 5 non-embedded fields:
name old time/op new time/op delta
Fields-16 293ns ± 1% 20ns ± 2% -93.13% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Fields-16 120B ± 0% 0B -100.00% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Fields-16 4.00 ± 0% 0.00 -100.00% (p=0.008 n=5+5)
Same struct but add an embedded struct with 2 fields:
name old time/op new time/op delta
Fields-16 389ns ± 1% 142ns ± 1% -63.53% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Fields-16 120B ± 0% 144B ± 0% +20.00% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Fields-16 4.00 ± 0% 2.00 ± 0% -50.00% (p=0.008 n=5+5)
I think the alloc/op went up because the "seen" map is no longer
allocated on the stack. There is more room for more optimization, but
it's probably not worth making things more complicated.
Change-Id: I6f9f2124334a8594ef9d6f9b5ac4b3a8aead5f49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223419
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Without the bug fix, you get ## followed by #### in the file.
Change-Id: I6af85debf4bc84f9e346732b5641eb3a4e7d7e83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223602
Reviewed-by: Andrew Bonventre <andybons@golang.org>
