add module search path to lsp server configuration

[LeonB]

Based of issue #9

About -L, that should be possible also but i'm not sure how it would be communicated to the lsp serve? thru extension option? some kind of config in jq file?

I think the first and easiest way to implement it, is to use the initializationOptions. There's also workspaceConfiguration and didChangeConfiguration.

The neovim lsp documentation states:

Most language servers expect to be sent client specified settings after initialization. Neovim does not make this assumption. A workspace/didChangeConfiguration notification should be sent to the server during on_init.

So I'm not sure if vim is sending the initializationOptions in the initialize request.

[LeonB]

Ah, I see the rpc parsing is done in jq itself :) Why is that if I may ask? The only options I see at the moment:

1. is do some pre-parsing of the json rpc calls in go

But I think your main goal here is to keep as much code in jq if I'm correct?

2. Use gojq.WithFunction to create a function that adds a path to the gojq.WithModuleLoader function

[wader]

Based of issue #9

About -L, that should be possible also but i'm not sure how it would be communicated to the lsp serve? thru extension option? some kind of config in jq file?

I think the first and easiest way to implement it, is to use the initializationOptions. There's also workspaceConfiguration and didChangeConfiguration.

The neovim lsp documentation states:

Most language servers expect to be sent client specified settings after initialization. Neovim does not make this assumption. A workspace/didChangeConfiguration notification should be sent to the server during on_init.

So I'm not sure if vim is sending the initializationOptions in the initialize request.

Yeap that could work for the LSP side, for vscode you can add some settings thingy to the lsp extension (https://github.com/wader/vscode-jq) but for other editors like neovim i don't know how it usually works?

How does for example gopls settings work with neovim?

[wader]

Ah, I see the rpc parsing is done in jq itself :) Why is that if I may ask? The only options I see at the moment:

The whole project is kind of experiment going to far :) i first did most of it in go but it just go very tedious to write the AST-code and i know jq would probably be a very good fit for doing exactly this so i tried and it work quite well. Now i'm not so sure anymore as it's not typed so making changes is a bit brittle. Maybe eventually worth doing a rewrite in go or even something else if gojq's parser is not suitable anymore, maybe something tree sitter based somehow?

1. is do some pre-parsing of the json rpc calls in go

But I think your main goal here is to keep as much code in jq if I'm correct?

Keep it in jq for now at least. And i think it should be possible, the jq rpc code has a state that gets passed/returned in the serve function, see https://github.com/wader/jq-lsp/blob/master/lsp/lsp.jq#L798-L812

2. Use gojq.WithFunction to create a function that adds a path to the gojq.WithModuleLoader function

Yes if it is complicated to maintain state in jq we could add some global state helpers to track things

[wader]

For ref neovim and gopls config https://github.com/golang/tools/blob/master/gopls/doc/vim.md#neovim-config but is lspconfig settings some generic lsp thing in lspconfig or per lsp? i'm know very little about it

@LeonB btw would you like to have a look at this? also the vscode/neovim part? i will of course help out

[LeonB]

I'm pretty sure the lspconfig settings is a generic lsp thing. I entered some random stuff in the lua lsp settings and checked the requests in the vim lsp log and saw the settings I entered.

• I'm going to debug a jq lsp sessions with custom settings and log the requests
• I'm going to do the same for vscode as soon as I figure out how to do custom settings in it

[LeonB]

neovim lspconfig only uses the didChangeConfiguration call:

["DEBUG",{"request":{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"search_path":["."]}}}}]
["DEBUG",{"request":{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"search_path":["."]}}}}]

neovim config:

require('lspconfig').jqls.setup({
    capabilities = capabilities,
    cmd = { '/Users/leon/Workspaces/go/src/github.com/LeonB/jq-lsp/jq-lsp' },
    -- autostart = false,
    root_dir = require('lspconfig').util.find_git_ancestor(),
    settings = {
        search_path = { "." },
    },
    settings_test = {
        test = "howdy"
    }
})

[wader]

neovim lspconfig only uses the didChangeConfiguration call:

["DEBUG",{"request":{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"search_path":["."]}}}}]
["DEBUG",{"request":{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"search_path":["."]}}}}]

neovim config:

require('lspconfig').jqls.setup({
    capabilities = capabilities,
    cmd = { '/Users/leon/Workspaces/go/src/github.com/LeonB/jq-lsp/jq-lsp' },
    -- autostart = false,
    root_dir = require('lspconfig').util.find_git_ancestor(),
    settings = {
        search_path = { "." },
    },
    settings_test = {
        test = "howdy"
    }
})

Nice! and you managed to figure how to enable debug 👍 sorry the code is not very dev friendly at the moment so please feel free to ask if something seems strange, it probably is :) and don't hesitate start a PR to have something to discuss about

[LeonB]

vscode took me a while... I think the api changed and the documentation isn't updated. Some examples in the repo made that clear:

["DEBUG",{"request":{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"languageServerExample":{"maxNumberOfProblems":200,"trace":{"server":"off"}}}}}}]

So vscode also uses params.settings but it looks like it always adds the lsp name in the settings. Which makes the lspconfig examples make sense now. Examples from https://github.com/neovim/nvim-lspconfig/blob/master/doc/server_configurations.md

require'lspconfig'.lua_ls.setup {
  settings = {
    Lua = {
      runtime = {
        -- Tell the language server which version of Lua you're using (most likely LuaJIT in the case of Neovim)
        version = 'LuaJIT',
      },
      diagnostics = {
        -- Get the language server to recognize the `vim` global
        globals = {'vim'},
      },
    },
  },
}

require'lspconfig'.pylsp.setup{
  settings = {
    pylsp = {
      plugins = {
        pycodestyle = {
          ignore = {'W391'},
          maxLineLength = 100
        }
      }
    }
  }
}

require'lspconfig'.rust_analyzer.setup{
  settings = {
    ['rust-analyzer'] = {
      diagnostics = {
        enable = false;
      }
    }
  }
}

So the jqls config should be something like:

require('lspconfig').jqls.setup({
    root_dir = require('lspconfig').util.find_git_ancestor(),
    settings = {
        jqls = {
            search_path = { "." },
        },
    },
})

[wader]

👍 nice digging. For vscode i understood it as there is some kind of config to put in package.json https://github.com/golang/vscode-go/blob/master/package.json#L1112 but haven't look close at it

[LeonB]

In the example client this is what does the sending of the LanguageClientOptions: https://github.com/matarillo/vscode-languageserver-csharp-example/blob/master/client/src/extension.ts#L44

I'm going to look at the state in lsp.jq and see if I can get that working.

[LeonB]

@wader how do you want the _import_env function to be adjusted?

Should I include a search_paths argument?

def _import_env($search_paths):

or maybe add it to the data taken in?

{"include_path":{"start":8,"stop":11,"str":"a", "search_paths": ["."]}}

[wader]

@wader how do you want the _import_env function to be adjusted?

Should I include a search_paths argument?

def _import_env($search_paths):

or maybe add it to the data taken in?

{"include_path":{"start":8,"stop":11,"str":"a", "search_paths": ["."]}}

Ah now i see, i did some ugly thing for the .jq-lsp thing. I wonder if it should be refactored to just take a string as input (and move .include_path.str // .import_path.str to the caller) and then maybe take search paths as an argument? feels jq-ish, but i'm not sure. What do you think?

Let me know if you have any jq issues, happy to help!

[LeonB]

That makes sense. I'll give it a go. But not today anymore :)

[wader]

Sounds good, glad someone wants to help out!

[LeonB]

I made some changes: https://github.com/LeonB/jq-lsp/commit/ddf01365aea7f5fea40f7be282c09375f3ee6e32

Curious what you think about them.

Only thing I'm having trouble with is feeding the settings.jqlsp.search_paths into _import_env

https://github.com/LeonB/jq-lsp/blob/master/lsp/lsp.jq#L447

@wader any pointers how I could best achieve that? For testing I provided it with a fixed path.

[wader]

Good progress. I think something like this could work:

diff --git a/lsp/lsp.jq b/lsp/lsp.jq
index 4a95583..4298683 100644
--- a/lsp/lsp.jq
+++ b/lsp/lsp.jq
@@ -223,7 +223,7 @@ def env_func_markdown:
   );

-def query_walk($uri; $start_env; f):
+def query_walk($uri; $search_paths; $start_env; f):
   def _t($start_env):
     def _pattern_env:
       def _f:
@@ -444,6 +444,7 @@ def query_walk($uri; $start_env; f):
     | ( (.imports // [])
       # import includes from document
       # @TODO: add $state.settings.search_paths
+      | debug({$search_paths})
       | map(_import_env([".", "/Users/leon/Workspaces/go/src/github.com/LeonB/jq-lsp"]))
       ) as $imports_envs
     | ( (.func_defs // [])
@@ -524,6 +525,7 @@ def handle($state):
         | ( $def_file.query
           | first(query_walk(
               $uri;
+              $state.settings.search_paths;
               builtin_env;
               ( query_token as $t
               | $t != null and
@@ -639,7 +641,12 @@ def handle($state):
                         uri: $doc.uri,
                         diagnostics:
                           [ $text_query
-                          | query_walk($doc.uri; builtin_env; .term.func) as {$env, $q}
+                          | query_walk(
+                              $doc.uri;
+                              $state.settings.search_paths;
+                              builtin_env;
+                              .term.func
+                            ) as {$env, $q}
                           | ($q | query_token.str) as $name
                           | ($q | query_args) as $args
                           | if isempty(
@@ -696,7 +703,7 @@ def handle($state):
         | result(
             # TODO: just traverse, no env
             [ $file.query
-            | query_walk($uri; []; .func_defs)
+            | query_walk($uri; $state.settings.search_paths; []; .func_defs)
             | .q.func_defs[] as $f
             | { name : ($f | func_def_signature),
                 kind: SymbolKindFunction,
diff --git a/tests/test.jq b/tests/test.jq
index c094319..23a0e52 100755
--- a/tests/test.jq
+++ b/tests/test.jq
@@ -47,7 +47,18 @@
     "method": "workspace/didChangeConfiguration",
     "params": {
       "settings": {
-        "jqls": {"search_path": ["."]}
+        "jqls": {"search_paths": ["."]}
+      }
+    }
+  },
+  {
+    "id": 3,
+    "jsonrpc": "2.0",
+    "method": "textDocument/definition",
+    "params": {
+      "position": {"character": 2, "line": 1},
+      "textDocument": {
+        "uri": "file://\(env.URI)/tests/b.jq"
       }
     }
   }

(change to test.jq was just to trigger call to query_walk with search_path set)

Some thought:

• It would be nice to be able to report that a include is not found somehow. Maybe can be done separately?
• Handle missing include_paths config. Set some default settings for "initialize" method etc?

[wader]

@LeonB did get any further? Hope your having a great weekend

[LeonB]

@wader thanks! I hope I have some time this weekend to work on it!

[LeonB]

Worked a bit on it! Sorry I can't work more on it.

Your suggested changes worked like a charm! That's the way I went but I wasn't sure that was the best way. Removed the hard-coded path and now only nvim settings are used. I'm going to test the changes this week with work assignments.

Next step is to see if I can add diagnostics when the module can't be found.

jq error:

echo '{}' | jq -f jqs/test.jq
jq: error: module not found: jqs/jq-test

gojq error:

echo '{}' | gojq -f jqs/test.jq
gojq: compile error: module not found: "jqs/jq-test"

So maybe split out the os stuff into something like lookup_module and use that for generating diagnostics. But I still have to look into how that's setup in jq-lsp.

[wader]

Worked a bit on it! Sorry I can't work more on it.

👍 no worries!

Your suggested changes worked like a charm! That's the way I went but I wasn't sure that was the best way. Removed the hard-coded path and now only nvim settings are used. I'm going to test the changes this week with work assignments.

Next step is to see if I can add diagnostics when the module can't be found.

jq error:

echo '{}' | jq -f jqs/test.jq
jq: error: module not found: jqs/jq-test

gojq error:

echo '{}' | gojq -f jqs/test.jq
gojq: compile error: module not found: "jqs/jq-test"

So maybe split out the os stuff into something like lookup_module and use that for generating diagnostics. But I still have to look into how that's setup in jq-lsp.

Sounds good. Have looked (and reverse engineered my own code) and thought a bit about how it could work. First maybe good to know how query_walk($uri; $start_env; f) works. You feed it a query AST (usually the whole file's AST) as input and it will output {q: <node>, env: <env>} objects where <node> is current AST node and <env> is environment object at that node. Arguments are $uri URI of source text document of input AST, $start_env initial start environment with functions and bindings (these are the builtins etc), f is filter to only output some interesting AST nodes, i guess it could have been skipped and just do query_walk() | select(...) instead.

So how to handle a missing import, i can see some ways it could work:

• Somehow make query_walk able to output "error" objects that we in didOpen/didChange can turn into a diagnostic objects.
• Throw some error object in query_walk and try/catch in didOpen/didChange
• Do import checks in a separate first step. As the import definitions are only on root AST nodes i guess it should not be that hard, but would still have to support recursive imports... and detect loops?

BTW if you want to play around with jq AST trees you can use https://github.com/wader/fq which has some internal functions to convert to/from AST, ex:

$ fq -rn '`import "a" as $a; include "b"; f` | _query_fromstring | ., (.imports += [{include_path: "c"}] | _query_tostring)'
{
  "imports": [
    {
      "import_alias": "$a",
      "import_path": "a"
    },
    {
      "include_path": "b"
    }
  ],
  "term": {
    "func": {
      "name": "f"
    },
    "type": "TermTypeFunc"
  }
}
import "a" as $a;
include "b";
include "c";
f

This outputs AST for import "a" as $a; include "b"; f adds a include "c" node and outputs the new AST a jq query.

Only difference to jq-lsp parser it that all tokens are just strings instead of {str: "..", start: 123, stop: 123}

Also note that fq has support for "raw"-string literals using `...` just like go, which is handy when doing these things :)