phpactor / language-server-phpactor-extensions

MIT License
0 stars 4 forks source link

Rename LSP support #21

Closed BladeMF closed 3 years ago

BladeMF commented 3 years ago

I am creating this PR so we can discuss the code. It's not going to get any better if I wait longer :-)

The code works by using Find all references to find everything that can be renamed.

  1. Rename type. If type has the same name as the file, rename the file.
  2. Rename local variable.
  3. Rename method.

https://github.com/phpactor/phpactor/issues/1140

BladeMF commented 3 years ago

Thank you for your notes and ideas! I appreciate your time with this.

Just so I am sure I understand your idea correctly, let me try to explain how it currently works and then we can get to me understanding your idea.

How it currently works

  1. Use the current DefinitionLocator to locate the definition using the current offset
  2. Use the current ReferenceFinder to locate all the references at the current offset
  3. Collect all the locations and group them by document
  4. For each document, for each location: 4.1. Check if file needs renaming (are we renaming a type with the same name as the file?) 4.2. Get the precise range for each edit
  5. Create a WorkspaceEdit object.

Your idea

While I agree that class moving should be somewhere in there, I have trouble seeing how we can use this architecture while using DefinitionLocator, ReferenceFinder because they currently implement the task of splitting between class, members variables and methods and variables.

Can you please elaborate more on how the above logic will go into three classes as it does not cite classes or members or variables with the exception of when deciding whether to rename the file. Do you want to reimplement the same reference finding logic in VariableRenamer, ClassRenamer and MethodRenamer or am I misunderstanding you?

weeman1337 commented 3 years ago

This one seems to be replaced by #24 ? Could it then be closed?

dantleech commented 3 years ago

yes, this has been extracted and implemented in other PRs :)