smjonas / inc-rename.nvim

Incremental LSP renaming based on Neovim's command-preview feature.
MIT License
637 stars 8 forks source link

Weid behavior in php #30

Closed phaalonso closed 10 months ago

phaalonso commented 1 year ago

When programming in PHP, the plugin has a weird behavior: image

Sorry for this monstrosity. I'm new to this codebase and still in the process of refactoring it and implementing best practices

Obs: working fine on other languages image

smjonas commented 1 year ago

Thanks for reporting! Which LSP server are you using (run :LspInfo)?

phaalonso commented 1 year ago

I'm using lsp-zero, I tried a few more times and the rename worked fine, the problem seem to be in the way that the plugin is rendering the new name

smjonas commented 1 year ago

lsp-zero is just a wrapper around the built-in LSP. Can you tell me the active LSP servers that the :LspInfo command is showing?

phaalonso commented 1 year ago

Sorry for the delay, the active lsp servers are intelephense and phpactor

smjonas commented 1 year ago

I can't reproduce this since neither intelephense nor phpactor seem to have the rename capability for me, so vim.lsp.buf.rename() always results in "Rename, no matching language servers with rename capability".

Could you tell me the output of the commands :lua =require("lsp-zero").build_options("intelephense", {}) and :lua =require("lsp-zero").build_options("phpactor", {})? (Maybe lsp-zero adds something to the config)

phaalonso commented 1 year ago

Sorry, I wasnt able to copy the output of the files so I recorded it

intelephense: Screencast from 01-27-2023 09:03:42 AM.webm

phpactor: Screencast from 01-27-2023 09:04:43 AM.webm

My dotfiles are in this repository, mayabe it will help in reproducing it

smjonas commented 1 year ago

Alright I only just now realized that only the paid version of intelephense supports renaming (source: https://intelephense.com/). Since I haven't bought the paid version I can't debug this further. Can you please use the branch debug (add branch = "debug" in your packer config), rename a variable in a PHP file, then tell me what is printed in :messages?

phaalonso commented 1 year ago

Strange, I also don't have the paid version and the renaming is working, maybe lsp-zero or my config is bypassing this somehow? 🤔

Well, this time I managed to extract the log running :put = execute('messages') in a empty file

Log ```log { { range = { ["end"] = { character = 15, line = 15 }, start = { character = 8, line = 15 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 19, line = 18 }, start = { character = 12, line = 18 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 19, line = 19 }, start = { character = 12, line = 19 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 23, line = 27 }, start = { character = 16, line = 27 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 73, line = 28 }, start = { character = 66, line = 28 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" } } LSP[phpactor][Info] ... scanned 0 references confirmed 2 ... LSP[phpactor][Info] Found 5 reference(s) { { range = { ["end"] = { character = 8, line = 15 }, start = { character = 8, line = 15 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 12, line = 18 }, start = { character = 12, line = 18 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 12, line = 19 }, start = { character = 12, line = 19 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 16, line = 27 }, start = { character = 16, line = 27 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" }, { range = { ["end"] = { character = 66, line = 28 }, start = { character = 66, line = 28 } }, uri = "file:///home/pedro/programacao/projetos/IxopPlay-Api/app/Http/Controllers/FabricantesController.php" } } Renamed 5 instances in 1 file ```
smjonas commented 1 year ago

To me it looks like the cause of this issue is that either phpactor or intelephense respond with incorrect values for the range.data.character value (notice how start and end are always the same although end.character should account for the variable's length):

range = {
      ["end"] = {
        character = 8,
        line = 15
      },
      start = {
        character = 8,
        line = 15
      }
    },

I have updated the debug branch, can you see if your issue is now fixed?

smjonas commented 1 year ago

Also, can you tell me whether this was caused by intelephense or by phpactor (e.g. by disabling either one of them on the main branch and seeing if the issue persists)? I'd like to file an issue there.

phaalonso commented 1 year ago

I just run some tests, and it seems to be caused by phpactor

phaalonso commented 1 year ago

image image

smjonas commented 1 year ago

Thanks! Is this fixed for you now with the latest update on the debug branch?

phaalonso commented 1 year ago

Sorry for the delay, at the moment the debug branch still has the bug but it's different than before. I noticied that the rename is running using the phpactor lsp and not intelephense

image

smjonas commented 1 year ago

Could you provide me with an example PHP project where I can test the renaming? I now installed phpactor but the LSP server won't attach to the root directory for some reason so I can't test your issue (also, single file support does not seem supported so I can't just use a single test file): image

I tested it with this PHP repo but still no luck.

phaalonso commented 1 year ago

Strange, I tried refactoring in the mentioned repository and is working on my computer. Maybe because of lsp-zero or mason

Can you give a try to this repository? If still doesn't work, I think that you will need configure the root path to a .git or composer.json

smjonas commented 10 months ago

Phpactor has now added a fix for this on master (https://github.com/phpactor/phpactor/pull/2352) so this issue should be fixed now 🎉.