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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: inline multiple deprecated function calls at once #66466

Open
stapelberg opened this issue Mar 22, 2024 · 2 comments
Open

x/tools/gopls: inline multiple deprecated function calls at once #66466

stapelberg opened this issue Mar 22, 2024 · 2 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@stapelberg
Copy link
Contributor

gopls version

golang.org/x/tools/gopls v0.15.2

go env

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/stapelberg/.cache/go-build'
GOENV='/home/stapelberg/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/stapelberg/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/stapelberg/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go-1.21'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go-1.21/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.6'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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-build1079342930=/tmp/go-build -gno-record-gcc-switches'

What did you do?

The situation was that I had a function that made many calls
to a deprecated function — let’s say it was ioutil.ReadFile:

package main

import "io/ioutil"

func main() {
	_, _ = ioutil.ReadFile("/tmp/foo")
	_, _ = ioutil.ReadFile("/tmp/bar")
}

What did you see happen?

When I place my Emacs point on ioutil.ReadFile and run M-x eglot-code-actions,
I do see the expected code action as the only completion:

image

But when I select multiple ioutil.ReadFile calls and run M-x eglot-code-actions,
I only get “Extract function” as the sole completion option:

image

What did you expect to see?

Would you happen to know if inlining multiple functions at once just isn’t possible in LSP?
Or is it possible in LSP, but not possible in Emacs’s eglot?

In my case, I resorted to string replacement, because it was quicker.
Ideally, the inliner would be even quicker to reach for :)

Editor and settings

Emacs 29 with builtin eglot

Logs

No response
@stapelberg stapelberg added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Mar 22, 2024
@gopherbot gopherbot added this to the Unreleased milestone Mar 22, 2024
@adonovan
Copy link
Member

Hi Michael, that's a good question.

LSP doesn't really have an opinion here: its "code actions" RPC requests the set of available commands for the selected region. If the client invokes one of these commands, it has the side effect of making the server send an "apply edits" downcall to the client. So there's nothing stopping the server from offering one "inline" code action per function call in the selected region, but it would be confusing to the user if the menu of code actions had potentially hundreds of alternative "inline" operations. That's why we only offer an inline code action if the selection is within a function call.

But I think you're asking for a way to "apply all" inline commands for all calls to a particular function within a region, which seems like a reasonable operation. It's just not obvious to me what the best UX would be for exposing it. We could detect that there are multiple ReadFile calls within the same source file and, if so, offer both "inline" and "inline all calls to ReadFile". But why stop at the file? What about the package, or module?

Or workspace: we have an open feature request for an "inline away" operation that would inline every call to ReadFile that can be found by a 'references' query. I wonder whether that is the real feature you are looking for.

There's a confounding factor, which is that complex inlinings don't always compose, but I suspect in practice simple cases like ReadFile, the edits are all non-conflicting.

@stapelberg
Copy link
Contributor Author

Ah! Yes, indeed, inline all references would have worked just fine in my case.
There’s still some value in inline-all-in-region (as it allows migrating piece by piece),
but probably inline-all-references is the more important feature to focus on :)

Thank you

@suzmue suzmue modified the milestones: Unreleased, gopls/backlog Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
4 participants
@stapelberg @adonovan @suzmue @gopherbot