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

Jump to bottom

x/tools/gopls: improvements to package renaming #56184

Closed
3 tasks done
findleyr opened this issue Oct 12, 2022 · 9 comments
Closed
3 tasks done

x/tools/gopls: improvements to package renaming #56184

findleyr opened this issue Oct 12, 2022 · 9 comments
Assignees
@dle8
Labels
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.12.0

Comments

@findleyr
Copy link
Contributor

findleyr commented Oct 12, 2022 •
edited by dle8

This issue tracks improvements to package renaming that we are aiming to land in gopls@v0.11.0, namely:

  • Add a test for the case when the renaming package's path contains "internal" as a segment.
  • Don't rename subpackages that don't belong to the same module with the renaming package (e.g. if we're renaming lib->lib1 and lib/submod is a sub module, then we should not rename packages in lib/submod -- this may actually work already but needs verification)
  • Check the go.mod files in the workspace to see if any replace directives need to be fixed if the renaming affects the locations of any go.mod files (e.g. in the previous step, any replace directives pointing at lib/submod need to be updated).
@findleyr findleyr added this to the gopls/v0.11.0 milestone Oct 12, 2022
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Oct 12, 2022
@bcmills
Copy link
Contributor

bcmills commented Oct 13, 2022

Reject renaming if the renamed subpackages don't belong to the same module with the renaming package

Hmm. I think it should be fine to rename across modules as long as both modules are in the workspace..?

@findleyr findleyr mentioned this issue Oct 20, 2022
Closed
@uh-zz
Copy link

uh-zz commented Nov 1, 2022

I was looking forward to this feature!

I tried package rename in the following environment and it seems that the renamed file is newly created and the original file is deleted.
Is this expected behavior?

Visual Studio Code v1.72.2
golang.org/x/tools/gopls v0.10.0

@findleyr
Copy link
Contributor Author

findleyr commented Nov 1, 2022

@uh-zz yes, this is how VS Code handles the RenameFile operation. Unfortunately, I believe this is out of our control.

@gopherbot
Copy link

Change https://go.dev/cl/450556 mentions this issue: gopls/internal/regtest: add a test for the case when the renaming package's path contains "internal" as a segment

@gopherbot
Copy link

Change https://go.dev/cl/450557 mentions this issue: gopls/internal/lsp: change PrepareRename behavior to prompt with the full package path

gopherbot pushed a commit to golang/tools that referenced this issue Dec 5, 2022
gopls/internal/regtest: add a test for the case when the renaming pac…
c9ea9a7 
…kage's path contains "internal" as a segment

For golang/go#56184.

Change-Id: Id33650adb5befb0a2cdead28425ad6ec8f7d9e0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/450556
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@findleyr findleyr mentioned this issue Dec 8, 2022
Open
3 tasks
@findleyr
Copy link
Contributor Author

findleyr commented Dec 8, 2022

I've updated the tasks here to separate the feature request of supporting arbitrary renaming of package paths into #57171.

@gopherbot
Copy link

Change https://go.dev/cl/459416 mentions this issue: gopls/internal/lsp: update replace directives in go.mod for package renaming

gopherbot pushed a commit to golang/tools that referenced this issue Dec 29, 2022
gopls/internal/lsp: update replace directives in go.mod for package r…
1a08d01 
…enaming

Check active go.mod files in the workspace to see if any replace directives need to be fixed if package renaming affects the replaced locations in any go.mod files.

For golang/go#56184.

Change-Id: I98ea07a602c39168d13f42f1b7a5f6738e194ece
Reviewed-on: https://go-review.googlesource.com/c/tools/+/459416
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@findleyr
Copy link
Contributor Author

@dle8 anything more to do here?

@dle8
Copy link

dle8 commented Jan 17, 2023

No I think we can close this issue!

@dle8 dle8 closed this as completed Jan 17, 2023
@golang golang locked and limited conversation to collaborators Jan 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dle8 dle8

Labels
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.12.0
Development

No branches or pull requests
5 participants
@bcmills @gopherbot @dle8 @uh-zz @findleyr