add Pyright langserver spec

[yuntan]

References

related to #534

Code changes

Added Pyright langserver spec.

User-facing changes

User can use Pyright with JupyterLab.

Backwards-incompatible changes

Nothing.

Chores

• linted
• tested
• documented
• changelog entry

I wonder how to add default configuration for language server. In this case, setting python.analysis.useLibraryCodeForTypes to true by default might helpful for users.

[krassowski]

Thanks for the PR!

I wonder how to add default configuration for language server. In this case, setting python.analysis.useLibraryCodeForTypes to true by default might helpful for users.

Good question! It is possible by changing one of the defaults in:

https://github.com/krassowski/jupyterlab-lsp/blob/e85d73f6577f586dfdc5080f12c420e4d3079b12/packages/jupyterlab-lsp/schema/plugin.json#L7-L36

It might be good to think about a better way in the future, but this would be fine for me if you just added it there.

[yuntan]

Thank you for your advice! I added default configuration and set this PR ready for review.

[krassowski]

Thank you! I think that it would be nice to show it in the documentation among other "NodeJS-based Language Servers" but it does not show up currently: https://jupyterlab-lsp--587.org.readthedocs.build/en/587/Language%20Servers.html

If I am not mistaken, it should be enough to add it to our devDependencies (in repo, not in the package):

https://github.com/krassowski/jupyterlab-lsp/blob/e85d73f6577f586dfdc5080f12c420e4d3079b12/package.json#L3

But we may need to think about it as I suspect it might be heavy. Thoughts @bollwyvl?

Also, feel free to add a changelog entry.

[krassowski]

Ah, adding it may cause the tests to fail if it gets picked up instead of pylsp for tests. We need to introduce the priority system to select which server should be used if multiple server supporting given language are available ASAP to make testing of multiple servers for given language possible on CI.

[bollwyvl]

priority system

yeah, a relatively quick band-aid that wouldn't require much spec rework would be to land it in user settings. Is this a place to start the idea of a regex-based approach for e.g. using different servers for notebooks and python files?

{
    "language_servers": {
      "python-lsp-server": {
         "priority": 100,
      },
      "pyright": {
         "priority": 1000,
         "serverSettings": {
            "python.analysis.useLibraryCodeForTypes": true
         },
         "_path_overrides": {
            "priority": {
               "ipynb$": 0
            }
         }
      }
   }
}

In this case, serverSettings would not be overrideable by path (since we would still just have one language server process per implementation).

This would give us a fair amount of flexibility until we figure out how to do multiple simultaneous sources properly, which is really the path forward...

[krassowski]

Looks good. In future we could just include something like:

{
    "language_servers": {
      "spellchecker-server": {
         "priority": 0,
         "secondaryServer": true
      },
      "python-lsp-server": {
         "priority": 100,
         "secondaryServer": true,
         "secondaryFeatures": ["textDocument/publishDiagnostics"]
      },
      "jedi-language-server": {
         "priority": 1000,
         "serverSettings": {}
      }
   }
}

where secondaryServer means that the server should run in tandem with whatever is running as a primary server (jedi-language-server in the example above) and the results from them should be combined. primary server. secondaryFeatures could limit that only specific features are used when running as secondary.

[bollwyvl]

secondaryServer

I'm thinking if we do 2, we might as well do n servers for m features on one file, all reporting at the same time. Case in point: I might want "the toml server" to provide well-formedness checks of a pyproject.toml and diagnostics from a "pyproject.toml server" that tells me if a requires package doesn't exist on my configured repositories and a spell checking server on my comments, description, etc.

[krassowski]

Agree. I did not mean to suggest that it is max two servers, I just could not think of a better name than "secondary" that would be obvious in meaning.

[krassowski]

Also updating Node as pyright requires >=12 (10 just reached EOL), but only bumping to 15 as 16 not yet on conda-forge: conda-forge/nodejs-feedstock#189

[krassowski]

The pyright diagnostics do not seem to be shown on Windows; this appears to be related to path encoding and the issues with #595.

Specifically we can see:

[W 2021-05-19 15:13:06.069 ServerApp] [pyright] Handling a message: {"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":12,"character":21}}}
[W 2021-05-19 15:13:06.069 ServerApp] [<jupyter_lsp.handlers.LanguageServerWebSocketHandler object at 0x000001B45DFB8130>] Handling a client message: {"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":12,"character":21}}}
[W 2021-05-19 15:13:06.498 ServerApp] [pyright] Handling a message: {"jsonrpc":"2.0","id":2,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":16,"character":21}}}
[W 2021-05-19 15:13:06.523 ServerApp] [<jupyter_lsp.handlers.LanguageServerWebSocketHandler object at 0x000001B45DFB8130>] Handling a client message: {"jsonrpc":"2.0","id":2,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":16,"character":21}}}
[D 2021-05-19 15:13:06.802 LabApp] 204 PUT /@est/lab/api/workspaces/default?1621437186737 (127.0.0.1) 3.00ms
[W 2021-05-19 15:13:06.804 ServerApp] [<LanguageServerSession(language_server=pyright, argv=['C:\\Miniconda3\\envs\\test\\node.EXE', 'D:\\a\\jupyterlab-lsp\\jupyterlab-lsp\\node_modules\\pyright\\langserver.index.js', '--stdio'])>] Handling a server message: {"jsonrpc":"2.0","id":1,"result":null}
[W 2021-05-19 15:13:06.805 ServerApp] [<LanguageServerSession(language_server=pyright, argv=['C:\\Miniconda3\\envs\\test\\node.EXE', 'D:\\a\\jupyterlab-lsp\\jupyterlab-lsp\\node_modules\\pyright\\langserver.index.js', '--stdio'])>] Handling a server message: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///d%3A/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py","diagnostics":[{"range":{"start":{"line":29,"character":8},"end":{"line":29,"character":14}},"message":"\"result\" is not defined","severity":1,"code":"reportUndefinedVariable","source":"Pyright","codeDescription":{"href":"https://github.com/microsoft/pyright/blob/main/docs/configuration.md"}}]}}
[W 2021-05-19 15:13:06.807 ServerApp] [<LanguageServerSession(language_server=pyright, argv=['C:\\Miniconda3\\envs\\test\\node.EXE', 'D:\\a\\jupyterlab-lsp\\jupyterlab-lsp\\node_modules\\pyright\\langserver.index.js', '--stdio'])>] Handling a server message: {"jsonrpc":"2.0","id":2,"result":null}
[D 2021-05-19 15:13:07.731 LabApp] 204 PUT /@est/lab/api/workspaces/default?1621437187726 (127.0.0.1) 3.98ms
[W 2021-05-19 15:13:48.164 ServerApp] [pyright] Handling a message: {"jsonrpc":"2.0","id":3,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":17,"character":23}}}
[W 2021-05-19 15:13:48.165 ServerApp] [<jupyter_lsp.handlers.LanguageServerWebSocketHandler object at 0x000001B45DFB8130>] Handling a client message: {"jsonrpc":"2.0","id":3,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py"},"position":{"line":17,"character":23}}}
[W 2021-05-19 15:13:48.237 ServerApp] [<LanguageServerSession(language_server=pyright, argv=['C:\\Miniconda3\\envs\\test\\node.EXE', 'D:\\a\\jupyterlab-lsp\\jupyterlab-lsp\\node_modules\\pyright\\langserver.index.js', '--stdio'])>] Handling a server message: {"jsonrpc":"2.0","id":3,"result":null}

So we request:

file:///d:/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py

But pyright returns:

file:///d%3A/a/jupyterlab-lsp/jupyterlab-lsp/atest/output/windows_39_4/home/n%C3%B6te%20b%C3%B2%C3%B3ks/example.py

[krassowski]

It appears that it was not intended behaviour two years ago: microsoft/pyright#124 but in a recent issue:
microsoft/pyright#1576 it appears that it became pretty standard for Pyright to encode : as c%3A.

[krassowski]

So green 🟢 :) Thank you @yuntan, merging now.