Because go/types is slightly more strict than gc about certain "soft"
errors (ones that aren't necessary to interpret a Go program), gorename
rejects programs that compile under gc. This change relaxes gorename's
error checks so that they are weaker than gc's.
This is a workaround for issue golang/go#14596 in gorename,
whose underlying problem is issue golang/go#8560 in gc.
Fixesgolang/go#14596
Change-Id: Ica5006c2376c0564a575224269093c1497348ee6
Reviewed-on: https://go-review.googlesource.com/29853
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the package, the added import declarations used backslashes.
In the test, filenames in warning messages used backslashes.
Now both use forward slash.
Fixesgolang/go#16384
Change-Id: I43116aab0b3209305f23ed9def7c4adf3259941e
Reviewed-on: https://go-review.googlesource.com/24943
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
There is no need to check for import conflicts when the package name
doesn't change. Add test.
Also, when reporting a (non-spurious) import conflict, make clear that
the error is just a warning. Add a test for same.
Change-Id: Idde0483b502cd041fd893230fec06b8533f54f3c
Reviewed-on: https://go-review.googlesource.com/24917
Reviewed-by: Michael Matloob <matloob@golang.org>
When displaying a diff the gorename command would panic because the
stdout variable was never set. These changes default stdout to
os.Stdout.
Fixes https://github.com/golang/go/issues/14009
Change-Id: I0902b2b4e2b8e1f46f8ab50e30dcfc841c925174
Reviewed-on: https://go-review.googlesource.com/18813
Reviewed-by: Alan Donovan <adonovan@google.com>
Cause: the absolute cwd, not ".", must be passed to build.Import.
Also: simplify the lookup logic in findFromObjects, and handle nil
gracefully.
(No unit test---not worth faking os.Getwd.)
Fixes issue #13866
Change-Id: I7e8eede55bc83d98e2ef5e42609c287e74c04501
Reviewed-on: https://go-review.googlesource.com/18453
Reviewed-by: Michael Matloob <matloob@golang.org>
A few files have been forked and tagged "go1.5,!go1.6" to work around
minor API changes between the two types packages:
- constant.Value.String() in oracle/describe.go and its tests;
- constant.ToInt must now be called before constant.Int64Val.
- types.Config{Importer: importer.Default()} in a number of places
- go/types/typeutil/import_test.go uses lowercase names to avoid 'import "C"'.
Files in go/types/typesutil, missing from my previous CL, have been
tagged !go1.5; these files will be deleted in February.
All affected packages were tested using 1.4.1, 1.5, and ~1.6 (tip).
Change-Id: Iec7fd370e1434508149b378438fb37f65b8d2ba8
Reviewed-on: https://go-review.googlesource.com/18207
Reviewed-by: Robert Griesemer <gri@golang.org>
This change will ensure that the tree continues to work with go1.4.1.
All files continue to depend on golang.org/x/tools/go/types, but in a
follow-up change, I will switch the primary files to depend on the
standard go/types package. Another (smaller) set of files will be
forked and tagged, this time !1.6, due to API differences between the
two packages.
All tests pass using 1.4.1, 1.5, and ~1.6 (tip).
Change-Id: Ifd75a6330e120957d646be91693daaba1ce0e8c9
Reviewed-on: https://go-review.googlesource.com/18333
Reviewed-by: Robert Griesemer <gri@golang.org>
The user may specify the diff tool using the -diffcmd flag.
+ test.
Also:
- eliminate redundant DryRun flag
- simplify Verbose messages using log.SetPrefix
Fixes issue #13355
Change-Id: I917edc73e31ddf0f5d5b9b30c43f826465529da1
Reviewed-on: https://go-review.googlesource.com/18208
Reviewed-by: Michael Matloob <matloob@golang.org>
Add an ;;;###autoload magic comment, so that the go-rename command can be used
without need to explicitly (require 'go-rename) first.
Change-Id: Ibdf9886fe98c55e1d948469aac972b568649f910
Reviewed-on: https://go-review.googlesource.com/16327
Reviewed-by: Alan Donovan <adonovan@google.com>
so that we can delete that package.
lexicalLookup reconstructs the lexical scope from the existing tree of
types.Scope blocks, using source position information to determine
which prefix of declarations are visible. (Inspired by Russ's
lookupAtPos in github.com/rsc/grind/grinder.)
forEachLexicalRef implements the part of the recursion from
refactor/lexical that enumerates the ast.Idents that use lexical
lookup. (I would still like to eliminate this redundant logic by
having go/types record environments, as in CL 9493.)
Change-Id: I040ab33b508aad2dc68fd48850fe92ec072045d1
Reviewed-on: https://go-review.googlesource.com/9544
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Rename the package name of the external test package, e.g. <from>_test to <to>_test.
Rewrite the import statements of external test packages which import the renamed package.
Fixes#10507
Change-Id: Iad702189469c54776e55ed4a821610bd9977618c
Reviewed-on: https://go-review.googlesource.com/12637
Reviewed-by: Alan Donovan <adonovan@google.com>
And in gomvpkg, don't stop just because some packages had errors.
This is inevitable in a large GOPATH tree.
Fixes issue golang/go#10907
Change-Id: I9a60b070228d06d44880202eeef54394e914f5d5
Reviewed-on: https://go-review.googlesource.com/10715
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Match gofmt's behaviour of closing the status buffer. Don't display the
buffer in case of success as we were going to hide it instantly again,
anyway. Also, instead of using delete-window, use
gofmt--kill-error-buffer. gofmt--kill-error-buffer uses quit-window if
there's a window for the buffer, kill-buffer otherwise.
quit-window only deletes the window if it was created explicitly for the
status buffer and if it is still displaying it. This way, we won't close
windows that the user created or repurposed himself.
Additionally, this change ensures that we don't leave a *go-rename*
buffer lying around when gorename succeeded.
Fixesgolang/go#10972.
Change-Id: Id1efb60d399f0062d870e925138aa827e12f3e58
Reviewed-on: https://go-review.googlesource.com/10453
Reviewed-by: Alan Donovan <adonovan@google.com>
This was not a visible bug since the only caller discards the relevant
result, so I also deleted the result.
Fixes#9999
Change-Id: I276d6523b2891d3cb9c8137448e1aed32a5fd197
Reviewed-on: https://go-review.googlesource.com/5921
Reviewed-by: Michael Matloob <michaelmatloob@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
This avoids littering the tree, and confusing some editors (e.g. Atom)
that expect the inode number to remain constant.
Change-Id: I2faeda1ed1b01e5e4cc720744ea3c99ab29e7333
Reviewed-on: https://go-review.googlesource.com/7664
Reviewed-by: Robert Griesemer <gri@golang.org>
The callbacks are intentionally concurrent, making this function very
easy to misuse (most clients so far have got it wrong, even my own).
Using a channel in the API makes the concurrency obvious, the
correct usage easy, and the client control flow simpler.
Change-Id: Ied38c3ed5c98b40eb1b322a984ed9dc092ac0918
Reviewed-on: https://go-review.googlesource.com/3250
Reviewed-by: Sameer Ajmani <sameer@golang.org>
...since the zero value is more useful by far.
This is a breaking API change, obviously. (One or two tests in this
CL have intentional been left using the zero value, i.e., they now
load source.)
Change-Id: I42287bfcdb1afef8ee84e5eac12534dd0a1fd5d2
Reviewed-on: https://go-review.googlesource.com/5653
Reviewed-by: Robert Griesemer <gri@golang.org>
The top-level moved package was skipped when doing import rewrites.
Don't skip that package.
Fixes#9811
Change-Id: I1c524ed44606586b5231e5adb6168079aa0e0228
Reviewed-on: https://go-review.googlesource.com/4470
Reviewed-by: Alan Donovan <adonovan@google.com>
Before this change, many kinds of error would cause the loader to stop.
making it brittle when analyzing large codebases, as in "godoc -analysis".
This change moves operations that used to occur during
configuration---(*build.Context).Import, loading, and parsing of
initial packages---into the Load call, and ensures that all failures
during Loading are reported at the end so that the maximum amount of
progress is made.
Also: redesign the tests and add many new cases.
Change-Id: Ia8cd99416af7c5d4a5fe133908adfa83676d401f
Reviewed-on: https://go-review.googlesource.com/3626
Reviewed-by: Robert Griesemer <gri@golang.org>
In the example, we use backslashes (not single quotes) to escape
double-quotes since it works on both Windows and POSIX.
Change-Id: Id883f5457bec4d8a36d5b12c759ad385125a98a6
Reviewed-on: https://go-review.googlesource.com/2862
Reviewed-by: Robert Griesemer <gri@golang.org>
- use import path not file path in go/buildutil.FakeContext OpenFile;
- use regexp to compare error messages in TestErrors, because
they contain windows file paths;
- use OS file path (not unix path), when checking move results
in TestMoves.
Change-Id: Ib62d344acb551fb612d8a0773ae1ab5f18341294
Reviewed-on: https://go-review.googlesource.com/3171
Reviewed-by: Alan Donovan <adonovan@google.com>
This refactoring of the tests of go/loader and refactor/rename made it
possible to write some loader tests I wanted, but the new tests reveal
bugs so they're commented out for now. I will fix them in a follow-up.
Change-Id: Iae3a20681a0a0791076debd4b82bb5ed74b0c577
Reviewed-on: https://go-review.googlesource.com/2825
Reviewed-by: Robert Griesemer <gri@golang.org>
This change adds a command mvpkg that will move a given package and
update all its imports. It uses similar logic to gorename to update
the imports.
Change-Id: Iebbd0b4c93c2302b0a71c3b99c68f6778106012a
Reviewed-on: https://go-review.googlesource.com/1973
Reviewed-by: Alan Donovan <adonovan@google.com>
Previously, gorename rejected all method renamings if it would
change the assignability relation.
Now, so long as the renaming was initiated at an abstract
method, the renaming proceeds, changing concrete methods (and
possibly other abstract methods) as needed. The user
intention is clear.
The intention of a renaming initiated at a concrete method is
less clear, so we still reject it if it would change the
assignability relation. The diagnostic advises the user to
rename the abstract method if that was the intention.
Additional safety checks are required: for each
satisfy.Constraint that couples a concrete type C and an
interface type I, we must treat it just like a set of implicit
selections C.f, one per abstract method f of I, and ensure the
selections' meanings are unchanged.
The satisfy package no longer canonicalizes types, since this
substitutes one interface for another (equivalent) one, which
is sound, but makes the type names random and the error
messages confusing.
Also, fixed a bug in 'satisfy' relating to map keys.
+ Lots more tests.
LGTM=sameer
R=sameer
CC=golang-codereviews
https://golang.org/cl/173430043
Such messages are more informative when the error occurs deep within a script.
Also: add tool name to digraph's usage messages.
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/173380043
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
