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/cmd/gorename: does not work on public functions and structs #30617

Closed
markvincentcaro opened this issue Mar 6, 2019 · 6 comments
Closed

Comments

@markvincentcaro
Copy link

markvincentcaro commented Mar 6, 2019

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

go version go1.12 windows/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\My Username\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=E:\GoCodes
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\MY USE~1\AppData\Local\Temp\go-build133335722=/tmp/go-build -gno-record-gcc-switches

What did you do?

My sample project has a package called mypkg. Inside mypkg is a file called util.go.

util.go:

package mypkg

func TwoPlusTwo() int {
    return 2 + 2
}

Function TwoPlusTwo() is used in main.go.

main.go:

package main

import (
    "fmt"
    "myproject/mypkg"
)

func main() {
    fmt.Println(mypkg.TwoPlusTwo())
}

I then renamed TwoPlusTwo() to GetTwoPlusTwo().

What did you expect to see?

I expected TwoPlusTwo() to be renamed to GetTwoPlusTwo() in all its usages.

What did you see instead?

Function TwoPlusTwo() only got renamed to GetTwoPlusTwo() inside mypkg.

It stayed as TwoPlusTwo() in main.go, which now triggers a compiler error because it can't find the function.

UPDATE: This issue is more serious than I thought because it seems to happen even for public Structs. Gorename does not rename their usages in other packages.

@gopherbot gopherbot added this to the Unreleased milestone Mar 6, 2019
@markvincentcaro
Copy link
Author

I think this is a pretty serious issue because using Find And Replace can have unexpected results even with "Match Whole Word" and "Match Case" turned on.

@markvincentcaro
Copy link
Author

UPDATE: This issue is more serious than I thought because it seems to happen even for public Structs. Gorename does not rename their usages in other packages.

@markvincentcaro markvincentcaro changed the title x/tools/cmd/gorename: Does not work on Global Functions x/tools/cmd/gorename: Does not work on Global Functions and Structs Mar 8, 2019
@markvincentcaro markvincentcaro changed the title x/tools/cmd/gorename: Does not work on Global Functions and Structs x/tools/cmd/gorename: Does not work on Public Functions and Structs Mar 8, 2019
@markvincentcaro markvincentcaro changed the title x/tools/cmd/gorename: Does not work on Public Functions and Structs x/tools/cmd/gorename: does not work on public functions and structs Apr 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

@ianthehat, what's the word on gorename?

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/170863 mentions this issue: cmd/gopherbot: CC triaged issues to owners

@ianthehat
Copy link

At this point gorename does what it does and we have no plans to make it do anything more. If there were actually a critical bug, we might fix it, but we don't plan to make gorename work in module mode, and we don't plan to add more features.
This counts as a feature request (expanding the scope of its rename powers) not a bug. It was always a best effort tool that may take manual clean up when finished.
If anyone else wants to improve it we will happily review the code and get it submitted, but fair warning that we plan to deprecate this tool and add renaming capabilities in a very different form to gopls instead.

@ianthehat ianthehat removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 16, 2019
@markvincentcaro
Copy link
Author

markvincentcaro commented Apr 16, 2019

@ianthehat So golang basically had an unfinished rename tool right from the start? That's kinda sad to learn. :(

>This counts as a feature request (expanding the scope of its rename powers) not a bug. It was always a best effort tool that may take manual clean up when finished.
This not counting as a bug because gorename was never designed to have project-wide scope to begin with puzzles me. What was the point of gorename scanning the whole gopath directory (and taking a very long time to do so, enough to warrant several issues being posted here about it) if it was not designed to rename files outside the current package?

In any case, seeing as how the Go team is now focusing on the new gopls tool, I wish you all the best of luck with it and hope that you awesome guys can at least make it work like a standard renaming tool and not a best effort one.

@bcmills bcmills closed this as completed Apr 16, 2019
@golang golang locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants