golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.52k stars 17.6k forks source link

x/tools/refactor/rename: provide a way to enumerate symbols in gorename notation #60831

Closed ghost closed 9 months ago

ghost commented 1 year ago

currently this package and this tool are available:

which accepts input like this:

"bytes".Buffer.Len

however to my knowledge, no package exists that can emit all the identifiers of a package, in a format friendly to gorename. this is somewhat close:

go doc -all -u bytes

but the output is not machine readable. this is somewhat close:

https://pkg.go.dev/cmd/api

but again not machine readable, and I dont think it supports anything outside the standard library.

ianlancetaylor commented 1 year ago

What about go doc -short bytes ?

dmitshur commented 1 year ago

CC @golang/tools-team.

adonovan commented 1 year ago

Ostensibly you are asking for a function that would enumerate all the methods (etc) of a package in the correct form for cmd/gorename so that you can then choose the one you want without having to think too hard about the syntax. Such a function would be easy to write, but I suspect that's not really the problem you're trying to solve. Do you perhaps wish gorename used a different syntax? Or offered some kind of menu or interactive choice? Or that there was a programmatic API through which you could express the renaming request? The solution to each of those problems is different.

ghost commented 1 year ago

Do you perhaps wish gorename used a different syntax? Or offered some kind of menu or interactive choice? Or that there was a programmatic API through which you could express the renaming request?

the tool could be better, but its probably good enough for now. the real issue is that the tool accepts some input, but some method should also be available to generate that input. that could be any of these:

if this new item was called "identifiers", someone could call it to generate all the identifiers for a package, then use the list as is, or narrow it down based on some criteria, and feed the result to x/tools/refactor/rename or gorename. as it currently stands, no tool or package is available to generate a list of identifiers, which makes using x/tools/refactor/rename and gorename painful for all but the most simple of use cases.

adonovan commented 1 year ago
  • an addition to the gorename tool

gorename is not well supported: it dates from a time before Go modules, and uses an old library to load source code that cannot work well with modules. We don't plan to extend it.

  • some new tool

We built gopls, among other reasons, to improve the experience for users of tools like this one. Have you tried it? If you want to rename from within your editor, you'll find it works much better than gorename.

gopls does have a command-line too, but it uses file names and byte offsets to identify the symbol to be renamed, for example: gopls rename file.go:#123 newname. On the one hand, these identifiers are not very readable, but on the other, they are easy to write. I'm not sure if that's better or worse from your point of view---you still haven't said what you're actually trying to do. ;-)

ghost commented 1 year ago

thats way, way worse. I dont have an editor that supports a language server (GVIM), or if it does, I dont know or care how to set it up or use it. I think a simpler case would be something like what I have already described. I am interested in doing mass renaming inside a package, so I dont wish to do that process via an editor. I would like to do it from the command line, either via a premade tool, or one I can conjure from a package that gets most of the way there.

If GoPLS has a way to accept or produce non line number based identifiers, then I would be interested.

adonovan commented 1 year ago

I dont have an editor that supports a language server (GVIM), or if it does, I dont know or care how to set it up or use it.

Vim does have an LSP client, for what it's worth, and if Gvim is fundamentally the same application then it probably does too.

I am interested in doing mass renaming inside a package, so I don't wish to do that process via an editor. I would like to do it from the command line, either via a premade tool, or one I can conjure from a package that gets most of the way there.

Ah! In that case you may be interested in https://github.com/rsc/rf, which is a scriptable batch refactoring tool. It's based on the same refactoring algorithm as gorename but has a much nicer batch interface.

If GoPLS has a way to accept or produce non line number based identifiers, then I would be interested.

Currently it does not, sorry. We could add one, but I wouldn't want to use the syntax of parseFromFlag in gorename: it's too hard to type (parens and quotation marks need escaping), and too hard to remember (what does :: mean?). The whole 'search' feature was added in pursuit of a generality (the ability to specify any object, even those declared within a function) that just isn't interesting in practice, since such a renaming is inherently local to that function. If users are going to go to the trouble of specifying 'from' symbols by some kind of name, it's because they have a larger scope. 'go doc' uses much more user-friendly heuristics to interpret dotted identifiers such as bytes.Buffer.Len, and I should have followed that approach.

If you want to take a crack at writing a function to print the names of package symbols in gorename syntax, I suggest you use this command as a starting point:

$ go run ./go/packages/gopackages -mode=types bytes
Go package "bytes":
    ...
    type Buffer struct{buf []byte; off int; lastRead readOp}
    method (*Buffer) Bytes() []byte
    func Clone(b []byte) []byte
    var ErrTooLarge error
    const MinRead untyped int
    ....

Unfortunately it is unlikely to be a priority for us, given the existing problems with gorename that I mentioned.

Best of luck.

ghost commented 1 year ago

@adonovan I checked both those suggestions, they both seems to have issues. I linked them above. Also regarding the actual renaming, I noticed that rsc.io/rf is about twice as fast as golang.org/x/tools/cmd/gorename:

> Measure-Command { rf 'mv FinishedHash _FinishedHash' }
TotalSeconds      : 1.6925358

> Measure-Command { rf 'mv FinishedHash _FinishedHash' }
TotalSeconds      : 1.7182022

> Measure-Command { gorename -from '\".\".FinishedHash' -to _FinishedHash }
TotalSeconds      : 3.5773823

> Measure-Command { gorename -from '\".\".FinishedHash' -to _FinishedHash }
TotalSeconds      : 3.4577449
adonovan commented 1 year ago

@adonovan I checked both those suggestions, they both seems to have issues. I linked them above.

Regarding the first problem (missing unexported declarations), try -mode=syntax, as explained in https://github.com/golang/go/issues/60995.

I noticed that rsc.io/rf is about twice as fast as golang.org/x/tools/cmd/gorename:

gorename uses the old go/loader package, and loading, parsing and type-checking is the dominant cost (not renaming itself), so I'm not surprised that it's slower.

ghost commented 1 year ago

Regarding the first problem (missing unexported declarations), try -mode=syntax, as explained in #60995.

thanks, that does fix it. one strange issue I noticed with x/tools/go/packages and the tool, is you seem to have no control in regards to the output of derivative types. so if you have this code:

package tls

import "math/big"

type DsaSignature struct {
   R, S *big.Int
}

type EcdsaSignature DsaSignature

you get this result:

> gopackages -mode syntax .
Go package "exports/tls":
        package tls
        has complete exported type info and typed ASTs
        file D:\Desktop\exports\tls\common.go
        import "math/big"
        type DsaSignature struct{R *math/big.Int; S *math/big.Int}
        type EcdsaSignature struct{R *math/big.Int; S *math/big.Int}

compared to something like go doc, which I think is a better representation of the type relationship:

> go doc -all
package tls // import "exports/tls"

TYPES

type DsaSignature struct {
        R, S *big.Int
}

type EcdsaSignature DsaSignature

what I mean is, the gopackages output makes it seem like you could rename the fields of one type without the other, when thats not the case. I believe go/ast gives you more control over this, but maybe I am missing something.

adonovan commented 1 year ago

one strange issue I noticed

The reason for this difference is that go doc is formatting the syntax tree, whereas gopackages is formatting the type information, and in the go/types representation of Named types resulting from type A struct { ... }; type B A) there is no relationship between A and B. (You can figure it out based on positions, but that's a bit of a hack.)

See #8178 for details.

ghost commented 1 year ago

I cobbled together my own solution using go/ast and Russ tool rf, with a fallback of x/tools/cmd/gorename. I am moving on. If any wants to know more feel free to comment. closing this. Alan you are welcome to reopen if you think its worth it.

adonovan commented 1 year ago

I am curious about approach you took in the end; if you're comfortable, please share a link to your code. Glad you got it working; sorry the process was not smoother.

ghost commented 1 year ago

heres the code I used in the end:

https://github.com/1268/exports

I tried to keep it as generic as possible, but it is somewhat specific to my use case - hopefully it can at least be a starting point to anyone in a similar situation - thanks for the help along the way

adonovan commented 1 year ago

What was the actual problem you were trying to solve? It seems like it may have been "rename unnecessarily exported identifiers to lowercase". In that case you may be interested in a tool I wrote (by chance) this week, golang.org/x/tools/internal/cmd/deadcode@master, that reports dead code (functions that cannot be reached from main), perhaps more precisely and efficiently than whatever tool you were using before.

ghost commented 1 year ago

I want to be able to take some arbitrary package and and remove everything that I personally dont care about. previously you could use StaticCheck with whole-program mode, but that was removed in 2020:

https://github.com/dominikh/go-tools/commit/5cfc85b

so now items are only reported as unused, if they are not exported. so to solve that, I needed some method to un export all items of an arbitrary package, which Russ RF and x/tools/cmd/gorename can do. however the missing piece is some method to report all exported items, to feed into one of those tools. thats what my package does. it sounds like your tool might be a better option, I will check it out. I can report back my findings if you want.

adonovan commented 1 year ago

Conceptually there are two parameters: the set of roots (executables and tests), and the filter subset. The packages arguments and the -test flag determine the roots; and the -filter flag determines what is shown.

deadcode -test ./hello/... reports all the functions that are linked into a executable or test in the hello tree but are unreachable. deadcode ./hello/world reports functions linked into the hello/world executable that are unreachable by it (but may be reachable by tests, or other binaries).

I'm open to suggestions for a better command-line interface; it worked for my needs but I sense it may be tricky for others.

adonovan commented 10 months ago

Was this issue reopened intentionally? Could you elaborate on what changed? Thanks.

gopherbot commented 9 months ago

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)