This changes the analysis code from that which was in unitchecker.go
to that in checker.go, so we can run actions that get facts for dependencies
concurrently.
Adds the rest of the traditional vet suite to the LSP.
TODO(matloob): test that facts are actually propagated between packages
Change-Id: I946082159777943af81bcf10e503fecc99da521e
Reviewed-on: https://go-review.googlesource.com/c/161671
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
text/scanner now uses the word "invalid" instead of "illegal".
See golang.org/cl/161199
Change-Id: I7db0ea2628760cba5c993bdec6e15e76b20b0e06
Reviewed-on: https://go-review.googlesource.com/c/162137
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change moves gopls from type-checking packages using the
go/packages API to type-checking from source. This is the first step in
adding caching to gopls.
Change-Id: I2a7dcfd8c9c0bfc6c35c86eadcdc6f9ce53d9be7
Reviewed-on: https://go-review.googlesource.com/c/161497
Reviewed-by: Ian Cottrell <iancottrell@google.com>
golang.org/cl/161317 set Tests to true in the packagestest go/packages
Config. Handle this in the go/packages tests.
Fixesgolang/go#30115
Change-Id: I6b4c0ca3dc2d73855239f3e7122e29da6ce81657
Reviewed-on: https://go-review.googlesource.com/c/161457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This starts hooking up the analysis framework into the LSP. It runs
the Tests analysis (which I think might be the only one that doesn't
need facts or results) and reports its diagnostics if there are
no parse or typecheck failures.
Next step: figure out how to pass through results.
Change-Id: I21702d1cf5d54da399df54437f556b9351caa864
Reviewed-on: https://go-review.googlesource.com/c/161358
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Name of file or URI must be checked ignorecase on Windows.
Change-Id: I61507b0aa95389c5e1f421f0702ba6ad17b79177
Reviewed-on: https://go-review.googlesource.com/c/160597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change allows testdata files to be *_test.go files.
Change-Id: Ic771ea7c89ff2d2aabd1af8be56f9c7286da9053
Reviewed-on: https://go-review.googlesource.com/c/161317
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The atomicalign checker detects non-64-bit aligned struct field
arguments to sync/atomic functions but currently misses out cases
where the struct variable identifier is a pointer to struct. This
is very common as it happens when the 64-bit field is accessed
in a method with pointer receiver, where the struct is itself the
method receiver. Add some tests to cover that new case.
While I'm at it, fix some typos.
Change-Id: I582cf5b7286b11285010f085045f58dc636ef3ee
Reviewed-on: https://go-review.googlesource.com/c/158999
Reviewed-by: Alan Donovan <adonovan@google.com>
Add an iexport.go (and corresponding iexport_test.go) file, which is an
adapted version of $GOROOT/src/cmd/compile/internal/gc/iexport.go. This
code writes exportdata for a *go/types.Package.
A majority of this code is directly copied from iexport.go, with a
change of types, while some of it had to be modified slightly.
Updates golang/go#28260
Change-Id: Ic7e8e99f0c6b886839280b410afffb037da8a79b
Reviewed-on: https://go-review.googlesource.com/c/156901
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
It was originally added to support goimports, but goimports
is going with another solution. We're going to disable it for
now, but not delete it, so that the goimports code path that
uses it can continue to be tested if and when we want to
use it.
We don't think there are any other users of name= but if
there are, please let us know and we'll work with you to
fix you, or we'll stop or revert this change. Thanks!
Change-Id: I73b7b6c0a5788148af5f3380189055b450f7b45e
Reviewed-on: https://go-review.googlesource.com/c/159702
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change synchronizes the absence of a special case entry for
go.googlesource.com import paths in cmd/go/internal/get to this
package. It seems it was added to x/tools/go/vcs in CL 180540043,
but it was never added to cmd/go/internal/get itself.
Having an go.googlesource.com entry here but not in cmd/go/internal/get
means the import path resolution logic diverges from that of the go
command, which is counter to the goal of this package.
After this change is applied, vcs.RepoRootForImportPathStatic reports
correct results for import paths like "go.googlesource.com/scratch.git"
(resolving it without an error) and "go.googlesource.com/scratch"
(reporting an error).
Updates golang/go#11490
Change-Id: I0b1c959675d9e20205a587a06d734fcd103ebf91
Reviewed-on: https://go-review.googlesource.com/c/159817
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
`stamblerre/gocode`, which is the fork of `mdempsky/gocode` that supports Go 1.11 modules, uses `packages.Load` to load all of the Go code for, among other things, IDE autocomplete support. A common aspect to IDE autocomplete is asking for suggestions very frequently (perhaps at every character typed). Once the user has typed another character, the previous request for autocomplete is invalid and can be canceled.
`packages.Load` already stops its `go list` component if the context is canceled, but if the context is canceled afterward, then it will parse all of the files that it found. This change stops the parsing of Go files once it detects that the context has been canceled. When a file has not been processed due to cancelation, its error will be set to that of the context.
This change dramatically improves the performance of the `stamblerre/gocode` fork when requests have been canceled.
Change-Id: Iba8c1e08eefa59137559ac9108238bfe5ba4ac21
GitHub-Last-Rev: 11a2210c8ce2ed9db9462ddc3e9676476f49f937
GitHub-Pull-Request: golang/tools#72
Reviewed-on: https://go-review.googlesource.com/c/159259
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
go/packages needs to call `go list` multiple times, which causes
redundant work and slows down goimports. If we reimplement `go list` in
memory, we can reuse state, saving time. `go list` also does work we
don't really need, like adding stuff to go.mod, and skipping that saves
more time.
We start with `go list -m`, which does MVS and such. The remaining work
is mostly mapping import paths and directories through the in-scope
modules to make sure we're giving the right answers. Unfortunately this
is quite subtle, and I don't know where all the traps are. I did my
best.
cmd/go already has tests for `go list`, of course, and packagestest is
not well suited to tests of this complexity. So I ripped off the script
tests in cmd/go that seemed relevant and made sure that our logic
returns the right stuff in each case. I'm sure that there are more cases
to cover, but this hit all the stuff I knew about and quite a bit I
didn't.
Since we may want to use the go/packages code path in the future, e.g.
for Bazel, I left that in place. It won't be used unless the magic env
var is set.
Files in internal and imports/testdata/mod were copied verbatim from
cmd/go.
Change-Id: I1248d99c400c1a0c7ef180d4460b9b8a3db0246b
Reviewed-on: https://go-review.googlesource.com/c/158097
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The atomicalign Analyzer checks the alignment of 64-bits variables
accessed atomically via sync/atomic functions on 32-bits
architectures. Per the sync/atomic BUG note those variables must
be 64-bits aligned, otherwise a runtime panic is issued.
The analyzer only shows and runs on 32-bits architectures.
This CL should not introduce any false positives.
Add some tests in testdata/src/a to verify the analyzer behavior
on affected architectures plus some very basic test to verify that
no warning is generated on non-affected ones.
Fixesgolang/go#11891
Change-Id: I02cfc574883564cd2a213a92d33bda3cc9a1ea98
Reviewed-on: https://go-review.googlesource.com/c/158277
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Many Analyzers need to measure the width of an integer and all today
use hacks. This change causes analysis.Pass to retain and expose the
type sizing function used during type checking.
This in turn requires go/packages to retain and expose the type sizing
function in Packages.TypesSizes, which addresses a longstanding need
among many of its clients.
Change-Id: Ia8362019bcde34c10cb4fbc38cfdfddcbef3eb5c
Reviewed-on: https://go-review.googlesource.com/c/158317
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This commit was merged earlier with some failing trybots, so it was
reverted. This is a re-submission.
Before this change, a test would fail:
$ GOCACHE=$HOME/go/cache go test
--- FAIL: TestLoadImportsGraph (0.36s)
--- FAIL: TestLoadImportsGraph/GOPATH (0.19s)
packages_test.go:191: golang.org/fake/subdir/d.test.Srcs = [4302876da86a8aae0c1669924daa223cafca60ef49ccaa060ae37e778d18f218-d], want [0.go]
--- FAIL: TestLoadImportsGraph/Modules (0.17s)
packages_test.go:191: golang.org/fake/subdir/d.test.Srcs = [4302876da86a8aae0c1669924daa223cafca60ef49ccaa060ae37e778d18f218-d], want [0.go]
FAIL
This is because it assumed that the user hadn't set their own GOCACHE,
and thus that all source files in the cache would be under the default
"go-build" cache directory.
We could fix this via os.Getenv("GOCACHE"), but a simpler mechanism is
to see if the source file has an extension. Source files don't have an
extension in GOCACHE, so that's much simpler to detect.
After this change:
$ GOCACHE=$HOME/go/cache go test
PASS
golist_fallback.go also had a bit of code to add "/go-build/" to the
added testmain.go path, to trick the tests on Go 1.10 to think the file
was in GOCACHE. Update that code too, to now not add ".go" to the path
instead.
While at it, gofmt.
Fixesgolang/go#29445.
Change-Id: I21fc59f13f00bea1f9a8a80e0438825f1a36ac3e
Reviewed-on: https://go-review.googlesource.com/c/156977
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
At least on the go list driver, which is the default fallback. Note that
we need to call 'go list' on Load("foo") and Load(), but not
Load("name=bar"). This is what the current logic was trying to
accomplish.
But it didn't take into account the case where there are zero initial
patterns, in which case we should still call 'go list'. Fix that, and
add a test.
Fixes#28767.
Change-Id: I40af9eb7f2407449c5683df1403928e2c57c86a4
Reviewed-on: https://go-review.googlesource.com/c/155898
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It triggers for all go versions before 1.11. We thought we might
be able to backport some of the necessary changes to 1.10.4, but
it was infeasible, so no 1.10.x versions will support the non-fallback
behavior.
Change-Id: Ib22be2715bf504464e4bd09c14453e5d132776f3
Reviewed-on: https://go-review.googlesource.com/c/156898
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit e7b5a6dfd8.
Reason for revert: merged by mistake
Change-Id: Ibf6ff4aa346b569aaa5e06b58770a93ca3eebdab
Reviewed-on: https://go-review.googlesource.com/c/156900
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
December 1, 2018 has come and gone.
If the timeframe should change that's fine; feel free to reject this and just update the date.
Change-Id: If6aaee30340ee0bfde7e578e054c5b8bc910a5b9
GitHub-Last-Rev: ec781ca1374a010168bc4a457d2ea5d0c023f8dc
GitHub-Pull-Request: golang/tools#64
Reviewed-on: https://go-review.googlesource.com/c/155237
Reviewed-by: Michael Matloob <matloob@golang.org>
Previously, if `packages.Load` failed, it would (sometimes? always?) return an error + no initial packages.
The moved `len(initial)` check was overriding the actual err with a much less useful one.
Example output before:
```
13:03:17.856866 load [./...]
13:03:28.403532 ./... matched no packages
```
And after:
```
13:03:30.942191 load [./...]
13:03:36.999506 go list repeated package [an internal package] with different values
```
Change-Id: I1a821e3855cbbbee904bcec9c29877e091c3e3a0
GitHub-Last-Rev: d1235fc2eceb0d4e947f47be99edc5ac96da5f84
GitHub-Pull-Request: golang/tools#65
Reviewed-on: https://go-review.googlesource.com/c/155745
Reviewed-by: Michael Matloob <matloob@golang.org>
Before this change, a test would fail:
$ GOCACHE=$HOME/go/cache go test
--- FAIL: TestLoadImportsGraph (0.36s)
--- FAIL: TestLoadImportsGraph/GOPATH (0.19s)
packages_test.go:191: golang.org/fake/subdir/d.test.Srcs = [4302876da86a8aae0c1669924daa223cafca60ef49ccaa060ae37e778d18f218-d], want [0.go]
--- FAIL: TestLoadImportsGraph/Modules (0.17s)
packages_test.go:191: golang.org/fake/subdir/d.test.Srcs = [4302876da86a8aae0c1669924daa223cafca60ef49ccaa060ae37e778d18f218-d], want [0.go]
FAIL
This is because it assumed that the user hadn't set their own GOCACHE,
and thus that all source files in the cache would be under the default
"go-build" cache directory.
We could fix this via os.Getenv("GOCACHE"), but a simpler mechanism is
to see if the source file has an extension. Source files don't have an
extension in GOCACHE, so that's much simpler to detect.
After this change:
$ GOCACHE=$HOME/go/cache go test
PASS
While at it, gofmt.
Fixes#29445.
Change-Id: I83c0afc20a527bb50a03f9946e555db36cc85efd
Reviewed-on: https://go-review.googlesource.com/c/155897
Reviewed-by: Michael Matloob <matloob@golang.org>
Now that we support checking for duplicate struct field tags across
anonymous struct fields, we must deal with the fact that the files
involved in such a warning may not be in the same package or directory.
This could lead to errors where a file was mentioned in a package where
it didn't exist. Or even worse, point at a location within an existing
file that doesn't contain the field we want, causing even further
confusion to the user.
To fix this, always make the "also at" positions relative to the current
warning, if possible.
Fixes#29130.
Change-Id: Iaa29b406978f1671bdfb2ddddb7058eeffec92a9
Reviewed-on: https://go-review.googlesource.com/c/155899
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
In module mode, the temp dir will contain a mod cache, which needs to be
made writeable before it can be deleted.
Change-Id: Ia979a8b06d1b4db47d25ffdfdf925ba8a0ac67de
Reviewed-on: https://go-review.googlesource.com/c/156078
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The list of recognized values for the `-debug` flag was old / incorrect.
Change-Id: Ic5f260e650e73512cca2f4db8155bb70bc6b6d17
GitHub-Last-Rev: af642385b81da00169ab9700abb26bed54bdf8ac
GitHub-Pull-Request: golang/tools#66
Reviewed-on: https://go-review.googlesource.com/c/155938
Reviewed-by: Alan Donovan <adonovan@google.com>
When run "go list" outside GOPATH, result should be empty
Change-Id: If832c46d3e4c32a2a87338cca95e3169ea3be055
Reviewed-on: https://go-review.googlesource.com/c/155557
Reviewed-by: Michael Matloob <matloob@golang.org>
The type is expect.Note, not expect.Comment.
Change-Id: I04338fee3b20a7507bbe2a6a15ea36cf1066fe88
Reviewed-on: https://go-review.googlesource.com/c/154580
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Pass -find whenever possible to speed up go list calls. Add an
environment variable, GOPACKAGESDEBUG, that controls debug logging
so that we don't have to tell users to recompile goimports to debug it.
Change-Id: If39ff7829279dafa4e066e74a024c27a8235154b
Reviewed-on: https://go-review.googlesource.com/c/155477
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Added a View interface to the source package, which allows for reading
of other files (in the same package or in other packages). We were
already reading files in jump to definition (to handle the lack of
column information in export data), but now we can also read files in
diagnostics, which allows us to determine the end of an identifier so
that we can report ranges in diagnostic messages.
Updates golang/go#29150
Change-Id: I7958d860dea8f41f2df88a467b5e2946bba4d1c5
Reviewed-on: https://go-review.googlesource.com/c/154742
Reviewed-by: Ian Cottrell <iancottrell@google.com>
refine needs to match packages back to roots, and it gets confused if
the same root appears twice. Deduplicate roots the same way we
deduplicate packages.
Introduce a new wrapper around driverResponse to make this easier, and
pass that around instead of the addPackage callback.
Fixesgolang/go#29297
Change-Id: I49ea37155c507af136391b9eb55a83b6dedfcc14
Reviewed-on: https://go-review.googlesource.com/c/155020
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Also fix packagesdriver to use "go list" if the gopackagesdriver
program is not available, rather than returning nil with no error.
Also fix some comments.
Change-Id: I0457cfa4eab4294760af71c780c62c05ad841c85
Reviewed-on: https://go-review.googlesource.com/c/154757
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Array offsets in recursive structures do not include the accumulated
offset. This diff fixes the mistake and adds a test.
Fixesgolang/go#29318
Change-Id: Iaa2a2f9404e4ed0e38b87e5e041709c1a8e25809
Reviewed-on: https://go-review.googlesource.com/c/154665
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Import test fix https://golang.org/cl/153831 from std lib
version (skips issue29198.go test for older versions of gccgo).
Updates golang/go#29198.
Change-Id: Iddd903068a73a307b1d19d93a7c03b1d00365022
Reviewed-on: https://go-review.googlesource.com/c/153870
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Import changes from std lib version, specifically
https://golang.org/cl/153821
which fixes a bug (#29198) in reading of import data.
Change-Id: I26d4da015b88bbc8f4f7775b0850bef5dffadbec
Reviewed-on: https://go-review.googlesource.com/c/153868
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Marcel van Lohuizen
Michael Matloob
Rebecca Stambler
Yasuhiro Matsumoto
Brad Fitzpatrick
Aurélien Rainone
Dmitri Shuralyov
Douglas Danger Manley
Heschi Kreinick
Ian Lance Taylor
Alan Donovan
Daniel Martí
Agniva De Sarker
fREW Schmidt
Steven L
Zac Bergquist
Ian Cottrell
Michael McLoughlin
David Symonds
Than McIntosh