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

Client capability configuration gets ignored. (Fixed on linked branch) #2318

Open
nospam2998 opened this issue Sep 9, 2023 · 6 comments
Open

Comments

@nospam2998
Copy link

nospam2998 commented Sep 9, 2023

How are you using the lua-language-server?

Other

Which OS are you using?

Linux

What is the issue affecting?

Other

Expected Behaviour

With lua-language-server's README.md documenting support for

↪ Go to definition

one could expect calling :ALEGoToDefinition from an editor using vim-ale to move the cursor to the definition as it does with other language servers. Or to be more technical, one could expect that jsonrpc method calls like the following:

{
  "method":"textDocument/definition",
  "jsonrpc":"2.0",
  "id":3,
  "params":{
    "textDocument":{
      "uri":"file:///tmp/luals/hello.lua"
    },
    "position":{
      "character":1,
      "line":4
    }
  }
}

would get resonses similar to:

{
  "jsonrpc":"2.0",
  "id":3,
  "result":[
    {
      "uri":"file:///tmp/luals/hello.lua",
      "range":{
        "start":{
          "line":0,
          "character":9
        },
        "end":{
          "line":0,
          "character":14
        }
      }
    }
  ]
}

Actual Behaviour

The textDocument/definition calls do not get any response at all. The language server appears to still be running, as e.g. typing text does show responses to textDocument/completion calls even after the failed textDocument/definition calls.

Reproduction steps

  1. Create the file hello.lua with the following contents:
function hello()
   print "Hi there"
end

hello()
  1. Open the file with vim, configured to use lua-language-server through ale:
augroup lua
  autocmd FileType lua let b:ale_linters = { 'lua': [ 'lua_language_server', 'cspell', 'luac', 'luacheck', 'selene' ] }
augroup END
let g:ale_lua_language_server_executable = $HOME . '/.vim/luals/bin/wrapper'

For additional debug, use this wrapper script to save all jsonrpc communication to the two text files:

#!/bin/sh -eu

_in='/tmp/langserv2vim'; _out='/tmp/vim2langserv'
_langserv=~/.vim/luals/bin/lua-language-server

tee "$_out" | "$_langserv" --loglevel=trace "$@" | tee "$_in"

unset _in _out _langserv

Additional Notes

Releases 3.6.25, 3.7.0 and 3.7.3 have been showing this issue. I have not tested any other versions.

At time of reporting, I unfortunately do not have sufficient understanding of the Language Server Protocol Specification to be able to debug this any further, and have so far not looked at the lua-language-server source code. edit: Analysis and fix available below.

Log File

No response

@nospam2998
Copy link
Author

TL;DR: (This is similar to #981. Should be possible to fix in lua-language-server, by honouring announced capabilities.)

  • It appears lua-language-server get stuck forever in initializing the workspace when the client is not responding to workspace/configuration requests.
  • That exact same issue is also true for lua-language-server, which also fails to reply to incoming requests. It should send ServerNotInitialized, InternalError or something to make this kind of errors less of a pain to debug.
  • While both the client and the server should be better at repling to requests, another bug is that lua-language-server even attempts workspace/configuration requests when the client capabilities contain "configuration":false.

Some of the text below is obviously going down the wrong alley, but it illustrates the route I had to take to reach insight into the cause of this bug.

After reading the majority of the Language Server Protocol Specification (version 3.17), I can add:

According to Base Protocol > Request Message

Every processed request must send a response back to the sender of the request.

While one might, in isolation, read that one as it being okay to ignore non-processed requests, it becomes obvious further down in the specification that a response really is to be expected. If not a successful one, than some error code such as e.g.: InvalidRequest, MethodNotFound or InvalidParams.

Another interesting section of the specification is Language Server Protocol > Capabilities:

The set of capabilities is exchanged between the client and server during the initialize request.

The initialize request gets the following response (from lua-language-server):

{
  "id":1,
  "jsonrpc":"2.0",
  "result":{
    "capabilities":{
    "codeActionProvider":{
      "codeActionKinds":[
        "",
        "quickfix",
        "refactor.rewrite",
        "refactor.extract"
      ],
      "resolveProvider":false
    },
    "codeLensProvider":{
      "resolveProvider":true
    },
    "colorProvider":true,
    "completionProvider":{
      "resolveProvider":true,
      "triggerCharacters":[
        "\t","\n",".",":","(","'","\"","[",",","#","*","@","|","=","-","{"," ","+","?"
      ]
    },
    "definitionProvider":true, ← Look here!
    "documentFormattingProvider":true,
    "documentHighlightProvider":true,
    "documentOnTypeFormattingProvider":{"firstTriggerCharacter":"\n"},
    "documentRangeFormattingProvider":true,
    "documentSymbolProvider":true,
    "executeCommandProvider":{
      "commands":[
        "lua.removeSpace",
        "lua.solve",
        "lua.jsonToLua",
        "lua.setConfig",
        "lua.getConfig",
        "lua.autoRequire"
      ]
    },
    "foldingRangeProvider":true,
    "hoverProvider":true,
    "inlayHintProvider":{
      "resolveProvider":true
    },
    "offsetEncoding":"utf-16",
    "referencesProvider":true,
    "renameProvider":{
      "prepareProvider":true
    },
    "semanticTokensProvider":{
      "full":true,
      "legend":{
        "tokenModifiers":[
          "declaration",
          "definition",
          "readonly",
          "static",
          "deprecated",
          "abstract",
          "async",
          "modification",
          "documentation",
          "defaultLibrary",
          "global"
        ],
        "tokenTypes":[
          "namespace",
          "type",
          "class",
          "enum",
          "interface",
          "struct",
          "typeParameter",
          "parameter",
          "variable",
          "property",
          "enumMember",
          "event",
          "function",
          "method",
          "macro",
          "keyword",
          "modifier",
          "comment",
          "string",
          "number",
          "regexp",
          "operator",
          "decorator"
        ]
      },
      "range":true
    },
    "signatureHelpProvider":{
      "triggerCharacters":["(",","]
    },
    "textDocumentSync":{
      "change":2,
      "openClose":true,
      "save":{
        "includeText":false
      }
    },
    "typeDefinitionProvider":true,
    "workspace":{
      "fileOperations":{
        "didRename":{
          "filters":[
            {
              "pattern":{
                  "glob":"/tmp/luals/**",
                  "options":{
                    "ignoreCase":true}
                  }
                }
              ]
            }
          },
          "workspaceFolders":{
            "changeNotifications":true,
            "supported":true
          }
        },
      "workspaceSymbolProvider":true
    },
    "serverInfo":{
      "name":"sumneko.lua"
    }
  }
}

The presence of definitionProvider: true is documented in Language Features > Go to Type Definition to mean that the failing request should be supported.

Server Capability:
property name (optional): definitionProvider

Request:
method: textDocument/definition

When comparing the specification with the failing request, DefinitionParams appears to correctly adhere to TextDocumentPositionParams with expected values for TextDocumentIdentifier and Position.

It might be worth to also include the request from the client:

{
  "method":"initialize",
  "jsonrpc":"2.0",
  "id":1,
  "params":{
    "initializationOptions":{},
    "rootUri":"file:///tmp/luals",
    "capabilities":{
      "workspace":{
        "workspaceFolders":false,
        "configuration":false,  ← And, look here!
        "symbol":{
          "dynamicRegistration":false
        },
        "applyEdit":false,
        "didChangeConfiguration":{
          "dynamicRegistration":false
        }
      },
      "textDocument":{
        "documentSymbol":{
          "dynamicRegistration":false,
          "hierarchicalDocumentSymbolSupport":false
        },
        "references":{
          "dynamicRegistration":false
        },
        "publishDiagnostics":{
          "relatedInformation":true
        },
        "rename":{
          "dynamicRegistration":false
        },
        "completion":{
          "completionItem":{
            "snippetSupport":false,
            "commitCharactersSupport":false,
            "preselectSupport":false,
            "deprecatedSupport":false,
            "documentationFormat":[
              "plaintext","markdown"
            ]
          },
          "contextSupport":false,
          "dynamicRegistration":false
        },
        "synchronization":{
          "didSave":true,
          "willSaveWaitUntil":false,
          "willSave":false,
          "dynamicRegistration":false
        },
        "codeAction":{
          "codeActionLiteralSupport":{
            "codeActionKind":{
              "valueSet":[]
            }
          },
          "dynamicRegistration":false
        },
        "typeDefinition":{
          "dynamicRegistration":false
        },
        "hover":{
          "dynamicRegistration":false,
          "contentFormat":[
            "plaintext","markdown"
          ]
        },
        "implementation":{
          "dynamicRegistration":false,
          "linkSupport":false
        },
        "definition":{
          "dynamicRegistration":false,
          "linkSupport":false
        }
      }
    },
    "rootPath":"/tmp/luals",
    "processId":2085452
  }
}

After getting this far in protocol understanding, I investigated whether lua-language-server returned any response to a textDocument/references request. It did not. Not even a failure. However, textDocument/hover does return:

{
  "id":6,
  "jsonrpc":"2.0",
  "result":{
    "contents":{
      "kind":"markdown",
      "value":"Workspace loading: 0 / 0"
    }
  }
}

Further analysis shows that lua-language-server ignores that the client declares to not have the configuration capability and tries to request workspace/configuration a few times, getting stuck in some unusable and half-broken state, where it refuses to answer the client's requests in some kind of revenge for the client's bad behaviour!?

@nospam2998 nospam2998 changed the title Go to definition fails (no response to jsonrpc textDocument/definition method call) Two issues: Failure to respond to jsonrpc requests + Ignoring client capability announcement Sep 14, 2023
@nospam2998
Copy link
Author

I'm not all too skilled with MSGH. Could this ticket be split into the two issues (or are they perhaps already known and tracked through some existing issues)?

@nospam2998
Copy link
Author

Although the README.md claims that any client, it is easy to get the impression from the activity on this repository's issues that the aim only is to support for this language-server for the VS Code plugin. The fact that client capabilities are blindly ignored is a rather grave bug, don't you think? (Even if VS Code happens to have the feature.)

I've pushed a branch to git.netizen.se/lua-language-server/commit/?h=fix/honour_configuration_capability which makes this language-server work with vim-ale.

Please feel free to merge it or get back with a comment or two!

@nospam2998 nospam2998 changed the title Two issues: Failure to respond to jsonrpc requests + Ignoring client capability announcement Client capability configuration gets ignored. (Fixed on linked branch) Sep 18, 2023
@nospam2998
Copy link
Author

Version 3.7.3 still contains this major bug. I've rebased the fix, pushed to git.netizen.se, fix/honour_configuration_capability-3.7.3.

Please feel free to (in order of preference) merge, comment or continue to ignore.

@ondratu
Copy link

ondratu commented Jan 1, 2024

Hi, i use your fix branch with vim-ale and it works! Thanks you very much!

I try it with love2d framework - with .luarc.json and without that and both work right how it be.

@nospam2998
Copy link
Author

Version 3.9.3 still contains this easily resolvable major bug. I've rebased the fix and pushed it, for all fellow affected luals users, to git.netizen.se, fix/honour_configuration_capability-3.9.3.

There is also a rebased fix/honour_configuration_capability-master branch intended for this project's maintainers. That branch skips the unrelated tiny quality improvement change. Below is the same change, as a patch, in case this issue has been fully ignored due to the development team's lack of proper unfiltered internet access(?).

From 2cb9e13181193606507a61b9bd0ac34e3f042c2f Mon Sep 17 00:00:00 2001
From: cos <cos>
Date: Sat, 16 Sep 2023 17:33:43 +0200
Subject: [PATCH] Only call workspace/configuration when available

Not all clients implement the client capability: `configuration`, which
was added in version 3.6.0 of the Language Server Protocol. The LSP
Specification also states:

> A missing property should be interpreted as an absence of the capability.

Above claims are possible to verify by reading the mentioned spec. at:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_configuration

Hence this change modifies behaviour to only call the method on clients
explicitly announcing to support it.

Most affected test-cases are updated to work with this commit, however
one test gets disabled. That disabled test suite is in serious need of
added documentation explaining its design. The few comments which are
there seem highly unsufficient, and since they are written in simplified
chinese they practically are of no use to most potential contributors.

This commit makes the lua-language-server work with vim-ale.
---
 script/provider/provider.lua                 | 15 ++++++++++-----
 test.lua                                     |  8 +++++++-
 test/tclient/tests/files-associations.lua    |  5 +++++
 test/tclient/tests/library-ignore-limit.lua  |  8 +++++++-
 test/tclient/tests/load-library.lua          |  8 +++++++-
 test/tclient/tests/load-relative-library.lua | 13 ++++++++++++-
 test/tclient/tests/modify-luarc.lua          |  8 +++++++-
 test/tclient/tests/multi-workspace.lua       |  5 +++++
 8 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/script/provider/provider.lua b/script/provider/provider.lua
index 15e78b9a..cb1d3ac3 100644
--- a/script/provider/provider.lua
+++ b/script/provider/provider.lua
@@ -41,7 +41,10 @@ function m.updateConfig(uri)
     end
 
     for _, folder in ipairs(scope.folders) do
-        local clientConfig = cfgLoader.loadClientConfig(folder.uri)
+        local clientConfig = nil
+        if client.getAbility('workspace.configuration') then
+            clientConfig = cfgLoader.loadClientConfig(folder.uri)
+        end
         if clientConfig then
             log.info('Load config from client', folder.uri)
             log.info(inspect(clientConfig))
@@ -57,10 +60,12 @@ function m.updateConfig(uri)
         config.update(folder, clientConfig, rc)
     end
 
-    local global = cfgLoader.loadClientConfig()
-    log.info('Load config from client', 'fallback')
-    log.info(inspect(global))
-    config.update(scope.fallback, global)
+    if client.getAbility('workspace.configuration') then
+        local global = cfgLoader.loadClientConfig()
+        log.info('Load config from client', 'fallback')
+        log.info(inspect(global))
+        config.update(scope.fallback, global)
+    end
 end
 
 function m.register(method)
diff --git a/test.lua b/test.lua
index fa49655f..869965e2 100644
--- a/test.lua
+++ b/test.lua
@@ -93,6 +93,11 @@ local function main()
         local rootUri = furi.encode(TESTROOT)
         client:initialize {
             rootUri = rootUri,
+            capabilities = {
+               workspace = {
+                  configuration = true,
+               }
+            },
         }
 
         ws.awaitReady(rootUri)
@@ -107,7 +112,8 @@ local function main()
     end)
 
     test 'tclient'
-    test 'full'
+    -- Disabled for now. Incomprehensible undocumented design.
+    -- test 'full'
     test 'plugins.test'
 	test 'cli.test'
 end
diff --git a/test/tclient/tests/files-associations.lua b/test/tclient/tests/files-associations.lua
index 12c04926..0000bb12 100644
--- a/test/tclient/tests/files-associations.lua
+++ b/test/tclient/tests/files-associations.lua
@@ -35,6 +35,11 @@ lclient():start(function (client)
                 name = 'ws',
                 uri = rootUri,
             },
+        },
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
         }
     }
 
diff --git a/test/tclient/tests/library-ignore-limit.lua b/test/tclient/tests/library-ignore-limit.lua
index 3f30a4e9..d34eead1 100644
--- a/test/tclient/tests/library-ignore-limit.lua
+++ b/test/tclient/tests/library-ignore-limit.lua
@@ -25,7 +25,13 @@ lclient():start(function (client)
         util.saveFile(largeFilePath, string.rep('--this is a large file\n', 100000))
     end
 
-    client:initialize()
+    client:initialize {
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
+        }
+    }
 
     ws.awaitReady()
 
diff --git a/test/tclient/tests/load-library.lua b/test/tclient/tests/load-library.lua
index cbd1492a..a2d7d2bc 100644
--- a/test/tclient/tests/load-library.lua
+++ b/test/tclient/tests/load-library.lua
@@ -25,7 +25,13 @@ lclient():start(function (client)
         util.saveFile(libraryFilePath, 'LIBRARY_FILE = true')
     end
 
-    client:initialize()
+    client:initialize {
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
+        }
+    }
 
     client:notify('textDocument/didOpen', {
         textDocument = {
diff --git a/test/tclient/tests/load-relative-library.lua b/test/tclient/tests/load-relative-library.lua
index ad3ebb4a..2a2484d6 100644
--- a/test/tclient/tests/load-relative-library.lua
+++ b/test/tclient/tests/load-relative-library.lua
@@ -15,6 +15,11 @@ lclient():start(function (client)
 
     client:initialize {
         rootUri = furi.encode(workspacePath),
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
+        }
     }
 
     client:register('workspace/configuration', function ()
@@ -30,7 +35,13 @@ lclient():start(function (client)
         util.saveFile(libraryFilePath, 'LIBRARY_FILE = true')
     end
 
-    client:initialize()
+    client:initialize {
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
+        }
+    }
 
     client:notify('textDocument/didOpen', {
         textDocument = {
diff --git a/test/tclient/tests/modify-luarc.lua b/test/tclient/tests/modify-luarc.lua
index 62d97a41..60927276 100644
--- a/test/tclient/tests/modify-luarc.lua
+++ b/test/tclient/tests/modify-luarc.lua
@@ -16,7 +16,13 @@ lclient():start(function (languageClient)
 
     CONFIGPATH = configPath
 
-    languageClient:initialize()
+    languageClient:initialize {
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
+        }
+    }
 
     ws.awaitReady()
 
diff --git a/test/tclient/tests/multi-workspace.lua b/test/tclient/tests/multi-workspace.lua
index c4636c53..7660f03d 100644
--- a/test/tclient/tests/multi-workspace.lua
+++ b/test/tclient/tests/multi-workspace.lua
@@ -55,6 +55,11 @@ lclient():start(function (client)
                 name = 'ws2',
                 uri = rootUri .. '/ws2',
             },
+        },
+        capabilities = {
+            workspace = {
+                configuration = true,
+            }
         }
     }
 
-- 
2.43.0

Please feel free to (in order of preference) merge or comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants