mfussenegger / nvim-jdtls

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

Bug: `didChangeWatchedFiles` causes file only to update on save. #645

Closed aarondill closed 3 months ago

aarondill commented 3 months ago

LSP client configuration

{
  capabilities = { --- require('cmp_nvim_lsp').default_capabilities()
    textDocument = {
      completion = {
        completionItem = {
          commitCharactersSupport = true,
          deprecatedSupport = true,
          insertReplaceSupport = true,
          insertTextModeSupport = {
            valueSet = { 1, 2 }
          },
          labelDetailsSupport = true,
          preselectSupport = true,
          resolveSupport = {
            properties = { "documentation", "detail", "additionalTextEdits", "sortText", "filterText", "insertText", "textEdit", "insertTextFormat", "insertTextMode" }
          },
          snippetSupport = true,
          tagSupport = {
            valueSet = { 1 }
          }
        },
        completionList = {
          itemDefaults = { "commitCharacters", "editRange", "insertTextFormat", "insertTextMode", "data" }
        },
        contextSupport = true,
        dynamicRegistration = false,
        insertTextMode = 1
      }
    }
  },
  cmd = { "/home/aaron/.local/share/nvim/mason/bin/jdtls", "-configuration", "/home/aaron/.cache/nvim/jdtls/java/config", "-data", "/home/aaron/.cache/nvim/jdtls/java/workspace" },
  init_options = {
    bundles = {}
  },
  root_dir = "/home/aaron/code/java"
}

Eclipse.jdt.ls version

v1.33.0

Steps to Reproduce

  1. Create File.java
// File.java
public class File {
  public static void main(String[] args) {
    int a = 10;
    int a = 9; // line 5
  }
}
  1. Notice diagnostic on line 5 (duplicate local)
  2. Change a to b on line 5
  3. Notice diagnostic changes (unused variable)
  4. Save the file :w
  5. Change b to a on line 5
  6. Notice the diagnostic does not change!
  7. Save the file :w
  8. Notice the diagnostic now changes.

Expected Result

The file should update in real-time as edited, not only on writes

Actual Result

The file diagnostics update only on writes. Note: using the default configuration, this is first noticeable in f3b916d6ee7bf526da78a628270a205f2ea9c32c:

diff --git a/lua/jdtls/setup.lua b/lua/jdtls/setup.lua
index 45816af..03e5ace 100644
--- a/lua/jdtls/setup.lua
+++ b/lua/jdtls/setup.lua
@@ -236,7 +236,7 @@ function M.start_or_attach(config)
   config.handlers['language/progressReport'] = config.handlers['language/progressReport'] or progress_report
   config.handlers['language/status'] = config.handlers['language/status'] or status_callback
   config.handlers['workspace/configuration'] = config.handlers['workspace/configuration'] or configuration_handler
-  local capabilities = vim.tbl_deep_extend('keep', config.capabilities, lsp.protocol.make_client_capabilities())
+  local capabilities = config.capabilities or lsp.protocol.make_client_capabilities()
   local extra_capabilities = {
     textDocument = {
       codeAction = {

because it changes capabilities to merge with vim.lsp.protocol.make_client_capabilities(), which includes workspace.didChangeWatchedFiles.dynamicRegistration = true

Log file with set_log_level(DEBUG) (nothing looks important): https://pastebin.com/P87ftUZu

aarondill commented 3 months ago

As a workaround, the user can set capabilities to disable didChangeWatchedFiles

capabilities = { workspace = { didChangeWatchedFiles = { dynamicRegistration = false } } },
aarondill commented 3 months ago

didChangeWatchedFiles currently also breaks LSP formatting, since jdt will only format the file based on the state on disk, so you have to write the file before calling lsp.buf.format or you risk improper formatting or even losing code (This has happened to me several times!)

mfussenegger commented 3 months ago

The given example works fine for me. Diagnostics change without saving, and I have didChangeWatchedFiles enabled. I also don't notice any difference between on/off in regards to how diagnostics update.

This also isn't really a nvim-jdtls issue as none of that is implemented in the plugin. It's either eclipse.jdt.ls or neovim core. Or you have a configuration issue, and there is something messed up with the client (not) attaching, or attaching multiple times.

I'd kinda rule out eclipse.jdt.ls itself, because vscode activates didChangeWatchedFiles, and I'm pretty sure this also isn't an issue in vscode-java - otherwise it would've been reported and likely addressed.

didChangeWatchedFiles currently also breaks LSP formatting, since jdt will only format the file based on the state on disk, so you have to write the file before calling lsp.buf.format or you risk improper formatting or even losing code

I also can't reproduce this.

It's also not how LSP works - or is supposed to. Neovim constantly informs the server about changes in the buffer. The file changes are complementary to that, and should mostly be used by the server to deal with changes that happened outside of an editor. E.g. eclipse.jdt.ls will detect folder creation, which can then show up in move refactorings.

aarondill commented 3 months ago

Using nvim -nu repro.lua File.java and repeating the above repro directions results in all diagnostics disappearing on save.

repro.lua ```lua -- DO NOT change the paths and don't remove the colorscheme local root = vim.fn.fnamemodify("./.repro", ":p") -- set stdpaths to use .repro for _, name in ipairs({ "config", "data", "state", "cache" }) do vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name end -- bootstrap lazy local lazypath = root .. "/plugins/lazy.nvim" if not vim.loop.fs_stat(lazypath) then vim.fn.system({ "git", "clone", "--filter=blob:none", "https://github.com/folke/lazy.nvim.git", lazypath }) end vim.opt.runtimepath:prepend(lazypath) -- install plugins local plugins = { "folke/tokyonight.nvim", -- Set up nvim-jdtls to attach to java files. { "williamboman/mason.nvim", build = ":MasonUpdate", opts = {} }, { "williamboman/mason-lspconfig.nvim", opts = { ensure_installed = { "jdtls" } }, dependencies = { "mason.nvim" } }, { "mfussenegger/nvim-jdtls", dependencies = { "williamboman/mason-lspconfig.nvim" }, ft = { "java" }, opts = function() local cache = vim.fn.stdpath("cache") assert(type(cache) == "string") return { -- How to find the root dir for a given filename. The default comes from -- lspconfig which provides a function specifically for java projects. root_dir = function() return require("jdtls.setup").find_root({ ".git" }) end, full_cmd = function(root_dir) ---@param root_dir? string local cmd = { vim.fn.exepath("jdtls") } if not root_dir then return cmd end local project_name = vim.fs.basename(root_dir) return vim.list_extend(cmd, { "-configuration", vim.fs.joinpath(cache, "jdtls", project_name, "config"), "-data", vim.fs.joinpath(cache, "jdtls", project_name, "workspace"), }) end, --- ! Uncomment this line to fix the issue ! -- capabilities = { workspace = { didChangeWatchedFiles = { dynamicRegistration = false } } }, settings = { redhat = { telemetry = { enabled = false } }, }, } end, config = function(_, opts) local function attach_jdtls() -- local has_cmp, cmp_nvim_lsp = pcall(require, "cmp_nvim_lsp") ---@type table local config = vim.deepcopy(opts) config.root_dir = opts.root_dir(vim.api.nvim_buf_get_name(0)) -- nil needs to be handled config.cmd = opts.full_cmd(config.root_dir) config.capabilities = vim.tbl_deep_extend( "force", vim.lsp.protocol.make_client_capabilities(), -- has_cmp and cmp_nvim_lsp.default_capabilities() or {}, opts.capabilities or {} ) return require("jdtls").start_or_attach(config) -- Existing server will be reused if the root_dir matches. end -- Attach the jdtls for each java buffer. HOWEVER, this plugin loads -- depending on filetype, so this autocmd doesn't run for the first file. -- For that, we call directly below. vim.api.nvim_create_autocmd("FileType", { callback = attach_jdtls, pattern = "java" }) -- Avoid race condition by calling attach the first time, since the autocmd won't fire. return attach_jdtls() end, }, } require("lazy").setup(plugins, { root = root .. "/plugins" }) vim.cmd.colorscheme("tokyonight-night") ```
aarondill commented 3 months ago

Note that I am using nvim nightly

> nvim -V1 -v
NVIM v0.10.0-dev-2702+g0c0be09ea-dirty
Build type: RelWithDebInfo
LuaJIT 2.1.1710088188
Compilation: /usr/sbin/cc -O2 -g -Og -g -flto=auto -fno-fat-lto-objects -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wvla -Wdouble-promotion -Wmissing-noreturn -Wmissing-format-attribute -Wmissing-prototypes -fsigned-char -fstack-protector-strong -Wno-conversion -fno-common -Wno-unused-result -Wimplicit-fallthrough -fdiagnostics-color=always  -DUNIT_TESTING -DHAVE_UNIBILIUM -D_GNU_SOURCE -DINCLUDE_GENERATED_DECLARATIONS -I/home/aaron/code/repos/neovim/.deps/usr/include/luajit-2.1 -I/home/aaron/code/repos/neovim/.deps/usr/include -I/home/aaron/code/repos/neovim/build/src/nvim/auto -I/home/aaron/code/repos/neovim/build/include -I/home/aaron/code/repos/neovim/build/cmake.config -I/home/aaron/code/repos/neovim/src -I/usr/include

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/local/share/nvim"

Run :checkhealth for more info
aarondill commented 3 months ago

Disabling didChangeWatchedFiles fixed this issue temporarily for me, but I'm now re-encountering the issue; so it's not that. I'm closing this issue, since didChangeWatchedFiles isn't the core problem.