From d3dd832e9b00c38410490087ad0843d8334a8ec8 Mon Sep 17 00:00:00 2001 From: Heejae Chang <1333179+heejaechang@users.noreply.github.com> Date: Fri, 10 Mar 2023 12:05:37 -0800 Subject: [PATCH] Expose client creation API for pylance (#20816) If new client change is available in pylance, made core extension to use pylance to do language server lifetime management. and also this PR removes all old notebook experiences so that it is inline with pylance (pylance already removed all those when moving client/middleware) --- .../activation/languageClientMiddleware.ts | 2 +- src/client/activation/node/analysisOptions.ts | 4 - .../activation/node/languageClientFactory.ts | 4 +- .../node/languageClientMiddleware.ts | 25 +--- .../activation/node/languageServerProxy.ts | 32 +++- .../activation/node/lspNotebooksExperiment.ts | 140 +----------------- src/client/activation/node/pylanceApi.ts | 29 ++++ src/client/api.ts | 12 +- src/client/browser/extension.ts | 44 ++++-- src/client/jupyter/jupyterIntegration.ts | 31 +++- .../pylanceLSExtensionManager.ts | 3 - src/client/languageServer/watcher.ts | 9 +- .../node/analysisOptions.unit.test.ts | 11 +- .../pylanceLSExtensionManager.unit.test.ts | 4 - src/test/languageServer/watcher.unit.test.ts | 17 --- 15 files changed, 156 insertions(+), 211 deletions(-) create mode 100644 src/client/activation/node/pylanceApi.ts diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts index 110d7461c615..c34aadc80208 100644 --- a/src/client/activation/languageClientMiddleware.ts +++ b/src/client/activation/languageClientMiddleware.ts @@ -42,7 +42,7 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { ); } - protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { + private shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { return jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled; } diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index f717a8e2f3f1..80dbc53a59ee 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -10,7 +10,6 @@ import { IExperimentService } from '../../common/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; -import { LspNotebooksExperiment } from './lspNotebooksExperiment'; import { traceWarn } from '../../logging'; const EDITOR_CONFIG_SECTION = 'editor'; @@ -22,7 +21,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, private readonly experimentService: IExperimentService, - private readonly lspNotebooksExperiment: LspNotebooksExperiment, ) { super(lsOutputChannel, workspace); } @@ -36,8 +34,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), - lspInteractiveWindowSupport: this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport(), autoIndentSupport: await this.isAutoIndentEnabled(), } as unknown) as LanguageClientOptions; } diff --git a/src/client/activation/node/languageClientFactory.ts b/src/client/activation/node/languageClientFactory.ts index ef2ac872c190..9543f265468f 100644 --- a/src/client/activation/node/languageClientFactory.ts +++ b/src/client/activation/node/languageClientFactory.ts @@ -11,7 +11,7 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { FileBasedCancellationStrategy } from '../common/cancellationUtils'; import { ILanguageClientFactory } from '../types'; -const languageClientName = 'Pylance'; +export const PYLANCE_NAME = 'Pylance'; export class NodeLanguageClientFactory implements ILanguageClientFactory { constructor(private readonly fs: IFileSystem, private readonly extensions: IExtensions) {} @@ -50,6 +50,6 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory { }, }; - return new LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); + return new LanguageClient(PYTHON_LANGUAGE, PYLANCE_NAME, serverOptions, clientOptions); } } diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 9c1d4c468191..a749ee9e02b9 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -36,7 +36,7 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { this.jupyterExtensionIntegration = serviceContainer.get( JupyterExtensionIntegration, ); - if (!this.notebookAddon && this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) { + if (!this.notebookAddon) { this.notebookAddon = new LspInteractiveWindowMiddlewareAddon( this.getClient, this.jupyterExtensionIntegration, @@ -44,32 +44,21 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { } } - protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { - return ( - super.shouldCreateHidingMiddleware(jupyterDependencyManager) && - !this.lspNotebooksExperiment.isInNotebooksExperiment() - ); - } - protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise { if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) { await this.lspNotebooksExperiment.onJupyterInstalled(); } - if (this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) { - if (!this.notebookAddon) { - this.notebookAddon = new LspInteractiveWindowMiddlewareAddon( - this.getClient, - this.jupyterExtensionIntegration, - ); - } - } else { - super.onExtensionChange(jupyterDependencyManager); + if (!this.notebookAddon) { + this.notebookAddon = new LspInteractiveWindowMiddlewareAddon( + this.getClient, + this.jupyterExtensionIntegration, + ); } } protected async getPythonPathOverride(uri: Uri | undefined): Promise { - if (!uri || !this.lspNotebooksExperiment.isInNotebooksExperiment()) { + if (!uri) { return undefined; } diff --git a/src/client/activation/node/languageServerProxy.ts b/src/client/activation/node/languageServerProxy.ts index dea261514702..45d1d1a17fee 100644 --- a/src/client/activation/node/languageServerProxy.ts +++ b/src/client/activation/node/languageServerProxy.ts @@ -9,6 +9,7 @@ import { LanguageClientOptions, } from 'vscode-languageclient/node'; +import { Extension } from 'vscode'; import { IExperimentService, IExtensions, IInterpreterPathService, Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -20,6 +21,7 @@ import { ILanguageClientFactory, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose, traceError } from '../../logging'; import { IWorkspaceService } from '../../common/application/types'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; +import { PylanceApi } from './pylanceApi'; // eslint-disable-next-line @typescript-eslint/no-namespace namespace InExperiment { @@ -56,6 +58,8 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { private lsVersion: string | undefined; + private pylanceApi: PylanceApi | undefined; + constructor( private readonly factory: ILanguageClientFactory, private readonly experimentService: IExperimentService, @@ -89,9 +93,16 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { interpreter: PythonEnvironment | undefined, options: LanguageClientOptions, ): Promise { - const extension = this.extensions.getExtension(PYLANCE_EXTENSION_ID); + const extension = await this.getPylanceExtension(); this.lsVersion = extension?.packageJSON.version || '0'; + const api = extension?.exports; + if (api && api.client && api.client.isEnabled()) { + this.pylanceApi = api; + await api.client.start(); + return; + } + this.cancellationStrategy = new FileBasedCancellationStrategy(); options.connectionOptions = { cancellationStrategy: this.cancellationStrategy }; @@ -111,6 +122,12 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { @traceDecoratorVerbose('Disposing language server') public async stop(): Promise { + if (this.pylanceApi) { + const api = this.pylanceApi; + this.pylanceApi = undefined; + await api.client!.stop(); + } + while (this.disposables.length > 0) { const d = this.disposables.shift()!; d.dispose(); @@ -203,4 +220,17 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { })), ); } + + private async getPylanceExtension(): Promise | undefined> { + const extension = this.extensions.getExtension(PYLANCE_EXTENSION_ID); + if (!extension) { + return undefined; + } + + if (!extension.isActive) { + await extension.activate(); + } + + return extension; + } } diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index d469cfb112df..de0acde0600e 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -1,16 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import * as semver from 'semver'; -import { Disposable, extensions } from 'vscode'; -import { IConfigurationService, IDisposableRegistry } from '../../common/types'; -import { sendTelemetryEvent } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; -import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constants'; -import { IExtensionSingleActivationService, LanguageServerType } from '../types'; -import { traceLog, traceVerbose } from '../../logging'; +import { IExtensionSingleActivationService } from '../types'; +import { traceVerbose } from '../../logging'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; -import { ILanguageServerWatcher } from '../../languageServer/types'; import { IServiceContainer } from '../../ioc/types'; import { sleep } from '../../common/utils/async'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; @@ -19,30 +12,18 @@ import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; export class LspNotebooksExperiment implements IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; - private pylanceExtensionChangeHandler: Disposable | undefined; - private isJupyterInstalled = false; - private isInExperiment: boolean | undefined; - - private supportsInteractiveWindow: boolean | undefined; - constructor( @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, - @inject(IConfigurationService) private readonly configurationService: IConfigurationService, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IJupyterExtensionDependencyManager) jupyterDependencyManager: IJupyterExtensionDependencyManager, ) { this.isJupyterInstalled = jupyterDependencyManager.isJupyterExtensionInstalled; } - public async activate(): Promise { - if (!LspNotebooksExperiment.isPylanceInstalled()) { - this.pylanceExtensionChangeHandler = extensions.onDidChange(this.pylanceExtensionsChangeHandler.bind(this)); - this.disposables.push(this.pylanceExtensionChangeHandler); - } - - this.updateExperimentSupport(); + // eslint-disable-next-line class-methods-use-this + public activate(): Promise { + return Promise.resolve(); } public async onJupyterInstalled(): Promise { @@ -50,103 +31,11 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService return; } - if (LspNotebooksExperiment.jupyterSupportsNotebooksExperiment()) { - await this.waitForJupyterToRegisterPythonPathFunction(); - this.updateExperimentSupport(); - } + await this.waitForJupyterToRegisterPythonPathFunction(); this.isJupyterInstalled = true; } - public isInNotebooksExperiment(): boolean { - return this.isInExperiment ?? false; - } - - public isInNotebooksExperimentWithInteractiveWindowSupport(): boolean { - return this.supportsInteractiveWindow ?? false; - } - - private updateExperimentSupport(): void { - const wasInExperiment = this.isInExperiment; - const isInTreatmentGroup = true; - const languageServerType = this.configurationService.getSettings().languageServer; - - this.isInExperiment = false; - if (languageServerType !== LanguageServerType.Node) { - traceLog(`LSP Notebooks experiment is disabled -- not using Pylance`); - } else if (!LspNotebooksExperiment.isJupyterInstalled()) { - traceLog(`LSP Notebooks experiment is disabled -- Jupyter disabled or not installed`); - } else if (!LspNotebooksExperiment.jupyterSupportsNotebooksExperiment()) { - traceLog(`LSP Notebooks experiment is disabled -- Jupyter does not support experiment`); - } else if (!LspNotebooksExperiment.isPylanceInstalled()) { - traceLog(`LSP Notebooks experiment is disabled -- Pylance disabled or not installed`); - } else if (!LspNotebooksExperiment.pylanceSupportsNotebooksExperiment()) { - traceLog(`LSP Notebooks experiment is disabled -- Pylance does not support experiment`); - } else if (!isInTreatmentGroup) { - traceLog(`LSP Notebooks experiment is disabled -- not in treatment group`); - // to avoid scorecard SRMs, we're also triggering the telemetry for users who meet - // the criteria to experience LSP notebooks, but may be in the control group. - sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); - } else { - this.isInExperiment = true; - traceLog(`LSP Notebooks experiment is enabled`); - sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); - } - - this.supportsInteractiveWindow = false; - if (!this.isInExperiment) { - traceLog(`LSP Notebooks interactive window support is disabled -- not in LSP Notebooks experiment`); - } else if (!LspNotebooksExperiment.jupyterSupportsLspInteractiveWindow()) { - traceLog(`LSP Notebooks interactive window support is disabled -- Jupyter is not new enough`); - } else if (!LspNotebooksExperiment.pylanceSupportsLspInteractiveWindow()) { - traceLog(`LSP Notebooks interactive window support is disabled -- Pylance is not new enough`); - } else { - this.supportsInteractiveWindow = true; - traceLog(`LSP Notebooks interactive window support is enabled`); - } - - // Our "in experiment" status can only change from false to true. That's possible if Pylance - // or Jupyter is installed after Python is activated. A true to false transition would require - // either Pylance or Jupyter to be uninstalled or downgraded after Python activated, and that - // would require VS Code to be reloaded before the new extension version could be used. - if (wasInExperiment === false && this.isInExperiment === true) { - const watcher = this.serviceContainer.get(ILanguageServerWatcher); - if (watcher) { - watcher.restartLanguageServers(); - } - } - } - - private static jupyterSupportsNotebooksExperiment(): boolean { - const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - return ( - jupyterVersion && (semver.gt(jupyterVersion, '2022.5.1001411044') || semver.patch(jupyterVersion) === 100) - ); - } - - private static pylanceSupportsNotebooksExperiment(): boolean { - const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; - return ( - pylanceVersion && - (semver.gte(pylanceVersion, '2022.5.3-pre.1') || semver.prerelease(pylanceVersion)?.includes('dev')) - ); - } - - private static jupyterSupportsLspInteractiveWindow(): boolean { - const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - return ( - jupyterVersion && (semver.gt(jupyterVersion, '2022.7.1002041057') || semver.patch(jupyterVersion) === 100) - ); - } - - private static pylanceSupportsLspInteractiveWindow(): boolean { - const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; - return ( - pylanceVersion && - (semver.gte(pylanceVersion, '2022.7.51') || semver.prerelease(pylanceVersion)?.includes('dev')) - ); - } - private async waitForJupyterToRegisterPythonPathFunction(): Promise { const jupyterExtensionIntegration = this.serviceContainer.get( JupyterExtensionIntegration, @@ -168,21 +57,4 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService traceVerbose(`Timed out waiting for Jupyter to call registerJupyterPythonPathFunction`); } } - - private static isPylanceInstalled(): boolean { - return !!extensions.getExtension(PYLANCE_EXTENSION_ID); - } - - private static isJupyterInstalled(): boolean { - return !!extensions.getExtension(JUPYTER_EXTENSION_ID); - } - - private async pylanceExtensionsChangeHandler(): Promise { - if (LspNotebooksExperiment.isPylanceInstalled() && this.pylanceExtensionChangeHandler) { - this.pylanceExtensionChangeHandler.dispose(); - this.pylanceExtensionChangeHandler = undefined; - - this.updateExperimentSupport(); - } - } } diff --git a/src/client/activation/node/pylanceApi.ts b/src/client/activation/node/pylanceApi.ts new file mode 100644 index 000000000000..72f20db140e4 --- /dev/null +++ b/src/client/activation/node/pylanceApi.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { + CancellationToken, + CompletionContext, + CompletionItem, + CompletionList, + Position, + TextDocument, + Uri, +} from 'vscode'; + +export interface PylanceApi { + client?: { + isEnabled(): boolean; + start(): Promise; + stop(): Promise; + }; + notebook?: { + registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void; + registerGetNotebookUriForTextDocumentUriFunction(func: (textDocumentUri: Uri) => Uri | undefined): void; + getCompletionItems( + document: TextDocument, + position: Position, + context: CompletionContext, + token: CancellationToken, + ): Promise; + }; +} diff --git a/src/client/api.ts b/src/client/api.ts index 78663feab712..8d618d6d053c 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -5,8 +5,11 @@ import { noop } from 'lodash'; import { Uri, Event } from 'vscode'; +import { BaseLanguageClient } from 'vscode-languageclient'; +import { LanguageClient } from 'vscode-languageclient/node'; +import { PYLANCE_NAME } from './activation/node/languageClientFactory'; import { IExtensionApi } from './apiTypes'; -import { isTestExecution } from './common/constants'; +import { isTestExecution, PYTHON_LANGUAGE } from './common/constants'; import { IConfigurationService, Resource } from './common/types'; import { IEnvironmentVariablesProvider } from './common/variables/types'; import { getDebugpyLauncherArgs, getDebugpyPackagePath } from './debugger/extension/adapter/remoteLaunchers'; @@ -33,6 +36,9 @@ export function buildApi( pylance: { getPythonPathVar: (resource?: Uri) => Promise; readonly onDidEnvironmentVariablesChange: Event; + createClient(...args: any[]): BaseLanguageClient; + start(client: BaseLanguageClient): Promise; + stop(client: BaseLanguageClient): Promise; }; } = { // 'ready' will propagate the exception, but we must log it here first. @@ -83,6 +89,10 @@ export function buildApi( return envs.PYTHONPATH; }, onDidEnvironmentVariablesChange: envService.onDidEnvironmentVariablesChange, + createClient: (...args: any[]): BaseLanguageClient => + new LanguageClient(PYTHON_LANGUAGE, PYLANCE_NAME, args[0], args[1]), + start: (client: BaseLanguageClient): Promise => client.start(), + stop: (client: BaseLanguageClient): Promise => client.stop(), }, }; diff --git a/src/client/browser/extension.ts b/src/client/browser/extension.ts index c9ecf8ac1212..5a0e8ae5ee14 100644 --- a/src/client/browser/extension.ts +++ b/src/client/browser/extension.ts @@ -10,22 +10,24 @@ import { LanguageServerType } from '../activation/types'; import { AppinsightsKey, PYLANCE_EXTENSION_ID } from '../common/constants'; import { EventName } from '../telemetry/constants'; import { createStatusItem } from './intellisenseStatus'; +import { PylanceApi } from '../activation/node/pylanceApi'; interface BrowserConfig { distUrl: string; // URL to Pylance's dist folder. } let languageClient: LanguageClient | undefined; +let pylanceApi: PylanceApi | undefined; export async function activate(context: vscode.ExtensionContext): Promise { - const pylanceExtension = vscode.extensions.getExtension(PYLANCE_EXTENSION_ID); + const pylanceExtension = vscode.extensions.getExtension(PYLANCE_EXTENSION_ID); if (pylanceExtension) { await runPylance(context, pylanceExtension); return; } const changeDisposable = vscode.extensions.onDidChange(async () => { - const newPylanceExtension = vscode.extensions.getExtension(PYLANCE_EXTENSION_ID); + const newPylanceExtension = vscode.extensions.getExtension(PYLANCE_EXTENSION_ID); if (newPylanceExtension) { changeDisposable.dispose(); await runPylance(context, newPylanceExtension); @@ -34,17 +36,35 @@ export async function activate(context: vscode.ExtensionContext): Promise } export async function deactivate(): Promise { - const client = languageClient; - languageClient = undefined; + if (pylanceApi) { + const api = pylanceApi; + pylanceApi = undefined; + await api.client!.stop(); + } + + if (languageClient) { + const client = languageClient; + languageClient = undefined; - await client?.stop(); - await client?.dispose(); + await client.stop(); + await client.dispose(); + } } async function runPylance( context: vscode.ExtensionContext, - pylanceExtension: vscode.Extension, + pylanceExtension: vscode.Extension, ): Promise { + context.subscriptions.push(createStatusItem()); + + pylanceExtension = await getActivatedExtension(pylanceExtension); + const api = pylanceExtension.exports; + if (api.client && api.client.isEnabled()) { + pylanceApi = api; + await api.client.start(); + return; + } + const { extensionUri, packageJSON } = pylanceExtension; const distUrl = vscode.Uri.joinPath(extensionUri, 'dist'); @@ -111,8 +131,6 @@ async function runPylance( ); await client.start(); - - context.subscriptions.push(createStatusItem()); } catch (e) { console.log(e); } @@ -196,3 +214,11 @@ function sendTelemetryEventBrowser( reporter.sendTelemetryEvent(eventNameSent, customProperties, measures); } } + +async function getActivatedExtension(extension: vscode.Extension): Promise> { + if (!extension.isActive) { + await extension.activate(); + } + + return extension; +} diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index aedc24b1e8bf..16da174f3178 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -9,7 +9,7 @@ import { dirname } from 'path'; import { CancellationToken, Event, Extension, Memento, Uri } from 'vscode'; import type { SemVer } from 'semver'; import { IWorkspaceService } from '../common/application/types'; -import { JUPYTER_EXTENSION_ID } from '../common/constants'; +import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../common/constants'; import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types'; import { GLOBAL_MEMENTO, @@ -34,7 +34,7 @@ import { } from '../interpreter/contracts'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { IDataViewerDataProvider, IJupyterUriProvider } from './types'; - +import { PylanceApi } from '../activation/node/pylanceApi'; /** * This allows Python extension to update Product enum without breaking Jupyter. * I.e. we have a strict contract, else using numbers (in enums) is bound to break across products. @@ -184,6 +184,8 @@ type JupyterExtensionApi = { export class JupyterExtensionIntegration { private jupyterExtension: Extension | undefined; + private pylanceExtension: Extension | undefined; + private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined; private getNotebookUriForTextDocumentUriFunction: ((textDocumentUri: Uri) => Uri | undefined) | undefined; @@ -300,6 +302,16 @@ export class JupyterExtensionIntegration { } private async getExtensionApi(): Promise { + if (!this.pylanceExtension) { + const pylanceExtension = this.extensions.getExtension(PYLANCE_EXTENSION_ID); + + if (pylanceExtension && !pylanceExtension.isActive) { + await pylanceExtension.activate(); + } + + this.pylanceExtension = pylanceExtension; + } + if (!this.jupyterExtension) { const jupyterExtension = this.extensions.getExtension(JUPYTER_EXTENSION_ID); if (!jupyterExtension) { @@ -316,8 +328,18 @@ export class JupyterExtensionIntegration { return undefined; } + private getPylanceApi(): PylanceApi | undefined { + const api = this.pylanceExtension?.exports; + return api && api.notebook && api.client && api.client.isEnabled() ? api : undefined; + } + private registerJupyterPythonPathFunction(func: (uri: Uri) => Promise) { this.jupyterPythonPathFunction = func; + + const api = this.getPylanceApi(); + if (api) { + api.notebook!.registerJupyterPythonPathFunction(func); + } } public getJupyterPythonPathFunction(): ((uri: Uri) => Promise) | undefined { @@ -326,6 +348,11 @@ export class JupyterExtensionIntegration { public registerGetNotebookUriForTextDocumentUriFunction(func: (textDocumentUri: Uri) => Uri | undefined): void { this.getNotebookUriForTextDocumentUriFunction = func; + + const api = this.getPylanceApi(); + if (api) { + api.notebook!.registerGetNotebookUriForTextDocumentUriFunction(func); + } } public getGetNotebookUriForTextDocumentUriFunction(): ((textDocumentUri: Uri) => Uri | undefined) | undefined { diff --git a/src/client/languageServer/pylanceLSExtensionManager.ts b/src/client/languageServer/pylanceLSExtensionManager.ts index 3865886880b2..c7df3318e3b7 100644 --- a/src/client/languageServer/pylanceLSExtensionManager.ts +++ b/src/client/languageServer/pylanceLSExtensionManager.ts @@ -5,7 +5,6 @@ import { promptForPylanceInstall } from '../activation/common/languageServerChan import { NodeLanguageServerAnalysisOptions } from '../activation/node/analysisOptions'; import { NodeLanguageClientFactory } from '../activation/node/languageClientFactory'; import { NodeLanguageServerProxy } from '../activation/node/languageServerProxy'; -import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment'; import { NodeLanguageServerManager } from '../activation/node/manager'; import { ILanguageServerOutputChannel } from '../activation/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; @@ -49,13 +48,11 @@ export class PylanceLSExtensionManager implements IDisposable, ILanguageServerEx fileSystem: IFileSystem, private readonly extensions: IExtensions, readonly applicationShell: IApplicationShell, - lspNotebooksExperiment: LspNotebooksExperiment, ) { this.analysisOptions = new NodeLanguageServerAnalysisOptions( outputChannel, workspaceService, experimentService, - lspNotebooksExperiment, ); this.clientFactory = new NodeLanguageClientFactory(fileSystem, extensions); this.serverProxy = new NodeLanguageServerProxy( diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index 8ed4a3e27334..d3eccb71144c 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -27,7 +27,6 @@ import { JediLSExtensionManager } from './jediLSExtensionManager'; import { NoneLSExtensionManager } from './noneLSExtensionManager'; import { PylanceLSExtensionManager } from './pylanceLSExtensionManager'; import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types'; -import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; @@ -65,7 +64,6 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang @inject(IFileSystem) private readonly fileSystem: IFileSystem, @inject(IExtensions) private readonly extensions: IExtensions, @inject(IApplicationShell) readonly applicationShell: IApplicationShell, - @inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment, @inject(IDisposableRegistry) readonly disposables: IDisposableRegistry, ) { this.workspaceInterpreters = new Map(); @@ -247,7 +245,6 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang this.fileSystem, this.extensions, this.applicationShell, - this.lspNotebooksExperiment, ); break; case LanguageServerType.None: @@ -265,11 +262,11 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang return lsManager; } - private async refreshLanguageServer(resource?: Resource): Promise { + private async refreshLanguageServer(resource?: Resource, forced?: boolean): Promise { const lsResource = this.getWorkspaceUri(resource); const languageServerType = this.configurationService.getSettings(lsResource).languageServer; - if (languageServerType !== this.languageServerType) { + if (languageServerType !== this.languageServerType || forced) { await this.stopLanguageServer(resource); await this.startLanguageServer(languageServerType, lsResource); } @@ -286,6 +283,8 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang workspacesUris.forEach(async (resource) => { if (event.affectsConfiguration(`python.languageServer`, resource)) { await this.refreshLanguageServer(resource); + } else if (event.affectsConfiguration(`python.analysis.pylanceLspClientEnabled`, resource)) { + await this.refreshLanguageServer(resource, /* forced */ true); } }); } diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 8d16f0c0d9c9..d4781f7e03e5 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -6,7 +6,6 @@ import { WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { DocumentFilter } from 'vscode-languageclient/node'; import { NodeLanguageServerAnalysisOptions } from '../../../client/activation/node/analysisOptions'; -import { LspNotebooksExperiment } from '../../../client/activation/node/lspNotebooksExperiment'; import { ILanguageServerOutputChannel } from '../../../client/activation/types'; import { IWorkspaceService } from '../../../client/common/application/types'; import { PYTHON, PYTHON_LANGUAGE } from '../../../client/common/constants'; @@ -33,7 +32,6 @@ suite('Pylance Language Server - Analysis Options', () => { let lsOutputChannel: typemoq.IMock; let workspace: typemoq.IMock; let experimentService: IExperimentService; - let lspNotebooksExperiment: typemoq.IMock; setup(() => { outputChannel = typemoq.Mock.ofType().object; @@ -45,14 +43,7 @@ suite('Pylance Language Server - Analysis Options', () => { lsOutputChannel = typemoq.Mock.ofType(); lsOutputChannel.setup((l) => l.channel).returns(() => outputChannel); experimentService = typemoq.Mock.ofType().object; - lspNotebooksExperiment = typemoq.Mock.ofType(); - lspNotebooksExperiment.setup((l) => l.isInNotebooksExperiment()).returns(() => false); - analysisOptions = new TestClass( - lsOutputChannel.object, - workspace.object, - experimentService, - lspNotebooksExperiment.object, - ); + analysisOptions = new TestClass(lsOutputChannel.object, workspace.object, experimentService); }); test('Workspace folder is undefined', () => { diff --git a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts index 0118cca0764f..751b26d37d3c 100644 --- a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts +++ b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import * as assert from 'assert'; -import { LspNotebooksExperiment } from '../../client/activation/node/lspNotebooksExperiment'; import { ILanguageServerOutputChannel } from '../../client/activation/types'; import { IWorkspaceService, ICommandManager, IApplicationShell } from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; @@ -38,7 +37,6 @@ suite('Language Server - Pylance LS extension manager', () => { {} as IFileSystem, {} as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, ); }); @@ -67,7 +65,6 @@ suite('Language Server - Pylance LS extension manager', () => { getExtension: () => ({}), } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, ); const result = manager.canStartLanguageServer(); @@ -95,7 +92,6 @@ suite('Language Server - Pylance LS extension manager', () => { getExtension: () => undefined, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, ); const result = manager.canStartLanguageServer(); diff --git a/src/test/languageServer/watcher.unit.test.ts b/src/test/languageServer/watcher.unit.test.ts index 61628257fc90..e86e19cf2055 100644 --- a/src/test/languageServer/watcher.unit.test.ts +++ b/src/test/languageServer/watcher.unit.test.ts @@ -5,7 +5,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { ConfigurationChangeEvent, Uri, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode'; import { JediLanguageServerManager } from '../../client/activation/jedi/manager'; -import { LspNotebooksExperiment } from '../../client/activation/node/lspNotebooksExperiment'; import { NodeLanguageServerManager } from '../../client/activation/node/manager'; import { ILanguageServerOutputChannel, LanguageServerType } from '../../client/activation/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; @@ -80,7 +79,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); @@ -130,7 +128,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -176,7 +173,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -253,7 +249,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -330,7 +325,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -410,7 +404,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -480,7 +473,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -544,7 +536,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -608,7 +599,6 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -667,7 +657,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -757,7 +746,6 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -837,7 +825,6 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -926,7 +913,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -1007,7 +993,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -1091,7 +1076,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register(); @@ -1175,7 +1159,6 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, - {} as LspNotebooksExperiment, disposables, ); watcher.register();