x/tools/gopls: Rename Symbol incorrectly renames symbols when code contains a syntax error

renthraysk commented 1 month ago

Go version

go version go1.22.5 linux/amd64 & golang.org/x/tools/gopls v0.16.1

Output of `go env` in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/ren/.cache/go-build'
GOENV='/home/ren/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/ren/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/ren/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/ren/dev/experiments/http/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1814768367=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Using Rename Symbol on the first attrs to rename to fhAttrs in the function below, results in the other usage of the symbol to be changed to afhAttrs0]

package main

import (
    "log/slog"
)

func Log(log *slog.Logger, buf []byte) {
    attrs := []slog.Attr{
        0:      {Key: "len"},
    }
    if len(buf) < 9 {
        log.
    }
    attrs[0].Value = slog.IntValue(len(buf))
}

What did you see happen?

package main

import (
    "log/slog"
)

func Log(log *slog.Logger, buf []byte) {
    fhAttrs := []slog.Attr{
        0:      {Key: "len"},
    }
    if len(buf) < 9 {
        log.
    }
    afhAttrs0].Value = slog.IntValue(len(buf))
}

What did you expect to see?

Either fail to rename the symbol due to the syntax error, or work as expected.

gabyhelp commented 1 month ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

hyangah commented 1 month ago

Thanks for the report with the repro. I could reproduce the issue. (Personally, I am surprised that rename works on sources in broken state :-))

gopls trace:

[Trace - 15:55:00.783 PM] Sending request 'textDocument/prepareRename - (39)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/xx/main.go"},"position":{"line":7,"character":3}}

[Trace - 15:55:00.784 PM] Received response 'textDocument/prepareRename - (39)' in 0ms.
Result: {"range":{"start":{"line":7,"character":1},"end":{"line":7,"character":6}},"placeholder":"attrs"}
...

[Trace - 15:55:04.343 PM] Sending request 'textDocument/rename - (42)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/xx/main.go"},"position":{"line":7,"character":3},"newName":"fhAttrs"}

[Trace - 15:55:04.344 PM] Received response 'textDocument/rename - (42)' in 0ms.
Result: {"documentChanges":[
{"textDocument":{
  "version":1,"uri":"file:///Users/hakim/xx/main.go"},
  "edits":[
     {"range":{"start":{"line":7,"character":1},"end":{"line":7,"character":6}},"newText":"fhAttrs"},
     {"range":{"start":{"line":13,"character":2},"end":{"line":13,"character":7}},"newText":"fhAttrs"}]}]}

tttoad commented 1 month ago

This is because the rename() function resolves this .

package main

import (
    "log/slog"
)

func Log(log *slog.Logger, buf []byte) {
    attrs := []slog.Attr{
        0:      {Key: "len"},
    }
    if len(buf) < 9 {
        log._   // Notice here. Added a placeholder.
    }
    attrs[0].Value = slog.IntValue(len(buf))
}

The protocol.NewMapper() function reads the original file without placeholders.

package main

import (
    "log/slog"
)

func Log(log *slog.Logger, buf []byte) {
    attrs := []slog.Attr{
        0:      {Key: "len"},
    }
    if len(buf) < 9 {
        log.
    }
    attrs[0].Value = slog.IntValue(len(buf))
}

gopherbot commented 1 month ago

Change https://go.dev/cl/599975 mentions this issue: gopls/rename: Fix rename failure when code contains syntax error.

golang / go