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>
fmt's godoc reads:
For compound objects, the elements are printed using these
rules, recursively, laid out like this:
struct: {field0 field1 ...}
array, slice: [elem0 elem1 ...]
maps: map[key1:value1 key2:value2 ...]
pointer to above: &{}, &[], &map[]
That is, a pointer to a struct, array, slice, or map, can be correctly
printed by fmt if the type pointed to can be printed without issues.
vet was only following this rule for pointers to structs, omitting
arrays, slices, and maps. Fix that, and add tests for all the
combinations.
This change was originally made to cmd/vet in
https://go-review.googlesource.com/c/147758
Updates #27672.
Change-Id: I7e25ecaeed619ae8b6ada79bccacba6b67171733
Reviewed-on: https://go-review.googlesource.com/c/149318
Reviewed-by: Michael Matloob <matloob@golang.org>
Per discussion with Russ,
the -all/-source/-v flags now silently do nothing, and
the -printffuncs (et al) shims now silently delegate to -printf.funcs, and
the -NAME.enable (et al) flags are now called just -NAME.
Various minor tweaks to command-line help messages.
Change-Id: If6587937f58446e605eca4d3a5be0aaf6287065d
Reviewed-on: https://go-review.googlesource.com/c/148879
Reviewed-by: Russ Cox <rsc@golang.org>
This is a workaround for a go list regression that broke
go/packages but went unnoticed by because of a missing
call to packages.PrintErrors, added here.
Updates golang/go#28749
Change-Id: I1819a6143134a422791106ac037d3458ef864322
Reviewed-on: https://go-review.googlesource.com/c/149237
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Use the packagestest framework to test completion. Add support for a
slice of token.Position to packagestest to support this.
Change-Id: Ie5ddece4446a3c74419727461a77faa3788cb040
Reviewed-on: https://go-review.googlesource.com/c/148197
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
The fallback driver wasn't returning the roots in a
deterministic order because it was using sticking them in
a map and then iterating over that map to get each element.
Put them into a slice instead (and make a few small
associated changes) to preserve behavior.
Fixesgolang/go#28040Fixesgolang/go#28609
Change-Id: Ib8f8c88d65b7a48b2b04ca91e2d3c316d5bb5803
Reviewed-on: https://go-review.googlesource.com/c/148880
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This mainly entails writing compiler output files to a temporary
directory, as well as the corrupted files in TestVersionHandling.
This is a backport of CL 146119.
Fixesgolang/go#28644
Change-Id: I5701fe3fda7d6364411eef8265c575c279dbf9a1
Reviewed-on: https://go-review.googlesource.com/c/149017
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This change adds to the list of standard library functions known to be
print or printf wrappers.
Although the printf Analyzer is capable of identifying wrapper
functions in the standard library, some drivers (e.g. Bazel) do not
apply analyzers to the standard packages. Really this is a bug
in those drivers but it is not likely to be fixed for a while.
Change-Id: I2032d0cb5fcb50e7b9933a75809becdd680380ec
Reviewed-on: https://go-review.googlesource.com/c/148572
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Details:
- Add -source, -v, and -all flags to vet-lite.
These have no effect and issue a warning.
- Add usage message to vet-lite that lists all
analyzers and explains -foo.enable and other flags.
- Factor this help message (common to vet-lite and
multichecker) into analysisflags.
- Add legacy aliases of new flags.
e.g. -printfuncs is now -printf.funcs
The old names work but issue a warning when used.
Also: update comments to say -vettool not$GOVETTOOL
I think we should probably do away with singlechecker
in a follow-up: a singleton multichecker is good enough,
and will allow us to remove cases in the flag-processing
logic.
Change-Id: Ib62f16b5e2f4c382a29e6300a6246b2db9e08049
Reviewed-on: https://go-review.googlesource.com/c/148559
Reviewed-by: Michael Matloob <matloob@golang.org>
When main.main returns, the process exits, so there's no need to cancel contexts.
This CL was originally reviewed as
https://go-review.googlesource.com/c/go/+/106915
and then retried in
https://go-review.googlesource.com/c/go/+/148758
but then reverted due to an embarrassing sequence
of careless moves.
Change-Id: Icdee0650996a442023e030697f10d2c31fd5fdff
Reviewed-on: https://go-review.googlesource.com/c/148877
Run-TryBot: Alan Donovan <adonovan@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
In vet, the shadow check is experimental, meaning not on by default.
The new analysis API has no concept of experimental, but you can
easily supply a different checker. By providing a shadow command, we
make it easy for users that want it to run it:
$ go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
$ go vet -vettool $(which shadow) my/project
Change-Id: I25dc7f3c830296121c7217e4615e8ff90e1b7c79
Reviewed-on: https://go-review.googlesource.com/c/148565
Reviewed-by: Michael Matloob <matloob@golang.org>
The printf checker is in the passes/ subdirectory.
Change-Id: I0a912231280bc954fee3088050541ba5ecb17dde
Reviewed-on: https://go-review.googlesource.com/c/148571
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The cgocall analyzer was originally written in vet to analyze "raw" cgo files,
which are type-checked on a best-effort basis. However, the go/analysis
API presents analyzers with "cooked" cgo files, which are well-typed
legal Go but obscure the relationship between a call C.f(...) and its arguments.
Prior to this CL, cgocall attempted to "uncook" the file, which was
as predictably fragile as it sounds, and it rapidly broke as the cgo
recipe evolved.
This change causes cgocall to parse, modify, type-check and analyze "raw"
cgo files. The approach (based on dot-importing the "cooked" package)
is rather too clever but should be more robust than the one it replaces.
Fixesgolang/go#28566
Change-Id: I3092a313c64d27153eaaa115fe8635abfed17023
Reviewed-on: https://go-review.googlesource.com/c/147317
Reviewed-by: Michael Matloob <matloob@golang.org>
This change makes AddNamedImport more symmetrical to DeleteNamedImport
in behavior, signature, and documentation.
In Go, it is valid to import a package (identified by an import path)
with different names in a single .go file¹. For example, this is ok,
and the program will build as long as there are no unused imports²:
import (
"path"
. "path"
_ "path"
pathpkg "path"
)
The four imports (represented by ast.ImportSpec type) have the same
import path, but different names.
It's currently possible to use DeleteNamedImport to delete the exact
{name, import path} ast.ImportSpec from an AST.
Previously, it wasn't possible to use AddNamedImport to add an exact
{name, import path} ast.ImportSpec if the AST already has another
ast.ImportSpec with the same import path (but different name).
This change fixes that, making it possible to use AddNamedImport as
expected.
Rename the ipath parameter to path in AddNamedImport for consistency
with DeleteNamedImport. Also change the language used in its
documentation to be more precise and consistent with DeleteNamedImport.
Add test cases for this behavior. The DeleteNamedImport test cases
were passing before any changes to the astutil code.
Add test coverage for the return bool value of {Add,Delete}NamedImport.
Document the behavior of DeleteNamedImport when there are duplicate
matching imports.
Simplify string(buf.Bytes()) to buf.String() in test helper code.
¹ https://golang.org/ref/spec#Import_declarations
² https://play.golang.org/p/bi6L34rQbcDFixesgolang/go#28605Fixesgolang/go#16411
Change-Id: I70e887f5174764ab1cf32a761c8c734e216eea67
Reviewed-on: https://go-review.googlesource.com/c/147448
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This will allow Sizes to be marshalled and unmarshalled. All the Sizes
we care about ane StdSizes anyways.
Change-Id: I79d1dcaebba32f7730de4375945e372eeefa78fe
Reviewed-on: https://go-review.googlesource.com/c/147978
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This is more reliable than looking in the environment.
Change-Id: I96c093b89faaece6b6256eefb4a4aac4d66b9cc9
Reviewed-on: https://go-review.googlesource.com/c/146757
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The new go/analysis/cmd/vet tool found a few issues. Fix them by
explicitly ignoring results where needed and renaming examples in
accordance to package testing's guidelines to make them appear on the
godoc pages.
This change doesn't touch go/ssa/interp/external.go because the
possible misuse of package unsafe there looks intentional.
Change-Id: Id63109e3797818a5e869fe8ed1bc1476959541f2
Reviewed-on: https://go-review.googlesource.com/c/147297
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This is based on 147340 and has the same test, but with a different fix.
We now produce a nice error if there is an empty root, instead of
crashing, and the cause of empty roots in go list has been fixed.
The underlying call to go list is returning the same package more than
once, and we only fix the first entry in the root list, so the second
one got left blank.
The fix was to not add the second duplicate copy to the output of the
driver at all.
Change-Id: I9f1b2f0fd63635ba101cdd3c8a5108530e968ba9
Reviewed-on: https://go-review.googlesource.com/c/147440
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
For performance, bail out of runNamedQueries before running go env if
there's nothing to do.
Add t.Helper() calls to packagestest.TestAll.
Escape # in packagestest.Export. The testing package adds #NN suffixes
to subtests that have redundant names.
Log how long gopathwalk.Walk took for name= queries when debug is on.
Change-Id: I37cb0ed11cf58e1693e29dea04697e5885ecc62b
Reviewed-on: https://go-review.googlesource.com/c/147203
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add some basic tests for diagnostics using the new
go/packages/packagestest framework.
Change-Id: I6a7bfba6c392928a9eb123ab71ceb73785c12600
Reviewed-on: https://go-review.googlesource.com/c/145697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When the name= query constructs the temporary module, it may find things
that don't resolve. In at least some cases (#28518), allowing go list
to access the network results in not just bad performance but actual
failures. Default GOPROXY to "off" when doing queries on the temporary
module to try to address that.
Also, add some more debug logging, including various environment
variables, so that it's easier to reproduce failing commands.
Change-Id: I1a6d3ffa5c845271ce48e9fe802a2491ccadcd7c
Reviewed-on: https://go-review.googlesource.com/c/146477
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The test correctly identified a problem.
Suppressing test to keep dashboard cleean, pending a fix.
Updates golang/go#28566
Change-Id: Ib3a8dbdd617c9f5701b5d6673434917d284dfb32
Reviewed-on: https://go-review.googlesource.com/c/147199
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This adds a library that allows markers and actions inside comments in go source
files.
It then also adds an easy way to use that library for tests using packagestest.Expect
This is used to easily write code inspection and manipulation tests in a
language that is common to all tests.
Change-Id: I755caaad1557c9b8779ad9ecda2b3309550d6976
Reviewed-on: https://go-review.googlesource.com/c/142998
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Also:
- add cmd/vet-lite, a version of cmd/vet that doesn't depend on
go/packages and must be run under "go vet". This will be vendored
into $GOROOT/src/cmd/vet.
- add an integration test for a unitchecker-based command under "go vet".
Change-Id: Id613dac2812816c6d6372fa6d1536c8d4e4c2676
Reviewed-on: https://go-review.googlesource.com/c/143418
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
We do insist that drivers are stable (not sorted, just stable) and that
refine is also stable, which allows us to promise a stable output.
I also changed refine so it returns the root set in the same order that
the original root id list was supplied, as this seems to be a strictly
better experience.
Change-Id: I8eb0bffd7547865d14a6c6f18646018b9af140bd
Reviewed-on: https://go-review.googlesource.com/c/145877
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
If a Module's Name ends with a version suffix (v2), create its module
with an appropriate version number (v2.0.0).
Change-Id: I1a16f054fac5717e79871ba1bf9a6343c0ce2f5b
Reviewed-on: https://go-review.googlesource.com/c/146097
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Treat the top-level package of a module specially so that the VCS
working dir name doesn't have to match the package name. Generally we
take the position that directory names should match packages, but for
the top-level package that's a pretty strict requirement. Now we always
check the top-level directory, just in case.
Change-Id: I2e96751bf46736fbaef434fa9788fdc1339c37b9
Reviewed-on: https://go-review.googlesource.com/c/144817
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The builders are currently flaky failing because of a root order being
wrong.
This causes all root lists to be sorted before being returned, so the
order is always stable no matter which underlying driver is running.
Fixesgolang/go#27594
Change-Id: I09db45c67ad00f23dfaec8e271acbd13fc338888
Reviewed-on: https://go-review.googlesource.com/c/143737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Package facts provides an implementation of the Import/Export methods
of the analysis.Pass interface and functions to encode and decode
facts, using Gob encoding, to a file. It will be part of the vet-lite
driver (invoked by go vet) but the same logic has been validated in
other build systems such as Blaze.
Change-Id: I60ef561e84e833b9a3b17c269ab358e7d0800ff3
Reviewed-on: https://go-review.googlesource.com/c/144737
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Instead of writing replace directives to link the test's modules
together, fill a GOPROXY dir and use `go mod download` to fill a real
GOCACHE with them. This makes the tests more realistic, since replace
directives will be somewhat unusual. It also gives the name= query
something to search.
Actually doing this in a way that's compatible with packagestest's
abstraction is a little tricky, since it wants to know where the files
will be. The actual files will be created by go mod download, so we have
to get Export to put them there to begin with, then move them out of the
way.
Since the GOPROXY zip format doesn't support symlinks, those will only
work in the primary module.
Change-Id: I6bc1d368f1c950d789e409213107d60bb1389802
Reviewed-on: https://go-review.googlesource.com/c/144498
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
I tried to introduce new modules where it made sense. I deleted the
weird goroot test because I removed the optimization it was testing in
an earlier CL.
Change-Id: I219ddaa4f462a4aeb640f62215d16f246511a5fe
Reviewed-on: https://go-review.googlesource.com/c/144497
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Darwin convertes the working directory to a real path which affects the
path a command is started with when you exec.
This causes a problem where the path the go list command is run with
does not match the path the outside program is trying to use, which
causes both undesirably changed paths in it's outputs, and sometimes
failures to cope with modules when source files are "outside" the module
root.
The go standard library has a special feature in os.Getwd where it
checks if the PWD environment variable is the same inode as the system
cwd, and if it is it returns the PWD instead. This change deliberately
sets the PWD before running go list so that behaviour is triggered,
which fixes all the paths.
This was breaking the mac build bots in a goimports test, it should be
fixed now.
Change-Id: I0f5d3c7d020b55749738036ba51c19884bb26598
Reviewed-on: https://go-review.googlesource.com/c/143517
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
packagestest.Modules is now available pre-go1.11 but doesn't add
itself to packagestest.All.
strings.ReplaceAll is not available in pre-go1.11
Change-Id: Ia8bf0e82bb853c6f29d31ca5c54651097342b19c
Reviewed-on: https://go-review.googlesource.com/c/143419
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is the new vet command. It can be run standalone:
$ vet my/project/...
or (soon) under go vet:
$ GOVETTOOL=$(which vet) go vet my/project/...
A forthcoming CL will add support for the second mode, and define a
vet-lite command that supports only that mode, but has fewer
dependencies; it is intended to be vendored into $GOROOT/src/cmd/vet.
Change-Id: I57696ae6d43aa31fd10b370247b7e7497f0f3597
Reviewed-on: https://go-review.googlesource.com/c/143417
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This CL brings over changes from https://golang.org/cl/74354/
which were not brought over.
The code was adjusted slightly such that a filename is still available
for ImportData even if a custom lookup function is provided (adjustments
are on lines 133, 188-193).
Change-Id: I58960e648c9aae1683eb4d7f8d7578f09349eca2
Reviewed-on: https://go-review.googlesource.com/c/143017
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
- eliminated special case for isAlias predicate
- appended code from bexport19_test.go to bexport_test.go
No other changes.
Change-Id: Icf10691510e51f4e618b2a48bc1a1215a98832b6
Reviewed-on: https://go-review.googlesource.com/c/142896
Reviewed-by: Alan Donovan <adonovan@google.com>
The analysisflags package provides a function to help
ensure that all drivers support consistent command-line
interfaces. In particular, -analyzer.enable flags use
tristate logic as in vet, and the -flags flag dumps
a list of flags in JSON for use by 'go vet' and other
build systems.
This code is in a separate package from internal/checker
(the common parts of multichecker, singlechecker)
because we don't want the forthcoming vet-lite (formerly
known as doctor) driver to have an unnecessary dependency
on go/packages. (When go/packages is promoted to the
standard library we can consolidate them.)
+ Test of tristate analyzer selection logic.
Change-Id: I5ea4e556e0f56505df06eb8fa9dd9eed884a1b47
Reviewed-on: https://go-review.googlesource.com/c/143197
Reviewed-by: Michael Matloob <matloob@golang.org>
Alan Donovan
Rebecca Stambler
Michael Matloob
Diogo Pinela
Daniel Martí
Michael Stapelberg
Nicholas Ng
Fazlul Shahriar
Brad Fitzpatrick
Dmitri Shuralyov
Ainar Garipov
Ian Cottrell
Heschi Kreinick
Robert Griesemer