rsc / rf

A refactoring tool for Go
BSD 3-Clause "New" or "Revised" License
669 stars 21 forks source link

rf: panic in Refactor.Load #10

Open rogpeppe opened 3 years ago

rogpeppe commented 3 years ago

commit 6fb3311e1224f1af9839bdef5161e043863fe93f

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
rogpeppe commented 3 years ago

I investigated this a bit. It seems that the reason for the panic is that there is no target set here which seems to be because there is no package in the current (top level) directory (this condition is never triggered.

Thus, adding a file dummy.go containing package dummy in the top level directory is a workaround for the panic.

What should the target be if there's no package in the current directory? I've probably misunderstood what "target" means here, but... does it even make sense to have a single target when the tests in every package have their own type universe?