It is possible to reconstruct the lexical environment at any point
using only Object and Scope positions; see CL 9544 for an example.
Change-Id: I1268f9490c072ceb1c4d044df2e9c49678d73dfd
Reviewed-on: https://go-review.googlesource.com/14131
Reviewed-by: Robert Griesemer <gri@golang.org>
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>
The optional Qualifier function determines what prefix to attach to
package-level names, enabling clients to qualify packages in different
ways, for example, using only the package name instead of its complete
path, or using the locally appropriate name for package given a set of
(possibly renaming) imports.
Prior to this change, clients wanting this behavior had to copy
hundreds of lines of complex printing logic.
Fun fact: (*types.Package).Path and (*types.Package).Name are valid
Qualifier functions.
We provide the RelativeTo helper function to create Qualifiers so that
the old behavior remains a one-liner.
Fixesgolang/go#11133
Change-Id: Ibd63f639c7b3aa1738826d6165f2d810efeb8293
Reviewed-on: https://go-review.googlesource.com/11692
Reviewed-by: Robert Griesemer <gri@golang.org>
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>
...to avoid namespace conflicts.
Also make its name "main", since it defines func main().
And fix 2 typos.
Change-Id: I7cf7894d6bed134907b3d2742255e5a82426071b
Reviewed-on: https://go-review.googlesource.com/3150
Reviewed-by: Robert Griesemer <gri@golang.org>
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
The main goroutine wasn't waiting for the two closed channels to drain.
Moral: with concurrency, never invent. D'oh.
LGTM=sameer
R=sameer
CC=golang-codereviews
https://golang.org/cl/178090043
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
(They may contain any character, after all.)
Also, allow but don't require parens and stars.
e.g. (*"encoding/json".Decoder).Decode or "encoding/json".Decoder.Decode
but not encoding/json.Decoder.Decode.
Since -from queries are now Go expressions, we use the Go parser.
(Thanks to Rog Peppe for the suggestion.)
LGTM=sameer
R=sameer
CC=golang-codereviews, gri, rogpeppe
https://golang.org/cl/154610043
Windows doesn't regard "/go/src" as an absolute path,
so we must explicitly define the virtual IsAbsPath method.
LGTM=gri
R=gri, sameer
CC=golang-codereviews
https://golang.org/cl/147100043
...by using buildutil.ContainingPackage instead of guessImportPath.
The former should be more portable.
(I meant to do this earlier, so this is also a nice cleanup.)
LGTM=gri
R=gri, sameer
CC=golang-codereviews
https://golang.org/cl/148050043
See the usage message in main.go for orientation.
To the best of my knowledge, the tool implements all required
soundness checks, except:
- the dynamic behaviour of reflection is obviously undecidable.
- it rejects method renamings that change the "implements" relation.
It should probably be more aggressive.
- actually it only checks the part of the "implements" relation
needed for compilation. Understanding the dynamic behaviour
of interfaces is obviously undecidable.
- a couple of minor gaps are indicated by TODO comments.
Also:
- Emacs integration.
- tests of all safety checks and (some) successful rewrites.
LGTM=dominik.honnef, sameer
R=gri, sameer, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/139150044
The type checker does not currently report the set of pairs of
types that are tested for assignability (though gri and I
agree that it should). This information is useful for many
applications. For example, refactoring tool needs to know the
minimal set of interface satisfaction constraints that must be
preserved during a refactoring.
This package is a stopgap measure to deduce the same
information using another pass. Unlike go/types, it is not
robust against inputs containing type errors.
LGTM=sameer
R=gri, sameer
CC=golang-codereviews
https://golang.org/cl/136470043
The Uses, Defs and Scope information provided by go/types is
inadequate for answering "what if?" queries about the
structure of the lexical environment.
In this code, for example,
var x int
func f() {
print(x)
x := ""
print(x)
}
the two referring Idents x appear at the same lexical depth,
inside the function f's Scope object, yet they resolve to
different objects.
This package associates a lexical.Environment instance with
every reference to capture these differences. Each
environment is a linked list of enclosing Blocks, and for each
block, a number indicating what prefix of its bindings are
visible. (Zero for the first 'x' reference above, 1 for the
second.)
+ Smoke test over stdlib.
This functionality could be integrated with the type checker
in lieu of the not-so-useful types.Info.Scopes data, at little
extra cost in code or in running time/space. We should talk
about that.
LGTM=sameer
R=gri, sameer
CC=golang-codereviews
https://golang.org/cl/143790043
Nodes in a strongly connected component (which includes most
stdlib packages) appear in results of both "forward" and
"reverse" searches from any other node in the same SCC.
LGTM=sameer
R=sameer
CC=golang-codereviews, gri
https://golang.org/cl/136470044
This CL is the first refactoring automated by "gorename". :)
Though I had to update the comments and run 'hg gofmt'. :(
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/142930043
Refactoring tools can use this to determine an upper bound on
the set of packages potentially affected by a refactoring.
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/141310043
