Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: panic in textDocument.rename #39614

Closed
leitzler opened this issue Jun 16, 2020 · 2 comments
Closed

x/tools/gopls: panic in textDocument.rename #39614

leitzler opened this issue Jun 16, 2020 · 2 comments

Comments

@leitzler
Copy link
Contributor

@leitzler leitzler commented Jun 16, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.4 darwin/amd64
$ go list -m golang.org/x/tools golang.org/x/tools/gopls
golang.org/x/tools v0.0.0-20200612022331-742c5eb664c2
golang.org/x/tools/gopls v0.0.0-20200612022331-742c5eb664c2

Does this issue reproduce with the latest release?

n/a

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
n/a

What did you do?

Tried to rename var foo in the following example.
Note the missing ) after make that results in a compilation error.

package x

func fn() {
    var foo bool
    make(map[string]bool
    if foo {
    }
}

request:

gopls.Rename() call; params:
&protocol.RenameParams{
    TextDocument:           protocol.TextDocumentIdentifier{URI:"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.ETgC2Ck2/x.go"},
    Position:               protocol.Position{Line:3, Character:8},
    NewName:                "bar",
    WorkDoneProgressParams: protocol.WorkDoneProgressParams{},
}

What did you expect to see?

No panic, other than that I'm not entirely sure. I appreciate the fact that it is possible to rename even if the source doesn't compile. But in an example like this the if-case foo isn't valid source so that one won't be renamed.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x13f04d1]

goroutine 139 [running]:
golang.org/x/tools/internal/lsp/source.deref(0x0, 0x0, 0xc0005ee4c0, 0x1fbf040)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/util.go:401 +0x41
golang.org/x/tools/internal/lsp/source.forEachLexicalRef.func1(0x1a457a0, 0xc000032800, 0x1a45b00)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename_check.go:319 +0x1c1
go/ast.inspector.Visit(0xc00007fae0, 0x1a457a0, 0xc000032800, 0x0, 0x0)
   /usr/local/go/src/go/ast/walk.go:373 +0x3a
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a457a0, 0xc000032800)
   /usr/local/go/src/go/ast/walk.go:52 +0x66
go/ast.walkExprList(0x1a422e0, 0xc00007fae0, 0xc000032840, 0x4, 0x4)
   /usr/local/go/src/go/ast/walk.go:26 +0x9e
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a45560, 0xc0000329c0)
   /usr/local/go/src/go/ast/walk.go:137 +0x1096
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a45920, 0xc0002008b0)
   /usr/local/go/src/go/ast/walk.go:196 +0x1b78
go/ast.walkStmtList(0x1a422e0, 0xc00007fae0, 0xc0002a2920, 0x2, 0x2)
   /usr/local/go/src/go/ast/walk.go:32 +0x9e
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a454e0, 0xc0004104e0)
   /usr/local/go/src/go/ast/walk.go:224 +0x1adf
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a45a60, 0xc000410510)
   /usr/local/go/src/go/ast/walk.go:344 +0xd98
go/ast.walkDeclList(0x1a422e0, 0xc00007fae0, 0xc000200900, 0x1, 0x1)
   /usr/local/go/src/go/ast/walk.go:38 +0x9e
go/ast.Walk(0x1a422e0, 0xc00007fae0, 0x1a459e0, 0xc00013a600)
   /usr/local/go/src/go/ast/walk.go:353 +0x264e
go/ast.Inspect(...)
   /usr/local/go/src/go/ast/walk.go:385
golang.org/x/tools/internal/lsp/source.forEachLexicalRef(0x1a63ec0, 0xc0002a1040, 0x1a65ea0, 0xc000215f90, 0xc0004139c0, 0x0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename_check.go:337 +0x177
golang.org/x/tools/internal/lsp/source.(*renamer).checkInLexicalScope(0xc00012e780, 0x1a65ea0, 0xc000215f90, 0x1a63ec0, 0xc0002a1040)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename_check.go:213 +0xd7
golang.org/x/tools/internal/lsp/source.(*renamer).checkInLocalScope(0xc00012e780, 0x1a65ea0, 0xc000215f90)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename_check.go:143 +0x95
golang.org/x/tools/internal/lsp/source.(*renamer).check(0xc00012e780, 0x1a65ea0, 0xc000215f90)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename_check.go:48 +0x1e7
golang.org/x/tools/internal/lsp/source.Rename(0x1a53ce0, 0xc0005e4120, 0x1a65900, 0xc00033c000, 0x1a57920, 0xc0002b8000, 0x4008000000000000, 0x4020000000000000, 0xc0000abcf0, 0x3, ...)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/source/rename.go:112 +0x5bf
golang.org/x/tools/internal/lsp.(*Server).rename(0xc0000b1880, 0x1a53c20, 0xc000204880, 0xc0002a4e00, 0x0, 0x0, 0xc0005d7080)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/rename.go:19 +0x117
golang.org/x/tools/internal/lsp.(*Server).Rename(0xc0000b1880, 0x1a53c20, 0xc000204880, 0xc0002a4e00, 0xc0002a4e00, 0x0, 0x0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/server_gen.go:148 +0x49
golang.org/x/tools/internal/lsp/protocol.serverDispatch(0x1a53c20, 0xc000204880, 0x1a6e1c0, 0xc0000b1880, 0xc0005e4000, 0x1a53e60, 0xc0002047c0, 0x0, 0x0, 0xbfb251e1d6e079d0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/protocol/tsserver.go:374 +0x1147
golang.org/x/tools/internal/lsp/protocol.ServerHandler.func1(0x1a53c20, 0xc000204880, 0xc0005e4000, 0x1a53e60, 0xc0002047c0, 0x0, 0x0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/protocol/protocol.go:62 +0xc0
golang.org/x/tools/internal/lsp/lsprpc.handshaker.func1(0x1a53c20, 0xc000204880, 0xc0005e4000, 0x1a53e60, 0xc0002047c0, 0x0, 0x0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/lsp/lsprpc/lsprpc.go:420 +0x40e
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1(0x1a53c20, 0xc000204880, 0xc00032ef20, 0x1a53e60, 0xc0002047c0, 0x18f94ce, 0x11)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/jsonrpc2/handler.go:35 +0xd3
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2(0xc000096840, 0xc00039cc00, 0xc00030f4c0, 0x1a53c20, 0xc000204880, 0xc00032ef20, 0x1a53e60, 0xc0002047c0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/jsonrpc2/handler.go:103 +0x86
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.0.0-20200612022331-742c5eb664c2/internal/jsonrpc2/handler.go:100 +0x171
@gopherbot gopherbot added this to the Unreleased milestone Jun 16, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 16, 2020

Change https://golang.org/cl/238042 mentions this issue: internal/lsp/source: avoid panic in rename check

gopherbot pushed a commit to golang/tools that referenced this issue Jun 16, 2020
In a case where the type info for an ast.CompositeLit isn't found in
forEachLexicalRef(...) gopls will panic.

e.g. trying to rename "foo" where it is declared in this case:
func fn() {
	var foo bool
	make(map[string]bool
	if foo {
	}
}

Note the missing ')' after make.

This change will skip ast.CompositeLits if the type can't be found, and
that fixes the panic. But it also do rename the identifier as long as it
is possible, and I'm not convinced that we should allow rename at all if
the source can't be compiled.

Updates golang/go#39614

Change-Id: Ibb50b15ce4b31056f2f1da52a4dcab7b8b91a320
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238042
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
@leitzler
Copy link
Contributor Author

@leitzler leitzler commented Jun 17, 2020

As CL238042 was merged this isn't an issue anymore, closing.

@leitzler leitzler closed this Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.