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: support change signature refactoring #38028

Open
stamblerre opened this issue Mar 23, 2020 · 5 comments
Open

x/tools/gopls: support change signature refactoring #38028

stamblerre opened this issue Mar 23, 2020 · 5 comments
Assignees
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@stamblerre
Copy link
Contributor

See microsoft/vscode-go#3122 for the original request.

Is your feature request related to a problem? Please describe.
I use IntelliJ and VS Code - both for development. VS Code usually for my open source contributions. I noticed that IntelliJ has this cool feature to change the definition / signature of a method or a function - where one can change the parameter's type, add a parameter with a default value for all the callers, and change the return parameters too

Describe the solution you'd like
VS Code to provide the feature to change the definition / signature of a method or function, which changes all the references of the method or function based on the change

Describe alternatives you've considered
Some features of the change signature feature can still be used currently - For example changing the name of the parameters - which will only cause a change inside the method or function and it can be done by doing just a rename. Other than that, the only alternate solution for me now is - to find all the references and change all the callers to add a parameter and do something similar for other things too.

Additional context
It would be cool for VS code to provide this feature. Not sure how it will be implemented. Is this a feature that should be provided by gopls? And if help is needed, I can pitch in - raise some PRs. I need to learn some stuff, which I can surely do. It would be a pleasure to raise PR for one of the greatest extensions for Golang 😄

@stamblerre stamblerre added FeatureRequest 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 23, 2020
@gopherbot gopherbot added this to the Unreleased milestone Mar 23, 2020
@stamblerre
Copy link
Contributor Author

We could also use change signature as a suggested fix for the fillreturns analyzer.

/cc @joshbaum

@stamblerre stamblerre removed this from the Unreleased milestone Jul 23, 2020
@nvinuesa
Copy link

nvinuesa commented Sep 9, 2020

I would greatly appreciate this feature! thanks for your work on gopls by the way 🙂

@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@goblinfactory
Copy link

goblinfactory commented Sep 3, 2021

(PING/KEEPALIVE)
Hi guys ...any news on this feature?
Given that GO doesnt have method overloads, this is a super critical need, since you cannot get away with search and replace to make these refactorings with a text editor/grep etc. Seems a bit overkill to have 1 person in the dev team manage all method refactorings with a single goland licence.

This thread looks dead, (no movement since October 2020) does this mean that as a noobie GOLANG user I've missed something (obvious to the wise grey haired folk) that makes this a non issue?

please advise...txs, Alan

@adonovan adonovan added the Refactoring Issues related to refactoring tools label Apr 24, 2023
@findleyr
Copy link
Contributor

FWIW this is now very feasible as a result of the inlining work @adonovan has been doing (e.g. #62650): a change signature refactoring can be implemented using inlining by creating a synthetic callee with the new signature, delegating the body of the current callee to the synthetic callee, and inlining the current callee.

In other words, Alan has done all the really hard (surprisingly hard) work of making inlining correct and safe, and change signature refactoring is now "just bookkeeping"...

Of course, it's not quite that easy:

  • Working with synthetic AST nodes is always difficult and buggy, due to loss of position information and comment associations.
  • Due to how inlining is implemented currently, we'd have to type-check the synthetic code, and so would have to first extract it into the callee package.
  • AFAIK there's no good LSP API for the type of change-signature UX one would presumably want. We could expose a custom command in gopls but would have to write client-side code to bind to that command.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/572296 mentions this issue: gopls/internal/golang: support removing unused parameters from methods

gopherbot pushed a commit to golang/tools that referenced this issue Mar 18, 2024
Make a minimal change to the removeparam algorithm to support removing
unused parameters of methods.

This doesn't address the problem of expanding the set of signatures or
calls based on interface satisfaction constraints: that is really a
separate concern, which also affects method renaming (golang/go#58461).
Were we to have a more general solution to that problem, it would be
relatively straightforward to also expand the change signature
algorithm.

For golang/go#38028

Change-Id: I296cb1f828f07d841c83b1fd33593ccd2fee3539
Reviewed-on: https://go-review.googlesource.com/c/tools/+/572296
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. 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