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.

Sign up for GitHub

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

Jump to bottom

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
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@leitzler
Copy link
Contributor

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 Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jun 16, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jun 16, 2020
@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Jun 16, 2020
@gopherbot
Copy link
Contributor

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
@leitzler @stamblerre
internal/lsp/source: avoid panic in rename check
dc31b40 
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

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

@golang golang locked and limited conversation to collaborators Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
2 participants
@leitzler @gopherbot