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>
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>
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>
This is mostly to allow access to exported file contents for tests
that need the source.
Change-Id: I0ef946d7bdd971b931e509d2cb54e2c59649fe47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/166883
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts CL 164837 (0f64db555a)
Reason for revert: breaks vetall against tip (and thus go tip's trybots)
Change-Id: I5109691481f44a9807675a6139f1619a03b0c58d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/165039
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Add the deepequalerrors analyzer to the vet command.
I don't really understand the comment in the file. I do want the analyzer to run
when the user explicitly calls go vet, but not automatically via go test.
I'm not sure this CL captures that.
Change-Id: Ie78ef110c7828ccbcc86735442c81dbb516dcf18
Reviewed-on: https://go-review.googlesource.com/c/tools/+/164837
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's possible to use a type which implements fmt.Formatter without
importing fmt directly, if the type is imported from another package
such as math/big.
On top of that, it's possible to use printf-like functions without
importing fmt directly, such as using testing.T.Logf.
These two scenarios combined can lead to the printf check not finding
the fmt.Formatter type, since it's not a direct dependency of the root
package.
fmt must still be in the import graph somewhere, so we could search for
it via types.Package.Imports. However, at that point it's simpler to
just look for the Format method manually via go/types.
Fixes#30399.
Change-Id: Id78454bb6a51b3c5e1bcb1984a7fbfb4a29a5be0
Reviewed-on: https://go-review.googlesource.com/c/163817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Our tests depend on the dependency graphs of the errors which
is not guaranteed to have a stable dependency graph across go versions.
Especially because we're planning on making changes to the errors package
in Go 1.13.
Instead, use the container/list package, which is completely useless
and won't be changed (unless/until Go gets generics).
Fixesgolang/go#30448
Change-Id: Ia5f4853d1da336dde2f025b1dd5e1d6223571dd6
Reviewed-on: https://go-review.googlesource.com/c/164298
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The new error value proposal (https://golang.org/design/29934-error-values)
adds stack frame information to the errors produced by errors.New
and fmt.Errorf. This will break any test that compares errors
with reflect.DeepEqual.
This vet check finds any call to reflect.DeepEqual both of whose
arguments are of type error, or are of a type that can store
of value of type error.
Change-Id: I55939339344ed5b4f61557a7296734a710211918
Reviewed-on: https://go-review.googlesource.com/c/162939
Reviewed-by: Damien Neil <dneil@google.com>
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>
Ian Lance Taylor
Brad Fitzpatrick
Steven L
Michael Matloob
Colin Arnott
Gergely Brautigam
Daniel Martí
David Chase
Ian Cottrell
Josh Bleecher Snyder
Elias Naur
Jonathan Amsterdam
Heschi Kreinick
Rebecca Stambler
Bryan C. Mills
Marcel van Lohuizen
Yasuhiro Matsumoto
Aurélien Rainone
Dmitri Shuralyov
Douglas Danger Manley
Alan Donovan
Agniva De Sarker