1
0
mirror of https://github.com/golang/go synced 2024-10-01 03:18:33 -06:00
Commit Graph

65 Commits

Author SHA1 Message Date
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
Josh Bleecher Snyder
6d32be89d5 imports: minor fixes
These are mostly comments for CL 24941 that didn't
get sent quickly enough.

strings.Trim(`\"`) works, but by accident.
It trims all leading and trailing "s and \s,
but there are never leading or trailing \s.

Semantic line breaks and punctuation cleanup.

The reflow of comments in the pkgName == "main" if
block is to silence this spurious vet failure:

fix.go:247: +build comment must appear before package clause and be followed by a blank line

Plain sync.Once values are enough.

Change-Id: I241f3025031b6f21605da78ea52066713a203327
Reviewed-on: https://go-review.googlesource.com/24983
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-07-16 22:16:59 +00:00
Brad Fitzpatrick
ffe4e61c64 imports: add configuration mechanism to exclude directories
Each $GOPATH entry may have a file $GOPATH/src/.goimportsignore which
may contain blank lines, #comment lines, or lines naming a directory
relative to the configuration file to ignore when scanning.  No
globbing or regex patterns are allowed.

Updates golang/go#16367 (goimports speed)
Fixes golang/go#16386 (add mechanism to ignore directories)

Change-Id: I8f1a88ae6c4d0ed3075444d70aec3e2228c5ce6a
Reviewed-on: https://go-review.googlesource.com/24971
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-07-15 19:48:34 +00:00
Brad Fitzpatrick
7c26c99973 imports: do less I/O finding the package name of an import
When goimports was run on a file like:

    package main

    import (
        "example.net/foo"
        "example.net/bar"
    )

    var _, _ = foo.Foo, bar.Bar

... even though there looks to be no work to do, it still needs to
verify that "example.net/foo" is really package "foo" (even though it
looks like it) and "example.net/bar" is really package
"bar". (Packages in the standard library are hard-coded since the
previous commit and not verified for consistency since they're always consistent)

To do that verification for non-std packages, go/build.Import was
being used before, but Import reads all files in the directory to make
sure they're consistent. That's unnecessary. Instead, stop after the
first file. If example.net/foo has foo.go with "package foo" and
just_kidding.go with "package other", we never read that far to find
the inconsistency. Oh well. Prefer speed.

Updates golang/go#16367

Change-Id: I9fc3fefbee0e8a6bc287bf2a565257fb9523fd5c
Reviewed-on: https://go-review.googlesource.com/24948
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2016-07-15 19:34:14 +00:00
Brad Fitzpatrick
e047ae774b cmd/goimports, imports: make goimports great again
I felt the burn of my laptop on my legs, spinning away while processing
goimports, and felt that it was time to make goimports great again.

Over the past few years goimports fell into a slow state of disrepair
with too many feature additions and no attention to the performance
death by a thousand cuts. This was particularly terrible on OS X with
its lackluster filesystem buffering.

This CL makes goimports stronger, together with various optimizations
and more visibility into what goimports is doing.

* adds more internal documentation

* avoids scanning $GOPATH for answers when running goimports on a file
  under $GOROOT (for Go core hackers)

* don't read all $GOROOT & $GOPATH directories' Go code looking for
  their package names until much later. Require the package name of
  missing imports to be present in the last two directory path
  components.  Then only try importing them in order from best to
  worst (shortest to longest, as before), so we can stop early.

* when adding imports, add names to imports when the imported package name
  doesn't match the baes of its import path. For example:
        import foo "example.net/foo/v1"

* don't read all *.go files in a package directory once the first file
  in a directory has revealed itself to be a package we're not looking
  for. For example, if we're looking for the right "client" for "client.Foo",
  we used to consider a directory "bar/client" as a candidate and read
  all 50 of its *.go files instead of stopping after its first *.go
  file had a "package main" line.

* add some fast paths to remove allocations

* add some fast paths to remove disk I/O when looking up the base
  package name of a standard library import (of existing imports in a
  file, which are very common)

* adds a special case for import "C", to avoid some disk I/O.

* add a -verbose flag to goimports for debugging

On my Mac laptop with a huge $GOPATH, with a test file like:

	package foo
	import (
	       "fmt"
	       "net/http"
	)

	/*

	*/
	import "C"

	var _ = cloudbilling.New
	var _ = http.NewRequest
	var _ = client.New

... this took like 10 seconds before, and now 1.3 seconds. (Still
slow; disk-based caching can come later)

Updates golang/go#16367 (goimports is slow)
Updates golang/go#16384 (refactor TestRename is broken on Windows)

Change-Id: I97e85d3016afc9f2ad5501f97babad30c7989183
Reviewed-on: https://go-review.googlesource.com/24941
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-07-15 05:29:07 +00:00
Brad Fitzpatrick
0835c73534 imports: special case rand.Read, prevent math/rand by chance
In Go 1.7, math/rand.Read was added. Previously, the only package
containing "rand.Read" was "crypto/rand".

goimports was updated to know that, and zstdlib.go contains a note
that it's ambiguous:

        "rand.Perm":        "math/rand",
        "rand.Prime":       "crypto/rand",
        "rand.Rand":        "math/rand",
        // "rand.Read" is ambiguous
        "rand.Reader":                   "crypto/rand",
        "rand.Seed":                     "math/rand",
        "rand.Source":                   "math/rand",

The intention originally was that such ambiguous things would
never be resolved, even randomly.

But a later change added support for build.Default.SrcDirs, which
meant GOROOT was also searched for ambiguous things. Or maybe I forget
the history.

In any case, when goimports tried to resolve "rand.Read", the
findImportStdlib check was returning nothing, which lead to the
$GOROOT being searched, where math/rand was picked by chance. That's a
dangerous default when the intentional might've been crypto/rand.

Special case it and prefer crypto/rand if there's no more specific
clue either way.

Change-Id: Ib5f8f297f72fa309d5ca9b15a37493df2e17567c
Reviewed-on: https://go-review.googlesource.com/24847
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
2016-07-14 05:39:27 +00:00
Filippo Valsorda
95963e031d imports: support symlinks in GOPATH
The go tool will import a package with a import path that traverses
symlinks with no problems, but goimports would remove that import
because it would fail to recognize the package as existent.

Fixes golang/go#14845

Note: if the file you are currently processing also has a name inside
the symlink, you might have to use the "long" path for vendoring to
work, as it wouldn't be recognized as "deeper" than the vendor folder
otherwise. For example in this tree:

.
├── myfile.go
└── x
    ├── vendor
    │   └── mypkg
    │       └── foo.go
    └── y -> ..

If myfile.go imports mypkg, you will have to process it as
./x/y/myfile.go, not ./myfile.go.

Change-Id: Ic8f41baed3f28d4e9b813160d91aef09ece1fc9f
Reviewed-on: https://go-review.googlesource.com/23803
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-06-09 03:45:06 +00:00
Cezar Sa Espinola
c86fe5956d imports: try importing pkg path from vendor when looking for pkg name
Since CL 17728 goimports was using the file path to try finding packages
inside its /vendor directory. This CL builds upon this previous work to
also consider packages inside /vendor when converting an import path to
a package name.

Change-Id: I173472ac51e3f681d758ec52add60ffc04c62c1a
Reviewed-on: https://go-review.googlesource.com/22020
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2016-05-10 17:11:45 +00:00
Brad Fitzpatrick
02e8ee6893 imports: add Options.FormatOnly
Fixes golang/go#14500

Change-Id: Ied9d772e5f606ce6716193faa2c1a285f0ab00b9
Reviewed-on: https://go-review.googlesource.com/21532
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-04-05 16:34:54 +00:00
Robert Griesemer
5a2fc32f44 imports: adjust vendored packages test to keep working against 1.6
The prior commit fixed this test for tip (1.7) but broke it for 1.6.

Change-Id: Ic39d215ac8bc60dba4b66201f1f16713b2e02d9c
Reviewed-on: https://go-review.googlesource.com/21144
Reviewed-by: Alan Donovan <adonovan@google.com>
2016-03-25 17:41:17 +00:00
Robert Griesemer
35f54f0ea0 imports: adjust tests to check vendored packages again (fix build)
Change-Id: I616dccc07c255bb7ca1146ad0d37d05999eeb772
Reviewed-on: https://go-review.googlesource.com/21142
Reviewed-by: Alan Donovan <adonovan@google.com>
2016-03-25 17:11:48 +00:00
Ahmy Yulrizka
6e94e8a89f imports: cleanup test due to assignment copies of sync.Once
go vet detected that the value of pkgIndexOnce
is copied through assignments in the test.
This patch fixes that by converting it to *sync.Once instead.

Change-Id: I90b2252942625b2f8a93eb5da24d16c6a8a56e4e
Reviewed-on: https://go-review.googlesource.com/20910
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-20 22:53:05 +00:00
Robert Griesemer
95e5e62424 imports: fix TestFindImportVendor for 1.5 (fix build)
TBR=adonovan

Change-Id: I7edeb180bdf09648e7539b9371db0d14272b9030
Reviewed-on: https://go-review.googlesource.com/20697
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-03-15 04:38:51 +00:00
Robert Griesemer
8852e49d27 imports: adjust tests to fix build
These changes will need to be reverted once we use
vendoring in the stdlib again, but it's trivial to
do so when the time comes.

TBR=adonovan

Fixes #14823.

Change-Id: I2173c48d5466874492affc679332f6484bf96592
Reviewed-on: https://go-review.googlesource.com/20695
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-03-15 04:38:37 +00:00
David R. Jenni
e852fdd89f go/ast/astutil: add function DeleteNamedImport.
DeleteNamedImport deletes the import with a given
name and path from a parsed file, if present.

imports uses this function to delete named imports.

Fixes golang/go#8149.

Change-Id: I84539d5627191c45f06db2d008507aee4d3b1eb2
Reviewed-on: https://go-review.googlesource.com/19951
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-02-26 17:37:10 +00:00
Russ Cox
bf084ef758 imports: add support for vendor directories
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>
2016-02-18 16:47:46 +00:00