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: Rename Symbol incorrectly renames symbols when code contains a syntax error #68465

Open
renthraysk opened this issue Jul 16, 2024 · 4 comments
Labels
gopls/parsing Issues related to parsing / poor parser recovery. gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@renthraysk
Copy link

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.

@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 Jul 16, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jul 16, 2024
@hyangah
Copy link
Contributor

hyangah commented Jul 18, 2024

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"}]}]}

@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 18, 2024
@hyangah hyangah modified the milestones: Unreleased, gopls/v0.17.0 Jul 18, 2024
@tttoad
Copy link

tttoad commented Jul 19, 2024

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
Copy link
Contributor

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

@findleyr findleyr modified the milestones: gopls/v0.17.0, gopls/v0.18.0 Nov 19, 2024
@findleyr findleyr added the gopls/parsing Issues related to parsing / poor parser recovery. label Dec 18, 2024
@findleyr findleyr modified the milestones: gopls/v0.18.0, gopls/v0.19.0 Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/parsing Issues related to parsing / poor parser recovery. gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants