We plan to reuse goimports' directory walking logic in the
implementation of go/packages. To prepare for that, refactor it to have
fewer global variables and a simpler interface.
This CL makes no functional changes, but may change performance
slightly. It always scans both $GOPATH and $GOROOT, and does so
serially. I expect that fastwalk's internal parallelism is enough to
keep the disk busy, and I don't think it's worth optimizing for people
hacking on Go itself.
Change-Id: Id797e1b8e31d52e2eae07b42761ac136689cec32
Reviewed-on: https://go-review.googlesource.com/c/135678
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing implementation detects import groups and tryies to
sort/regroup only the last one. Ignore existing grouping and
applying the sort function to all imports fix this.
Fixesgolang/go#20818
Change-Id: I5db46c6dc8fabd9299b79349880994be5c1b8195
Reviewed-on: https://go-review.googlesource.com/c/116795
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
The corpus needs to be init'd before generating the index.
Regression caused by CL 141397.
Updates golang/go#25443.
Change-Id: I04d19bf5d279031cc8ddf0f850d81eb06fc5efa8
Reviewed-on: https://go-review.googlesource.com/c/141717
Run-TryBot: Chris Broadfoot <cbro@golang.org>
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>
The several functions not exported, Use "The xxx function ..." rule.
Change-Id: Iaba2490b4a9675213f70d055bd9ded9e5a245aa5
Reviewed-on: https://go-review.googlesource.com/c/47872
Reviewed-by: Alan Donovan <adonovan@google.com>
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>
httptest assumes it's running a test, it registers its own flags, which
means godoc ends up with mysterious flags.
By implement an http.ResponseWriter, we do not need to use httptest.
Fixesgolang/go#28138
Change-Id: Ia0de8597c3edb0e7bdea6d8b3b2f1618a12f9239
Reviewed-on: https://go-review.googlesource.com/c/141417
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
- This shaves off 12.8KB (unzipped) / 3.8KB (zipped)
downloadable data and around 6ms of javascript parse time
when the files are not injected.
- Also, conditionally rendering the callgraph html only when
callgraph is actually present.
- Removed the "text/javascript" attribute as it is the default
value now.
Change-Id: I70c030bdda2a054c181b2b3a693842bafed88d1a
Reviewed-on: https://go-review.googlesource.com/c/115875
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The input[type="search"] has a pseudo element,
it's ::-webkit-search-decoration and has an original margin.
To get rid of the margin, this commit specifies -webkit-appearance
property to none.
Fixesgolang/go#27210
Change-Id: I09bbae5e0b811a3b3fa3a047b97ea45190e75f46
Reviewed-on: https://go-review.googlesource.com/c/131317
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
godoc is now just a webserver (the one that runs golang.org)
A future change might rename this to something else, but not now.
Fixesgolang/go#25443
Change-Id: Ib95d0589e13f4c7a717ff28d694f1472e427eef4
Reviewed-on: https://go-review.googlesource.com/c/141397
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The code as written would use JavaScript to show the "Play" button in
the menu when the Playground is enabled, making the button appear
after the page first renders, causing a visible "bounce" in the menu.
This change checks the condition (whether this is not the front page, or
"wide") in the template code, instead of with JavaScript after the page
has loaded.
Change-Id: Iccf0a344116372f3207158793be8c7734e67a4ae
Reviewed-on: https://go-review.googlesource.com/c/140798
Reviewed-by: Andrew Bonventre <andybons@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>
Without changing the behavior of the present command for local
usage (using the local socket for running examples, defaulting to
the current directory for all content). Add flags and set them to
the appropriate values if running on App Engine.
Notably, since the Go files must be in the same directory as
app.yaml, the content root must be ./content/ to avoid listing
the present source files.
It also defaults to running example snippets via the HTTPTransport
(https://play.golang.org/compile) instead of locally when on App
Engine.
There are also some small cleanup code changes.
Update golang/go#28080
Change-Id: I40bb7923107614f88d2bfdffd34a824d4bacb3a1
Reviewed-on: https://go-review.googlesource.com/c/140841
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
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 moves the ARG declarations immediately before they're used to
make this omission less likely for future additions.
Change-Id: Id52a936d978f96d3c629feff69fc9dc4ae1b8463
Reviewed-on: https://go-review.googlesource.com/c/140377
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Calling golang.org/{compile|share} just forwards it to
play.golang.org/{compile|share}. Avoid the extra jump for the
request.
Change-Id: I95e8e79fe4623346663d0c11204f196c75716177
Reviewed-on: https://go-review.googlesource.com/c/140839
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Currently, when a POST request is made using code with build tag
+build !appengine it doesn't set the content-type header passed
to the function. This was breaking the case where a url-encoded
body (a code snippet's body and protocol version) was being POSTed
to golang.org/compile since it never set the content-type of the
request and the corresponding form values parsed out were empty as
a result.
Update golang/go#28080
Change-Id: I677ca01b2f5aecedbd13d1faa7a838ddc0199244
Reviewed-on: https://go-review.googlesource.com/c/140838
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@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>
Adds version information for package docs for the production version of
godoc running on golang.org.
Updates golang/go#5778.
Change-Id: I8b56e8152b20b34104f274263a6c0b5a0180093b
Reviewed-on: https://go-review.googlesource.com/c/139557
Reviewed-by: Devon H. O'Dell <devon.odell@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
Deploys no longer depend on Docker.
With only Make and gcloud installed, the following should deploy a new version:
$ git clone https://go.googlesource.com/tools
$ cd tools
$ cd cmd/godoc
$ make cloud-build deploy
Updates golang/go#27205.
Change-Id: I5cc1142e02dc288450d55dbd4da4b30c0a080bd5
Reviewed-on: https://go-review.googlesource.com/c/139240
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also rename `make build` and `make push` to `make docker-build` and
`make docker-push` in preparation to introduce Cloud Build (removing
the dependency on Docker).
Updates golang/go#27205.
Change-Id: Iae19b9a6f77d09246a1332c7ec9eceec449cdba8
Reviewed-on: https://go-review.googlesource.com/c/139239
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
