mfussenegger / nvim-jdtls

Extensions for the built-in LSP support in Neovim for eclipse.jdt.ls
GNU General Public License v3.0
1.07k stars 61 forks source link

Rename or/and Move of class does not work #283

Closed asmodeus812 closed 1 year ago

asmodeus812 commented 2 years ago

LSP client configuration

        return {
            cmd = config.cmd,
            handlers = {
                ["language/progressReport"] = get_progress,
                ['language/status'] = get_status
            },
            flags = {
                debounce_text_changes = 150,
                allow_incremental_sync = true,
            },
            handlers = {},
            on_attach = require("plugin.lsp.initializer").on_lsp_attached,
            capabilities = require("plugin.lsp.configure").capabilities,
            root_dir = root_loc,
            settings = {
                java = {
                    trace = {
                        server = "message",
                    },
                    signatureHelp = {
                        enabled = false
                    },
                    referenceCodeLens = {
                        enabled = true
                    },
                    format = {
                        enabled = true,
                        settings = {
                            profile = "GoogleStyle",
                            url = "https://raw.githubusercontent.com/google/styleguide/gh-pages/eclipse-java-google-style.xml"
                        },
                    },
                    saveActions = {
                        organizeImports = true
                    },
                    completion = {
                        importOrder = {
                            "javax",
                            "java",
                            "com",
                            "org"
                        }
                    },
                    codeGeneration = {
                        tostring = {
                            listArrayContents = true,
                            skipNullValues = true,
                            template = "${object.className}{${member.name()}=${member.value}, ${otherMembers}}",
                        },
                        useBlocks = true,
                        hashCodeEquals = {
                            useInstanceof = true,
                            useJava7Objects = true
                        },
                        generateComments = true,
                        insertLocation = true
                    },
                    autobuild = {
                        enabled = true
                    },
                    progressReports = {
                        enabled = false
                    },
                    eclipse = {
                        downloadSources = true
                    },
                    maven = {
                        downloadSources = true,
                        updateSnapshots = true
                    }
                }
            }
        }

Lsp Info reports the following - image

Eclipse.jdt.ls version

1.9.0 Milestone build

Steps to Reproduce

Use case 1:

  1. Clone the following project - https://github.com/jenkins-docs/simple-java-maven-app.git
  2. Open the App class and move the cursor over the class name
  3. Use "lua vim.lsp.buf.rename()", input AppRenamed
  4. Initial rename of class works
  5. Go to the variable message
  6. Rename it to messageRenamed
  7. Go to class AppRenamed
  8. Try to rename it to AppRenamedAgain
  9. Observe the following error after trying step 8 image

Use case 2:

  1. Open the App class
  2. Create folder/package named another like (under com.mycompany.another)
  3. Open actions, select Move 'App'
  4. New package is missing (another) from the list
  5. Unable to proceed with moving the class to the new package (!not sure if move UI accepts user input instead of just being a filter box)

Use case 3:

  1. Do not open a java file directy, instead open the readme (e.g)
  2. Create folder/package named another like (under com.mycompany.another)
  3. Open the App class (jdtls, first attaches)
  4. Open actions, select Move 'App'
  5. New package is visible, select it
  6. Nothing happens, (jdtls does something, i.e fidget reports progress for a brief second) but the package of App does not change, neither is it moved in the another package directory
  7. Note this also seems to break the rename operation - i.e if we now try to rename the class (which was not moved) the error from use case 1 is reported again.

Expected Result

Use cases from above should work as expected.

Actual Result

See use cases.

asmodeus812 commented 2 years ago

Any more details are needed, let me know.

asmodeus812 commented 2 years ago

@mfussenegger is it possible to have a look at this, still facing these issues.

asmodeus812 commented 1 year ago

@mfussenegger hello, this is still reproducible on any version of jdtls.

EpsilonKu commented 1 year ago

@asmodeus812 Its more like jdt.ls problem not this plugin. Try https://github.com/tobias-z/java-util.nvim, but I dont think it will help. Did u tried to repeat it on vs code?

lidulibai commented 1 year ago

Case 1 works for me. Eclipse.jdt.ls version 1.15.0

asmodeus812 commented 1 year ago

@asmodeus812 Its more like jdt.ls problem not this plugin. Try https://github.com/tobias-z/java-util.nvim, but I dont think it will help. Did u tried to repeat it on vs code?

Yeah, i just tested that in VS Code, it does work, all use cases work actually, it takes like 20 seconds to set up the Java env. With all the extensions installations. Both vscode and nvim use the same jdtls version as well.

mfussenegger commented 1 year ago

Use case 1)

Can't reproduce this. It works for me. But there were some fixes within Neovim related to the LSP capabilities needed to make this work.

Use case 2)

How are you creating the folder, package? There is currently no filesystem-watcher capability in Neovim, so there is no way for the language server to detect the new package. You'd have to create a new java file in a new package for it to notice it.

There is a PR pending to add filesystem watcher capabilities to Neovim, but could also be that more than that is missing/required to make it work. But in any case it's likely unrelated to nvim-jdtls itself.

Use case 3)

Works for me too. But again, this might have been fixed meanwhile in Neovim itself.

asmodeus812 commented 1 year ago

@mfussenegger I just cloned that repo again and tried to replay the use cases. Using vim 0.8 with jdtls .19 The first use case - failed on the initial rename, even with the is out of sync with the filesystem error. Note that i deleted the workspace folder before hand and let jdtls create it again. Also note that if i delete the folder between renames it seems to work sometimes, so there is something going on with the workspace folder, which for me is in the home dir in wsl (see below)

The second case - the folder is created through vim from nvim-tree / netrw etc. I was expecting that maybe that the list will be fetched / resolved only when you open that move dialog, on the fly should be able to see that new folder right ? Seems like it is statically fetched when the server attaches to the project, meaning that if you make new packages after jdtls attaches the first time (i.e you open any file from your project), you won't be able to move into them, which seems like a problem.

The third case - still able to reproduce it.

I am using wsl and tests are done on the native wsl fs, nvim is started from wsl as well. The config being used for jdtls you can find below.

        local result = {
            -- See: https://github.com/eclipse/eclipse.jdt.ls#running-from-the-command-line for jdtls configuration
            cmd = cmd,
            root_dir = root_loc,
            filetypes = {
                "java"
            },
            init_options = {
                extendedClientCapabilities = {
                    progressReportProvider = false,
                    classFileContentsSupport = true;
                    generateToStringPromptSupport = true;
                    hashCodeEqualsPromptSupport = true;
                    advancedExtractRefactoringSupport = true;
                    advancedOrganizeImportsSupport = true;
                    generateConstructorsPromptSupport = true;
                    generateDelegateMethodsPromptSupport = true;
                    moveRefactoringSupport = true;
                    overrideMethodsPromptSupport = true;
                    inferSelectionSupport = {
                        "extractMethod",
                        "extractVariable",
                        "extractConstant"
                    };
                },
                bundles = utils.get_bundles({
                    ["java-test"] = "extension/server/*",
                    ["java-debug-adapter"] = "extension/server/*",
                }),
            },
            -- See https://github.com/eclipse/eclipse.jdt.ls/wiki/running-the-java-ls-server-from-the-command-line#initialize-request
            settings = {
                java = {
                    trace = {
                        server = "message",
                    },
                    signatureHelp = {
                        enabled = true
                    },
                    referenceCodeLens = {
                        enabled = true
                    },
                    format = {
                        -- See https://raw.githubusercontent.com/google/styleguide/gh-pages/eclipse-java-google-style.xml for code style
                        enabled = true,
                        settings = formatter,
                    },
                    saveActions = {
                        organizeImports = true
                    },
                    favoriteStaticMembers = {
                        "org.junit.jupiter.api.DynamicTest.*",
                        "org.junit.jupiter.api.Assertions.*",
                        "org.junit.jupiter.api.Assumptions.*",
                        "org.junit.jupiter.api.DynamicContainer.*",
                        "org.junit.Assert.*",
                        "org.junit.Assume.*",
                        "java.util.Objects.*",
                        "org.mockito.ArgumentMatchers.*",
                        "org.mockito.Mockito.*",
                        "org.mockito.Answers.*",
                    },
                    filteredTypes = {
                        "java.awt.*",
                        "com.sun.*",
                        "jdk.*",
                        "sun.*",
                    },
                    completion = {
                        importOrder = {
                            "javax",
                            "java",
                            "com",
                            "org"
                        }
                    },
                    codeGeneration = {
                        tostring = {
                            listArrayContents = true,
                            skipNullValues = true,
                            template = "${object.className}{${member.name()}=${member.value}, ${otherMembers}}",
                        },
                        useBlocks = true,
                        hashCodeEquals = {
                            useInstanceof = true,
                            useJava7Objects = true
                        },
                        generateComments = false,
                        insertLocation = true
                    },
                    autobuild = {
                        enabled = true
                    },
                    progressReports = {
                        enabled = false
                    },
                    eclipse = {
                        downloadSources = true
                    },
                    maven = {
                        downloadSources = true,
                        updateSnapshots = true
                    },
                    sources = {
                        organizeImports = {
                            starThreshold = 9999,
                            staticStarThreshold = 9999
                        }
                    }
                }
            },
        }
asmodeus812 commented 1 year ago

@mfussenegger since i have been facing this issue for a while now, and it is replicable on both macos and wsl. Can you at least point me in some direction as to where this out of sync error is coming from, who is logging it, is it the plugin or the server. I will have to try to solve it on my own it seems.

mfussenegger commented 1 year ago

Use-case 2 works as well in neovim 0.9 with:

      workspace = {
        didChangeWatchedFiles = {
          dynamicRegistration = true
        }
      }

It will detect folders created outside neovim / via plugins, and offer new packages as destinations.

For 1 & 3, given that I don't use macOS / windows, can't reproduce it, and given that nvim-jdtls is only looking up the destinations, provide the dialog to select the target, and then calls into vim.lsp.util.apply_workspace_edit - I'm closing this here.

You could activate trace-logging, see what the server returns on java/move requests, and double check if the apply_workspace_edit call does the right thing. If it doesn't, it is a neovim core issue.