The imports package's public API is build.Default, but that doesn't mean
we need to use it in the internal implementation or the tests. Now we
have a new type, fixEnv, that contains everything relevant from
build.Context, as well as the various global vars that were only used
for testing.
Don't worry too much about the new function parameters; they mostly
move into the resolvers in the next CL.
Refactoring only; no user-visible changes intended.
Change-Id: I0d4c904955c5854dcdf904009cb3413c734baf88
Reviewed-on: https://go-review.googlesource.com/c/158437
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Redesign fixImports to have a clearer workflow, and hopefully create
clear places to plug in go/packages. This change is mostly
performance/functionality neutral, but does clean up some corner cases.
The new flow centers around the pass type, which encapsulates the
process of loading information about the current code, adding possible
new imports, and trying to apply them. I'm hoping that it's easy to
understand what's happening just by reading fixImports, and that new
sources of information (e.g. a network service) fit well into that flow.
Where possible, I left the functions near where they were extracted in
hopes of making review easier, but it's probably not going to be easy.
Sorry. I might move them into a more reasonable order in a followup CL.
Notable modifications:
- The stdlib cache is restructured to match pass' internal storage.
- Sibling imports with conflicting names are considered.
- Package name lookups are batched, hopefully making it easier to plug
in go/packages.
Questions that might be worth answering:
- Should findImportGoPath really scan $GOROOT? Unless the user is working
on a development copy, it's totally redundant with the cache.
- What is the best way to combine candidates from multiple sources?
Right now the first one wins, and findStdlibCandidates relies on that
to get crypto/rand ahead of math/rand.
- In the third pass, should it assume sibling imports or should it
actually go load the exports? It didn't load them before, but that seems
arbitrary.
Change-Id: Ie4ad0b69bfbe9b16883f2b0517b1278575c9f540
Reviewed-on: https://go-review.googlesource.com/c/150339
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The comment of Process() implies src can be nil, but it didn't handle
nil src correctly before because parse() doesn't expect nil src.
Passing []byte(nil) to parser.ParseFile() results in error and parse()
tries to parse again by modifying src if src is statement list.
This problem isn't related with cmd/goimports because goimports doesn't
pass []byte(nil) to Process() as src.
Fixesgolang/go#19676
Change-Id: Idbaa6646c3907690763eabc798860c56bb9963d4
Reviewed-on: https://go-review.googlesource.com/38613
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previous work to resolvegolang/go#18201 increased the maximum line
length that goimports could handle to 1MiB (CL83800), but generated code
can result in Go files with longer lines. Use a bufio.Reader instead of
a bufio.Scanner to support arbitrarily long lines, as permitted by the
Go spec.
Change-Id: If719e531859804304d60a8c00db6304ab3d5fe5e
Reviewed-on: https://go-review.googlesource.com/93439
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change set fixes the issue by specifying a max buffer size of ~1 megabyte for Scanner.
Previously the max was not set resulting in the default max which is ~64k. This increased max should not increase
memory use for smaller, normal files because it is a max and the code expands the buffer as needed for large tokens.
The change set includes an additional change to return an error from the Scanner which was ignored previously.
Fixesgolang/go#18201
Change-Id: I11be39af74d5eb3b353ad81ba1cb5404207aa65d
Reviewed-on: https://go-review.googlesource.com/83800
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change remove an unnecessary string conversion of
a variable that is already a string.
Change-Id: I64d9a6c059276fa22f1be9f2dba02bbeb63cb8fb
Reviewed-on: https://go-review.googlesource.com/35560
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Editor modes that invoke the goimports command on temporary copies
of actual source files will need to invoke goimports -srcdir now to say
where the real source directory is. Otherwise goimports will not consider
vendored or internal packages when looking for new imports.
In lieu of a test for cmd/goimports (because it has no tests),
a command transcript:
$ cd /tmp
$ cat x.go
package p
var _ = hpack.HuffmanDecode
$
$ GOPATH= goimports < x.go
package p
var _ = hpack.HuffmanDecode
$ GOPATH= goimports x.go
package p
var _ = hpack.HuffmanDecode
$
But with the new flag:
$ GOPATH= goimports -srcdir $GOROOT/src/math < x.go
package p
import "golang.org/x/net/http2/hpack"
var _ = hpack.HuffmanDecode
$ GOPATH= goimports -srcdir $GOROOT/src/math x.go
package p
import "golang.org/x/net/http2/hpack"
var _ = hpack.HuffmanDecode
$
The tests in this CL and the above transcript assume that
$GOROOT/src/vendor/golang.org/x/net/http2/hpack exists.
It did in 40a26c9, but it does not today.
It will again soon (once Go 1.7 opens).
For golang/go#12278 (original request).
Change-Id: I27b136041f54edcde4bf474215b48ebb0417f34d
Reviewed-on: https://go-review.googlesource.com/17728
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Rewrite performed with this command:
sed -i '' 's_code.google.com/p/go\._golang.org/x/_g' \
$(grep -lr 'code.google.com/p/go.' *)
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/170920043
Fixes 7866. Adds a test case.
When trying to add newlines before certain imports via text manipulation,
a regex is used to iterate over all imports. The regex failed to match
dot imports because \w doesn't match a literal dot. This changes the regex
to accept a dot as well.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/99400043
Assuming:
1) package declaration does not exist
2) the Fragment option is set
3) a main function exists
We will assume it is a main package and add the declaration.
This change also sets the Fragment option in goimports.
LGTM=crawshaw, bradfitz
R=bradfitz, crawshaw
CC=golang-codereviews
https://golang.org/cl/96850044
Because goimports also sorts stdlib-vs-external differently
(whereas gofmt just sorts string-wise), we need to put a blank
line between imports of different classes. This happened in
some cases, but not all.
Fixesgolang/go#7132
LGTM=kamil.kisiel
R=golang-codereviews, kamil.kisiel
CC=golang-codereviews
https://golang.org/cl/60870047
