Address a few irritating glitches in the go list debug logging.
- Print a fully runnable command line, with args like "a" "b" "c"
instead of [a b c].
- Include stderr in the debug logs for cases where the command fails.
- Print the correct PWD environment var from cmd instead of cfg.
Change-Id: I58e77b370baf8378a21377b81ee2ba5d21a557ab
Reviewed-on: https://go-review.googlesource.com/c/163497
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Switch from test to use net/rpc instead of text/scanner, since net/rpc
is a frozen package that is guaranteed not to change.
Change-Id: I6ece679bc16737fa9a75ac896f9eb2190317a498
Reviewed-on: https://go-review.googlesource.com/c/163204
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
text/scanner was used in tests because it was unlikely to change between
releases, but of course, a line number changed, breaking the tests. Use
a different constant for testing that remained on the same line.
Change-Id: I41425728e0918ffe388b3c8df9fd71e8371eb38e
Reviewed-on: https://go-review.googlesource.com/c/163197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
In CL 149609, a file was added to
src/cmd/vendor/golang.org/x/tools/go/analysis/internal/analysisflags/patch.go
to override the behavior of the V flag for cmd/vet.
That modification causes the behavior of cmd/vet to change when a
pristine copy of x/tools is vendored in, and module-mode vendoring
will only support pristine copies (see golang/go#30240).
Instead, allow cmd/vet to override the V flag by defining its own V
flag before it invokes unitchecker.Main.
Tested manually (by patching into cmd/vendor).
Updates golang/go#30240
Updates golang/go#30241
Updates golang/go#26924
Updates golang/go#30228
Change-Id: I10e4523e1f4ede94fbfc745012dadeefef48e927
Reviewed-on: https://go-review.googlesource.com/c/162989
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
These tests tried to access the user's GOPATH, which can't work if you're
running from a module! So use packagestest to set up a fake GOPATH.
Also remove a TODO that's never going to be done because we won't
be making more investments in go/build.
Change-Id: I379c74345ace91e6c4282a42ff62106bf325d0e0
Reviewed-on: https://go-review.googlesource.com/c/162757
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was using a mix of stdout and stderr. Most users won't notice, but
it's inconsistent for no apparent reason. In particular, I noticed as
some of my tool execution tests started failing.
Change-Id: I9afe5f5bed0a575d3ba20e8dc1cc593c35565cf9
Reviewed-on: https://go-review.googlesource.com/c/162717
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This means the test excercise modules and gopath workspaces. That will
allow this test to run once tools becomes a module...
The test still doesn't pass under Windows.
Change-Id: Ic5e46b9b92c5aac909a6ceb56f30851832fc09ac
Reviewed-on: https://go-review.googlesource.com/c/162404
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
This is needed to write the more advanced test cases for editor integrated
tools.
The expectation system also uses the overlay rather than the file if it is
supplied.
Change-Id: I8fd21f4efe5ac5869fa6e25d3cd0d5096051e5e5
Reviewed-on: https://go-review.googlesource.com/c/153240
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
we check length of the map, and a non empty map forces source mode.
Change-Id: Id314ad87b96d9154198122586fd8672a70c5ca95
Reviewed-on: https://go-review.googlesource.com/c/153198
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
files specified in file= queries will now also be checked for in overlay
packages.
Fixesgolang/go#29048
Change-Id: I096d02c3777e6c7decedfcb97c0d011ce9b47b28
Reviewed-on: https://go-review.googlesource.com/c/152797
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This CL updates the importer to match the original code in the std lib
but for the necessary changes to make the code work in x/tools and
with older versions of the std lib.
Notably, it brings over changes from:
https://go-review.googlesource.com/c/152378
Change-Id: Icaa05303a05a226c710b5923d8574f255651fe47
Reviewed-on: https://go-review.googlesource.com/c/152577
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The interface is now stable.
Change-Id: I7dc3feb70131cddb003f9320272a0fbd9b314048
Reviewed-on: https://go-review.googlesource.com/c/152597
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
importer.For does not populate the caller's token.FileSet
leading to spurious position information in diagnostics.
This change is the upstream fix for
https://go-review.googlesource.com/c/go/+/152258.
Fixesgolang/go#28995
Change-Id: I9307d4f1f25c2b0877558426d4d71b3f1df99505
Reviewed-on: https://go-review.googlesource.com/c/152578
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The first issue is that %b and %o can print pointers, but printVerbs
didn't reflect that:
The %b, %d, %o, %x and %X verbs also work with pointers,
formatting the value exactly as if it were an integer.
The second issue is that arrays can never be printed as pointers. This
was previously reported as part of #27672.
The third issue is that only %p can print all slices, maps, and
functions as if they were pointers. This differs from verbs like %b or
%o, which can't print these types as pointers.
Fix all of the issues above, and add extensive test cases covering all
the combinations. Verified all of them with an executed program. The
amount of test cases is perhaps overkill, but this is not the first time
we've gotten the printf pointer logic wrong.
Updates #27672.
Fixes#28858.
Change-Id: I62eb79d505fd1e250a16b90bda3c68b702f35a29
Reviewed-on: https://go-review.googlesource.com/c/149979
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Previously running "go tool vet" said
vet: invalid command: want .cfg file (this reduced version of vet is
intended to be run only by the 'go vet' command)
With this change it says:
vet: invoking "go tool vet" directly is unsupported; use "go vet"
Updates golang/go#28869
Change-Id: I603ab2f75bb52d860e5cd7466e89d051dfbf3f08
Reviewed-on: https://go-review.googlesource.com/c/152217
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This CL updates the importer to match the original code in the std lib
but for the necessary changes to make the code work in x/tools and
with older versions of the std lib.
Notably, it brings over changes from:
https://go-review.googlesource.com/c/152078https://go-review.googlesource.com/c/152077https://golang.org/cl/151997https://golang.org/cl/151557https://golang.org/cl/149957
including test fixes (we want tests to run when gccgo is available,
not just when all go tools are gccgo-based), bug fixes (primarily
related to aliases), performance enhancements, and new code to read
the V3 export data emitted by the most recent gccgo.
Change-Id: I2d34bace23769e62795599b93db8295169076594
Reviewed-on: https://go-review.googlesource.com/c/151717
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This support is not perfect, but should produce more accurate results than
the previous behavior, which is to silently drop those files.
Change-Id: Ia00c042bf303e9ec0fb1cbd579c0fccb29073de0
Reviewed-on: https://go-review.googlesource.com/c/151999
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The expectation langauge is LL(1) but the scanner does not support true
lookahead
This change adds a true LL(1) token stream and rewrites the parser in terms of
it.
Also clean up the error handling and use the behaviour to fix all the broken
edge cases, and then change the tests to cover the now correct behaviour.
Change-Id: If3d602cda490ed2f4732efce400eb8eabce8a8ec
Reviewed-on: https://go-review.googlesource.com/c/151998
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This allows for multi-line comments where a new line is allowed between
expectations.
It also allows trailing new lines and commas in expectation lists.
Change-Id: I714f8ad483a0238281f26ceb74ce9e73d922af69
Reviewed-on: https://go-review.googlesource.com/c/151718
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Our tests compare import graphs from go/packages to expected graphs,
and one of the test cases imports math/bits. But in tip math/bits
picked up a dependency on unsafe, which means the expected graph
is different when run against a Go version >= go1.11. Remove that edge
before comparing against the expected graph to work around the breakage.
Change-Id: Ic586a75ba530741d251df9f87d0817a8e37d92ea
Reviewed-on: https://go-review.googlesource.com/c/151657
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When running go list for a file= query, the go/packages code previously ran "go list ." command in the directory of the provided file. This was necessary for compatibility with the fallback, but caused problems when running file= queries for files in the module cache. We now use absolute directory paths (supported in Go >=1.11) in the non-fallback case and retain the previous behavior in the fallback case.
Change-Id: I3a9feaaeda12e068ec3d510b5f78437482c65b4c
Reviewed-on: https://go-review.googlesource.com/c/148199
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is needed because testing.TB does not have a "Run" method for sub
tests, and the signature of the function it takes needs to match the
testing parameter.
Change-Id: I9201dd75891ed1bda300a4247fe7eac1e25bd30d
Reviewed-on: https://go-review.googlesource.com/c/150857
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Pass in parameters to the driver in stdin rather than through argv.
This allows us to more easily route parameters to the driver and add
more parameters without breaking users. The driver doesn't need to
have the same interface as go list, because it's unlikely the driver interface
will converge with the go list interface.
We still pass in the query "words" to the driver in the argv, because
that's as good a place as any, but there's no "list" command.
Question: should we add a "command" option to the driverRequest struct?
Change-Id: Ifdbb3f84b6bfd04259f5ab63e756341d7f69de9b
Reviewed-on: https://go-review.googlesource.com/c/150337
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This makes our internal version of go to definition be tested with the
same test data that godef now uses
Change-Id: I04e488b6b9b2d891181f202ea1125b823a079c50
Reviewed-on: https://go-review.googlesource.com/c/150045
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The vet-lite tool was useful for developing the new cmd/vet but no
longer needs to exist. This changes removes the command and moves the
main.go file to the unitchecker directory where it serves as an
example and can still be built for testing and debugging.
See also https://go-review.googlesource.com/c/go/+/150297.
Change-Id: Ic10c7cd3aeeaa2e1397dd81939616c6877f7005d
Reviewed-on: https://go-review.googlesource.com/c/150298
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This allows it to be used in benchmarks as well as tests.
Change-Id: I1eb7307a0a7d27c541b14dd8b84c4bc2c770f3c9
Reviewed-on: https://go-review.googlesource.com/c/150257
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
As Alan rightfully guessed, porting the stdmethods check to use go/types
required the use of types.TypeString not only when printing signatures
in warnings, but also when matching them.
Added a simple test case too.
Fixesgolang/go#28792.
Change-Id: Ifbbdd4b1a2f1090d6f9a1674d52b8f0887a67d06
Reviewed-on: https://go-review.googlesource.com/c/149977
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
All of these were quite heavily indented for no good reason; breaking or
returning early makes the code easier to read and follow.
Change-Id: Ic539517b07604d71495277b16f1b7eb60d2e3d3c
Reviewed-on: https://go-review.googlesource.com/c/149978
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The -json and -c=N flags, formerly belonging only to the
go/packages-based {single,multi}checkers, are now supported by
unitchecker as well.
The no-op -source, -v, -all, and -tags flags, formerly belonging only
to unitchecker, have moved to the analysisflags package, which is
common to all checkers.
The -flags flag now reports all registered flags (except the
{single,multi}checker-only debugging flags) rather than just those
related to analyzers, allowing one to say: 'go vet -json' or 'go vet -c=1'.
The code for printing diagnostics, either plain or in JSON, has been
factored and moved into the common analysisflags package.
This CL depends on https://go-review.googlesource.com/c/go/+/149960 to
cmd/go, which causes 'go vet' to populate the ID field of the *.cfg.
This field is used as a key in the JSON tree.
Added basic tests of the new -json and -c unitchecker flags.
Change-Id: Ia7a3a9adc86de067de060732d2c200c58be3945a
Reviewed-on: https://go-review.googlesource.com/c/150038
Reviewed-by: Michael Matloob <matloob@golang.org>
It's only for debugging.
Change-Id: Ic2aacc6bcb52607c253f02b963e0e281213142b0
Reviewed-on: https://go-review.googlesource.com/c/150039
Reviewed-by: Michael Matloob <matloob@golang.org>
The suite used by this tool matters to GOROOT/src/cmd/vet/all and the
'vetall' builder. Add a comment to this effect.
Change-Id: I2e16eb670b03a7bae8224625baaebd1298e2424c
Reviewed-on: https://go-review.googlesource.com/c/150040
Reviewed-by: Michael Matloob <matloob@golang.org>
The recursive stringer check should report cases such as
func (x T) String() string { return fmt.Sprint(x) }
in which the receiver x (or possibly &x) was passed into a fmt print call.
However, in translating it from the go/ast to the go/types representation,
I inadvertently made it report any situation in which a value of type T
was passed to fmt, even when the value is not x, as in:
func (cons *cons) String() string {
... fmt.Sprint(cons.cdr) ...
}
Fixed and tested.
Change-Id: I57e88755c9989deaaad45cc306a604f3db4ee269
Reviewed-on: https://go-review.googlesource.com/c/149616
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
(By "vet lite", we mean static tools that must be invoked by a build
system, such as 'go vet'.)
This CL publishes the former internal/unitchecker package.
Its misnamed Main function is renamed Run, and it has a new Main
that does the steps of a real main (log, flag, etc).
The motivation for this change is to reduce cmd/vet-lite to the point
of triviality to simplify the maintenance of the vendored copy of
x/tools in GOROOT, because GOROOT/src/cmd/vet will need a copy of that
logic. It is now essentially a one-liner.
Also, improve usage messages; analysisflags.PrintUsage wasn't
appropriate for all callers so it has been eliminated.
Each of {single,multi,unit}checker prints its own 1-line usage message.
Change-Id: I214c0e4ae7a2923eee8df3f7548341f2320cad2b
Reviewed-on: https://go-review.googlesource.com/c/149742
Reviewed-by: Michael Matloob <matloob@golang.org>
- add a no-op -tags flag for legacy compatibility.
Tags processing is done by go vet, but it passes the flag on.
Exercised by cmd/go TestGoVetWithTags.
- rename OtherFiles to NonGoFiles in the JSON *.cfg file, to match
the name actually used for this field (see github.com/golang/go/issues/27665).
We really need to publish the types for this protocol.
Exercised by cmd/go TestScript/vet_asm.
- suppress diagnostics in cfg.VetxOnly mode.
Exercised by cmd/go TestTestVet.
Change-Id: I63259f1bd01531d110362e38190a220389b2ec4b
Reviewed-on: https://go-review.googlesource.com/c/149608
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This exposes the ability to add markers to the public interface, and
changes the way markers are collected to make it so a standard call to
Expect can replicate the internal behaviour.
This allows custom rules to also add marks.
Also add a special EOF identifier that acts like a mark at the end of
the file in which it occurs.
Change-Id: Ic5e41cbc5b7ae3c4d1c5b8baba980147c1d22ef1
Reviewed-on: https://go-review.googlesource.com/c/149610
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change causes singlechecker and multichecker to exit with the
correct error code: 0 for success, 1 for load/analysis and other
errors, 3 for diagnostics. (We avoid 2 because the flag package uses
it.)
In JSON mode, errors in package loading, parsing, typechecking and
analysis are successfully in JSON format, with exit code 0.
+ Test.
Change-Id: Iaf130ed3d4cb3e747a628af6da8dc97d065aa869
Reviewed-on: https://go-review.googlesource.com/c/149603
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Type.String prints named types using the complete package path: "dir/pkg.T"
The notation used by canonicalMethod, and the cmd/vet/all whitelist,
and the one users want to see, uses only the package name: "pkg.T".
Change-Id: If2334a8cca1fb80e947cb105530b946a5a8dec7b
Reviewed-on: https://go-review.googlesource.com/c/149597
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Michael Matloob <matloob@golang.org>
Vet issues a warning (non-error diagnostic) when, for example, it
cannot check an assembly file because the Go and asm symbols are in
different packages. The new analysis API has no concept of warnings:
any diagnostic always causes a non-zero exit.
This change turns the asmdecl diagnostics back into warnings using
log.Print, which is not ideal, but is necessary to pacify cmd/vet/all
and its whitelist during the transition. Better solutions would be for
the new analysis API to have a concept of warning, or for asmdecl to
be silent and cmd/vet/all's whitelist not to expect these messages.
Also, fix a bug in the "cross-check" predicate: cmd/vet confuses the
name of a package and its path. The a∕b∕c names (using Unicode
division slash) that appear in assembly correspond directly to the
path.
The only effective test of this change will be cmd/vet/all itself.
Change-Id: I2e402d48717df723e2efdc2379636ec9b204031d
Reviewed-on: https://go-review.googlesource.com/c/149598
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Michael Matloob <matloob@golang.org>
Now that vet can rely on go/types, there's no reason to do extra work to
avoid using it. The rewrite lets us get rid of the field list flattening
code, as well as the slight verbosity that comes with go/printer.
While at it, make the testdata/method.go expected errors be more
specific, to make sure that we're not breaking the warnings that are
printed.
This change was originally made to cmd/vet in
https://go-review.googlesource.com/c/148919
Change-Id: I123e64d369e521199712c9807583c53d428534ac
Reviewed-on: https://go-review.googlesource.com/c/149418
Reviewed-by: Michael Matloob <matloob@golang.org>
Pointers to compound objects (structs, slices, arrays, maps) are only
followed by fmt if the pointer is at the top level of an argument. This
is to minimise the chances of fmt running into loops.
However, vet did not follow this rule. It likely doesn't help that fmt
does not document that restriction well, which is being tracked in
#28625.
This change was originally made to cmd/vet as
https://go-review.googlesource.com/c/147997.
Updates #27672.
Change-Id: I65944cf355baedb4578af57046e2bbfd3fe6a9dc
Reviewed-on: https://go-review.googlesource.com/c/149319
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Some of the Analyzers' names were changed during the refactoring.
These legacy flags ensure the old names continue to work.
Change-Id: I466aa38ec55071c944fb73571915aa7afb42dbc2
Reviewed-on: https://go-review.googlesource.com/c/149417
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Josh Bleecher Snyder
Heschi Kreinick
Rebecca Stambler
Bryan C. Mills
Michael Matloob
Daniel Martí
Marcel van Lohuizen
Yasuhiro Matsumoto
Brad Fitzpatrick
Aurélien Rainone
Dmitri Shuralyov
Douglas Danger Manley
Ian Lance Taylor
Alan Donovan
Agniva De Sarker
fREW Schmidt
Steven L
Zac Bergquist
Ian Cottrell
Michael McLoughlin
David Symonds
Than McIntosh