Skip to content

Remove the need for reloading VS Code when changing language servers

Kim-Adeline Miguel edited this page Apr 19, 2022 · 11 revisions

This is a design document for the refactoring needed in order to not require a reload when updating the python.languageServer setting (issue: 18509, PR: #18884).

Table of contents

Previous design

Language server-specific code was registered on extension activation depending on the value of the python.languageServer setting: For example, if python.languageServer was set to Jedi, only the Jedi-specific classes would be available in the extension. This meant that whenever python.languageServer was updated, we needed to reload the extension (and VS Code) in order to register the classes needed by the new language server.

Language server checks (if the setting is set to the default value, if the interpreter is Python 2.7, if the workspace is untrusted) were also done during activation on language server start.

Jupyter support

Notebook support in the Language Server Protocol is still in a proposed state, meaning that it isn't included in LSP v3.16, only in the pre-release 3.17 version.

Until notebook support lands in the stable API, we have a custom-made middleware in the Python extension that will intercept LSP calls and send them to the Jupyter LSP package. Since this middleware isn't tied to how we launch language servers, it shouldn't be impacted by this work.

As part of the Python extension API for the Jupyter extension, we provide a way to get a reference to the language server, via the ILanguageServerCache interface, which will be refactored as part of this work.

Existing classes and interfaces

Pylance classes

  • NodeLanguageServerAnalysisOptions: Class that implements ILanguageServerAnalysisOptions, provides an analysis options object that will be used by NodeLanguageServerProxy to start the language server
  • NodeLanguageServerActivator: Higher-level class that makes sure Pylance is installed and displays an error message otherwise, and inherits from LanguageServerActivatorBase, which provides common behaviour to call the server manager to create and start the language server;
  • NodeLanguageServerManager: Will dispose of everything, connect/disconnect to/from the middleware, register the command to restart the language server, and will call NodeLanguageServerProxy.start() to start it
  • NodeLanguageClientFactory: Creates a language client, used in NodeLanguageServerProxy.start()
  • NodeLanguageServerProxy: Communicates directly with the language server
  • NodeLanguageServerFolderService: MPLS (language server that predates Pylance) remnants, currently only provides a helper function to retrieve the Pylance extension directory, which was necessary back when the MPLS had to be downloaded by the Python extension, instead of being a standalone extension
  • LanguageServerChangeHandler: Registers a handler for when extensions change, and displays a prompt to reload VS Code if Pylance just finished installing.

Jedi classes

JediLanguageServerAnalysisOptions, JediLanguageClientFactory, JediLanguageServerManager and JediLanguageServerProxy have the same functionality as their Pylance counterparts. Since the Jedi language server is shipped with the Python extension, we don't need to listen for extension changes, or check if any extension is installed, so JediLanguageServerActivator doesn't have add any extra behaviour outside of its parent class LanguageServerActivatorBase.

Other classes

Activation classes, and classes that are not directly Jedi or Pylance-related.

  • LanguageServerExtensionActivationService: Called on extension activation since it implements IExtensionActivationService. Instantiates the LanguageServerChangeHandler, adds watchers to workspaceService.onDidChangeConfiguration, workspaceService.onDidChangeWorkspaceFolders, and interpreterService.onDidChangeInterpreter and is the outermost class that will start the language server creation and startup process. It also implements the ILanguageServerCache interface, which provides a get method to be injected in JupyterExtensionIntegration;
  • RefCountedLanguageServer: Implements ILanguageServerActivator like the LS-specific activators, but is only a wrapper around implemented activators (Jedi, Pylance and None). It is used in LanguageServerExtensionActivationService to keep track of language servers, so that we don't dispose of the servers too soon. Why would we dispose of them too soon? I don't know, probably because of some old manual tracking of the language server and/or something MPLS-related, since this part of the code has literally not been touched in 2 years;
  • ILanguageServerActivator: Interface implemented by all the *Activator classes, defines 3 methods: start (start the language server), activate (connect the language server manager to the middleware) and deactivate (disconnect);
  • LanguageServerActivatorBase: Abstract class that implements the ILanguageServerActivator, and that the Jedi and Pylance *Activator classes inherit from since it provides common behaviour;
  • LanguageClientMiddlewareBase: Implements the Middleware interface from vscode-languageclient, does middleware stuff (inserting telemetry calls), and is used when running Pylance in the browser;
  • LanguageClientMiddleware: Extends LanguageClientMiddlewareBase, and is used in the language server proxy classes;

Proposed design

The general idea of this refactoring is to not rely on dependency injection to register language server classes anymore, and instead enabling the extension to start and stop language servers on the fly.

We can also simplify the reference counting part of the code, since it seems like remnants of old MPLS days, along with the NodeLanguageServerFolderService class, which functionality can be achieved using VS Code APIs.

While we're here, old MPLS telemetry events and settings can also be removed.

General architecture

This is the current dependency chain to start the language server:

LanguageServerExtensionActivationService -> [Node|Jedi]LanguageServerActivator -> [Node|Jedi]LanguageServerManager -> [Node|Jedi]LanguageServerProxy -> [Node|Jedi]LanguageClientFactory

With all these classes being registered on extension load, and the server returned by the language server activator being wrapped in a RefCountedLanguageServer instance.

The general architecture following this refactoring would be:

LanguageServerExtensionActivationService -> LanguageServerWatcher -> [Node|Jedi]LanguageServerExtensionManager with references to a Proxy, ClientFactory and ServerManager

This would introduce 2 new interfaces:

  • ILanguageServerWatcher
  • ILanguageServerExtensionManager (still needs a better name)

The ILanguageServerWatcher interface would be implemented by a LanguageServerWatcher singleton, which would have the following responsibilities:

  • Holds a reference to an ILanguageServerManager implementation
  • Implements the ILanguageServerCache interface for Jupyter support
  • On extension activation:
    • Checks LS settings, and instantiates the correct ILanguageServerExtensionManager
    • Asks the LS extension manager if the language server can be started (basically, if the relevant LS extension is installed), and if yes, start the language server via the LS extension manager
  • Watches for python.languageServer and interpreter changes:
    • If the setting or the interpreter changed, asks the LS extension manager to stop the current language server and dispose of it
    • Instantiates a new LS extension manager, asks if the LS can be started, and starts it

TODO

The ILanguageServerExtensionManager interface would be implemented by all possible language server options (that would be Jedi, Node/Pylance and None), and would have the following responsibilities:

  • Exposes:
    • startLS
    • stopLS
    • canStartLS: throw error + display prompt
  • Responsibilities:
    • create analysis options instead of delegating to a subclass (jedi analysis options require a couple of DI deps, so pass the service container to the constructor)
    • instantiate proxy subclass
    • instantiate server manager subclass
    • instantiate client factory subclass

All in all, this would mean the following for existing classes:

  • LanguageServerActivationService gets trimmed down a lot, RefCountedLanguageServer disappears
  • Language server activator classes would get refactored in LanguageServerWatcher
  • NodeLanguageServerManager, NodeLanguageServerProxy and NodeLanguageClientFactory would not be registered with the dependency injection system anymore, and would instead be instantiated inside the PylanceLanguageServerManager
  • NodeLanguageServerFolderService gets removed

Considerations

TODO

  • multiroot workspace
  • extension change handler
  • original plan was to put jedilsp in its own extension, hence the LS extension manager name
  • make sure it still works in codespaces, vscode.dev and jupyter

Note that the [Node|Jedi]LanguageServerAnalysisOptions classes are not mentioned here because they are a dependency of the Proxy classes, and even though they are needed to instantiate the language server, they are not explicitly called when starting it.

Clone this wiki locally