1
0
mirror of https://github.com/golang/go synced 2024-11-18 21:44:45 -07:00
Commit Graph

80 Commits

Author SHA1 Message Date
Heschi Kreinick
9c309ee22f imports: stop using go/packages for modules
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>
2019-01-22 20:29:12 +00:00
Heschi Kreinick
16909d206f imports: remove globals, stop using build.Default
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>
2019-01-18 19:33:59 +00:00
Heschi Kreinick
8a60511975 imports: don't look for, or find, empty packages
Fix a pair of bugs that combined to cause golang/go#29520. First, don't go
looking for a package if we've only seen unexported identifiers selected
from it. It's probably a typo. Second, don't find packages with no files
in them, e.g. because they're all build tagged out. We can't know what
package they form, so we have no business considering them.

Test only for the first, since without the first bug the second has no
observable effect on behavior, and I don't want to test the private API.

Fixes golang/go#29520

Change-Id: I5b797940bec051be5945b9c5cb4e7bf28527a939
Reviewed-on: https://go-review.googlesource.com/c/156178
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-01-03 20:59:43 +00:00
Heschi Kreinick
d4971274fe imports: don't remove imports that conflict with globals
When a package contains both a var and an import that produce the same
identifier, the compiler will complain. I had thought that it made sense
to remove the redundant imports in that case. However, we can't reliably
tell whether a global from one file is in scope in another file. Most
notably, having multiple main packages in the same directory is pretty
common, and if someone declares a var in one that matches an import in
another, we don't want to remove the import.

Change-Id: I49f58fccdb8a8542ec85cf4d80d3e20d3159d2c7
Reviewed-on: https://go-review.googlesource.com/c/154740
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2018-12-18 20:40:10 +00:00
Heschi Kreinick
bbccd8cae4 imports: use go/packages, support modules
When in module mode (go env GOMOD is nonempty), use go/packages to load
package information. This adds module support, without slowing down
non-module mode. Functionality should be the same in both cases.

Since we intend to use go/packages for everything eventually, the tests
run in three modes: GOPATH, using the old code paths; GOPATH_GoPackages,
using go/packages on a GOPATH layout; and Modules, using go/packages on
a module layout. They do take a little while to run.

The code path for go/packages is a lot more convoluted than it should be,
since it mixes in and out of the older path. I expect we'll clean that
up after deleting the old one.

Change-Id: I5b54bd470e091e6fb429597dfbfd1c8fa03f5b59
Reviewed-on: https://go-review.googlesource.com/c/152317
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>
2018-12-13 19:03:29 +00:00
Heschi Kreinick
5f4a60f04f imports: fix renamed sibling imports more
This is the fix I should have made in CL 153440. What I get for being in
a rush.

Fixes golang/go#29180

Change-Id: I7ee3e26173b86c70574b9710f84094e46db27a37
Reviewed-on: https://go-review.googlesource.com/c/153859
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2018-12-12 18:25:43 +00:00
Heschi Kreinick
6a3e9aa2ab imports: fix renamed sibling imports
Fix some logic errors around renamed sibling imports. The one in
findMissingImport was just a silly mistake, the one in addCandidate
was because I hadn't thought about renamed imports.

Change-Id: Iecd0b4e6151bff7b8cb6ad21065f017fb1245bfd
Reviewed-on: https://go-review.googlesource.com/c/153440
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-12-10 22:52:55 +00:00
Heschi Kreinick
4c53570e04 imports: ignore globals in different packages
Some people put multiple packages' files in the same directory, most
commonly commands and code generators. Global variables from one package
should be ignored in other packages.

Change-Id: I9a5d27778570183dfe391dd3273dfa8277a29bf2
Reviewed-on: https://go-review.googlesource.com/c/153419
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-12-10 19:38:48 +00:00
Heschi Kreinick
ee45598d2f imports: create named imports for name/path mismatches (again)
For clarity and performance reasons, we want the fast path of goimports
to be purely syntactic. Packages whose import paths don't match their
package names make this harder. Before this CL, we parsed each imported
package to get its real package name. Now, we make named imports for
such packages, and on subsequent runs we don't have to do the extra
work.

A package name matches its import path if the name is the last segment
of the path, or the next-to-last before a version suffix vNN. gopkg.in
style .vNN suffixes are considered mismatching.

The previous attempt (CL 145699) failed because I assumed we'd be able
to find all packages from scratch. That's not true if the import path is
completely unrelated to the package name. To avoid that problem, rather
than removing and re-adding mismatched imports, we just literally add
names to them.

Fixes golang/go#28428

Change-Id: I93bd820f5956162ae9c99afa73bdcfc570a2e7b4
Reviewed-on: https://go-review.googlesource.com/c/152000
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-12-03 18:35:04 +00:00
Heschi Kreinick
a28dfb48e0 revert "imports: create named imports for name/path mismatches"
This reverts CL 145699 (commit 864069cfd1)

Reason for revert: If the mismatch is bad enough that goimports can't find
it again, then the import is just removed, even if the user adds it back again.

Fixes #28645.

Change-Id: I6c8fc5434c2b56c73b260bcec2c12d8746fac4ad
Reviewed-on: https://go-review.googlesource.com/c/148237
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-07 22:50:58 +00:00
Heschi Kreinick
864069cfd1 imports: create named imports for name/path mismatches
For clarity and performance reasons, we want the fast path of goimports
to be purely syntactic. Packages whose import paths don't match their
package names make this harder. Before this CL, we parsed each imported
package to get its real package name. Now, we make named imports for
such packages, and on subsequent runs we don't have to do the extra
work.

A package name matches its import path if the name is the last segment
of the path, or the next-to-last before a version suffix vNN. gopkg.in
style .vNN suffixes are considered mismatching.

goimports already had almost exactly the desired logic, but only when
adding a new import. So the bulk of this change is simply removing the
logic that allowed it to recognize that a mismatched import satisfied
some uses. With that gone, it will remove those imports as unused, then
add a new renamed import. Some comments may be destroyed.

Fixes golang/go#28428

Change-Id: I53846e6046affb420f41719f84c71086c5b9e5e6
Reviewed-on: https://go-review.googlesource.com/c/145699
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-07 19:53:05 +00:00
Heschi Kreinick
a0a13e073c imports: make tests compatible with modules
Tweak the goimports tests to be more module-compatible. This should be
the last big change to the tests; they all pass with the new
implementation.

The primary change is to avoid using packages not provided by the test's
modules. Other modules will be downloaded at test time, which is
nonhermetic and quite slow.

Other miscellanea:

- The appengine grouping tests have to be split out so
they can be GOPATH-only, because modules have to have dots in their
names.
- The tests for .goimportsignore and node_modules are GOPATH-only
because we decided not to include those behaviors in go/packages in
module mode.
- Some vendoring tests are GOPATH-only because vendoring is not a thing
in module mode.
- TestFindImportInLocalGoFiles changes content, because the existing
test was incorrect: bogus.net/bytes was a viable candidate even though
it isn't on disk. I'm not sure why it wasn't flaky.

Change-Id: I35a3aac92d3fb7f70a1a8f027f0b423282420a4d
Reviewed-on: https://go-review.googlesource.com/c/145138
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-10-30 00:07:16 +00:00
Heschi Kreinick
e74f1bd585 imports: port tests to packagestest
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>
2018-10-26 18:23:19 +00:00
Brad Fitzpatrick
a2dc47679d Revert "imports: support repairing import grouping/ordering"
This reverts commit CL 116795 12a7c317e8.

Reason for revert: mangles comments in imports. See comments
on issue golang/go#20818.

Updates golang/go#20818

Change-Id: Iff82f8dc310dceb982b48d82b26176ea279fef10
Reviewed-on: https://go-review.googlesource.com/c/144339
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-10-24 17:12:08 +00:00
Heschi Kreinick
bf693efdb3 imports: make tests use a helper, read from disk more
Replace the repeated Process/err check/output block with a test helper
eliminating a lot of copy/pasting. Change many tests to get their input
from disk rather than the argument to Process; this is mostly me being
selfish: go/packages doesn't support creating new packages with overlays
and that breaks a lot of these tests.

Change-Id: Ib1b7908c3d86d17323a94f1ee4ce074e264327b4
Reviewed-on: https://go-review.googlesource.com/c/141680
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-10-16 19:29:16 +00:00
Heschi Kreinick
d1d6d0cbb6 imports: refactor tests
We plan to change goimports to use go/packages, which requires changing
its internal design. Having tests use the external interface makes those
changes easier. After this change almost all testing is through Process.

Broadly speaking, the changes are:

- Switch to subtests wherever possible. This involved making up many
names, which I hope are accurate.

- Convert tests that used findImport directly to use Process instead.
This often made them slightly larger but not unduly IMO.

- Replace simple tests with entries in the giant table at the top.

- Remove uses of custom goroots, which are troublesome for
go/packages' use of the go command. Almost none of them were actually
necessary. I left one in TestGoRootPrefixOfGoPath; I'm not sure how
to handle it yet.

Change-Id: I7b810750f72842b58223f102097ccbb51b82bf39
Reviewed-on: https://go-review.googlesource.com/c/140840
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-10-12 19:02:34 +00:00
Heschi Kreinick
19e2aca3fd internal/gopathwalk: create
Extract goimports' logic for walking Go source directories into a
separate package, suitable for use in go/packages. No functional
changes. Added a convenience feature to fastwalk, allowing the user to
say that they're done with a directory and stop receiving callbacks for
it.

Testing is a little light; I expect goimports' tests to cover most
everything we care about.

Change-Id: If047ada4414f5f282637d11fd07e8342fadc9c33
Reviewed-on: https://go-review.googlesource.com/c/138877
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-10-12 18:13:39 +00:00
Heschi Kreinick
1d07bcb7f9 imports: refactor directory walking
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>
2018-10-12 17:53:56 +00:00
Serhii Aheienko
12a7c317e8 imports: support repairing import grouping/ordering
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.

Fixes golang/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>
2018-10-12 15:41:53 +00:00
Rebecca Stambler
cf99646d84 imports: stub out dirPackageInfo function in tests
TestFixImports did not have to explicitly add "strings" to the
simplePkgs map - this only happened because sibling files were searched
and the strings import was found through them. If we stub out the
dirPackageInfo function, we have to explicitly add the strings import.

Change-Id: I149e522fad37039fc790e6a855834d86ff050674
Reviewed-on: https://go-review.googlesource.com/129036
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2018-08-10 19:55:50 +00:00
LE Manh Cuong
8cb83b71b4 imports: fix lost line between package and import statement
Fixes golang/go#26290

Change-Id: Ide797a46bf1d0d6070940fb5b9db3e76502bc528
Reviewed-on: https://go-review.googlesource.com/122736
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-10 02:38:27 +00:00
LE Manh Cuong
d600f31f81 imports: fix unexpected blank line after package comment
The fix in golang/go#23709 introduced a separate bug where extra blank
lines were sometimes inserted. This fixes that newly introduced bug.

Fixes golang/go#26246

Change-Id: I78131cc1d01ae246922ed9e4336ebb31d1c6cfa1
Reviewed-on: https://go-review.googlesource.com/122538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-09 17:31:08 +00:00
Russ Cox
893c2b1ff5 imports: assume import x/y/v2 is package y
This will be true in general for Go modules, so it's the right fallback.
Note that if the package can be found in GOPATH, the code still
uses the actual package name from GOPATH, so this only changes
the fallback path. The fallback path is what currently executes
when using modules (because they are not in GOPATH).

Change-Id: I3d48517583eae9431e139371d363ce354c89340a
Reviewed-on: https://go-review.googlesource.com/122616
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-09 16:22:11 +00:00
LE Manh Cuong
b23eb6252f imports: fixup comments on import lines correctly
The current implementation uses the added import specs EndPos to fixup
the comments position after import specs is sorted. If two or more
import specs have the same EndPos, a comment associated with one of them
is always added to the last import spec.

This commit uses the current import spec position to compute new
position for next import spec. So there is never two or more specs have
the same EndPos.

Fixes golang/go#23709

Change-Id: I60ace9431d871e94a2b3d90892aa80d0671aeea0
Reviewed-on: https://go-review.googlesource.com/121878
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-02 20:27:37 +00:00
Ian Lance Taylor
f96157268c imports: fix test to work with gccgo
Change-Id: I382905b7b7bd8189c169048ba05080ee96e1da5b
Reviewed-on: https://go-review.googlesource.com/117995
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-06-11 19:17:03 +00:00
Bryan C. Mills
165bdd618e imports: fix races in findImportGoPath
Before this change, findImportGoPath used a field within the
(otherwise read-only) structs in the dirScan map to cache the distance
from the importing package to the candidate package to be imported. As
a result, the top-level imports.Process function was not safe to call
concurrently: one goroutine could overwrite the distances while
another was attempting to sort by them. Furthermore, there were some
internal write-after-write races (writing the same cached distance to
the same address) that otherwise violate the Go memory model.

This change fixes those races, simplifies the concurrency patterns,
and clarifies goroutine lifetimes. The functions in the imports
package now wait for the goroutines they spawn to finish before
returning, eliminating the need for an awkward test-only mutex that
could otherwise mask real races in the production code paths.

See also:
https://golang.org/wiki/CodeReviewComments#goroutine-lifetimes
https://golang.org/wiki/CodeReviewComments#synchronous-functions

Fixes golang/go#25030.

Change-Id: I8fec735e0d4ff7abab406dea9d0c11d1bd93d775
Reviewed-on: https://go-review.googlesource.com/109156
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-04-30 20:17:55 +00:00
haya14busa
1c0c7a8cce imports: Process should accept nil src
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.

Fixes golang/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>
2018-04-12 23:47:31 +00:00
Nick Miyake
c41d143952 imports: support match without trailing slash for -local flag
Make it so that import prefixes specified using the -local flag are considered
a match for an import path if the prefix ends with a '/' and the import path
matches exactly the prefix without a slash. For example, specifying
"golang.org/x/tools/" as a prefix would match the import for the package
"golang.org/x/tools".

Fixes golang/go#24368

Change-Id: I0302db72fda63ad24d7b964aa73f78aa0ebccb37
Reviewed-on: https://go-review.googlesource.com/100460
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-03-15 06:52:34 +00:00
Ian Cottrell
c1547a3f90 imports: Limit the amount of text printed in the large token case
If the test fails, it tries to print a million characters.
This limits it to just the start of got and want, which is enough to see
the imports block anyway.

Change-Id: I2c58db8e96e73da436ca16fa8a57c820a95242ca
Reviewed-on: https://go-review.googlesource.com/100216
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-03-13 18:49:02 +00:00
Nishanth Shanmugham
059bec968c cmd/goimports: support multiple comma-separated imports in -local flag
In cmd/goimports, allow for the -local flag to accept a comma-separated
list of import path prefixes. Also, update the imports package
accordingly to support this.

Fixes golang/go#19188

Change-Id: I083d584df8c3a77532f0f66e9c5d970960180e0d
Reviewed-on: https://go-review.googlesource.com/85397
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-03-05 19:26:41 +00:00
Antonio Antelo
6c27c68f27 imports: fix mangled comments after package clause insertion
Fixes golang/go#12097

Change-Id: Ie6a6aa997e89700e49d703b7fd00f515b03ad6f8
Reviewed-on: https://go-review.googlesource.com/93235
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-14 03:58:58 +00:00
Daniel Martí
90b807ada4 all: fix a few issues found by unparam
In cmd/present, a mode was being passed to the function parse, but it
wasn't actually being used. Use it.

In go/ssa, checkFinalInstr received an idx integer but it doesn't
actually need it. Get rid of it.

Lastly, in imports, findImportStdlib always returned rename==false. Get
rid of that result parameter.

Change-Id: I719006b69ee80a3ef4b0ea24c1c206016a7e304b
Reviewed-on: https://go-review.googlesource.com/93596
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-13 22:52:51 +00:00
Nikhil Benesch
c0251d31d2 imports: handle arbitrarily long lines
Previous work to resolve golang/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>
2018-02-13 22:26:52 +00:00
Michael Fraenkel
8cab8a1319 imports: sibling imports must have matching references
When selecting a sibling's import, the unresolved reference must have
been also used otherwise use the normal search to determine the best
possible package to import.

Fixes golang/go#23001

Change-Id: I38a983569991464970ad5921fe7f280dd3e35a2c
Reviewed-on: https://go-review.googlesource.com/82875
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2018-01-05 20:15:57 +00:00
Steve Gilbert
96b5a5404f imports: support files with longer lines
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.

Fixes golang/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>
2018-01-03 17:47:52 +00:00
Michael Fraenkel
9c57063f67 imports: prioritize closer packages
Prefer imports that are closer to the current package.

Fixes golang/go#17557

Change-Id: Iec55a294d396feac6234be307e08608b8559f65c
Reviewed-on: https://go-review.googlesource.com/37070
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-13 21:23:40 +00:00
haya14busa
84a35ef54d go/ast/astutil: new third-party imports shouldn't go in the std group
Before this change, astutil would only do a prefix match of a new import
with all the existing ones, to try to place it in the correct group. If
none was found, the new import would be placed at the beginning of the
first import group.

This works well for new std imports, but it doesn't work well for new
third-party packages that don't share any prefix with any of the
existing imports.

Example:

	import (
		"time"

		"github.com/golang/snappy"
	)

When adding "golang.org/x/sys/unix" with astutil.AddImport, the import
is inserted as follows:

	import (
		"golang.org/x/sys/unix"
		"time"

		"github.com/golang/snappy"
	)

And goimports reorganizes the imports to separate std and third-party
packages:

	import (
		"time"

		"golang.org/x/sys/unix"

		"github.com/golang/snappy"
	)

We usually don't want to introduce a new import group; in most cases,
the desired behavior is separating std from third-party packages.

With this CL, new imports that don't share prefix with any existing ones
will be placed with the first group of third-party imports, if any
exist. If no third-party import group exists, a new one will be added.
In the case of our example above, this will be the new outcome:

	import (
		"time"

		"github.com/golang/snappy"
		"golang.org/x/sys/unix"
	)

Fixes golang/go#19190.

Change-Id: Id4630015c029bd815234a6c8726cb97f4af16f1c
Reviewed-on: https://go-review.googlesource.com/37552
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2017-08-15 00:39:00 +00:00
Brad Fitzpatrick
6e57528ade imports: fix reading from stdin on Windows
Don't panic when reading from stdin on Windows. This is a regression
from https://golang.org/cl/43454

Also fix some weird behavior with stdin processing I noticed during
reviewing the code: don't allow the -w (write) flag, and adust the
filename shown with the -d (diff) flag.

Fixes golang/go#20941

Change-Id: I73d0a1dc74c919238a3bb72823585bbf1b7daba1
Reviewed-on: https://go-review.googlesource.com/47810
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Jones <rbjones@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-07-07 17:20:05 +00:00
Brad Jones
e011c1062a imports: prefer paths imported by sibling files.
Adds an Imports field to packageInfo with the imports used by sibling
files, and uses it preferentially if it matches a missing import.

Example: if foo/foo.go imports "local/log", it's a reasonable assumption
that foo/bar.go will also want "local/log" instead of "log".

Change-Id: Ifb504ed5e00ff18459f19d8598cc2c94099ae563
Reviewed-on: https://go-review.googlesource.com/43454
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-06-09 17:18:19 +00:00
Brad Fitzpatrick
f60b69ed8c imports: fix symlink loop detection, deflaking TestImportSymlinks
Change the shouldTraverse function to no longer keep a global map of
which inodes it's seen. Instead, whenever a symlink is seen for a path
name, check every directory entry in that path name and see if any are
the same inode as the current one, detecting any loop just from the
name itself.

More details of why the test was flaky are in the bug.

Fixes golang/go#18142

Change-Id: I869f7a13d130c63d78b7af81802a16c4b4b2f3bd
Reviewed-on: https://go-review.googlesource.com/37947
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-03-08 23:27:09 +00:00
haya14busa
8524ce5143 imports, go/ast/astutil: do not make grouped imports non-grouped when removing
import (
		"fmt"
		"strings"
	)

When deleting "fmt" import statement, code should be converted to the
following code.

	import (
		"strings"
	)

Instead of

	import "strings"

Diff becomes nicer by this change and it avoids that rewriting grouped
imports non-grouped may result in confusion comments.

Example:

	// comment 1
	import (
		"fmt"
	        // comment 2
		"strings"
	)

should be

	// comment 1
	import (
	        // comment 2
		"strings"
	)

instead of

	// comment 1
        // comment 2
	import "strings"

Fixes golang/go#18051

Change-Id: I3c07b70b657191eacf83c3197a965e587286c950
Reviewed-on: https://go-review.googlesource.com/36853
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-02-13 18:56:44 +00:00
Chris Stockton
4c6345e8dc imports: respect ignoredDirs for symlinks
Fixes golang/go#17916

Added tests.

Change-Id: Ie44e4bcbec267b6c16249336c5d48bae86acc2b5
Reviewed-on: https://go-review.googlesource.com/33246
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-11-23 02:24:47 +00:00
Brian Kennedy
62f0028207 imports: properly handle global variables when creating imports
Fixes golang/go#7463

Change-Id: Ib8b511a34c562a2b3cdce6bb143caa7cf453bd00
Reviewed-on: https://go-review.googlesource.com/23444
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2016-08-11 06:06:14 +00:00
Brad Fitzpatrick
1c6f639aae imports: don't ignore GOPATH if GOROOT is a prefix of GOPATH
Fixes golang/go#16458

Change-Id: I1aaec5d115dd703dd702101f6bec37bf8b02a73d
Reviewed-on: https://go-review.googlesource.com/25192
Reviewed-by: Andrew Gerrand <adg@golang.org>
2016-07-26 13:45:23 +00:00
Keith Rarick
ed69e84b15 cmd/goimports, imports: add -local flag
This allows the caller to indicate they want certain
import paths to sort into another group after 3rd-party
imports when added by goimports. For example, running
'goimports -local example.com/' might produce

    import (
        "database/sql"
        "io"
        "strconv"

        "golang.org/x/net/context"

        "example.com/foo/bar"
        "example.com/foo/baz"
    )

Resolves golang/go#12420

Change-Id: If6d88599f6cca2f102313bce95ba6ac46ffec1fe
Reviewed-on: https://go-review.googlesource.com/25145
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-07-23 01:23:54 +00:00
Alex Brainman
682b241254 imports: make filepath.Rel work on windows
filepath.Rel will always fail if one parameter starts with
drive letter and the other does not. Make both filepath.Rel
parameters absolute paths, to give it chance to succeed.

Fixes broken tools build on windows.

Change-Id: Ibefcfe283f28977503323f01bc3a698478227f84
Reviewed-on: https://go-review.googlesource.com/25120
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-07-21 14:33:34 +00:00
Brad Fitzpatrick
d4a8e583a1 imports: skip "node_modules" directories
Updates golang/go#16417

Change-Id: Ia4a5f997036274d09cca2ff10e500e403c1725ba
Reviewed-on: https://go-review.googlesource.com/25044
Reviewed-by: Andrew Gerrand <adg@golang.org>
2016-07-19 16:05:27 +00:00
Brad Fitzpatrick
edf8e6fef8 cmd/goimports, imports: optimize directory scanning and other things
This brings goimports from 160ms to 100ms on my laptop, and under 50ms
on my Linux machine.

Using cmd/trace, I noticed that filepath.Walk is inherently slow.
See https://golang.org/issue/16399 for details.

Instead, this CL introduces a new (private) filepath.Walk
implementation, optimized for speed and avoiding unnecessary work.

In addition to avoid an Lstat per file, it also reads directories
concurrently. The old goimports code did that too, but now that logic
is removed from goimports and the code is simplified.

This also adds some profiling command line flags to goimports that I
found useful.

Updates golang/go#16367 (goimports is slow)
Updates golang/go#16399 (filepath.Walk is slow)

Change-Id: I708d570cbaad3fa9ad75a12054f5a932ee159b84
Reviewed-on: https://go-review.googlesource.com/25001
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-07-19 03:21:45 +00:00
Brad Fitzpatrick
caebc7a51c imports: ignore case and hyphens when lexically filtering candidate dirs
This means a directory /gopath/src/foo/Go-Bar is considered as maybe
containing package "bar" or even "gobar".

Also, more tests.

Fixes golang/go#16402

Change-Id: I14208d738e3a081cb6d9bcd83d777280e118f4e7
Reviewed-on: https://go-review.googlesource.com/25030
Reviewed-by: Andrew Gerrand <adg@golang.org>
2016-07-18 23:38:47 +00:00
Brad Fitzpatrick
29481a356d imports: skip test on plan9
Change-Id: I608a1ba47a049b7eb5f4ded481744d8b25e68800
Reviewed-on: https://go-review.googlesource.com/24988
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2016-07-17 16:31:09 +00:00