Skip to content

x/tools/gopls: rename doesn't propagate to the implementors #65098

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

Closed
denpeshkov opened this issue Jan 14, 2024 · 4 comments
Closed

x/tools/gopls: rename doesn't propagate to the implementors #65098

denpeshkov opened this issue Jan 14, 2024 · 4 comments
Assignees
Labels
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. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@denpeshkov
Copy link

Go version

devel go1.22-1d45a7ef56 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/denpeshkov/Library/Caches/go-build'
GOENV='/Users/denpeshkov/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/denpeshkov/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/denpeshkov/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/denpeshkov/src/go.googlesource.com/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/denpeshkov/src/go.googlesource.com/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='devel go1.22-1d45a7ef56 Wed Jan 10 03:29:50 2024 +0000'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/denpeshkov/src/github.com/denpeshkov/greenlight/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/nw/fts0xshx033fq3q8rn4rhjhc0000gn/T/go-build755587796=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

==> ./foo/foo.go <==

package foo

type I interface {
	F()
}

==> ./bar/bar.go <==

package bar

type S struct{}

func (s S) F() {}

==> ./qux/qux.go <==

package qux

import (
	"github.com/denpeshkov/doodles/bar"
	"github.com/denpeshkov/doodles/foo"
)

var _ foo.I = bar.S{}

The renaming of the function foo/I.F() to foo/I.FF() doesn't propagate to bar/S.F()

Checked with both [email protected] in VSCode and gorename

What did you see happen?

bar/S.F() renamed to bar/S.FF()

What did you expect to see?

bar/s.F() is not renamed

@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 Jan 14, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jan 14, 2024
@adonovan
Copy link
Member

adonovan commented Jan 16, 2024

Thanks for the report; I can reproduce and confirm this bug using gopls@master. Possibly a dup or relative of #58461.

FWIW, improving refactoring (including renaming) will be a major focus of our team this year.

@adonovan adonovan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Refactoring Issues related to refactoring tools labels Jan 16, 2024
@hyangah hyangah modified the milestones: Unreleased, gopls/v0.16.0 Jan 17, 2024
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 May 23, 2024
@findleyr findleyr modified the milestones: gopls/v0.17.0, gopls/v0.18.0 Oct 22, 2024
@dennypenta
Copy link

@adonovan any update on design this feature?
what do you think, is it possible to also implement the other way around, renaming a single implementor to rename the interface and the rest implementors?

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/667135 mentions this issue: gopls/internal/golang: add test for (unfixed) issue 65098

@adonovan
Copy link
Member

any update on design this feature?

I just confirmed that this indeed a dup of #58461, and added a test, but still no fix yet unfortunately. Perhaps for gopls v0.20.

is it possible to also implement the other way around, renaming a single implementor to rename the interface and the rest implementors?

No, we rely on the interface/implementation to infer the user's intent: renaming an interface method causes all methods to be renamed; but renaming a concrete method causes an error if we encounter an interface method that matches. This allows users to control how broad a renaming they want.

@adonovan adonovan closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2025
gopherbot pushed a commit to golang/tools that referenced this issue Apr 22, 2025
The test confirms the reported problematic behavior,
and investigation shows it to be (as suspected) a dup of

Also, a couple of minor cleanups from static analyzers.

Updates golang/go#58461
Updates golang/go#65098

Change-Id: I316c2f3bd23005526ccd9da461a18f1198373fe4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/667135
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. Refactoring Issues related to refactoring tools 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