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: implement "remove a parameter" refactoring #63534

Closed
findleyr opened this issue Oct 13, 2023 · 4 comments
Closed

x/tools/gopls: implement "remove a parameter" refactoring #63534

findleyr opened this issue Oct 13, 2023 · 4 comments
Assignees
@findleyr
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge 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/v0.14.0

Comments

@findleyr
Copy link
Member

Belatedly filing an issue to track a project I've been pursuing for the past two weeks: using the new inlining technology to implement change signature refactoring (#38028).

As a first instance of this, I'm implementing a refactoring to remove unused parameters, which is more tractable than other rewrites for reasons that I will explain in the commit message.
@findleyr findleyr 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 Oct 13, 2023
@findleyr findleyr added this to the gopls/v0.14.0 milestone Oct 13, 2023
@findleyr findleyr self-assigned this Oct 13, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/532495 mentions this issue: gopls/internal/lsp: add code actions to remove unused parameters

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/534918 mentions this issue: internal/refactor/inline: less hacky solution for eliding braces

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/535335 mentions this issue: internal/refactor/inline: actually check for last uses of free vars

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/535795 mentions this issue: gopls/internal/lsp/source: abort change signature on overlapping calls

gopherbot pushed a commit to golang/tools that referenced this issue Oct 16, 2023
@findleyr @gopherbot
gopls/internal/lsp/source: abort change signature on overlapping calls
8a71c39 
The heuristic used to identify remaining uninlined calls by counting
fails when the inlining operation changes the order of "original" calls,
as may be the case with overlapping calls, following CL 535456.

I spent around an hour trying to fix this properly (by tracking calls
that were "just" inlined), but it's too hard because we must also track
calls that had previously been inlined. It needs more theory and
thought. For now, remove support for overlapping calls, as in all other
cases the top-to-bottom counting heuristic should still work.

For golang/go#63534

Change-Id: I59fe4aa5d20e56b0eb9dcbc01d29dd2ece082c17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535795
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Oct 16, 2023
@findleyr
internal/refactor/inline: less hacky solution for eliding braces
61bb3e9 
Joint with Alan Donovan (CL 535455) :)

For golang/go#63534

Change-Id: Ia4fdc617edf6699da285f56670a51efac1817834
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534918
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Oct 16, 2023
@findleyr
internal/refactor/inline: improve check for last uses of free vars
53e637b 
This showed up when I used the inliner for removing parameters: we
should be more precise about detecting the last use of a local var.

A TODO is left to make this perfect, which will be done in a subsequent
CL.

For golang/go#63534

Change-Id: Id5c753f3e7ae51e07db1d29a59e82e51c6d5952c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535335
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@findleyr findleyr added the FeatureRequest Issues asking for a new feature that does not need a proposal. label Oct 16, 2023
@golang golang locked and limited conversation to collaborators Oct 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@findleyr findleyr

Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge 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/v0.14.0
Development

No branches or pull requests
2 participants
@gopherbot @findleyr