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>
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>
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.
Fixesgolang/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>
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>
The following tag
// +build linux,!appengine darwin
Translated to (linux AND !appengine) OR (darwin)
Causing issues on darwin when attempting to run dev_appserver.
Change this to always exclude these files from appengine builds.
Change-Id: Ifd63a884747001797d0b0e828f0c9c391bc7c73c
Reviewed-on: https://go-review.googlesource.com/60911
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
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"
)
Fixesgolang/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>
The import path is ambiguous in the presence of vendoring (e.g. golang/go#20610)
Change-Id: I22f372b233b8554e3d9210b383a7df7a6a0f3eee
Reviewed-on: https://go-review.googlesource.com/53470
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We're at beta 2 already, so the API is likely stable.
If anything gets rolled back, we can update imports again.
Change-Id: I87e0adde062e45cab16e2bee2f929c1ac1bb828d
Reviewed-on: https://go-review.googlesource.com/49150
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
Fixesgolang/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>
golint suggested:
comment on exported var LocalPrefix should be of the form "LocalPrefix ..."
Change-Id: I1d2f92ba76e3c091440f676936dbb1fc1ce07f3a
Reviewed-on: https://go-review.googlesource.com/46432
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
In some cases walkFn is being called after the fastWalk function has
returned. This often happens when an error was encountered early on in
scanning directories with many entries.
It is caused by fastWalk not waiting for its workers to complete their
work. A sync.WaitGroup is used to wait for all workers to finish when
the function returns.
Updates golang/go#16399
Change-Id: I695d30c18e4878b789520b9d8a650f9688d896ac
Reviewed-on: https://go-review.googlesource.com/40092
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
Fixesgolang/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>
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>
This CL only copies files and updates build tags.
Substantive changes will come in follow-ups.
This is a workaround for git's lack of rename/copy tracking.
Tested with go1.6, go1.7, and tip (go1.8).
Change-Id: Id88a05273fb963586b228d5e5dfacab32133a960
Reviewed-on: https://go-review.googlesource.com/32630
Reviewed-by: Robert Griesemer <gri@golang.org>
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"
)
Resolvesgolang/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>
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>
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>
This means a directory /gopath/src/foo/Go-Bar is considered as maybe
containing package "bar" or even "gobar".
Also, more tests.
Fixesgolang/go#16402
Change-Id: I14208d738e3a081cb6d9bcd83d777280e118f4e7
Reviewed-on: https://go-review.googlesource.com/25030
Reviewed-by: Andrew Gerrand <adg@golang.org>
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>
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)
Fixesgolang/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>
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>
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>
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>
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.
Fixesgolang/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>
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>
