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 renaming a package #41567

Open
golopot opened this issue Sep 23, 2020 · 8 comments
Open

x/tools/gopls: support renaming a package #41567

golopot opened this issue Sep 23, 2020 · 8 comments
Assignees
Labels
FeatureRequest gopls/refactoring gopls help wanted Tools
Milestone

Comments

@golopot
Copy link

@golopot golopot commented Sep 23, 2020

What did you do?

  1. Open a go file in vscode
  2. Put keyboard cursor on the line package somepackage.
  3. Press F2 (rename symbol)

What did you expect to see?

Let me rename package.

What did you see instead?

This element cannot be renamed.

Build info

golang.org/x/tools/gopls v0.5.0
go version go1.15 linux/amd64

@gopherbot gopherbot added Tools gopls labels Sep 23, 2020
@gopherbot gopherbot added this to the Unreleased milestone Sep 23, 2020
@stamblerre stamblerre changed the title x/tools/gopls: rename package feature request x/tools/gopls: support renaming a package Sep 23, 2020
@stamblerre stamblerre removed this from the Unreleased milestone Sep 23, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@msAlcantara
Copy link

@msAlcantara msAlcantara commented Jun 26, 2021

There is some tips to implement this? I think that this could be super useful and I would like to implements. I think that the implementation should be in qualifiedObjsAtProtocolPos function, I'm in the right way?

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 28, 2021

@msAlcantara: Thanks for your interest! I think this is slightly more complicated than that because the package name declaration does not have a types.Object, so I don't think it would work in the framework of qualifiedObjsAtProtocolPos. Still, renaming a package should be a lot simpler than a standard rename, because you can access the position of the package name by looking at the AST of each file in the package (https://pkg.go.dev/go/ast#File.Package). There will be some complexity with test packages, but hopefully it's as simple as updating their names too. Does that help at all?

@msAlcantara
Copy link

@msAlcantara msAlcantara commented Jul 7, 2021

Thanks very much for your awnser, but I have some question about it.

  1. There something that tell me that the rename is of package name?
  2. Do you think that is better change qualifiedObjsAtProtocolPos to support rename a package or is better do something like this: if isPackageRename(...) { ... }?

Sorry is this questions is too dump, I'm a just getting started with gopls source code and I'm trying to understand

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 13, 2021

Please don't apologize--this is a tricky issue! I think the easiest thing would be to do something like if isPackageRename(...) in the rename logic itself (not in qualifiedObjsAtProtocolPos).

@jan-xyz
Copy link

@jan-xyz jan-xyz commented Apr 8, 2022

Should an implementation for this also rename the directory and the package comment? And if "yes", is this still possible by looking at the position in the AST? What would happen if the package declaration and directory are already different? Should they become harmonised or only rename the package declaration in that case? We probably also need to account for blackbox test packages, right (e.g. package foo_test)?

@findleyr
Copy link
Contributor

@findleyr findleyr commented Apr 12, 2022

Hi I think this is something that @dle8 might look into.

Should an implementation for this also rename the directory and the package comment?

IMO yes it should do both (note that LSP supports renaming files via TextDocumentEdits, so I think this should be possible).

To start we can implement textDocument/references on package names (package foo).

@findleyr findleyr removed this from the gopls/unplanned milestone Apr 19, 2022
@findleyr findleyr added this to the gopls/on-deck milestone Apr 19, 2022
@findleyr
Copy link
Contributor

@findleyr findleyr commented Apr 20, 2022

Right now our protocol package only supports DocumentChanges that are TextDocumentEdits, that needs to be fixed in order to support renaming files/directories. CC @pjweinb

It's probably also worth researching exactly how well supported renaming files is among LSP clients, as I can imaging this might be a tricky operation.

@gopherbot
Copy link

@gopherbot gopherbot commented May 25, 2022

Change https://go.dev/cl/408714 mentions this issue: internal/lsp: support renaming a package

gopherbot pushed a commit to golang/tools that referenced this issue Jun 21, 2022
Update References to detect if the package is referenced and a regtest to test within and external package references.

Updates golang/go#41567

Change-Id: I607a47bf15f1c9f8236336f795fcef081db49d6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408714
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls/refactoring gopls help wanted Tools
Projects
None yet
Development

No branches or pull requests

8 participants