With this change (finally, after a lot of detours) if you run the lsp tests with `-race -pipe` then you
can reliably reproduce the race in golang/go#30091
Change-Id: Ibd9fda5e07409a15d1bc8d14cb46fde41155aa6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169999
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This connects up the configuration message, and uses it to allow the client to set the environment
in the config passed to packages.Load
Change-Id: I75e03c01c74e9b11c8b4c47b9cbdd0574cddf778
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169704
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Instead of using a simple path map we now attempt to match files with
os.SameFile with fast paths for exact path matches. This should fix issues both
with symlinked directories (the mac tmp folder) and with case sensitivity
(windows)
Change-Id: I014dd01f89d08a348e7de7697cbc3a2512a6e5b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169699
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The options are all unexported, and this CL is (almost) a no-op:
the one difference is that since needImports and needSyntax are now
independently specified, LoadSyntax and LoadAllSyntax are equivalent,
because LoadSyntax needs both the needImports and needSyntax bits.
I want to pin down the options that we want to split into, and
future CLs can allow the options to be used individually...
Updates golang/go#29429
Updates golang/go#29427
Change-Id: I5b2913e2c53e7ade56905e46912b076ccc339827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/162140
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This is the first step of fixing gopls on Windows.
Change-Id: I20cea63051f448cba86996edaf7a8347b0d20bd1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169444
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
go/token.File.Offset panics on invalid positions, which is causing
crashes on invalid ASTs.
Change-Id: I6c698760ae92f3076a056c3560bea2e8eeb528db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169443
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change uses go/types information to get the types for the
different symbols. It also groups the symbols according to their kinds,
though this doesn't seem to be reflected in the actual VSCode UI...
Updates golang/go#30915
Change-Id: I2caefe01f9834aaad6b9e81cd391d461405ef725
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169438
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add a vet pass that checks that the second argument to errors.As
is a pointer to a type implementing error.
Change-Id: I0924e634cbea0664c8728f0e74213b924f8498e6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168938
Reviewed-by: Damien Neil <dneil@google.com>
This upgrades the current gopls query definition tests to use the main testdata
folder. This considerably increases the coverage and also sets us up to better
test the other command line features as we add them.
Change-Id: If722f3f6d0270104000f1451d20851daf0757874
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169159
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Stop fighting the behavior of the go tool when run in these directories.
Updates golang/go#30790
Change-Id: I32dfeb0bafa3ed3664500f1768b2293e5257d09b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168757
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
exec.Command already runs exec.LookPath when given a name that contains
no path separators. There's no need to call exec.LookPath a second time
to detect that cmd.Run failed because of a missing executable file.
It can be detected from the returned error. Do so because it's cleaner.
Also improve the error text to say that the problem was that the go
executable file was not found in $PATH (or %PATH%, etc., depending on
the underlying operating system). In the general case, we can't know if
Go is or isn't installed.
Example error text on macOS:
gopackages: 'go list' driver requires 'go', but executable file not found in $PATH
Updates golang/go#29552
Change-Id: I769553f125240dccd02098c22641f6a1ed10063c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168897
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
The go/ssa/interp tests have been a maintenance nightmare for years
because the interpreter requires intrinsics for all low-level or
non-Go code functions, and the set of such functions in the standard
library naturally changes from day to day.
This CL finally drops support for interpreting real packages (which
has anyway been broken for ages) and restricts the test suite to small
programs that use a handful of simple functions in packages bytes,
strings, errors, runtime, reflect, and unicode. These functions are
declared in a tiny fake standard libary in testdata/src, and the
implementations of these functions are provided by interpreter
intrinsics that delegate to the real Go implementation---all their
parameters and results are basic datatypes.
The test suite is now very fast and should be easy to maintain going
forward. It is still possible that a change to some file in
$GOROOT/test/*.go adds a dependency to a symbol not present in our
standard library, but this is rare. I will either delete the test or
add the intrinsic on a case-by-case basis.
We no longer attempt to interpret major functionality like
fmt.Sprintf or "testing".
The interpreter always pretends to be in linux/amd64 mode.
Happy Christmas, Brad. ;)
Fixesgolang/go#27292
Change-Id: I715cf63e3534e2e0dab4666a5d7c669bf1d92674
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168898
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Only warn if the method is missing for all types.
Fixesgolang/go#30971
Change-Id: I94169ad3266f68ca20378a8dc5538aed2541a773
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168803
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
`codeFact` returns `nil, err` on errors, which results in error messages like:
panic: internal error: encoding of nil fact failed in [analyzer]
Though that and the stacktrace are often enough to identify the cause, the nil
hides the actual source of the problem.
Change-Id: Iddcdee386a5c64c6567d2727ebe7a77fe21927e9
GitHub-Last-Rev: 92163c2a5a631817319c992f7445f86d95130514
GitHub-Pull-Request: golang/tools#78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167997
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
If we can't execute the go command, check to see if it exists. If it doesn't,
say so in the error.
Fixesgolang/go#29552
Change-Id: Iffd2db5fc8f1daa2e458eaa326b27d9a0d971b6b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168777
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Make it slightly more clear that this is not a user error, but an
internal error.
Updates golang/go#30519
Change-Id: I7adb3b5bb1548eab8e46db48946d55f9d59a4311
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168657
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Alternative build systems like blaze don't always provide a way to determine
the relationship between a package and the package it's testing. This means
that sometimes the check for misspelled Example function names over-reports
because it doesn't find the object being exemplified. Don't report errors
unless a object can't be found in any of the imports. This means that there
won't be any false positives though of course this comes at the cost of
false positives.
Change-Id: I7435eeb2333b6dd72e06bb6383fff2ac17bee845
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168404
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
go list functionality changed in 1.11 and compatibility logic was added
to support 1.10 and before. Now that Go 1.12 has been released, support
for those version has ended and we can remove the legacy code.
Change-Id: Ifdd5c566dbbfe4fade5be27ad9ae20052d604c15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/166537
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
third_party only had JavaScript for cmd/heapview, which was deleted in 3c4dfba64b.
Change-Id: I7378e04e8eea74c32fcad4f7759b4e3b399faabc
GitHub-Last-Rev: 0f6ad4a47d85fe2ad00adb4b851ac01ba1881463
GitHub-Pull-Request: golang/tools#79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168577
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The test fails if the GOROOT directory contains a dash, e.g. as in
/usr/lib/google-golang on corporate workstations, or on certain
builders.
Change-Id: If08c438764f4b76fff7fb43ccf3e4fd56afeff1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168063
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL fixes packages ignoring errors regarding files that have
non .go extensions. Packages can be called with just file names
or path which includes files. These aren't checked at all by
packages if they are go files or not, but it fails silently because
of it.
In more detail, go list fails with named files error in STDERR.
However, that is ignored, because go list notoriously abused STDERR
for non-error messages.
Fixesgolang/go#29899
Change-Id: Ie4dc39da0b87200ebd23e6c607396557685e2807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/164663
Reviewed-by: Michael Matloob <matloob@golang.org>
Historically, vet had always been unhappy about encoding/xml itself:
method MarshalXML(e *xml.Encoder, start xml.StartElement) error should
have signature MarshalXML(*xml.Encoder, xml.StartElement) error
This dates back to the time when vet couldn't depend on type
information. It compared the type expressions directly as strings, which
was a problem when the code was in encoding/xml itself. There, the
function parameters are *Encoder and StartElement, not *xml.Encoder and
xml.StartElement.
However, vet has been depending on type information for a while, so this
restriction no longer makes sense. The analyzer almost got it right, but
the only stopgap was a piece of the old code that tried to compare type
expression strings.
Remove it; typeString already deals with these edge cases for us. To
ensure vet remains happy with encoding/xml, add a very simple test for
it. The package now has zero reports, so the fact that its source has
zero "// want" comments is appropriate.
Finally, remove some long unused parameters from matchParamType.
Change-Id: Iab3ed57da7bc4a80522ae21e62b67e7828b97c89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168058
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
When constant values change but stringer has not
been run again, we can get misleading string values.
Protect against this by generating code that will fail
with a compiler error when this happens.
Most compilers should be smart enough to omit the
code containing the checks.
Change-Id: I7a36d20f014cba0e7d88851d1b649a098ee30d76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/163637
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
If GO111MODULE=on but there's no go.mod, GOMOD will be set to /dev/null
or NUL and there will be no main module. goimports should handle this
case roughly the same way the go command does.
Fixesgolang/go#30855
Change-Id: I6fbf4c056000db5abd8788a6014ae5f13b1c8cd4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167860
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Some of the tests on the name query operate on test modules or module cache
trees that are checked in as testdata. When the tests run, they can modify
the go.mod files and the cache tree directories. Copy the directories
to a temporary directory to avoid getting spurious git diffs showing up.
Change-Id: I991a4510201988d596833faea88425a335d3228b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167859
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Change span to hide its fields and have validating accessors
This catches the cases where either the offset or the position is being used
when it was not set.
It also normalizes the forms as the API now controls them, and allows us to
simplify some of the logic.
The converters are now allowed to return an error, which lets us cleanly
propagate bad cases.
The lsp was then converted to the new format, and also had some error checking
of its own added on the top.
All this allowed me to find and fix a few issues, most notably a case where the
wrong column mapper was being used during the conversion of definition results.
Change-Id: Iebdf8901e8269b28aaef60caf76574baa25c46d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167858
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL ensures that a "." inside a string literal will return an empty
completion list.
Fixesgolang/go#30477
Change-Id: I1442d0acab4c12a829047805f745c4729d69c208
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167857
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This change brings back handling for circular imports, which was removed
because I originally thought that go/packages would handle that.
However, since we are type-checking from source, we still end up having
to deal with that.
Additionally, we propagate the errors of type-checking to the
diagnostics so that the user can actually see some of the problems.
Change-Id: I0139bcaae461f1bcaf95706532bc5026f2430101
Reviewed-on: https://go-review.googlesource.com/c/tools/+/166882
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The language client must support the preselect feature, so as of right
now, I don't think that this change has any effect. However, ultimately,
we should preselect the first completion item we suggest, as we rank
items.
Change-Id: I977cce26157504595a0193ab551685e21a3df155
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167466
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
An ignored error in toProtocolDiagnostics could result in empty
diagnostics being (incorrectly) sent to the user.
Change-Id: I34c86a1f5bbf28888bedad094f596cc27a52b86d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167714
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
