We had previously worked around a VS Code URI bug by unescaping URIs.
This is incorrect, so stop doing it and then add a specific workaround
just for that one bug.
Fixesgolang/go#36999
Change-Id: I92f1a5f71749af7a6b1020eee1272586515f7084
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217599
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We seem to be leaking cache entries. A simple status page will help us
confirm that.
Change-Id: I485bfff6ebfb5d30655554487583e15a3f49f9a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217597
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
source.Identifier previously was used for references and rename, so it
needed to take a package policy. Now, it's only used for definition and
hover, so it should always be the narrowest package handle. We can use
this fact to determine if the identifier is located in its declaring
package, and if that package is a test variant, we don't link to the
documentation on pkg.go.dev, since it doesn't exist.
Change-Id: I5686828858a3feafb8ff2e4c5964b562f66db9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change aggregates all the diagnostics before publishing them to the client. This also updates the lsp_test to use lsp.Diagnose vs source.FileDiagnostics, unless lsp.Diagnose doesn't return any results.
Change-Id: I39959b50af3f65fce5d8e15fb138ccc019ce8073
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217338
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change will skip packages.load calls when a go.mod file changes if that go.mod file is not the go.mod file associated with the view.
Change-Id: I23a214a89203dd58417f3e2f69725ce3b669a5ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217238
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds diagnostics to imports in .go files that are missing the dependency in the go.mod file. The quick fix that adds the appropriate require statement is coming in a follow up CL.
Updates golang/go#31999
Change-Id: I314cdbe8e3dd27da744ca0391e7a6e5dc1ebaa77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216277
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When we stopped returning an error from awaitInitialized, we didn't
handle this case in LookupBuiltin.
Fixesgolang/go#36975.
Change-Id: I9668a7148f60ca1f9cad953d46caf6ec5500541e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217399
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Since parsing modfiles does not give us positions relative to the fileset, we need to account for this by adding the file's offset within the fileset to the positions of the markers within the file.
Change-Id: I826cec05cd77d37900d32bce2fb6e5edcbfea936
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217341
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
err must be nil, as the code checks in line 117.
Change-Id: I23d87bdc9489c9ed249bfa1ac61b1ed722121c6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217402
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In preparation for some meaningful changes, rework a few things:
- rename "fix" to "fixAST"
- separate "parseExpr" into "parseStmt" and "parseExpr"
- pull out "walkASTWithParent" function
Change-Id: If6c8a249441feda95704f37bc9bde3ef2b64cbd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216481
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix type inference to expect a type name for the first "make()"
parameter and an integer for later parameters. For example:
// Previously we expected "[]int{}", now we expect "[]int".
var _ []int = make(<>)
Note that we don't currently support actually completing to unnamed
type names like "[]int", but this improvement at least eliminates
nonsensical completion suggestions.
// Previously we had no expectation, now we expect an int.
var _ []int = make([]int, <>)
Change-Id: Ifd349767662ab6902d3a3ea9e52de7df70cb37c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217310
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Env is used in the debug template, but not available.
Change-Id: I8e270464aecd927bc553bfbf9e7c85a477833890
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217085
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change adds a check inside of the types.ImporterFunc to see if the context has been cancelled.
Updates golang/go#34683
Change-Id: I0f12da0f8158ecda0eec00150ed6ff772c2f89c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217257
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds some basic marker support for go.mod files inside of go/expect. It requires all markers to be of the form "//@mark()", where mark can be anything. It is the same format as .go files, only difference is that it needs to have "//" since that is the only comment marker that go.mod files recognize.
Updates golang/go#36091
Change-Id: Ib9e325e01020181b8cee1c1be6bb257726ce913d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216838
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
DefaultOptions was a value type, but held map values. This CL changes it
to a function that returns an Options value that has new instances of
all reference types. It would be better if this function returned a
pointer, but that change ended up being too large. I will need to
refactor handling of options later anyway, in order to support sessions
with differing options for golang.org/issues/34111.
This fixes a race in internal/lsp/tests: internal/hooks/analysis.go
mutates the Analyzers map.
See for example the trybots result at:
https://storage.googleapis.com/go-build-log/0d34f5f0/linux-amd64-race_4ecdf9c8.log
Change-Id: I41be450b590a3f3104ac9a1cb9cb312ea3ff7ff4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217077
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We should never be converting to a URI directly, in case it's encoded or
needs Windows-specific fixes.
Change-Id: I8af272f878e2e4faf428bd7b74b7cba4f65093a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217086
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Even though users shouldn't depend on the order of the package list,
go/packages.Load should still return packages in a deterministic order.
golang.org/cl/216721 broke the determinism by getting package
order using a map. Fix that by sorting the packages.
Updates golang/go#36908
Change-Id: Icf8476009abfc27009d494eb57712caeb0775343
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217087
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Drive letters are always case-insensitive, so we should standardize them
by always keeping them uppercase.
Updates golang/go#36904
Change-Id: I8de25b175790b01627f947600c1511edf38c316c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217080
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
It had a 50/50 chance of checking the test package for the import
instead of the non-test package. Find the right package before
checking it now.
Fixesgolang/go#36908
Change-Id: I3a044d83f55093961cf5221b315a54028fdd540d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217084
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
go list -e exits with a non-zero exit status if pkg-config
fails. Add a work-around to that to our long list of go list -e
work-arounds.
Also add a moderately complicated test case that installs a fake
pkg-config.
Fixesgolang/go#36770
Change-Id: I1187357e567eefde528fe30b27abde4ae346170f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217078
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is a hack that checks for the extra "error" package that go list
generates when there's are some invalid imports, and attaches the error
to the importing package. This should eventually be taken out once
go list can return multiple errors on a package, and reports this error
on the importing package.
Fixesgolang/go#36188
Change-Id: If355552d7d9bd2ed74a27ddb55133f8d60973d36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216721
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
It was added in Go 1.11, so technically we don't need to support Go
1.10, but we probably still should.
Change-Id: I8d5f0d7e5ae822f9536d9c332319186c703cdb34
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216939
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We can assume that any package we load directly will be a package in the
workspace, so it's reasonable to set workspace packages at that point.
We're guaranteed not to directly load dependencies, or we may end up
with indirect dependencies in the go.mod file.
Change-Id: I5d406e54da2bc3278b139c75b436d111b5564418
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216726
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The test that checks to make sure a user's go.mod file does not change if tempModfile=true is not doing anything. At some point, the go.mod file to test added a go directive. This change removes the go directive from the go.mod file.
Change-Id: I1c4b6b326e99d8fec5c21ac779dcdcd73610f48c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216840
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Some housekeeping. The shouldLoad function doesn't actually make sense
in load.go.
Change-Id: Idcfade57ab853cdae809c688fa21026a0a432c60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216727
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Any context that came in through the lsp protocol package will work, live or dead.
Change-Id: I7566ec07b09e1c8e54a5255ebda8553843cfe974
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216846
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We check cancellation pretty early on in analysis, but not before we
read Options. Checking earlier avoids a race in the command tests.
Fixesgolang/go#36699.
Change-Id: I557c0441d70c80e122655be00a03b47080ddb7f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216844
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now that overlays are sorted before being processed, we can assume a
consistent ordering of non-test files before test files. Some of the
code in the overlay processing therefore becomes unneeded.
Change-Id: Ic7a20ceb7013af7ce876a280c975eadf717157d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216722
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The nilness analysis gives us diagnostics with invalid start Pos. We can
just ignore those and log them.
Add a missing error check while I'm in here.
Change-Id: I4a205f253a9e47ec1513ff6299479f52e414a48c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216724
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The core of processGolistOverlay was a map iteration, which is
nondeterministic. When creating both a non-test and test package, we
would sometimes encounter the test file before the non-test file. In
that case, for reasons I didn't bother tracking down, we would never
create the non-test package.
Rather than doing complicated bookkeeping to fix the problem, simply
process non-test before test, and make the loop deterministic to save
all of our sanity.
Updates golang/go#36661.
Change-Id: I1f76869fa52794ac8ae96e22ad06a2b1e1861995
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216717
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If an orphaned file is used to recover a workspace package, we should
remove the initialization error and treat the view as correctly
initialized.
Also, stop caching metadata for packages with no files. We have no way
to invalidate it, and it's useless, so just re-load those files as
needed.
Fixesgolang/go#36795.
Fixesgolang/go#36671.
Fixesgolang/go#36772.
Change-Id: I0aee5a43401517b6073d27136cca533160effef2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is just to ensure that arguments are always ordered when passed to
packages.Load.
Change-Id: I2c74673eff31efc31854dc0d306809890f633789
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go command gets mad if go.mod has changed since it started, e.g.
because a new dependency was added by a concurrent go list call. Retry
loads if they hit a concurrency problem. See the comment for more
details.
Testing this is awkward. I ran a background script that constantly
modified the go.mod file and observed that gopls waited until it was
killed and then recovered.
Updates golang/go#36772.
Change-Id: I5636c99a5a94b415c4a6fbb71869b07e31d3fed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216543
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change sets up the infrastructure to surface the dependencies that are missing in a go.mod file. In a follow up CL, we will use these to add diagnostics to the imports of .go files telling the user to add the dependency to their go.mod file.
Updates golang/go#31999
Change-Id: I697df0d7c6eabfdc2f0c75cb36aa35794647fd13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214700
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were reloading all known files, which resulted in modifications to
user's go.mod files.
Change-Id: I14e86af896d1e75f3fdaaa00b9af8d7fb1d1e9e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216542
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mostly because I find it very annoying.
But I imagine lots of people shadow variables intentionally, and it's
very noisy.
Also, fix an error caught by the nilness check while I'm thinkng about
analyses.
Change-Id: I1867c8613194028815666efd879899bb5065c9d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216541
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The command line tool doesn't go through JSON RPC, so it retains type
information that's stripped in the tests. Cut the parameters down to
basic types manually for now. But I don't know why the command line
tests send RPCs if the real thing doesn't.
Fixesgolang/go#36769.
Change-Id: I428b40478557ca35a7dae8d02bbcd990411f714f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216539
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go/packages workaround to hide errors for overlay packages doesn't
seem to work well. It's easier to just hide list errors in gopls
diagnostics unless the package genuinely failed to type-check. Check if
the package has missing dependencies as an approximation of if it is
well-typed.
This required some additional special casing for the import cycle error
detection, which now causes them to have duplicate diagnostics. It's a
rare enough case that this doesn't concern me, but we should clean this
up at some point.
Fixesgolang/go#36754.
Change-Id: If12c92fb9a0e0b69b711ae9a509ecb1b2a32255c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216310
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that we are reloading metadata for workspace packages by package
path, ad-hoc packages get their metadata reloaded incorrectly. If an
ad-hoc package has no metadata, reload it by reloading the entire
directory.
Fixesgolang/go#36753
Change-Id: Ie440f6f76a220009d487b7ceadcf40594643e969
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216307
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I had originally thought I might be able to use exprAtPos for this,
which is why I ended up eliminating that function when I saw it only had
one use.
One test also had to change in order to fit better with the spec.
Specifically: "If [the active parameter is] omitted or the value
lies outside the range of `signatures[activeSignature].parameters`
it defaults to 0 if the active signature has parameters."
Fixesgolang/go#36766.
Change-Id: I400d5b2db2985bfaa5efbcd91225151ca8b5f46a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216309
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The LSP already supports a bunch of analyses we have less confidence in
than the vet suite so we should add these too.
Updates golang/go#36639
Change-Id: Ifc37d09e3acd73de021be7b45b3d80fe8c00e0d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215677
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
