Can't do module tests on a pre-modules version of Go.
Change-Id: I8e3e22d472f37ebc9f930a68c9a6c0f4b7ba7ea0
Reviewed-on: https://go-review.googlesource.com/c/142700
Run-TryBot: Heschi Kreinick <heschi@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
...and other trivial cleanups.
Multi-line doc comments have been moved to exported Doc constants for
the sake of godoc.
Change-Id: Ib1cbec5806c699d51283c34685c4cd96953f5384
Reviewed-on: https://go-review.googlesource.com/c/142360
Reviewed-by: Michael Matloob <matloob@golang.org>
Add an implementation of name= for go list. It will be used to
implement goimports and godoc-like lookups by package name.
Imported a copy of the semver package from the stdlib to do version
comparison, and tweaked the gopathwalk API to include a hint about what
kind of source directory is being traversed.
Note that the tests, despite my best efforts, are not hermetic: go list
insists on doing version lookups in situations where it seems to me like
it shouldn't need to.
I think this implementation is ready for serious use. The one thing I'm
nervous about is that it currently does a substring match when looking
for a package name, so if you look up a package named "a" you will get
a huge number of results. This matches goimports' behavior but I don't
know if it's suitable for general use.
Change-Id: I2b7f823b74571fe30d3bd9c7dfafb4e6a40df5d3
Reviewed-on: https://go-review.googlesource.com/c/138878
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Guide to changes:
- The -printfuncs flag is renamed -printf.funcs.
It no longer supports "pkg.T.f" form, requiring instead
"(dir/pkg.T).f" or "(*dir/pkg.T).f".
The legacy ":%d" suffix is no longer supported.
- (*testing.T).Errorf and friends are removed from the isPrint map
because they are discovered by induction while analyzing package
"testing".
- localPrintfLike map operations are replaced by the Fact mechanism.
- The go/types representation is used instead of strings/ast.Nodes in
various places. For example:
pkgpath, name string -> *types.Func (to identify a function)
format, args *ast.Field -> *types.Var (to identify format/args params)
This was required to fix a latent bug in maybePrintfWrapper's
handling of format string parameters` declared using "factored"
syntax, such as: func f(foo, format string, args...interface{}).
See L253 of the original testdata file for a testcase that ensured
the buggy (?) behavior.
- func printfLike is removed as it was deadcode.
- isFormatter is rewritten to avoid a global variable.
- "if verbose { warn }" is replaced by "if false { report }" for now.
- recursive stringer is rewritten more simply in term of go/types.
Change-Id: Ia6ee827117b611c686e38207916a21fe1fc296e2
Reviewed-on: https://go-review.googlesource.com/c/142239
Reviewed-by: Michael Matloob <matloob@golang.org>
Get the value of GOARCH from the config instead.
It should have been set to the same as the one from the environment by
default, but may have been overidden by the caller.
Change-Id: If9a6c0ae998c1c72ad2a68fe83c8bb9f5614a189
Reviewed-on: https://go-review.googlesource.com/c/142361
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>
We agreed on a different set of invalid queries than what the test
tests. The change got lost in a merge. Fix it.
Change-Id: I812e561d924f5dbd0c29e3a6ec5fb53022d09487
Reviewed-on: https://go-review.googlesource.com/c/142359
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This is just a rename for ease of review.
The logic change will follow.
Change-Id: I8856b22f2157d63c6983c0f00a12c87e5d5dd1a4
Reviewed-on: https://go-review.googlesource.com/c/142357
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
CL 121876 made sync.noCopy implement sync.Locker and
added this as an assumption to vet. But now that copylock
is no longer in the standard library it cannot
assume that it is analyzing a recent standard library
in which noCopy has an Unlock method.
Change-Id: I5a30b3711ae6cc0855eb246fdd93b1906779bdde
Reviewed-on: https://go-review.googlesource.com/c/141683
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This file is nominally part of the core vet tool but in reality it is
only used by the printf checker.
This is just a rename; the actual change will come in a follow-up.
Change-Id: I5497d6888228e6781cf0d3ae62b7c6b5b723e151
Reviewed-on: https://go-review.googlesource.com/c/142240
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This is a straight rename to simplify review of the actual change in a
follow-up CL. (This is a workaround for git diff's weak support for
renames.)
Change-Id: If63336d4f782e12066fce83a848465371013cdde
Reviewed-on: https://go-review.googlesource.com/c/142237
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
In go1.10, go/packages falls back to loading all packages
from source but not typechecking function bodies for imports.
The ssautil.Packages function would nonetheless provide
the partially-typed ASTs to the SSA builder, which would crash.
Now Packages only passes syntax trees to the SSA builder for
the initial packages, which are the only ones guaranteed to be
fully typed.
It is impossible to discern whether the caller of Packages intends to
build SSA code for dependencies, as in some clients such as
cmd/callgraph, so we add a new function, AllPackages, that expresses
this intent.
Fixesgolang/go#28106
Change-Id: I6a88b7c7545e9de90b61f5bee0e6de3d2e21b548
Reviewed-on: https://go-review.googlesource.com/c/141686
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add a guarantee for breaking changes.
Change-Id: I7c176eb0c3a309ad187e3a33a645996e397d09be
Reviewed-on: https://go-review.googlesource.com/c/141684
Reviewed-by: Alan Donovan <adonovan@google.com>
This change updates the language accepted through the arguments
to packages.Load to make it more consistent. There are now two layers.
A pattern containing an "=" is considered to be a special query, and the
part of the pattern up to the first "=" is considered the query type. All
other patterns are to be interpreted as the build system interprets it.
For now two special queries exist. file= has the behavior that contains:
did: finding packages containing the given file. The query type pattern=
is used to pass through a pattern to be interpreted by the build system.
pattern= is a type of escaping to allow passing through patterns that
contain an "=" to be interpreted by the underlying buildsystem. To allow
for new query types to be introduced, packages.Load will report an
error if the qury type is not understood. We expect name= to be added in
an upcoming change.
"contains:" changes to "file=". A new
Change-Id: I1b208d1c998c67d5556cdc872d7694273cedb7e4
Reviewed-on: https://go-review.googlesource.com/c/141681
Reviewed-by: Alan Donovan <adonovan@google.com>
Now that asmdecl is not in the standard repo, we must not assume
that types.SizesFor knows about all architectures and panic if it
does not. This change makes it print a warning and assume 64-bit
norms.
Change-Id: Idacad350b2fc9343adfb32539fec7003b39380ed
Reviewed-on: https://go-review.googlesource.com/c/141679
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The typechecker's Sizes function is not currently set correctly.
The correct answer requires information known only to the build
system's query tool (go list, blaze query, etc), and we need to
add a mechanism for it to return this information.
In the meantime, we use this simple workaround: if the GOARCH
environment variable is set, we use that to determine sizes according
to the conventions of gc. Otherwise, we use the architecture for which
the application was compiled. Both could easily be incorrect, but this
is nonetheless progress.
This change should fix the tests of go/analysis/passes/shift,
which are currently broken for GOARCH=386 because the analysistest
driver uses go/packages, which ignores GOARCH.
Change-Id: Iabe3211ad513a9a94eadd6d8f4b2068f7abdd053
Reviewed-on: https://go-review.googlesource.com/c/141757
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Duplicate root analyzers caused duplicate flag registration and other problems.
Change-Id: Id0c2761529c57ed1f9a63b669e62401ebf035cc2
Reviewed-on: https://go-review.googlesource.com/c/141159
Reviewed-by: Michael Matloob <matloob@golang.org>
This checker needed some reworking because whereas vet sees
unprocessed cgo source files (with partial type information), the
analysis API sees cgo-processed files with complete type information.
However, that means the checker must effectively undo some of the
transformations done by cgo, making it more fragile during changes to
cgo.
Change-Id: I3a243260f59b16e2e546e8f3e4585b93d3731192
Reviewed-on: https://go-review.googlesource.com/c/141157
Reviewed-by: Michael Matloob <matloob@golang.org>
The analysistest.Run function now applies a single analysis to
a set of packages, not just one, as this is necessary for testing
the "tests" Analyzer. The Run function also returns a richer
Result for each package, allowing a test to perform additional
checks if necessary.
I really don't understand how Gerrit decides whether to render
a file such as passes/tests/tests.go as a mv+edit or an add;
small changes to the CL seem to perturb the heuristic.
When reviewing these CLs please inspect the logical diff of
passes/vet/tests.go -> passes/tests/tests.go
Change-Id: I7812837278b20c8608ccbb6c709c675588a84db1
Reviewed-on: https://go-review.googlesource.com/c/140457
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
And rename from "bool" to "bools".
Using analysistest unearthed a minor bug,
github.com/golang/go/issues/28086.
To avoid complicating the diff we work
around it in the tests for now.
Change-Id: I682f33506de778dfdfe97841cd2b16e3d47062b8
Reviewed-on: https://go-review.googlesource.com/c/140737
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Also: don't abort loading just because there were parse/type errors.
It's the driver's job to decide whether to fail due to errors.
Change-Id: I055033fb89319d957b328c4fa4a30144afc7457c
Reviewed-on: https://go-review.googlesource.com/c/140738
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Also, rename to stdmethods and add more tests.
Change-Id: I09b65899dc02a8062f3ec1d909c2eae45472e236
Reviewed-on: https://go-review.googlesource.com/c/140761
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Also: create internal/analysisutil package for
trivial helper functions shared by many vet analyzers.
(Eventually we may want to export some of these functions.)
Change-Id: I2b721a16989826756d0426bc7f70089dfb1ef9ce
Reviewed-on: https://go-review.googlesource.com/c/140577
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This allows users of go/packages to replace the contents of already
existing files, to support use-cases such as unsaved files in editors.
BREAKING CHANGE: This CL changes the signature of the function provided
to Config.ParseFile.
Change-Id: I6ce50336060832679e9f64f8d201b44651772e0b
Reviewed-on: https://go-review.googlesource.com/c/139798
Reviewed-by: Alan Donovan <adonovan@google.com>
This analysis was renamed from "rangeloops" since it applies equally
to non-range for-loops.
Change-Id: I441378b29d36aaf7fe102913c5b3aaa7cfd351a0
Reviewed-on: https://go-review.googlesource.com/c/140578
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Also: extend analysistest to extract '// want ...' comments
out of non-Go source files such as assembly.
This change also moves asm8.s file into the right directory
so that it gets exercised by the test.
All the .s files were git-mv'd and then the ERROR comments
were changed to 'want'; also +build vet_test tags were removed.
Sadly Gerrit reports the old and new files as unrelated...
Change-Id: I8a2ecd6dd6fb0e20630f0ba6205c4378e4e912b3
Reviewed-on: https://go-review.googlesource.com/c/140120
Reviewed-by: Michael Matloob <matloob@golang.org>
buildtag checker:
- This checker has been modified from the version in vet to handle Go
and non-Go files differently, to avoid having to re-read-and-parse
Go files in the common case.
- The old cmd/vet driver would run this check on all the files in
a directory whereas new drivers will run it only on the files
selected for a particular configuration, so some of the checks
(those in checkArguments) will never fire. But this is not a regression
relative to 'go vet', because it too presents cmd/vet with only the
files selected as part of the package.
analysistest:
- fix bug that processed a block of //-comments as one.
- treat "...// want..." within a //-comment as a want comment.
This is required for adding expectations on lines that are already comments.
Change-Id: Iacf3684864e07532f77176481afbf059a9638f3b
Reviewed-on: https://go-review.googlesource.com/c/139797
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
The ctrlflow Analyzer builds a control-flow graph (see
golang.org/x/tools/go/cfg) for each named and unnamed function in the
package.
It computes for each function whether it can never return, either
because the function is an intrinsic that stops the thread (e.g.
os.Exit), or because control never reaches a return statement, or
because the function inevitably calls another function that never
returns. For each such function it exports a noReturn fact.
This change also:
- adds 'inspect', another Analyzer that builds an optimized AST
traversal table for use by nearly every other Analyzer.
- changes analysistest.Run to return the analysis result to enable
further testing.
(This required changing it to analyze one package at a time,
which is no less efficient, and is the typical case.)
Change-Id: I877e2b2363a365a9976aa9c2719ad3fba4df2634
Reviewed-on: https://go-review.googlesource.com/c/139478
Reviewed-by: Michael Matloob <matloob@golang.org>
This test is unsurprisingly failing on the nocgo builder because
the cgo packages don't exist on those builders.
Updates golang/go#28040
Change-Id: I633b73bb48e76824645e4e8dd141fb42c9adc19f
Reviewed-on: https://go-review.googlesource.com/c/140121
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
internal/checker
- don't display "[name.category]" in diagnostic
output. Most users don't care which analyzer reported the message.
The -json output retains this information.
- print better log messages for analyze -debug=f.
- print (non-JSON) text output to standard error, like a compiler would.
passes/pkgfact
- fix a nil deref panic when encountering non-renaming imports.
- require names to have underscores before and after (_x_)
as this avoids a huge number of spurious matches in (e.g.) the
syscall package.
- don't export empty facts.
Change-Id: I86c003b96521334e371f9d5fcea1323cd779d7f0
Reviewed-on: https://go-review.googlesource.com/c/139657
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 139320 unset these env vars for the whole process in two tests,
but the correct fix is to unset them for subprocesses forked by go/packages.
Change-Id: I35e3ab9e424b00326e9e813e4daf0ae92ec36e26
Reviewed-on: https://go-review.googlesource.com/c/139477
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fixes 'go test ./...' in the root of the tools repo with GO111MODULE=on.
Updates golang/go#27858
Change-Id: I7492d2a2406997a399fe2badd24882fcb19d37c5
Reviewed-on: https://go-review.googlesource.com/c/139320
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change adds support for testing the facts produced by an Analyzer
using a similar mechanism to the way it checks for diagnostics.
A "// want ..." comment may now contain a mixture of expectations for
diagnostics and facts. Diagnostics are indicated by a string literal,
as before. Facts are indicated by name:"regexp" where name identifies
the object (declared on the same line) with which the fact is
associated.
func neverReturns() { // want neverReturns:"noReturn"
for {}
}
Also:
- analysistest: report errors during package loading.
(We don't yet have a way to test RunDespiteErrors Analyzers in the
face of errors.)
- tests for Facts produced by findcall and pkgfacts.
(Findcall now produces facts just for testing.)
- Add String method to various Fact types.
Should the Fact interface have this method?
Change-Id: Ifa15fbd49d6ec3042b5fe9d3ebf22f4bdfdc8769
Reviewed-on: https://go-review.googlesource.com/139157
Reviewed-by: Michael Matloob <matloob@golang.org>
Some users may set GO111MODULE=on, and we will eventually want to be able to
build x/tools itself in module mode.
Updates golang/go#27858
Updates golang/go#27852
Change-Id: Iaf488b2a89e6526471530245cb580f1f0391a770
Reviewed-on: https://go-review.googlesource.com/137815
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Some applications (for example, diagnostics shown to a user in an
editor) may want to distinguish between errors generated by
the driver, parser, and type-checker. The Error struct did not have any
mechanism for doing this, so add an ErrorKind field and set it in
appendError.
Change-Id: If347163225d1e3a567e98610e9ba8a0930e4659c
Reviewed-on: https://go-review.googlesource.com/139317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
TestLoadImportsC just works on Go 1.10. And there's no good reason
that TestLoadAllSyntaxImportErrors shouldn't work, even though
it's currently always skipped.
Change-Id: Icd8d311f12c5731cc635937a00251eab0a3077ec
Reviewed-on: https://go-review.googlesource.com/139117
Reviewed-by: Alan Donovan <adonovan@google.com>
A previous change (golang.org/cl/137096) that made absolute package paths relative to
GOROOT or GOPATH entries also accidentally applied to .go filesnames.
Filter those out of the list of paths considered to make relative to
search path.
(package paths that don't start with './' or '/' are relative to GOROOT or GOPATH,
but filenames are not.)
Change-Id: I67fbd0e5caa7e53f3ab5b77f55d6841fe2132578
Reviewed-on: https://go-review.googlesource.com/c/138880
Reviewed-by: Alan Donovan <adonovan@google.com>
Heschi Kreinick
Alan Donovan
Ian Cottrell
Michael Matloob
Bryan C. Mills
Rebecca Stambler