rsc / rf

A refactoring tool for Go

rf is an experimental refactoring tool. It is very much a work in progress.

rf is incredibly rough and likely to be buggy and change incompatibly.

rf's Issues

rf: rename all variables with matching name

One thing I would like to do in my refactoring is change a local variable naming convention. e.g., in package runtime, rename all variables of type *m with name _m_ to mp.

This can obviously already be done one-by-one (e.g., rf 'mv canpanic._m_ canpanic.mp'), but I still need to manually identify all instances to construct mv commands. Ideally rf could help make this easier.

The first question is whether this is even a feature that makes sense for rf? To me it seems nice if it can fit in nicely. My use-case is renaming the final remaining uses of old naming conventions in the runtime that are left over from the C-to-Go conversion. Another theoretical use-case I could imagine is a codebase that has been naming their context.Context variables c, and now wants to change to the more standard ctx.

Secondly, how should this be expressed? A few approaches I've considered:

ex command

ex {
  named var _m_ *m
  named var mp *m
  _m_ -> mp
}

This is actually the first thing I tried (without the named keyword) before realizing that the names in an ex command are relevant only for references within the block. For pattern matching, only the types matter. Thus is example (without named) would in theory run but be a no-op since the types are the same.

The named keyword would add the variable name as part of the pattern matching key (i.e., only match variables of type *m that are named _m_). mp won't already exist in the matched program, so it might also need another keyword (implicit?).

This approach is nice because it explicitly takes type into account. In the context.Context case this could be particularly important, as there are likely to be other unrelated variables named c.

mv -all command

Right now, mv takes the addr to change, and an addr uniquely defines a single thing in the package (right?). A fuzzy addr could match all instances of something. e.g., mv -all _m_ mp or perhaps mv *._m_ *.mp matches all addrs that end in _m_ and renames them in the same scope.

This is perhaps simpler to write for simple renames, but doesn't take type into account, which is unfortunate.

list helper

Perhaps rf shouldn't even try to directly support this case, but could instead provide an rf -list helper that lists the addr for all identifiers, plus their kind (Func, Type, etc) and type (if applicable). A simple script could then be used to filter this output to look for all instances of _m_ I care about and construct a set of trivial mv commands to perform the renames.

rf: ex introduces double-qualified pkg.pkg.Name identifiers

I expect the test case below should succeed, but it currently fails because ex instead tries to substitute "sub1.sub1.X" in place of "m.X" in sub2/2.go.

/cc @rsc

ex . ./sub2 {
  import "m/sub1"
  X -> sub1.X
}
-- m.go --
package m

import "m/sub1"

var X = 42

func init() { sub1.X = X }

var _ = X
-- sub1/1.go --
package sub1

var X int
-- sub2/2.go --
package sub2

import "m"

var _ = m.X
-- stdout --
diff old/m.go new/m.go
--- old/m.go
+++ new/m.go
@@ -4,6 +4,6 @@

 var X = 42

-func init() { sub1.X = X }
+func init() { sub1.X = sub1.X }

-var _ = X
+var _ = sub1.X
diff old/sub2/2.go new/sub2/2.go
--- old/sub2/2.go
+++ new/sub2/2.go
@@ -1,5 +1,7 @@
 package sub2

-import "m"
+import (
+	"m/sub1"
+)

-var _ = m.X
+var _ = sub1.X

rf: does not refactor test files

In the test case below, rf currently only refactors the x.go and p/x.go. It doesn't refactor any of the _test files.

It seems like the logic around test packages in rf/refactor needs fixing (or maybe you had fixed this in your local x/tools copy?). I can try digging into this, but thought I'd file an issue first in case you know what needs to be done already.

mv X Y
-- go.mod --
module example.com/m
-- x.go --
package m

var X int
var _ = X
-- x_test.go --
package m

var _ = X
-- y_test.go --
package m_test

import "example.com/m"

var _ = m.X
-- p/x.go --
package p

import "example.com/m"

var _ = m.X
-- p/x_test.go --
package p

import "example.com/m"

var _ = m.X
-- p/y_test.go --
package p_test

import "example.com/m"

var _ = m.X

rf: GoCompiledFiles is empty when running rf on the runtime package

When trying to run rf on the runtime package I got a bunch of failures. I dug down and noticed that GoCompiledFiles (a field that appears in the JSON output of go list when -compiled is passed) is empty when trying to use rf in std.

I worked around this with the following patch:

diff --git a/refactor/snap.go b/refactor/snap.go
index fe543ed..ae0ffbe 100644
--- a/refactor/snap.go
+++ b/refactor/snap.go
@@ -307,7 +307,22 @@ func (r *Refactor) Load() (*Snapshot, error) {

                // Set up for loading from source code.
                p.Export = "" // Remember NOT to load from export data.
-               for _, name := range jp.CompiledGoFiles {
+               files := jp.CompiledGoFiles
+               lowLevelPkgs := map[string]struct{}{
+                       "runtime/internal/math": struct{}{},
+                       "internal/bytealg":      struct{}{},
+                       "runtime":               struct{}{},
+               }
+               if _, ok := lowLevelPkgs[p.PkgPath]; ok {
+                       // For any number of reasons, `go list` doesn't
+                       // actually return a populated CompiledGoFiles
+                       // for any of these packages, preventing anything
+                       // useful from being done. Use GoFiles because
+                       // it's already filtered out all the right arch-specific
+                       // files and that's good enough for the runtime.
+                       files = jp.GoFiles
+               }
+               for _, name := range files {
                        if strings.HasSuffix(name, ".s") { // surprise!
                                continue
                        }

AFAICT GoFiles does already filter by GOOS and GOARCH, so I'm not totally sure what the benefit of using GoCompiledFiles over GoFiles is in general.

@prattmic noted that this problem disappears if the -export flag is removed, but then he encountered

../crypto/ecdsa/ecdsa.go:123:19: cannot use r (variable of type *big.Int) as *big.Int value in argument to b.AddASN1BigInt
../crypto/ecdsa/ecdsa.go:124:19: cannot use s (variable of type *big.Int) as *big.Int value in argument to b.AddASN1BigInt
rf: errors found after script

when trying to run rf on the runtime. Not sure yet what's going wrong there. I'll dig deeper a little deeper later, but first I wanted to just get this down in writing.

mv broken

using this file:

package widevine

import (
   "crypto"
   "crypto/rsa"
   "strings"
)

func Signed_Request() {
   rsa.SignPSS(
      strings.NewReader("hello world"), nil, crypto.SHA1, nil, nil,
   )
}

I get this result:

> rf 'mv Signed_Request _Signed_Request'
rf: widevine.go:11:46: cannot use crypto.SHA1 (constant 3 of type crypto.Hash)
as crypto.Hash value in argument to rsa.SignPSS
errors found before executing script

Tests are failing with go1.19 and newer

There might a behavior regression in Go, since go1.16 can run the tests successfully, but go1.19 and gotip fail with panic: lost receiver name.

go1.16.10 test ./...
ok      rsc.io/rf       28.153s
ok      rsc.io/rf/diff  0.167s
?       rsc.io/rf/git-generate  [no test files]
ok      rsc.io/rf/refactor      0.168s

go version
go version go1.19.3 darwin/amd64

go test ./...

panic executing: mv x.go f.go
--- FAIL: TestRun (8.76s)
    --- FAIL: TestRun/mv_file4.txt (0.70s)
        rf_test.go:85: testdata/mv_file4.txt
panic: lost receiver name [recovered]
        panic: lost receiver name [recovered]
        panic: lost receiver name

goroutine 434 [running]:
testing.tRunner.func1.2({0x12a71c0, 0x135e9d0})
        /usr/local/opt/go/libexec/src/testing/testing.go:1396 +0x24e
testing.tRunner.func1()
        /usr/local/opt/go/libexec/src/testing/testing.go:1399 +0x39f
panic({0x12a71c0, 0x135e9d0})
        /usr/local/opt/go/libexec/src/runtime/panic.go:884 +0x212
rsc.io/rf.run.func1()
        /Users/vearutop/dev/rf/rf.go:76 +0xa5
panic({0x12a71c0, 0x135e9d0})
        /usr/local/opt/go/libexec/src/runtime/panic.go:884 +0x212
rsc.io/rf/refactor.(*Snapshot).addPkgDeps(0x0?, 0xc0008db909?, 0xc009f09ad0)
        /Users/vearutop/dev/rf/refactor/pkgref.go:223 +0x46f
rsc.io/rf/refactor.(*Snapshot).DepsGraph(0xc00012aab0, 0x2)
        /Users/vearutop/dev/rf/refactor/pkgref.go:147 +0x6d
rsc.io/rf.mvCode(0xc00012aab0, {0xc00c8a6bb0, 0x1, 0xc00ad91da0?}, 0xc000058110?, 0xc00ae8aea0)
        /Users/vearutop/dev/rf/mvcode.go:122 +0x59d
rsc.io/rf.cmdMv(0xc00012aab0, {0xc004838303?, 0x12f9946?})
        /Users/vearutop/dev/rf/mv.go:127 +0xb7a
rsc.io/rf.run(0xc0031f27e0, {0xc0048382a0?, 0xd?})
        /Users/vearutop/dev/rf/rf.go:133 +0x2d1
rsc.io/rf.TestRun.func1(0xc008f82b60)
        /Users/vearutop/dev/rf/rf_test.go:121 +0x345
testing.tRunner(0xc008f82b60, 0xc00beae6a0)
        /usr/local/opt/go/libexec/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
        /usr/local/opt/go/libexec/src/testing/testing.go:1493 +0x35f
FAIL    rsc.io/rf       8.927s
ok      rsc.io/rf/diff  (cached)
?       rsc.io/rf/git-generate  [no test files]
ok      rsc.io/rf/refactor      (cached)
FAIL

rf: type unification for type patterns

The normal semantics for var t T in an ex rule is that t will match any expression whose type is assignable to T. In the example below, i and v are both assignable to I, which implements interface { Equal(I) bool }, so I would expect all of the equals to be rewritten. However, currently only the first gets rewritten:

ex {
  type T interface { Equal(T) bool }
  var t1, t2 T
  t1 == t2 -> t1.Equal(t2)
}
-- x.go --
package p

type I interface { Equal(I) bool }
var i I

type V int
func (V) Equal(I) bool
var v V
var _ I = v

var _ = i == i
var _ = i == v
var _ = v == i
var _ = v == v

I suspect matcher.matchWildcardType needs to be extended to support type unification rather than requiring strict type identity for back matches.

There's probably some subtlety here for preferring that the unification to prefer unifying to I rather than interface { Equal(I) bool }. Might want to learn from how types2 handles this.

In the mean time, it's possible to manually workaround this without too many additional rules:

ex {
  type A interface{ Equal(A) bool }
  type B interface{ Equal(A) bool }

  var a1, a2 A
  var b1, b2 B

  a1 == a2 -> a1.Equal(a2)
  a1 == b2 -> a1.Equal(b2)
  b1 == a2 -> b1.Equal(a2)
  b1 == b2 -> b1.Equal(b2)
}
-- x.go --
package p

type I interface{ Equal(I) bool }
var i I

type V int
func (V) Equal(I) bool
var v V
var _ I = v

var _ = i == i
var _ = i == v
var _ = v == i
var _ = v == v

allow specifying custom build tags

wire relies on custom build tags in initializer files to generate code. The files that are loaded/built by default are part of the generated output and the files that rf should really be refactoring are not built unless the wireinject built tag is set. There should be a way to tell rf to use this build tag when loading and refactoring code.

There may be a larger question of how to refactor every version of code that relies heavily on build tags, but I don't want to open that can of worms. I hope it's simple enough to specify a single set of tags to use when loading/refactoring code.

rf: type checking fails when it should not

When I try to run rf on our code repo, I get lots of errors like this:

../../objectstore/common/fetch_exported_test.go:22:71: cannot use hasher (variable of type *common.Hasher) as *common.Hasher value in argument to inmem.NewObjectStoreBucket

The package paths are identical and the types should be identical but the types.Named.obj fields differ, which I guess means that the types have come from different type checking sessions.

For the record, the actual command I was running was this:

rf 'ex {import "github.com/influxdata/idpe/models"; []models.Point -> models.Points}'

rf: does not refactor definitions inside *pkg*_test files

Got following files:

$ ls
foo_test.go  go.mod  go.sum

$ cat foo_test.go
package foo_test

var X int

$ rf -diff 'mv X Y'
cannot find X
rf: errors found during: mv X Y

There is a similar issue #1 got resolved. The discrepency of this issue is that X is defined in the test package, rather than defined elsewhere.
Additionally, changing the package name from foo_test to foo, then this issue doesn't exist.

is this tool still maintained?

rf: review use of types.Info

The fix in 71eddf1 was due to using the wrong package's types.Info, so it was missing information for non-exported declarations. I had used "info" because I saw it used for similar code elsewhere in that function, so I thought it was safe to use.

It would be good to double check the other cases, and write tests cases for any that are incorrect. I suspect some may work today only because many tests only use a single package.

specifying a different source package

I haven't found a way to specify a different source package than the current one when invoking commands like mv A B. I'm working around this limitation by calling rf in a shell script that changes the working directory before invoking it. In a large codebase, each invocation of rf requires about 15-30 seconds to load everything before it does any refactoring. In my case, I wanted to rename structs in 130 packages and this took over an hour.

Example:

go.mod

module example.com/p

go 1.16

p.go

package p

var nothing string

a/a.go

package a

var A int

If I understand code addresses correctly, I would expect to be able to write something like this:

rf 'mv example.com/p/a:A A'

rf: panic: type check did not complete

I'd like to convert my old usages of unsafe casts to use unsafe.Add. I started with the simplest of rules to begin this:

ex {
        import "unsafe";
        var p unsafe.Pointer;
        var o uintptr;
        (unsafe.Pointer)(uintptr(p) + o) -> unsafe.Add(p, o)
}

But rf becomes upset:

panic executing: 
panic: type check did not complete [recovered]
        panic: type check did not complete

goroutine 1 [running]:
main.run.func1()
        /tmp/rf/rf.go:76 +0xa5
panic({0x63e3e0, 0x6d1100})
        /usr/lib/go/src/runtime/panic.go:1038 +0x215
rsc.io/rf/refactor.(*Snapshot).typeCheck(0xc0000ea120)
        /tmp/rf/refactor/snap.go:588 +0x91c
rsc.io/rf/refactor.(*Refactor).Load(0xc0000c43c0)
        /tmp/rf/refactor/snap.go:356 +0x180a
main.run(0xc0000c43c0, {0x7ffc50738a4d, 0xc000062750})
        /tmp/rf/rf.go:101 +0x224
main.main()
        /tmp/rf/rf.go:44 +0x165

rf: ex and replace one statement with a couple of statements

hi,

I've tried to use rf ex to replace one statement with a pair of statements.

I tried the following:

      ex {
            import "gioui.org/op";
            import "gioui.org/op/clip";
    
            var o *op.Ops;
            var p clip.Path;
            var sty clip.StrokeStyle;
            var width float32;
    
            p.Stroke(width, sty).Add(o) ->  \
                    p.Op().Add(o)           \
                    clip.StrokeOp{          \
                            Width: width,   \
                            Cap: sty.Cap,   \
                            Join: sty.Join, \
                    }.Add(o);
      }

ie: replacing:

p.Stroke(width, sty).Add(o)

with:

p.Op().Add(o)
clip.StrokeOp{
    Width: width,
    Cap: sty.Cap,
    Join: sty.Join,
}

trying to run the above ex script, I get:

$> rf ex ...
ex: missing substitution in example: p.Stroke(width, sty).Add(o) ->

is this something that should be possible to achieve with rf ex (or with rf at all) ?

rf: panic: needParen outer *ast.MapType

In module github.com/zephyrtronium/xirho, running rf 'ex { System -> *System }' panics:

panic executing: ex { System -> *System }
panic: needParen outer *ast.MapType [recovered]
        panic: needParen outer *ast.MapType

goroutine 1 [running]:
main.run.func1(0xc0043d7ed0)
        /home/zephyr/go/src/rsc.io/rf/rf.go:74 +0xe8
panic(0x670020, 0xc00c3bbae0)
        /usr/local/go/src/runtime/panic.go:969 +0x1b9
main.needParen(0x713c60, 0xc00c377a00, 0xc0036d0590, 0x7, 0x7, 0xc00c3e03c0)
        /home/zephyr/go/src/rsc.io/rf/ex.go:748 +0x8df
main.(*matcher).applySubst(0xc003657800, 0x713c60, 0xc00c377a00, 0xc0036d0590, 0x7, 0x7, 0xc00bab73b0, 0x1, 0x713fa0, 0xc00bae9270)
        /home/zephyr/go/src/rsc.io/rf/ex.go:701 +0x2dd
main.(*exArgs).run.func1(0xc0036d0590, 0x7, 0x7)
        /home/zephyr/go/src/rsc.io/rf/ex.go:534 +0x506
rsc.io/rf/refactor.walkRange.func1(0x0, 0x0, 0x649c01)
        /home/zephyr/go/src/rsc.io/rf/refactor/syntax.go:178 +0xb0
go/ast.inspector.Visit(0xc00c3e0340, 0x0, 0x0, 0x7129c0, 0xc00c3e0340)
        /usr/local/go/src/go/ast/walk.go:373 +0x3a
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x713820, 0xc0036269e0)
        /usr/local/go/src/go/ast/walk.go:367 +0x189
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x713a20, 0xc003624fc0)
        /usr/local/go/src/go/ast/walk.go:176 +0x1565
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x713460, 0xc00362abc0)
        /usr/local/go/src/go/ast/walk.go:102 +0x43a
go/ast.walkExprList(0x7129c0, 0xc00c3e0340, 0xc003621090, 0x1, 0x1)
        /usr/local/go/src/go/ast/walk.go:26 +0x9e
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x7130e0, 0xc00362ac00)
        /usr/local/go/src/go/ast/walk.go:207 +0x1ebe
go/ast.walkStmtList(0x7129c0, 0xc00c3e0340, 0xc003627c60, 0x2, 0x2)
        /usr/local/go/src/go/ast/walk.go:32 +0x9e
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x713260, 0xc003625c50)
        /usr/local/go/src/go/ast/walk.go:224 +0x18e5
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x7136e0, 0xc003625c80)
        /usr/local/go/src/go/ast/walk.go:344 +0xcbf
go/ast.walkDeclList(0x7129c0, 0xc00c3e0340, 0xc0035db180, 0x7, 0x8)
        /usr/local/go/src/go/ast/walk.go:38 +0x9e
go/ast.Walk(0x7129c0, 0xc00c3e0340, 0x713660, 0xc0035db400)
        /usr/local/go/src/go/ast/walk.go:353 +0x2346
go/ast.Inspect(...)
        /usr/local/go/src/go/ast/walk.go:385
rsc.io/rf/refactor.walkRange(0x713660, 0xc0035db400, 0x0, 0x7fffffffffffffff, 0xc00c3d3e00, 0xc00c3dc540)
        /home/zephyr/go/src/rsc.io/rf/refactor/syntax.go:175 +0x135
rsc.io/rf/refactor.WalkPost(...)
        /home/zephyr/go/src/rsc.io/rf/refactor/syntax.go:154
main.(*exArgs).run(0xc00bf30f30)
        /home/zephyr/go/src/rsc.io/rf/ex.go:468 +0x25d
main.cmdEx(0xc00014a2a0, 0xc0000165c3, 0x15)
        /home/zephyr/go/src/rsc.io/rf/ex.go:54 +0x16c
main.run(0xc000062360, 0x7ffe33e8b09a, 0x18, 0x0, 0x0)
        /home/zephyr/go/src/rsc.io/rf/rf.go:131 +0x331
main.main()
        /home/zephyr/go/src/rsc.io/rf/rf.go:44 +0x1b4

rf: local variable rename panic

Attempting to rename a variable in a function results in a panic in rewriteDefn. This occurs for the documentation example of mv F.who F.greeter, or as a test case:

mv F.x F.y
-- x.go --
package m

func F() {
        x := 123
        _ = x
}
-- stdout --
diff old/x.go new/x.go
--- old/x.go
+++ new/x.go
@@ -1,6 +1,6 @@
 package m

 func F() {
-       x := 123
-       _ = x
+       y := 123
+       _ = y
 }

$ go test
panic executing: mv F.x F.y
--- FAIL: TestRun (13.06s)
    --- FAIL: TestRun/mv_localvar.txt (0.05s)
        rf_test.go:85: testdata/mv_localvar.txt
panic: [*ast.Ident *ast.AssignStmt *ast.BlockStmt *ast.FuncDecl *ast.File] [recovered]
        panic: [*ast.Ident *ast.AssignStmt *ast.BlockStmt *ast.FuncDecl *ast.File] [recovered]
        panic: [*ast.Ident *ast.AssignStmt *ast.BlockStmt *ast.FuncDecl *ast.File]

goroutine 510 [running]:
testing.tRunner.func1.2(0x82a440, 0xc00577c190)
        /usr/lib/google-golang/src/testing/testing.go:1144 +0x332
testing.tRunner.func1(0xc00966c600)
        /usr/lib/google-golang/src/testing/testing.go:1147 +0x4b6
panic(0x82a440, 0xc00577c190)
        /usr/lib/google-golang/src/runtime/panic.go:965 +0x1b9
rsc.io/rf.run.func1(0xc00541dd90)
        /usr/local/google/home/mpratt/src/rf/rf.go:76 +0xe7
panic(0x82a440, 0xc00577c190)
        /usr/lib/google-golang/src/runtime/panic.go:965 +0x1b9
rsc.io/rf.rewriteDefn(0xc005778000, 0xc005776340, 0xc00577a07c, 0x1)
        /usr/local/google/home/mpratt/src/rf/mv.go:284 +0x78c
rsc.io/rf.cmdMv(0xc005778000, 0xc004bbf3e3, 0x7)
        /usr/local/google/home/mpratt/src/rf/mv.go:190 +0x123f
rsc.io/rf.run(0xc00305fc20, 0xc004bbf3d0, 0xb, 0x0, 0x0)
        /usr/local/google/home/mpratt/src/rf/rf.go:133 +0x30f
rsc.io/rf.TestRun.func1(0xc00966c600)
        /usr/local/google/home/mpratt/src/rf/rf_test.go:121 +0x48a
testing.tRunner(0xc00966c600, 0xc004bc5880)
        /usr/lib/google-golang/src/testing/testing.go:1194 +0xef
created by testing.(*T).Run
        /usr/lib/google-golang/src/testing/testing.go:1239 +0x2b3
exit status 2
FAIL    rsc.io/rf       13.093s

(Note I hand-edited the diff to what I expect the output to be, but it might be slightly off).

method to list identifiers

I would like to list either the exported identifiers, or all identifier for a package. Is this possible with the RF package or tool?

ex does not like rules including unused variables

For example, the script below could be used to simplify obtaining a map key, rewriting foo, _ := bar[idx] into foo := bar[idx]:

$ rf 'ex { var m map[string]int; var s string; x, _ := m[s] -> x := m[s] }'
ex.go:6:2: x declared but not used
ex.go:7:3: x declared but not used
ex: errors in example:
	package ipld
	func _() {
	var m map[string]int
	var s string
	{
	{x, _ := m[s] }
	{ x := m[s]}
	}
	}

This fails, because the rule snippet alone does not use the declared variable. Perhaps we should turn off such "not used" errors when loading rules, or perhaps we should add a special way to write rules to match declarations. A special way could then also match var foo, _ = bar[idx] and foo, _ = bar[idx], for example.

rf: panic in Refactor.Load

commit 6fb3311

While trying to create minimal example to demonstrate #9, I found the following panic.
Reproduce by running the testscript command:

exec rf 'ex {import "example/c"; Foo -> Bar}'
-- a/a.go --
package a

import (
	"example/b"
	"example/c"
)

var F = b.F

func init() {
	b.F = new(c.Foo)
}
-- a/a_test.go --
package a_test

import (
	"example/b"
	"example/c"
	"testing"
)

func TestFoo(t *testing.T) {
	b.F = new(c.Foo)
}
-- b/b.go --
package b

import "example/c"

var F *c.Foo
-- c/c.go --
package c

type Foo int
-- go.mod --
module example

go 1.17

I see this panic:

panic executing: 
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x64e7ce]

goroutine 1 [running]:
main.run.func1(0xc000179ed0)
	/home/rogpeppe/other/rf/rf.go:76 +0xe7
panic(0x6b28e0, 0x8905e0)
	/home/rogpeppe/go/src/runtime/panic.go:972 +0x1d4
rsc.io/rf/refactor.(*Refactor).Load(0xc00005a480, 0xc000070930, 0xc000014210, 0x2)
	/home/rogpeppe/other/rf/refactor/snap.go:336 +0x19ce
main.run(0xc00005a480, 0x7ffff469abc9, 0x23, 0x0, 0x0)
	/home/rogpeppe/other/rf/rf.go:101 +0x238
main.main()
	/home/rogpeppe/other/rf/rf.go:44 +0x1bb

rf: ex fails randomly with new loader code

The new loader code has broken ex commands that use packages that have _test.go in them (i.e., TestGoFiles). The test case below will fail about half of the time because rf doesn't think sub.Zero in x.go needs to be replaced. It seems to non-deterministically choose either the test or non-test variant of imported sub package for matching references to sub.Zero.

The other half of the time, there's a panic about overlapping edits. Replacing constant sub.Zero with function sub.Zero() works though.

ex import "example.com/sub"; sub.Zero -> 0
-- go.mod --
module example.com
-- x.go --
package m

import "example.com/sub"

var _ = sub.Zero
-- sub/sub.go --
package sub

const Zero = 0
-- sub/sub_test.go --
package sub
-- stdout --
diff old/x.go new/x.go
--- old/x.go
+++ new/x.go
@@ -1,5 +1,3 @@
 package m

-import "example.com/sub"
-
-var _ = sub.Zero
+var _ = 0

/cc @rsc