From 2b8c3ff5dc6c67a4579a921cc52e0dff0a128f33 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Thu, 14 Apr 2022 12:47:25 -0700 Subject: [PATCH 01/41] WIP --- .../activation/common/analysisOptions.ts | 4 ++-- src/client/activation/jedi/manager.ts | 3 +++ .../activation/languageClientMiddleware.ts | 13 ++++++++-- .../languageClientMiddlewareBase.ts | 2 +- src/client/activation/node/analysisOptions.ts | 24 +++++++++++++++++-- .../activation/node/lspNotebooksExperiment.ts | 21 ++++++++++++++++ src/client/activation/node/manager.ts | 12 ++++++++++ src/client/activation/types.ts | 3 ++- src/client/jupyter/jupyterIntegration.ts | 8 ++++++- .../node/analysisOptions.unit.test.ts | 6 ++--- 10 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 src/client/activation/node/lspNotebooksExperiment.ts diff --git a/src/client/activation/common/analysisOptions.ts b/src/client/activation/common/analysisOptions.ts index f2839a25399d..d23b34b08a62 100644 --- a/src/client/activation/common/analysisOptions.ts +++ b/src/client/activation/common/analysisOptions.ts @@ -36,7 +36,7 @@ export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServ @traceDecoratorError('Failed to get analysis options') public async getAnalysisOptions(): Promise { const workspaceFolder = this.getWorkspaceFolder(); - const documentSelector = this.getDocumentFilters(workspaceFolder); + const documentSelector = await this.getDocumentFilters(workspaceFolder); return { documentSelector, @@ -54,7 +54,7 @@ export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServ return undefined; } - protected getDocumentFilters(_workspaceFolder?: WorkspaceFolder): DocumentFilter[] { + protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { return this.workspace.isVirtualWorkspace ? [{ language: PYTHON_LANGUAGE }] : PYTHON; } diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index 29f1106c5f8c..e2e12f185cfe 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -110,6 +110,9 @@ export class JediLanguageServerManager implements ILanguageServerManager { } } + // eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function + public setNotebookMiddleware(_notebookAddon: Middleware & Disposable): void {} + @debounceSync(1000) protected restartLanguageServerDebounced(): void { this.restartLanguageServer().ignoreErrors(); diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts index 7474c020217b..bd0ccefdf141 100644 --- a/src/client/activation/languageClientMiddleware.ts +++ b/src/client/activation/languageClientMiddleware.ts @@ -10,6 +10,7 @@ import { LanguageClientMiddlewareBase } from './languageClientMiddlewareBase'; import { LanguageServerType } from './types'; import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; +import { LspNotebooksExperiment } from './node/lspNotebooksExperiment'; export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { public constructor(serviceContainer: IServiceContainer, serverType: LanguageServerType, serverVersion?: string) { @@ -26,7 +27,11 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { const extensions = serviceContainer.get(IExtensions); // Enable notebook support if jupyter support is installed - if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) { + if ( + jupyterDependencyManager && + jupyterDependencyManager.isJupyterExtensionInstalled && + !LspNotebooksExperiment.isInNotebooksExperiment + ) { this.notebookAddon = createHidingMiddleware(); } disposables.push( @@ -34,7 +39,11 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { if (jupyterDependencyManager) { if (this.notebookAddon && !jupyterDependencyManager.isJupyterExtensionInstalled) { this.notebookAddon = undefined; - } else if (!this.notebookAddon && jupyterDependencyManager.isJupyterExtensionInstalled) { + } else if ( + !this.notebookAddon && + jupyterDependencyManager.isJupyterExtensionInstalled && + !LspNotebooksExperiment.isInNotebooksExperiment + ) { this.notebookAddon = createHidingMiddleware(); } } diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 8a2981b6d9dd..8d09d9042273 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -104,7 +104,7 @@ export class LanguageClientMiddlewareBase implements Middleware { return this.connectedPromise.promise; } - protected notebookAddon: (Middleware & Disposable) | undefined; + public notebookAddon: (Middleware & Disposable) | undefined; private connectedPromise = createDeferred(); diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 7518666aa9d0..532a0cb9c0f5 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,11 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { LanguageClientOptions } from 'vscode-languageclient'; +import { inject, injectable } from 'inversify'; +import { WorkspaceFolder } from 'vscode'; +import { DocumentFilter } from 'vscode-languageserver-protocol'; import { IWorkspaceService } from '../../common/application/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; +import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor @@ -18,6 +21,23 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - } as unknown) as LanguageClientOptions; + lspNotebooksSupport: await LspNotebooksExperiment.isInNotebooksExperiment(), + }; + } + + protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { + let filters = await super.getDocumentFilters(_workspaceFolder); + + if (await LspNotebooksExperiment.isInNotebooksExperiment()) { + return [ + ...filters, + { + notebookDocument: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, + cellLanguage: 'python', + }, + ]; + } + + return filters; } } diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts new file mode 100644 index 000000000000..0488daba1e34 --- /dev/null +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { inject, injectable } from 'inversify'; +import { IExperimentService } from '../../common/types'; + +@injectable() +export class LspNotebooksExperiment { + private static _isInNotebooksExperiment?: boolean; + + @inject(IExperimentService) private static experiments: IExperimentService; + + public static async isInNotebooksExperiment(): Promise { + if (LspNotebooksExperiment._isInNotebooksExperiment === undefined) { + LspNotebooksExperiment._isInNotebooksExperiment = await LspNotebooksExperiment.experiments.inExperiment( + 'pylanceLspNotebooksEnabled', + ); + } + + return LspNotebooksExperiment._isInNotebooksExperiment; + } +} diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index e2a67f6fecdb..e754ac372f48 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -2,6 +2,9 @@ // Licensed under the MIT License. import '../../common/extensions'; +import { inject, injectable, named } from 'inversify'; + +import { Disposable } from 'vscode'; import { ICommandManager } from '../../common/application/types'; import { IDisposable, IExtensions, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; @@ -19,6 +22,7 @@ import { } from '../types'; import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; +import { Middleware } from 'vscode-languageclient'; export class NodeLanguageServerManager implements ILanguageServerManager { private resource!: Resource; @@ -100,6 +104,14 @@ export class NodeLanguageServerManager implements ILanguageServerManager { } } + // QUESTIONS: + // Better way to install the injected middleware rather than leveraging existing LanguageClientMiddleware.notebookAddon? + public setNotebookMiddleware(notebookAddon: Middleware & Disposable): void { + if (this.middleware) { + this.middleware.notebookAddon = notebookAddon; + } + } + @debounceSync(1000) protected restartLanguageServerDebounced(): void { this.restartLanguageServer().ignoreErrors(); diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 3ca10b29255d..2ad8b295fb42 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -14,7 +14,7 @@ import { RenameProvider, SignatureHelpProvider, } from 'vscode'; -import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node'; +import { LanguageClient, LanguageClientOptions, Middleware } from 'vscode-languageclient/node'; import * as lsp from 'vscode-languageserver-protocol'; import type { IDisposable, IOutputChannel, Resource } from '../common/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; @@ -125,6 +125,7 @@ export interface ILanguageServerManager extends IDisposable { start(resource: Resource, interpreter: PythonEnvironment | undefined): Promise; connect(): void; disconnect(): void; + setNotebookMiddleware(notebookAddon: Middleware): void; } export const ILanguageServerProxy = Symbol('ILanguageServerProxy'); diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index 0d41e98505c0..4ab4120d60de 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -9,7 +9,8 @@ import { dirname } from 'path'; import { CancellationToken, Disposable, Event, Extension, Memento, Uri } from 'vscode'; import * as lsp from 'vscode-languageserver-protocol'; import type { SemVer } from 'semver'; -import { ILanguageServerCache, ILanguageServerConnection } from '../activation/types'; +import { Middleware } from 'vscode-languageclient'; +import { ILanguageServerCache, ILanguageServerConnection, ILanguageServerManager } from '../activation/types'; import { IWorkspaceService } from '../common/application/types'; import { JUPYTER_EXTENSION_ID } from '../common/constants'; import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types'; @@ -157,6 +158,8 @@ type PythonApiForJupyterExtension = { resource: Resource, interpreter?: PythonEnvironment, ): Promise; + + injectMiddlewareHook(middleware: Middleware): void; }; type JupyterExtensionApi = { @@ -194,6 +197,7 @@ export class JupyterExtensionIntegration { @inject(IComponentAdapter) private pyenvs: IComponentAdapter, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(ICondaService) private readonly condaService: ICondaService, + @inject(ILanguageServerManager) private readonly languageServerManager: ILanguageServerManager, ) {} public registerApi(jupyterExtensionApi: JupyterExtensionApi): JupyterExtensionApi | undefined { @@ -275,6 +279,8 @@ export class JupyterExtensionIntegration { getCondaVersion: () => this.condaService.getCondaVersion(), getEnvironmentActivationShellCommands: (resource: Resource, interpreter?: PythonEnvironment) => this.envActivation.getEnvironmentActivationShellCommands(resource, interpreter), + injectMiddlewareHook: (middleware: Middleware & Disposable) => + this.languageServerManager.setNotebookMiddleware(middleware), }); return undefined; } diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 3e14130c650e..0b03adca2d25 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -17,7 +17,7 @@ suite('Pylance Language Server - Analysis Options', () => { return super.getWorkspaceFolder(); } - public getDocumentFilters(workspaceFolder?: WorkspaceFolder): DocumentFilter[] { + public async getDocumentFilters(workspaceFolder?: WorkspaceFolder): Promise { return super.getDocumentFilters(workspaceFolder); } @@ -51,10 +51,10 @@ suite('Pylance Language Server - Analysis Options', () => { expect(filter).to.be.equal(PYTHON); }); - test('Document filter matches all python language schemes when in virtual workspace', () => { + test('Document filter matches all python language schemes when in virtual workspace', async () => { workspace.reset(); workspace.setup((w) => w.isVirtualWorkspace).returns(() => true); - const filter = analysisOptions.getDocumentFilters(); + const filter = await analysisOptions.getDocumentFilters(); assert.deepEqual(filter, [{ language: PYTHON_LANGUAGE }]); }); From c26e4cc1904d78775211cb0a87925e9062042aff Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 20 Apr 2022 23:37:34 -0700 Subject: [PATCH 02/41] Setting instead of experiment, fix LspNotebooksExperiment, inject func instead of middleware --- package.json | 9 ++++++++ src/client/activation/jedi/manager.ts | 5 +++- .../activation/languageClientMiddleware.ts | 5 ++-- .../languageClientMiddlewareBase.ts | 15 ++++++++++-- src/client/activation/node/analysisOptions.ts | 10 +++++--- .../activation/node/lspNotebooksExperiment.ts | 23 ++++++++++--------- src/client/activation/node/manager.ts | 9 ++++---- src/client/activation/serviceRegistry.ts | 3 +++ src/client/activation/types.ts | 5 ++-- src/client/common/configSettings.ts | 2 ++ src/client/common/types.ts | 1 + src/client/jupyter/jupyterIntegration.ts | 7 +++--- .../node/analysisOptions.unit.test.ts | 6 ++++- 13 files changed, 69 insertions(+), 31 deletions(-) diff --git a/package.json b/package.json index 150657f638d4..aeaf919e7db1 100644 --- a/package.json +++ b/package.json @@ -906,6 +906,15 @@ "scope": "machine-overridable", "type": "string" }, + "python.pylanceLspNotebooksEnabled": { + "type": "boolean", + "default": false, + "description": "Determines if pylance's experimental LSP notebooks support is used or not.", + "scope": "machine", + "tags": [ + "experimental" + ] + }, "python.sortImports.args": { "default": [], "description": "Arguments passed in. Each argument is a separate item in the array.", diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index e2e12f185cfe..a6096dece64e 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -5,6 +5,9 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import '../../common/extensions'; +import { inject, injectable, named } from 'inversify'; + +import { Uri } from 'vscode'; import { ICommandManager } from '../../common/application/types'; import { IDisposable, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; @@ -111,7 +114,7 @@ export class JediLanguageServerManager implements ILanguageServerManager { } // eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function - public setNotebookMiddleware(_notebookAddon: Middleware & Disposable): void {} + public registerJupyterPythonPathFunction(_func: (uri: Uri) => Promise): void {} @debounceSync(1000) protected restartLanguageServerDebounced(): void { diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts index bd0ccefdf141..06270b38a029 100644 --- a/src/client/activation/languageClientMiddleware.ts +++ b/src/client/activation/languageClientMiddleware.ts @@ -25,12 +25,13 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { ); const disposables = serviceContainer.get(IDisposableRegistry) || []; const extensions = serviceContainer.get(IExtensions); + const lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); // Enable notebook support if jupyter support is installed if ( jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled && - !LspNotebooksExperiment.isInNotebooksExperiment + !lspNotebooksExperiment.isInNotebooksExperiment() ) { this.notebookAddon = createHidingMiddleware(); } @@ -42,7 +43,7 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { } else if ( !this.notebookAddon && jupyterDependencyManager.isJupyterExtensionInstalled && - !LspNotebooksExperiment.isInNotebooksExperiment + !lspNotebooksExperiment.isInNotebooksExperiment() ) { this.notebookAddon = createHidingMiddleware(); } diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 8d09d9042273..a419ad3a5760 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -86,7 +86,12 @@ export class LanguageClientMiddlewareBase implements Middleware { const settingDict: LSPObject & { pythonPath: string; _envPYTHONPATH: string } = settings[ i ] as LSPObject & { pythonPath: string; _envPYTHONPATH: string }; - settingDict.pythonPath = configService.getSettings(uri).pythonPath; + + if (uri?.scheme === 'vscode-notebook-cell' && this.jupyterPythonPathFunction) { + settingDict.pythonPath = (await this.jupyterPythonPathFunction(uri)) ?? ''; // TODO: How to handle undefined? + } else { + settingDict.pythonPath = configService.getSettings(uri).pythonPath; + } const env = await envService.getEnvironmentVariables(uri); const envPYTHONPATH = env.PYTHONPATH; @@ -100,11 +105,17 @@ export class LanguageClientMiddlewareBase implements Middleware { }, }; + private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; + + public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise) { + this.jupyterPythonPathFunction = func; + } + private get connected(): Promise { return this.connectedPromise.promise; } - public notebookAddon: (Middleware & Disposable) | undefined; + protected notebookAddon: (Middleware & Disposable) | undefined; private connectedPromise = createDeferred(); diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 532a0cb9c0f5..11de6f88233a 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -12,7 +12,11 @@ import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor - constructor(lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService) { + constructor(lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, + @inject(ILanguageServerOutputChannel) lsOutputChannel: ILanguageServerOutputChannel, + @inject(IWorkspaceService) workspace: IWorkspaceService, + @inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment, + ) { super(lsOutputChannel, workspace); } @@ -21,14 +25,14 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - lspNotebooksSupport: await LspNotebooksExperiment.isInNotebooksExperiment(), + lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), }; } protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { let filters = await super.getDocumentFilters(_workspaceFolder); - if (await LspNotebooksExperiment.isInNotebooksExperiment()) { + if (this.lspNotebooksExperiment.isInNotebooksExperiment()) { return [ ...filters, { diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 0488daba1e34..b7b9b15f4433 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -1,21 +1,22 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { IExperimentService } from '../../common/types'; +import { IConfigurationService } from '../../common/types'; +import { IExtensionSingleActivationService } from '../types'; @injectable() -export class LspNotebooksExperiment { - private static _isInNotebooksExperiment?: boolean; +export class LspNotebooksExperiment implements IExtensionSingleActivationService { + public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; - @inject(IExperimentService) private static experiments: IExperimentService; + private _isInNotebooksExperiment?: boolean; - public static async isInNotebooksExperiment(): Promise { - if (LspNotebooksExperiment._isInNotebooksExperiment === undefined) { - LspNotebooksExperiment._isInNotebooksExperiment = await LspNotebooksExperiment.experiments.inExperiment( - 'pylanceLspNotebooksEnabled', - ); - } + constructor(@inject(IConfigurationService) private readonly configurationService: IConfigurationService) {} - return LspNotebooksExperiment._isInNotebooksExperiment; + public async activate(): Promise { + this._isInNotebooksExperiment = this.configurationService.getSettings().pylanceLspNotebooksEnabled; + } + + public isInNotebooksExperiment(): boolean { + return !this._isInNotebooksExperiment; } } diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index e754ac372f48..1893bdefc43d 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -7,12 +7,13 @@ import { inject, injectable, named } from 'inversify'; import { Disposable } from 'vscode'; import { ICommandManager } from '../../common/application/types'; import { IDisposable, IExtensions, Resource } from '../../common/types'; +import { Uri } from 'vscode'; +import { IDisposable, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; import { IServiceContainer } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { Commands } from '../commands'; import { LanguageClientMiddleware } from '../languageClientMiddleware'; import { ILanguageServerAnalysisOptions, @@ -104,11 +105,9 @@ export class NodeLanguageServerManager implements ILanguageServerManager { } } - // QUESTIONS: - // Better way to install the injected middleware rather than leveraging existing LanguageClientMiddleware.notebookAddon? - public setNotebookMiddleware(notebookAddon: Middleware & Disposable): void { + public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { if (this.middleware) { - this.middleware.notebookAddon = notebookAddon; + this.middleware.registerJupyterPythonPathFunction(func); } } diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index 53042df2c4de..d7512b533cad 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -16,6 +16,7 @@ import { LoadLanguageServerExtension } from './common/loadLanguageServerExtensio import { PartialModeStatusItem } from './partialModeStatus'; import { ILanguageServerWatcher } from '../languageServer/types'; import { LanguageServerWatcher } from '../languageServer/watcher'; +import { LspNotebooksExperiment } from './node/lspNotebooksExperiment'; export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(IExtensionActivationService, PartialModeStatusItem); @@ -36,4 +37,6 @@ export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(ILanguageServerWatcher, LanguageServerWatcher); serviceManager.addBinding(ILanguageServerWatcher, IExtensionActivationService); serviceManager.addBinding(ILanguageServerWatcher, ILanguageServerCache); + serviceManager.addSingleton(LspNotebooksExperiment, LspNotebooksExperiment); + serviceManager.addBinding(LspNotebooksExperiment, IExtensionSingleActivationService); } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 2ad8b295fb42..732a56473a36 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -13,8 +13,9 @@ import { ReferenceProvider, RenameProvider, SignatureHelpProvider, + Uri, } from 'vscode'; -import { LanguageClient, LanguageClientOptions, Middleware } from 'vscode-languageclient/node'; +import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node'; import * as lsp from 'vscode-languageserver-protocol'; import type { IDisposable, IOutputChannel, Resource } from '../common/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; @@ -125,7 +126,7 @@ export interface ILanguageServerManager extends IDisposable { start(resource: Resource, interpreter: PythonEnvironment | undefined): Promise; connect(): void; disconnect(): void; - setNotebookMiddleware(notebookAddon: Middleware): void; + registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void; } export const ILanguageServerProxy = Symbol('ILanguageServerProxy'); diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 2c095797b1c0..71182b7c518e 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -96,6 +96,8 @@ export class PythonSettings implements IPythonSettings { public poetryPath = ''; + public pylanceLspNotebooksEnabled = false; + public devOptions: string[] = []; public linting!: ILintingSettings; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 84ce8e753ca5..e30794e55ddd 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -180,6 +180,7 @@ export interface IPythonSettings { readonly condaPath: string; readonly pipenvPath: string; readonly poetryPath: string; + readonly pylanceLspNotebooksEnabled: boolean; readonly devOptions: string[]; readonly linting: ILintingSettings; readonly formatting: IFormattingSettings; diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index 4ab4120d60de..6448654d9743 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -9,7 +9,6 @@ import { dirname } from 'path'; import { CancellationToken, Disposable, Event, Extension, Memento, Uri } from 'vscode'; import * as lsp from 'vscode-languageserver-protocol'; import type { SemVer } from 'semver'; -import { Middleware } from 'vscode-languageclient'; import { ILanguageServerCache, ILanguageServerConnection, ILanguageServerManager } from '../activation/types'; import { IWorkspaceService } from '../common/application/types'; import { JUPYTER_EXTENSION_ID } from '../common/constants'; @@ -159,7 +158,7 @@ type PythonApiForJupyterExtension = { interpreter?: PythonEnvironment, ): Promise; - injectMiddlewareHook(middleware: Middleware): void; + registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void; }; type JupyterExtensionApi = { @@ -279,8 +278,8 @@ export class JupyterExtensionIntegration { getCondaVersion: () => this.condaService.getCondaVersion(), getEnvironmentActivationShellCommands: (resource: Resource, interpreter?: PythonEnvironment) => this.envActivation.getEnvironmentActivationShellCommands(resource, interpreter), - injectMiddlewareHook: (middleware: Middleware & Disposable) => - this.languageServerManager.setNotebookMiddleware(middleware), + registerJupyterPythonPathFunction: (func: (uri: Uri) => Promise) => + this.languageServerManager.registerJupyterPythonPathFunction(func), }); return undefined; } diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 0b03adca2d25..4a3ed1d9eb4f 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -6,6 +6,7 @@ import { 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'; @@ -31,6 +32,7 @@ suite('Pylance Language Server - Analysis Options', () => { let outputChannel: IOutputChannel; let lsOutputChannel: typemoq.IMock; let workspace: typemoq.IMock; + let lspNotebooksExperiment: typemoq.IMock; setup(() => { outputChannel = typemoq.Mock.ofType().object; @@ -38,7 +40,9 @@ suite('Pylance Language Server - Analysis Options', () => { workspace.setup((w) => w.isVirtualWorkspace).returns(() => false); lsOutputChannel = typemoq.Mock.ofType(); lsOutputChannel.setup((l) => l.channel).returns(() => outputChannel); - analysisOptions = new TestClass(lsOutputChannel.object, workspace.object); + lspNotebooksExperiment = typemoq.Mock.ofType(); + lspNotebooksExperiment.setup((l) => l.isInNotebooksExperiment()).returns(() => false); + analysisOptions = new TestClass(lsOutputChannel.object, workspace.object, lspNotebooksExperiment.object); }); test('Workspace folder is undefined', () => { From 0478e8b89438a53b8365902391ebf8317650f1fc Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Thu, 21 Apr 2022 16:37:25 -0700 Subject: [PATCH 03/41] API is now being called but func is not saved correctly --- src/client/activation/languageClientMiddleware.ts | 10 +++++++--- src/client/activation/node/analysisOptions.ts | 4 ++-- src/client/activation/node/lspNotebooksExperiment.ts | 4 ++-- src/client/activation/node/manager.ts | 11 +++++++++-- src/client/common/configSettings.ts | 5 +++-- src/client/common/types.ts | 3 ++- 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts index 06270b38a029..9c95c8bd4dc4 100644 --- a/src/client/activation/languageClientMiddleware.ts +++ b/src/client/activation/languageClientMiddleware.ts @@ -31,19 +31,23 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { if ( jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled && - !lspNotebooksExperiment.isInNotebooksExperiment() + lspNotebooksExperiment.isInNotebooksExperiment() != true ) { this.notebookAddon = createHidingMiddleware(); } disposables.push( extensions?.onDidChange(() => { if (jupyterDependencyManager) { - if (this.notebookAddon && !jupyterDependencyManager.isJupyterExtensionInstalled) { + if ( + this.notebookAddon && + (!jupyterDependencyManager.isJupyterExtensionInstalled || + lspNotebooksExperiment.isInNotebooksExperiment() == true) + ) { this.notebookAddon = undefined; } else if ( !this.notebookAddon && jupyterDependencyManager.isJupyterExtensionInstalled && - !lspNotebooksExperiment.isInNotebooksExperiment() + lspNotebooksExperiment.isInNotebooksExperiment() != true ) { this.notebookAddon = createHidingMiddleware(); } diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 11de6f88233a..c76f3827922d 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -25,14 +25,14 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), + lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment() == true, }; } protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { let filters = await super.getDocumentFilters(_workspaceFolder); - if (this.lspNotebooksExperiment.isInNotebooksExperiment()) { + if (this.lspNotebooksExperiment.isInNotebooksExperiment() == true) { return [ ...filters, { diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index b7b9b15f4433..73334fea69c0 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -16,7 +16,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService this._isInNotebooksExperiment = this.configurationService.getSettings().pylanceLspNotebooksEnabled; } - public isInNotebooksExperiment(): boolean { - return !this._isInNotebooksExperiment; + public isInNotebooksExperiment(): boolean | undefined { + return this._isInNotebooksExperiment; } } diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index 1893bdefc43d..7681027d75d1 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -37,6 +37,7 @@ export class NodeLanguageServerManager implements ILanguageServerManager { private connected = false; private lsVersion: string | undefined; + private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined; private started = false; @@ -106,8 +107,13 @@ export class NodeLanguageServerManager implements ILanguageServerManager { } public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { - if (this.middleware) { - this.middleware.registerJupyterPythonPathFunction(func); + this.jupyterPythonPathFunction = func; + this.applyJupyterPythonPathFunction(); + } + + private applyJupyterPythonPathFunction() { + if (this.jupyterPythonPathFunction && this.middleware) { + this.middleware.registerJupyterPythonPathFunction(this.jupyterPythonPathFunction); } } @@ -135,6 +141,7 @@ export class NodeLanguageServerManager implements ILanguageServerManager { const options = await this.analysisOptions.getAnalysisOptions(); this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Node, this.lsVersion); options.middleware = this.middleware; + this.applyJupyterPythonPathFunction(); // Make sure the middleware is connected if we restart and we we're already connected. if (this.connected) { diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 71182b7c518e..eeeed9e6550b 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -96,8 +96,6 @@ export class PythonSettings implements IPythonSettings { public poetryPath = ''; - public pylanceLspNotebooksEnabled = false; - public devOptions: string[] = []; public linting!: ILintingSettings; @@ -118,6 +116,8 @@ export class PythonSettings implements IPythonSettings { public globalModuleInstallation = false; + public pylanceLspNotebooksEnabled = false; + public experiments!: IExperiments; public languageServer: LanguageServerType = LanguageServerType.Node; @@ -296,6 +296,7 @@ export class PythonSettings implements IPythonSettings { this.disableInstallationChecks = pythonSettings.get('disableInstallationCheck') === true; this.globalModuleInstallation = pythonSettings.get('globalModuleInstallation') === true; + this.pylanceLspNotebooksEnabled = pythonSettings.get('pylanceLspNotebooksEnabled') === true; const sortImportSettings = systemVariables.resolveAny(pythonSettings.get('sortImports'))!; if (this.sortImports) { diff --git a/src/client/common/types.ts b/src/client/common/types.ts index e30794e55ddd..fa4d6b16bc61 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -180,7 +180,6 @@ export interface IPythonSettings { readonly condaPath: string; readonly pipenvPath: string; readonly poetryPath: string; - readonly pylanceLspNotebooksEnabled: boolean; readonly devOptions: string[]; readonly linting: ILintingSettings; readonly formatting: IFormattingSettings; @@ -191,6 +190,8 @@ export interface IPythonSettings { readonly envFile: string; readonly disableInstallationChecks: boolean; readonly globalModuleInstallation: boolean; + readonly autoUpdateLanguageServer: boolean; + readonly pylanceLspNotebooksEnabled: boolean; readonly onDidChange: Event; readonly experiments: IExperiments; readonly languageServer: LanguageServerType; From dfa043cb7fa384861f2b4101b2c0ae0ea4b4728a Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 22 Apr 2022 10:41:12 -0700 Subject: [PATCH 04/41] Save func on API object; not getting notebookDocument/did* --- package.json | 4 +++- src/client/activation/jedi/manager.ts | 4 ---- src/client/activation/node/analysisOptions.ts | 3 ++- src/client/activation/node/manager.ts | 20 +++++++------------ src/client/activation/types.ts | 2 -- src/client/browser/extension.ts | 1 + src/client/common/constants.ts | 14 ++++++------- src/client/jupyter/jupyterIntegration.ts | 15 +++++++++++--- 8 files changed, 32 insertions(+), 31 deletions(-) diff --git a/package.json b/package.json index aeaf919e7db1..95b2ea867043 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,9 @@ "enabledApiProposals": [ "quickPickSortByLabel", "testObserver", - "notebookEditor" + "notebookEditor", + "tabs", + "notebookCellExecutionState" ], "author": { "name": "Microsoft Corporation" diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index a6096dece64e..252dad873286 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -7,7 +7,6 @@ import '../../common/extensions'; import { inject, injectable, named } from 'inversify'; -import { Uri } from 'vscode'; import { ICommandManager } from '../../common/application/types'; import { IDisposable, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; @@ -113,9 +112,6 @@ export class JediLanguageServerManager implements ILanguageServerManager { } } - // eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function - public registerJupyterPythonPathFunction(_func: (uri: Uri) => Promise): void {} - @debounceSync(1000) protected restartLanguageServerDebounced(): void { this.restartLanguageServer().ignoreErrors(); diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index c76f3827922d..d9a7d5c2db11 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -34,10 +34,11 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt if (this.lspNotebooksExperiment.isInNotebooksExperiment() == true) { return [ - ...filters, + { language: 'python' }, { notebookDocument: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, cellLanguage: 'python', + sync: true, // HACK to activate notebook matching in client.js match() }, ]; } diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index 7681027d75d1..d2ce3873a0f6 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -24,6 +24,7 @@ import { import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { Middleware } from 'vscode-languageclient'; +import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; export class NodeLanguageServerManager implements ILanguageServerManager { private resource!: Resource; @@ -37,7 +38,6 @@ export class NodeLanguageServerManager implements ILanguageServerManager { private connected = false; private lsVersion: string | undefined; - private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined; private started = false; @@ -49,6 +49,7 @@ export class NodeLanguageServerManager implements ILanguageServerManager { private readonly languageServerProxy: ILanguageServerProxy, commandManager: ICommandManager, private readonly extensions: IExtensions, + @inject(JupyterExtensionIntegration) private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, ) { if (NodeLanguageServerManager.commandDispose) { NodeLanguageServerManager.commandDispose.dispose(); @@ -106,17 +107,6 @@ export class NodeLanguageServerManager implements ILanguageServerManager { } } - public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { - this.jupyterPythonPathFunction = func; - this.applyJupyterPythonPathFunction(); - } - - private applyJupyterPythonPathFunction() { - if (this.jupyterPythonPathFunction && this.middleware) { - this.middleware.registerJupyterPythonPathFunction(this.jupyterPythonPathFunction); - } - } - @debounceSync(1000) protected restartLanguageServerDebounced(): void { this.restartLanguageServer().ignoreErrors(); @@ -141,7 +131,11 @@ export class NodeLanguageServerManager implements ILanguageServerManager { const options = await this.analysisOptions.getAnalysisOptions(); this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Node, this.lsVersion); options.middleware = this.middleware; - this.applyJupyterPythonPathFunction(); + + const jupyterPythonPathFunction = this.jupyterExtensionIntegration.getJupyterPythonPathFunction(); + if (jupyterPythonPathFunction) { + this.middleware.registerJupyterPythonPathFunction(jupyterPythonPathFunction); + } // Make sure the middleware is connected if we restart and we we're already connected. if (this.connected) { diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 732a56473a36..3ca10b29255d 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -13,7 +13,6 @@ import { ReferenceProvider, RenameProvider, SignatureHelpProvider, - Uri, } from 'vscode'; import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node'; import * as lsp from 'vscode-languageserver-protocol'; @@ -126,7 +125,6 @@ export interface ILanguageServerManager extends IDisposable { start(resource: Resource, interpreter: PythonEnvironment | undefined): Promise; connect(): void; disconnect(): void; - registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void; } export const ILanguageServerProxy = Symbol('ILanguageServerProxy'); diff --git a/src/client/browser/extension.ts b/src/client/browser/extension.ts index 2effb94d4e03..576e25f6bbac 100644 --- a/src/client/browser/extension.ts +++ b/src/client/browser/extension.ts @@ -83,6 +83,7 @@ async function runPylance( const client = new LanguageClient('python', 'Python Language Server', clientOptions, worker); languageClient = client; + languageClient.registerProposedFeatures(); context.subscriptions.push( vscode.commands.registerCommand('python.viewLanguageServerOutput', () => client.outputChannel.show()), diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 8659c14dfd8f..14f34b56e83c 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -9,16 +9,16 @@ export const InteractiveScheme = 'vscode-interactive'; export const PYTHON = [ { scheme: 'file', language: PYTHON_LANGUAGE }, { scheme: 'untitled', language: PYTHON_LANGUAGE }, - { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, - { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, + // { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, + // { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, ]; -export const PYTHON_NOTEBOOKS = [ - { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, - { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, - { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, -]; +// export const PYTHON_NOTEBOOKS = [ +// { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, +// { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, +// { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, +// ]; export const PVSC_EXTENSION_ID = 'ms-python.python'; export const PYLANCE_EXTENSION_ID = 'ms-python.vscode-pylance'; diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index 6448654d9743..63e3194891b8 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -9,7 +9,7 @@ import { dirname } from 'path'; import { CancellationToken, Disposable, Event, Extension, Memento, Uri } from 'vscode'; import * as lsp from 'vscode-languageserver-protocol'; import type { SemVer } from 'semver'; -import { ILanguageServerCache, ILanguageServerConnection, ILanguageServerManager } from '../activation/types'; +import { ILanguageServerCache, ILanguageServerConnection } from '../activation/types'; import { IWorkspaceService } from '../common/application/types'; import { JUPYTER_EXTENSION_ID } from '../common/constants'; import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types'; @@ -184,6 +184,8 @@ type JupyterExtensionApi = { export class JupyterExtensionIntegration { private jupyterExtension: Extension | undefined; + private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined; + constructor( @inject(IExtensions) private readonly extensions: IExtensions, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @@ -196,7 +198,6 @@ export class JupyterExtensionIntegration { @inject(IComponentAdapter) private pyenvs: IComponentAdapter, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(ICondaService) private readonly condaService: ICondaService, - @inject(ILanguageServerManager) private readonly languageServerManager: ILanguageServerManager, ) {} public registerApi(jupyterExtensionApi: JupyterExtensionApi): JupyterExtensionApi | undefined { @@ -279,7 +280,7 @@ export class JupyterExtensionIntegration { getEnvironmentActivationShellCommands: (resource: Resource, interpreter?: PythonEnvironment) => this.envActivation.getEnvironmentActivationShellCommands(resource, interpreter), registerJupyterPythonPathFunction: (func: (uri: Uri) => Promise) => - this.languageServerManager.registerJupyterPythonPathFunction(func), + this.registerJupyterPythonPathFunction(func), }); return undefined; } @@ -325,4 +326,12 @@ export class JupyterExtensionIntegration { } return undefined; } + + private registerJupyterPythonPathFunction(func: (uri: Uri) => Promise) { + this.jupyterPythonPathFunction = func; + } + + public getJupyterPythonPathFunction(): ((uri: Uri) => Promise) | undefined { + return this.jupyterPythonPathFunction; + } } From ac28d1121f4208a4be02eb8f81fdfee1e793ebac Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 22 Apr 2022 22:29:01 -0700 Subject: [PATCH 05/41] Upgrade to latest LSP --- package.json | 6 +----- src/client/activation/node/analysisOptions.ts | 6 +++--- src/client/browser/extension.ts | 5 +++++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 95b2ea867043..5e471343fd15 100644 --- a/package.json +++ b/package.json @@ -19,11 +19,7 @@ "languageServerVersion": "0.5.30", "publisher": "ms-python", "enabledApiProposals": [ - "quickPickSortByLabel", - "testObserver", - "notebookEditor", - "tabs", - "notebookCellExecutionState" + "notebookDocumentEvents" ], "author": { "name": "Microsoft Corporation" diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index d9a7d5c2db11..6a2d0eb4a7f8 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -36,9 +36,9 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return [ { language: 'python' }, { - notebookDocument: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, - cellLanguage: 'python', - sync: true, // HACK to activate notebook matching in client.js match() + notebook: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, + language: 'python', + // sync: true, // HACK to activate notebook matching in client.js match() }, ]; } diff --git a/src/client/browser/extension.ts b/src/client/browser/extension.ts index 576e25f6bbac..7fccbebaa085 100644 --- a/src/client/browser/extension.ts +++ b/src/client/browser/extension.ts @@ -113,6 +113,11 @@ async function runPylance( await client.start(); +<<<<<<< HEAD +======= + await languageClient.start(); + +>>>>>>> f8f322539 (Upgrade to latest LSP) context.subscriptions.push(createStatusItem()); } catch (e) { console.log(e); From 20f1416f2d6d17df7094a8297a28b2fcbff54aa4 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 22 Apr 2022 23:24:24 -0700 Subject: [PATCH 06/41] Remove sync property hack --- src/client/activation/node/analysisOptions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 6a2d0eb4a7f8..27fdb4eb4ea7 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -38,7 +38,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt { notebook: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, language: 'python', - // sync: true, // HACK to activate notebook matching in client.js match() }, ]; } From 980372b5b7be5a32a321780a75b53ea68edf3ba3 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 26 Apr 2022 11:51:57 -0700 Subject: [PATCH 07/41] Fix usage of LanguageClient.start/stop --- src/client/activation/jedi/languageServerProxy.ts | 8 ++++++++ src/client/browser/extension.ts | 5 ----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index f8b5aaf9d920..2a7794a8369f 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -56,6 +56,11 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { interpreter: PythonEnvironment | undefined, options: LanguageClientOptions, ): Promise { + if (this.languageServerTask) { + await this.languageServerTask; + return; + } + this.lsVersion = (options.middleware ? (options.middleware).serverVersion : undefined) ?? '0.19.3'; @@ -91,6 +96,9 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { const d = this.disposables.shift()!; d.dispose(); } + + this.languageServerTask = this.languageClient.start(); + await this.languageServerTask; } // eslint-disable-next-line class-methods-use-this diff --git a/src/client/browser/extension.ts b/src/client/browser/extension.ts index 7fccbebaa085..576e25f6bbac 100644 --- a/src/client/browser/extension.ts +++ b/src/client/browser/extension.ts @@ -113,11 +113,6 @@ async function runPylance( await client.start(); -<<<<<<< HEAD -======= - await languageClient.start(); - ->>>>>>> f8f322539 (Upgrade to latest LSP) context.subscriptions.push(createStatusItem()); } catch (e) { console.log(e); From 7dce31f47b9eeff16b679331743ae74c35df739a Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 26 Apr 2022 11:52:28 -0700 Subject: [PATCH 08/41] Remove unnecessary doc filters change --- src/client/common/constants.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 14f34b56e83c..8659c14dfd8f 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -9,16 +9,16 @@ export const InteractiveScheme = 'vscode-interactive'; export const PYTHON = [ { scheme: 'file', language: PYTHON_LANGUAGE }, { scheme: 'untitled', language: PYTHON_LANGUAGE }, - // { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, - // { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, + { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, + { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, ]; -// export const PYTHON_NOTEBOOKS = [ -// { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, -// { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, -// { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, -// ]; +export const PYTHON_NOTEBOOKS = [ + { scheme: 'vscode-notebook', language: PYTHON_LANGUAGE }, + { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE }, + { scheme: InteractiveInputScheme, language: PYTHON_LANGUAGE }, +]; export const PVSC_EXTENSION_ID = 'ms-python.python'; export const PYLANCE_EXTENSION_ID = 'ms-python.vscode-pylance'; From 42bb8b3f14fa88870ab21368288285b47de9b31e Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 26 Apr 2022 13:47:34 -0700 Subject: [PATCH 09/41] Determine if experiment is supported via extension versions; telemetry for supported case --- .../activation/node/lspNotebooksExperiment.ts | 26 ++++++++++++++++++- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 5 ++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 73334fea69c0..cf99c1804a77 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -1,8 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { inject, injectable } from 'inversify'; +import { extensions } from 'vscode'; +import * as semver from 'semver'; import { IConfigurationService } from '../../common/types'; import { IExtensionSingleActivationService } from '../types'; +import { sendTelemetryEvent } from '../../telemetry'; +import { EventName } from '../../telemetry/constants'; +import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constants'; @injectable() export class LspNotebooksExperiment implements IExtensionSingleActivationService { @@ -10,13 +15,32 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private _isInNotebooksExperiment?: boolean; + private _jupyterVersion: string | undefined; + + private _pylanceVersion: string | undefined; + constructor(@inject(IConfigurationService) private readonly configurationService: IConfigurationService) {} public async activate(): Promise { this._isInNotebooksExperiment = this.configurationService.getSettings().pylanceLspNotebooksEnabled; + this._jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; + this._pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; + + if (this._supportsNotebooksExperiment()) { + sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); + } } public isInNotebooksExperiment(): boolean | undefined { - return this._isInNotebooksExperiment; + return this._isInNotebooksExperiment && this._supportsNotebooksExperiment(); + } + + private _supportsNotebooksExperiment(): boolean { + return ( + this._jupyterVersion !== undefined && + semver.satisfies(this._jupyterVersion, '>=2022.4.100') && + this._pylanceVersion !== undefined && + semver.satisfies(this._pylanceVersion, '>=2022.4.4-pre.1 || 9999.0.0-dev') + ); } } diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 81305350b958..67117fe0c167 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -61,6 +61,7 @@ export enum EventName { UNITTEST_DISABLED = 'UNITTEST.DISABLED', PYTHON_EXPERIMENTS_INIT_PERFORMANCE = 'PYTHON_EXPERIMENTS_INIT_PERFORMANCE', + PYTHON_EXPERIMENTS_LSP_NOTEBOOKS = 'PYTHON_EXPERIMENTS_LSP_NOTEBOOKS', PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS = 'PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS', EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c2daec3dab5c..c16b3f19bf32 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1409,6 +1409,11 @@ export interface IEventNamePropertyMapping { "create_new_file_command" : { "owner": "luabud" } */ [EventName.CREATE_NEW_FILE_COMMAND]: unknown; + /** + * Telemetry event sent when the installed versions of Python, Jupyter, and Pylance are all capable + * of supporting the LSP notebooks experiment. This does not indicate that the experiment is enabled. + */ + [EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS]: unknown; /** * Telemetry event sent once on session start with details on which experiments are opted into and opted out from. */ From 02fbe944a39a75aece64b132266cd648e1beabe8 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 26 Apr 2022 14:38:37 -0700 Subject: [PATCH 10/41] Debug logging in LspNotebooksExperiment --- .../activation/node/lspNotebooksExperiment.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index cf99c1804a77..25b2c2b1310b 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -1,13 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { extensions } from 'vscode'; import * as semver from 'semver'; -import { IConfigurationService } from '../../common/types'; +import { IConfigurationService, IOutputChannel } from '../../common/types'; import { IExtensionSingleActivationService } from '../types'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constants'; +import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; @injectable() export class LspNotebooksExperiment implements IExtensionSingleActivationService { @@ -19,7 +19,10 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private _pylanceVersion: string | undefined; - constructor(@inject(IConfigurationService) private readonly configurationService: IConfigurationService) {} + constructor( + @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel, + ) {} public async activate(): Promise { this._isInNotebooksExperiment = this.configurationService.getSettings().pylanceLspNotebooksEnabled; @@ -29,6 +32,10 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService if (this._supportsNotebooksExperiment()) { sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); } + + this.output.appendLine( + `LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInNotebooksExperiment()}`, + ); } public isInNotebooksExperiment(): boolean | undefined { From 6c17863ae3f8d59a947339adc8e50993407c5a3b Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 27 Apr 2022 10:44:04 -0700 Subject: [PATCH 11/41] Special handling for config requests on ipynb files --- src/client/activation/languageClientMiddlewareBase.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index a419ad3a5760..d48380f01d9b 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -87,8 +87,11 @@ export class LanguageClientMiddlewareBase implements Middleware { i ] as LSPObject & { pythonPath: string; _envPYTHONPATH: string }; + // TODO: Remove cell branch? if (uri?.scheme === 'vscode-notebook-cell' && this.jupyterPythonPathFunction) { settingDict.pythonPath = (await this.jupyterPythonPathFunction(uri)) ?? ''; // TODO: How to handle undefined? + } else if (uri?.fsPath?.endsWith('.ipynb') && this.jupyterPythonPathFunction) { + settingDict.pythonPath = (await this.jupyterPythonPathFunction(uri)) ?? ''; // TODO: How to handle undefined? } else { settingDict.pythonPath = configService.getSettings(uri).pythonPath; } From 4062eb4bfd74f92c9ccb23df8ae7db9c7b852659 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 27 Apr 2022 18:34:15 -0700 Subject: [PATCH 12/41] Post-rebase fixes --- src/client/activation/jedi/manager.ts | 2 -- src/client/activation/node/analysisOptions.ts | 8 +++----- src/client/activation/node/manager.ts | 6 +----- src/client/common/types.ts | 1 - 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index 252dad873286..29f1106c5f8c 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -5,8 +5,6 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import '../../common/extensions'; -import { inject, injectable, named } from 'inversify'; - import { ICommandManager } from '../../common/application/types'; import { IDisposable, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 27fdb4eb4ea7..598326231e64 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { LanguageClientOptions } from 'vscode-languageclient'; import { WorkspaceFolder } from 'vscode'; import { DocumentFilter } from 'vscode-languageserver-protocol'; import { IWorkspaceService } from '../../common/application/types'; @@ -13,9 +13,7 @@ import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor constructor(lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, - @inject(ILanguageServerOutputChannel) lsOutputChannel: ILanguageServerOutputChannel, - @inject(IWorkspaceService) workspace: IWorkspaceService, - @inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment, + private readonly lspNotebooksExperiment: LspNotebooksExperiment, ) { super(lsOutputChannel, workspace); } @@ -26,7 +24,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt experimentationSupport: true, trustedWorkspaceSupport: true, lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment() == true, - }; + } as unknown) as LanguageClientOptions; } protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index d2ce3873a0f6..c8daeb821928 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -4,11 +4,8 @@ import '../../common/extensions'; import { inject, injectable, named } from 'inversify'; -import { Disposable } from 'vscode'; import { ICommandManager } from '../../common/application/types'; import { IDisposable, IExtensions, Resource } from '../../common/types'; -import { Uri } from 'vscode'; -import { IDisposable, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; import { IServiceContainer } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -23,7 +20,6 @@ import { } from '../types'; import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; -import { Middleware } from 'vscode-languageclient'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; export class NodeLanguageServerManager implements ILanguageServerManager { @@ -49,7 +45,7 @@ export class NodeLanguageServerManager implements ILanguageServerManager { private readonly languageServerProxy: ILanguageServerProxy, commandManager: ICommandManager, private readonly extensions: IExtensions, - @inject(JupyterExtensionIntegration) private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, + private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, ) { if (NodeLanguageServerManager.commandDispose) { NodeLanguageServerManager.commandDispose.dispose(); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index fa4d6b16bc61..d83420548955 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -190,7 +190,6 @@ export interface IPythonSettings { readonly envFile: string; readonly disableInstallationChecks: boolean; readonly globalModuleInstallation: boolean; - readonly autoUpdateLanguageServer: boolean; readonly pylanceLspNotebooksEnabled: boolean; readonly onDidChange: Event; readonly experiments: IExperiments; From 3f3275527b43cf0eb3e5f3ee2535d9d0b4bf9d6d Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 27 Apr 2022 19:08:17 -0700 Subject: [PATCH 13/41] Adjust to server startup changes --- src/client/activation/node/manager.ts | 3 +-- .../pylanceLSExtensionManager.ts | 11 +++++++- src/client/languageServer/watcher.ts | 6 +++++ .../pylanceLSExtensionManager.unit.test.ts | 8 ++++++ src/test/languageServer/watcher.unit.test.ts | 26 +++++++++++++++++++ 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index c8daeb821928..d0f7ae84c138 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -2,8 +2,6 @@ // Licensed under the MIT License. import '../../common/extensions'; -import { inject, injectable, named } from 'inversify'; - import { ICommandManager } from '../../common/application/types'; import { IDisposable, IExtensions, Resource } from '../../common/types'; import { debounceSync } from '../../common/utils/decorators'; @@ -11,6 +9,7 @@ import { IServiceContainer } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; +import { Commands } from '../commands'; import { LanguageClientMiddleware } from '../languageClientMiddleware'; import { ILanguageServerAnalysisOptions, diff --git a/src/client/languageServer/pylanceLSExtensionManager.ts b/src/client/languageServer/pylanceLSExtensionManager.ts index 6c1791a72414..5cf0e4d1c931 100644 --- a/src/client/languageServer/pylanceLSExtensionManager.ts +++ b/src/client/languageServer/pylanceLSExtensionManager.ts @@ -5,6 +5,7 @@ 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'; @@ -22,6 +23,7 @@ import { Pylance } from '../common/utils/localize'; import { IEnvironmentVariablesProvider } from '../common/variables/types'; import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; +import { JupyterExtensionIntegration } from '../jupyter/jupyterIntegration'; import { traceLog } from '../logging'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { LanguageServerCapabilities } from './languageServerCapabilities'; @@ -50,10 +52,16 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities fileSystem: IFileSystem, private readonly extensions: IExtensions, readonly applicationShell: IApplicationShell, + lspNotebooksExperiment: LspNotebooksExperiment, + jupyterExtensionIntegration: JupyterExtensionIntegration, ) { super(); - this.analysisOptions = new NodeLanguageServerAnalysisOptions(outputChannel, workspaceService); + this.analysisOptions = new NodeLanguageServerAnalysisOptions( + outputChannel, + workspaceService, + lspNotebooksExperiment, + ); this.clientFactory = new NodeLanguageClientFactory(fileSystem, extensions); this.serverProxy = new NodeLanguageServerProxy( this.clientFactory, @@ -69,6 +77,7 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities this.serverProxy, commandManager, extensions, + jupyterExtensionIntegration, ); } diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index b3f01a97a672..6165d9137d73 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -34,6 +34,8 @@ import { JediLSExtensionManager } from './jediLSExtensionManager'; import { NoneLSExtensionManager } from './noneLSExtensionManager'; import { PylanceLSExtensionManager } from './pylanceLSExtensionManager'; import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types'; +import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment'; +import { JupyterExtensionIntegration } from '../jupyter/jupyterIntegration'; const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -74,6 +76,8 @@ export class LanguageServerWatcher @inject(IFileSystem) private readonly fileSystem: IFileSystem, @inject(IExtensions) private readonly extensions: IExtensions, @inject(IApplicationShell) readonly applicationShell: IApplicationShell, + @inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment, + @inject(JupyterExtensionIntegration) private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, @inject(IDisposableRegistry) readonly disposables: IDisposableRegistry, ) { this.workspaceInterpreters = new Map(); @@ -239,6 +243,8 @@ export class LanguageServerWatcher this.fileSystem, this.extensions, this.applicationShell, + this.lspNotebooksExperiment, + this.jupyterExtensionIntegration, ); break; case LanguageServerType.None: diff --git a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts index 418aa7812795..0f64dca09439 100644 --- a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts +++ b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts @@ -2,6 +2,7 @@ // 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'; @@ -14,6 +15,7 @@ import { import { IEnvironmentVariablesProvider } from '../../client/common/variables/types'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { IServiceContainer } from '../../client/ioc/types'; +import { JupyterExtensionIntegration } from '../../client/jupyter/jupyterIntegration'; import { PylanceLSExtensionManager } from '../../client/languageServer/pylanceLSExtensionManager'; suite('Language Server - Pylance LS extension manager', () => { @@ -37,6 +39,8 @@ suite('Language Server - Pylance LS extension manager', () => { {} as IFileSystem, {} as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, ); }); @@ -66,6 +70,8 @@ suite('Language Server - Pylance LS extension manager', () => { getExtension: () => ({}), } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, ); const result = manager.canStartLanguageServer(); @@ -93,6 +99,8 @@ suite('Language Server - Pylance LS extension manager', () => { getExtension: () => undefined, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, ); const result = manager.canStartLanguageServer(); diff --git a/src/test/languageServer/watcher.unit.test.ts b/src/test/languageServer/watcher.unit.test.ts index 6eac31dd2d83..33eaa7d2570b 100644 --- a/src/test/languageServer/watcher.unit.test.ts +++ b/src/test/languageServer/watcher.unit.test.ts @@ -5,6 +5,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { ConfigurationChangeEvent, Disposable, 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'; @@ -19,6 +20,7 @@ import { LanguageService } from '../../client/common/utils/localize'; import { IEnvironmentVariablesProvider } from '../../client/common/variables/types'; import { IInterpreterHelper, IInterpreterService } from '../../client/interpreter/contracts'; import { IServiceContainer } from '../../client/ioc/types'; +import { JupyterExtensionIntegration } from '../../client/jupyter/jupyterIntegration'; import { JediLSExtensionManager } from '../../client/languageServer/jediLSExtensionManager'; import { NoneLSExtensionManager } from '../../client/languageServer/noneLSExtensionManager'; import { PylanceLSExtensionManager } from '../../client/languageServer/pylanceLSExtensionManager'; @@ -75,6 +77,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); }); @@ -124,6 +128,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, disposables, ); @@ -171,6 +177,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, disposables, ); @@ -245,6 +253,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -319,6 +329,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -397,6 +409,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -466,6 +480,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -528,6 +544,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -591,6 +609,8 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -648,6 +668,8 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -736,6 +758,8 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -814,6 +838,8 @@ suite('Language server watcher', () => { ({ showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, + {} as LspNotebooksExperiment, + {} as JupyterExtensionIntegration, [] as Disposable[], ); From 4cd555fb7f6e7e0c949e5afe614197047a7eb0bf Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 27 Apr 2022 23:00:04 -0700 Subject: [PATCH 14/41] Get JupyterExtensionIntegration via container.get since it's created too late for injection into watcher --- src/client/activation/node/manager.ts | 6 ++++-- .../languageServer/pylanceLSExtensionManager.ts | 3 --- src/client/languageServer/watcher.ts | 3 --- .../pylanceLSExtensionManager.unit.test.ts | 4 ---- src/test/languageServer/watcher.unit.test.ts | 13 ------------- 5 files changed, 4 insertions(+), 25 deletions(-) diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index d0f7ae84c138..dda8d93f2b57 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -44,7 +44,6 @@ export class NodeLanguageServerManager implements ILanguageServerManager { private readonly languageServerProxy: ILanguageServerProxy, commandManager: ICommandManager, private readonly extensions: IExtensions, - private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, ) { if (NodeLanguageServerManager.commandDispose) { NodeLanguageServerManager.commandDispose.dispose(); @@ -127,7 +126,10 @@ export class NodeLanguageServerManager implements ILanguageServerManager { this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Node, this.lsVersion); options.middleware = this.middleware; - const jupyterPythonPathFunction = this.jupyterExtensionIntegration.getJupyterPythonPathFunction(); + const jupyterExtensionIntegration = this.serviceContainer.get( + JupyterExtensionIntegration, + ); + const jupyterPythonPathFunction = jupyterExtensionIntegration.getJupyterPythonPathFunction(); if (jupyterPythonPathFunction) { this.middleware.registerJupyterPythonPathFunction(jupyterPythonPathFunction); } diff --git a/src/client/languageServer/pylanceLSExtensionManager.ts b/src/client/languageServer/pylanceLSExtensionManager.ts index 5cf0e4d1c931..faa1bb75c4bc 100644 --- a/src/client/languageServer/pylanceLSExtensionManager.ts +++ b/src/client/languageServer/pylanceLSExtensionManager.ts @@ -23,7 +23,6 @@ import { Pylance } from '../common/utils/localize'; import { IEnvironmentVariablesProvider } from '../common/variables/types'; import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; -import { JupyterExtensionIntegration } from '../jupyter/jupyterIntegration'; import { traceLog } from '../logging'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { LanguageServerCapabilities } from './languageServerCapabilities'; @@ -53,7 +52,6 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities private readonly extensions: IExtensions, readonly applicationShell: IApplicationShell, lspNotebooksExperiment: LspNotebooksExperiment, - jupyterExtensionIntegration: JupyterExtensionIntegration, ) { super(); @@ -77,7 +75,6 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities this.serverProxy, commandManager, extensions, - jupyterExtensionIntegration, ); } diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index 6165d9137d73..7e37243621f2 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -35,7 +35,6 @@ import { NoneLSExtensionManager } from './noneLSExtensionManager'; import { PylanceLSExtensionManager } from './pylanceLSExtensionManager'; import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types'; import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment'; -import { JupyterExtensionIntegration } from '../jupyter/jupyterIntegration'; const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -77,7 +76,6 @@ export class LanguageServerWatcher @inject(IExtensions) private readonly extensions: IExtensions, @inject(IApplicationShell) readonly applicationShell: IApplicationShell, @inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment, - @inject(JupyterExtensionIntegration) private readonly jupyterExtensionIntegration: JupyterExtensionIntegration, @inject(IDisposableRegistry) readonly disposables: IDisposableRegistry, ) { this.workspaceInterpreters = new Map(); @@ -244,7 +242,6 @@ export class LanguageServerWatcher this.extensions, this.applicationShell, this.lspNotebooksExperiment, - this.jupyterExtensionIntegration, ); break; case LanguageServerType.None: diff --git a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts index 0f64dca09439..1a51c93d4783 100644 --- a/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts +++ b/src/test/languageServer/pylanceLSExtensionManager.unit.test.ts @@ -15,7 +15,6 @@ import { import { IEnvironmentVariablesProvider } from '../../client/common/variables/types'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { IServiceContainer } from '../../client/ioc/types'; -import { JupyterExtensionIntegration } from '../../client/jupyter/jupyterIntegration'; import { PylanceLSExtensionManager } from '../../client/languageServer/pylanceLSExtensionManager'; suite('Language Server - Pylance LS extension manager', () => { @@ -40,7 +39,6 @@ suite('Language Server - Pylance LS extension manager', () => { {} as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, ); }); @@ -71,7 +69,6 @@ suite('Language Server - Pylance LS extension manager', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, ); const result = manager.canStartLanguageServer(); @@ -100,7 +97,6 @@ suite('Language Server - Pylance LS extension manager', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, ); const result = manager.canStartLanguageServer(); diff --git a/src/test/languageServer/watcher.unit.test.ts b/src/test/languageServer/watcher.unit.test.ts index 33eaa7d2570b..f8fa8b2e7b7e 100644 --- a/src/test/languageServer/watcher.unit.test.ts +++ b/src/test/languageServer/watcher.unit.test.ts @@ -20,7 +20,6 @@ import { LanguageService } from '../../client/common/utils/localize'; import { IEnvironmentVariablesProvider } from '../../client/common/variables/types'; import { IInterpreterHelper, IInterpreterService } from '../../client/interpreter/contracts'; import { IServiceContainer } from '../../client/ioc/types'; -import { JupyterExtensionIntegration } from '../../client/jupyter/jupyterIntegration'; import { JediLSExtensionManager } from '../../client/languageServer/jediLSExtensionManager'; import { NoneLSExtensionManager } from '../../client/languageServer/noneLSExtensionManager'; import { PylanceLSExtensionManager } from '../../client/languageServer/pylanceLSExtensionManager'; @@ -78,7 +77,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); }); @@ -129,7 +127,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, disposables, ); @@ -178,7 +175,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, disposables, ); @@ -254,7 +250,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -330,7 +325,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -410,7 +404,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -481,7 +474,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -545,7 +537,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -610,7 +601,6 @@ suite('Language server watcher', () => { showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -669,7 +659,6 @@ suite('Language server watcher', () => { } as unknown) as IExtensions, {} as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -759,7 +748,6 @@ suite('Language server watcher', () => { showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); @@ -839,7 +827,6 @@ suite('Language server watcher', () => { showWarningMessage: () => Promise.resolve(undefined), } as unknown) as IApplicationShell, {} as LspNotebooksExperiment, - {} as JupyterExtensionIntegration, [] as Disposable[], ); From ebf1251b5d997c40339d474ca2da7aa7eb0076a2 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 29 Apr 2022 16:24:04 -0700 Subject: [PATCH 15/41] Call jupyterPythonPathFunction for all URIs, not just ipynb files --- src/client/activation/languageClientMiddlewareBase.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index d48380f01d9b..7b724f6e36cd 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -87,12 +87,11 @@ export class LanguageClientMiddlewareBase implements Middleware { i ] as LSPObject & { pythonPath: string; _envPYTHONPATH: string }; - // TODO: Remove cell branch? - if (uri?.scheme === 'vscode-notebook-cell' && this.jupyterPythonPathFunction) { - settingDict.pythonPath = (await this.jupyterPythonPathFunction(uri)) ?? ''; // TODO: How to handle undefined? - } else if (uri?.fsPath?.endsWith('.ipynb') && this.jupyterPythonPathFunction) { - settingDict.pythonPath = (await this.jupyterPythonPathFunction(uri)) ?? ''; // TODO: How to handle undefined? - } else { + if (uri && this.jupyterPythonPathFunction) { + settingDict.pythonPath = await this.jupyterPythonPathFunction(uri); + } + + if (settingDict.pythonPath === undefined) { settingDict.pythonPath = configService.getSettings(uri).pythonPath; } From 07ed0db3c55664a60eb845623067f3c5e9974610 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 29 Apr 2022 16:26:44 -0700 Subject: [PATCH 16/41] Fix issues in analysisOptions.ts --- src/client/activation/node/analysisOptions.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 598326231e64..fcc7795dad72 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { LanguageClientOptions } from 'vscode-languageclient'; import { WorkspaceFolder } from 'vscode'; +import { LanguageClientOptions } from 'vscode-languageclient'; import { DocumentFilter } from 'vscode-languageserver-protocol'; import { IWorkspaceService } from '../../common/application/types'; @@ -12,7 +12,9 @@ import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor - constructor(lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, + constructor( + lsOutputChannel: ILanguageServerOutputChannel, + workspace: IWorkspaceService, private readonly lspNotebooksExperiment: LspNotebooksExperiment, ) { super(lsOutputChannel, workspace); @@ -23,14 +25,14 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment() == true, + lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment() === true, } as unknown) as LanguageClientOptions; } protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { - let filters = await super.getDocumentFilters(_workspaceFolder); + const filters = await super.getDocumentFilters(_workspaceFolder); - if (this.lspNotebooksExperiment.isInNotebooksExperiment() == true) { + if (this.lspNotebooksExperiment.isInNotebooksExperiment() === true) { return [ { language: 'python' }, { From 4b1870ba016852df7a2069f81ea6f443191f0c10 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 29 Apr 2022 17:31:49 -0700 Subject: [PATCH 17/41] Log interpreter path provided by Jupyter --- src/client/activation/languageClientMiddlewareBase.ts | 5 +++++ src/client/common/utils/localize.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 7b724f6e36cd..e158a950ea74 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -14,9 +14,11 @@ import { import { HiddenFilePrefix } from '../common/constants'; import { IConfigurationService } from '../common/types'; import { createDeferred, isThenable } from '../common/utils/async'; +import { Interpreters } from '../common/utils/localize'; import { StopWatch } from '../common/utils/stopWatch'; import { IEnvironmentVariablesProvider } from '../common/variables/types'; import { IServiceContainer } from '../ioc/types'; +import { traceLog } from '../logging'; import { EventName } from '../telemetry/constants'; import { LanguageServerType } from './types'; @@ -89,6 +91,9 @@ export class LanguageClientMiddlewareBase implements Middleware { if (uri && this.jupyterPythonPathFunction) { settingDict.pythonPath = await this.jupyterPythonPathFunction(uri); + if (settingDict.pythonPath) { + traceLog(Interpreters.pythonInterpreterPathFromJupyter().format(settingDict.pythonPath)); + } } if (settingDict.pythonPath === undefined) { diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index c58011adf004..cb1eae04327d 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -271,6 +271,11 @@ export namespace Interpreters { 'Interpreters.selectInterpreterTip', 'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar', ); + export const pythonInterpreterPath = localize('Interpreters.pythonInterpreterPath', 'Python interpreter path: {0}'); + export const pythonInterpreterPathFromJupyter = localize( + 'Interpreters.pythonInterpreterPathFromJupyter', + 'Jupyter provided interpreter path override: {0}', + ); } export namespace InterpreterQuickPickList { From 4a18117bafe328037d7b9a4272366a23d02f693c Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 29 Apr 2022 17:32:06 -0700 Subject: [PATCH 18/41] Update to latest LSP --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 5e471343fd15..f88501e1fb1a 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,6 @@ "languageServerVersion": "0.5.30", "publisher": "ms-python", "enabledApiProposals": [ - "notebookDocumentEvents" ], "author": { "name": "Microsoft Corporation" From 9b888565290d9dd4d411689b2275006ece0a9e88 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 2 May 2022 11:23:09 -0700 Subject: [PATCH 19/41] Remove registerProposedFeatures accidentally added back during rebase --- src/client/browser/extension.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/browser/extension.ts b/src/client/browser/extension.ts index 576e25f6bbac..2effb94d4e03 100644 --- a/src/client/browser/extension.ts +++ b/src/client/browser/extension.ts @@ -83,7 +83,6 @@ async function runPylance( const client = new LanguageClient('python', 'Python Language Server', clientOptions, worker); languageClient = client; - languageClient.registerProposedFeatures(); context.subscriptions.push( vscode.commands.registerCommand('python.viewLanguageServerOutput', () => client.outputChannel.show()), From 5915be051faed077034f3da4e40beafad60aa39b Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 2 May 2022 12:40:33 -0700 Subject: [PATCH 20/41] Cleanup for PR --- package.json | 5 ++++- src/client/activation/common/analysisOptions.ts | 4 ++-- src/client/activation/node/analysisOptions.ts | 4 ++-- src/test/activation/node/analysisOptions.unit.test.ts | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index f88501e1fb1a..6d5241397978 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,9 @@ "languageServerVersion": "0.5.30", "publisher": "ms-python", "enabledApiProposals": [ + "quickPickSortByLabel", + "testObserver", + "notebookEditor" ], "author": { "name": "Microsoft Corporation" @@ -906,7 +909,7 @@ "python.pylanceLspNotebooksEnabled": { "type": "boolean", "default": false, - "description": "Determines if pylance's experimental LSP notebooks support is used or not.", + "description": "Determines if Pylance's experimental LSP notebooks support is used or not.", "scope": "machine", "tags": [ "experimental" diff --git a/src/client/activation/common/analysisOptions.ts b/src/client/activation/common/analysisOptions.ts index d23b34b08a62..f2839a25399d 100644 --- a/src/client/activation/common/analysisOptions.ts +++ b/src/client/activation/common/analysisOptions.ts @@ -36,7 +36,7 @@ export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServ @traceDecoratorError('Failed to get analysis options') public async getAnalysisOptions(): Promise { const workspaceFolder = this.getWorkspaceFolder(); - const documentSelector = await this.getDocumentFilters(workspaceFolder); + const documentSelector = this.getDocumentFilters(workspaceFolder); return { documentSelector, @@ -54,7 +54,7 @@ export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServ return undefined; } - protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { + protected getDocumentFilters(_workspaceFolder?: WorkspaceFolder): DocumentFilter[] { return this.workspace.isVirtualWorkspace ? [{ language: PYTHON_LANGUAGE }] : PYTHON; } diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index fcc7795dad72..a8141590f6b0 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -29,8 +29,8 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt } as unknown) as LanguageClientOptions; } - protected async getDocumentFilters(_workspaceFolder?: WorkspaceFolder): Promise { - const filters = await super.getDocumentFilters(_workspaceFolder); + protected getDocumentFilters(_workspaceFolder?: WorkspaceFolder): DocumentFilter[] { + const filters = super.getDocumentFilters(_workspaceFolder); if (this.lspNotebooksExperiment.isInNotebooksExperiment() === true) { return [ diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 4a3ed1d9eb4f..f608e9bec3da 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -18,7 +18,7 @@ suite('Pylance Language Server - Analysis Options', () => { return super.getWorkspaceFolder(); } - public async getDocumentFilters(workspaceFolder?: WorkspaceFolder): Promise { + public getDocumentFilters(workspaceFolder?: WorkspaceFolder): DocumentFilter[] { return super.getDocumentFilters(workspaceFolder); } From 6ab13b21118d27583819468254d2ab06e2ee30ee Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Thu, 5 May 2022 11:49:24 -0700 Subject: [PATCH 21/41] Split LanguageClientMiddleware into Node/Jedi versions --- .../jedi/languageClientMiddleware.ts | 14 +++ .../activation/jedi/languageServerProxy.ts | 5 +- src/client/activation/jedi/manager.ts | 13 +-- .../activation/languageClientMiddleware.ts | 58 ------------- .../languageClientMiddlewareBase.ts | 16 +--- .../node/languageClientMiddleware.ts | 85 +++++++++++++++++++ src/client/activation/node/manager.ts | 13 +-- 7 files changed, 114 insertions(+), 90 deletions(-) create mode 100644 src/client/activation/jedi/languageClientMiddleware.ts delete mode 100644 src/client/activation/languageClientMiddleware.ts create mode 100644 src/client/activation/node/languageClientMiddleware.ts diff --git a/src/client/activation/jedi/languageClientMiddleware.ts b/src/client/activation/jedi/languageClientMiddleware.ts new file mode 100644 index 000000000000..6e1de4bfe3ff --- /dev/null +++ b/src/client/activation/jedi/languageClientMiddleware.ts @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { IServiceContainer } from '../../ioc/types'; +import { sendTelemetryEvent } from '../../telemetry'; + +import { LanguageClientMiddlewareBase } from '../languageClientMiddlewareBase'; +import { LanguageServerType } from '../types'; + +export class JediLanguageClientMiddleware extends LanguageClientMiddlewareBase { + public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { + super(serviceContainer, LanguageServerType.Jedi, sendTelemetryEvent, serverVersion); + } +} diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index 2a7794a8369f..dd24cc97f9c8 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -14,11 +14,11 @@ import { IInterpreterPathService, Resource } from '../../common/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { LanguageClientMiddleware } from '../languageClientMiddleware'; import { ProgressReporting } from '../progress'; import { ILanguageClientFactory, ILanguageServerProxy } from '../types'; import { killPid } from '../../common/process/rawProcessApis'; import { traceDecoratorError, traceDecoratorVerbose, traceError } from '../../logging'; +import { JediLanguageClientMiddleware } from './languageClientMiddleware'; export class JediLanguageServerProxy implements ILanguageServerProxy { public languageClient: LanguageClient | undefined; @@ -62,7 +62,8 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { } this.lsVersion = - (options.middleware ? (options.middleware).serverVersion : undefined) ?? '0.19.3'; + (options.middleware ? (options.middleware).serverVersion : undefined) ?? + '0.19.3'; this.languageClient = await this.factory.createLanguageClient(resource, interpreter, options); this.registerHandlers(); diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index 29f1106c5f8c..2b1600483524 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -14,21 +14,16 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { Commands } from '../commands'; -import { LanguageClientMiddleware } from '../languageClientMiddleware'; -import { - ILanguageServerAnalysisOptions, - ILanguageServerManager, - ILanguageServerProxy, - LanguageServerType, -} from '../types'; +import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose, traceVerbose } from '../../logging'; +import { JediLanguageClientMiddleware } from './languageClientMiddleware'; export class JediLanguageServerManager implements ILanguageServerManager { private resource!: Resource; private interpreter: PythonEnvironment | undefined; - private middleware: LanguageClientMiddleware | undefined; + private middleware: JediLanguageClientMiddleware | undefined; private disposables: IDisposable[] = []; @@ -132,7 +127,7 @@ export class JediLanguageServerManager implements ILanguageServerManager { @traceDecoratorVerbose('Starting language server') protected async startLanguageServer(): Promise { const options = await this.analysisOptions.getAnalysisOptions(); - this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Jedi, this.lsVersion); + this.middleware = new JediLanguageClientMiddleware(this.serviceContainer, this.lsVersion); options.middleware = this.middleware; // Make sure the middleware is connected if we restart and we we're already connected. diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts deleted file mode 100644 index 9c95c8bd4dc4..000000000000 --- a/src/client/activation/languageClientMiddleware.ts +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { IJupyterExtensionDependencyManager } from '../common/application/types'; -import { IDisposableRegistry, IExtensions } from '../common/types'; -import { IServiceContainer } from '../ioc/types'; -import { sendTelemetryEvent } from '../telemetry'; - -import { LanguageClientMiddlewareBase } from './languageClientMiddlewareBase'; -import { LanguageServerType } from './types'; - -import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; -import { LspNotebooksExperiment } from './node/lspNotebooksExperiment'; - -export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { - public constructor(serviceContainer: IServiceContainer, serverType: LanguageServerType, serverVersion?: string) { - super(serviceContainer, serverType, sendTelemetryEvent, serverVersion); - - if (serverType === LanguageServerType.None) { - return; - } - - const jupyterDependencyManager = serviceContainer.get( - IJupyterExtensionDependencyManager, - ); - const disposables = serviceContainer.get(IDisposableRegistry) || []; - const extensions = serviceContainer.get(IExtensions); - const lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); - - // Enable notebook support if jupyter support is installed - if ( - jupyterDependencyManager && - jupyterDependencyManager.isJupyterExtensionInstalled && - lspNotebooksExperiment.isInNotebooksExperiment() != true - ) { - this.notebookAddon = createHidingMiddleware(); - } - disposables.push( - extensions?.onDidChange(() => { - if (jupyterDependencyManager) { - if ( - this.notebookAddon && - (!jupyterDependencyManager.isJupyterExtensionInstalled || - lspNotebooksExperiment.isInNotebooksExperiment() == true) - ) { - this.notebookAddon = undefined; - } else if ( - !this.notebookAddon && - jupyterDependencyManager.isJupyterExtensionInstalled && - lspNotebooksExperiment.isInNotebooksExperiment() != true - ) { - this.notebookAddon = createHidingMiddleware(); - } - } - }), - ); - } -} diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index e158a950ea74..3aac748a6986 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -14,11 +14,9 @@ import { import { HiddenFilePrefix } from '../common/constants'; import { IConfigurationService } from '../common/types'; import { createDeferred, isThenable } from '../common/utils/async'; -import { Interpreters } from '../common/utils/localize'; import { StopWatch } from '../common/utils/stopWatch'; import { IEnvironmentVariablesProvider } from '../common/variables/types'; import { IServiceContainer } from '../ioc/types'; -import { traceLog } from '../logging'; import { EventName } from '../telemetry/constants'; import { LanguageServerType } from './types'; @@ -89,12 +87,7 @@ export class LanguageClientMiddlewareBase implements Middleware { i ] as LSPObject & { pythonPath: string; _envPYTHONPATH: string }; - if (uri && this.jupyterPythonPathFunction) { - settingDict.pythonPath = await this.jupyterPythonPathFunction(uri); - if (settingDict.pythonPath) { - traceLog(Interpreters.pythonInterpreterPathFromJupyter().format(settingDict.pythonPath)); - } - } + settingDict.pythonPath = await this.getPythonPathOverride(uri); if (settingDict.pythonPath === undefined) { settingDict.pythonPath = configService.getSettings(uri).pythonPath; @@ -112,10 +105,9 @@ export class LanguageClientMiddlewareBase implements Middleware { }, }; - private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; - - public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise) { - this.jupyterPythonPathFunction = func; + // eslint-disable-next-line class-methods-use-this + protected async getPythonPathOverride(_uri: Uri | undefined): Promise { + return undefined; } private get connected(): Promise { diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts new file mode 100644 index 000000000000..b3a25cef6eaf --- /dev/null +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; +import { Uri } from 'vscode'; +import { IJupyterExtensionDependencyManager } from '../../common/application/types'; +import { IConfigurationService, IDisposableRegistry, IExtensions } from '../../common/types'; +import { Interpreters } from '../../common/utils/localize'; +import { IServiceContainer } from '../../ioc/types'; +import { traceLog } from '../../logging'; +import { sendTelemetryEvent } from '../../telemetry'; + +import { LanguageClientMiddlewareBase } from '../languageClientMiddlewareBase'; +import { LanguageServerType } from '../types'; + +import { LspNotebooksExperiment } from './lspNotebooksExperiment'; + +export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { + private readonly _jupyterDependencyManager: IJupyterExtensionDependencyManager; + + private readonly _disposables: IDisposableRegistry; + + private readonly _lspNotebooksExperiment: LspNotebooksExperiment; + + private _jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; + + public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { + super(serviceContainer, LanguageServerType.Node, sendTelemetryEvent, serverVersion); + + this._jupyterDependencyManager = serviceContainer.get( + IJupyterExtensionDependencyManager, + ); + this._disposables = serviceContainer.get(IDisposableRegistry) || []; + this._lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); + + const extensions = serviceContainer.get(IExtensions); + const config = serviceContainer.get(IConfigurationService); + + this._updateHidingMiddleware(); + + this._disposables.push(extensions?.onDidChange(() => this._updateHidingMiddleware())); + this._disposables.push(config.getSettings()?.onDidChange(() => this._updateHidingMiddleware())); + } + + private _updateHidingMiddleware() { + // Enable notebook support if jupyter support is installed + if (this._jupyterDependencyManager) { + if ( + this.notebookAddon && + (!this._jupyterDependencyManager.isJupyterExtensionInstalled || + this._lspNotebooksExperiment.isInNotebooksExperiment() === true) + ) { + this.notebookAddon = undefined; + } else if ( + !this.notebookAddon && + this._jupyterDependencyManager.isJupyterExtensionInstalled && + this._lspNotebooksExperiment.isInNotebooksExperiment() !== true + ) { + this.notebookAddon = createHidingMiddleware(); + } + } + } + + public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { + this._jupyterPythonPathFunction = func; + } + + protected async getPythonPathOverride(uri: Uri | undefined): Promise { + if ( + uri === undefined || + this._jupyterPythonPathFunction === undefined || + this._lspNotebooksExperiment.isInNotebooksExperiment() !== true + ) { + return undefined; + } + + const result = await this._jupyterPythonPathFunction(uri); + + if (result) { + traceLog(Interpreters.pythonInterpreterPathFromJupyter().format(result)); + } + + return result; + } +} diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index dda8d93f2b57..9de400396932 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -10,23 +10,18 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { Commands } from '../commands'; -import { LanguageClientMiddleware } from '../languageClientMiddleware'; -import { - ILanguageServerAnalysisOptions, - ILanguageServerManager, - ILanguageServerProxy, - LanguageServerType, -} from '../types'; +import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; +import { NodeLanguageClientMiddleware } from './languageClientMiddleware'; export class NodeLanguageServerManager implements ILanguageServerManager { private resource!: Resource; private interpreter: PythonEnvironment | undefined; - private middleware: LanguageClientMiddleware | undefined; + private middleware: NodeLanguageClientMiddleware | undefined; private disposables: IDisposable[] = []; @@ -123,7 +118,7 @@ export class NodeLanguageServerManager implements ILanguageServerManager { @traceDecoratorVerbose('Starting language server') protected async startLanguageServer(): Promise { const options = await this.analysisOptions.getAnalysisOptions(); - this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Node, this.lsVersion); + this.middleware = new NodeLanguageClientMiddleware(this.serviceContainer, this.lsVersion); options.middleware = this.middleware; const jupyterExtensionIntegration = this.serviceContainer.get( From d2c1f55d27f79db1f9fe19080cf42ac2f082090c Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 10:05:00 -0700 Subject: [PATCH 22/41] First attempt at handling late Pylance/Jupyter install --- .../node/languageClientMiddleware.ts | 6 + .../activation/node/lspNotebooksExperiment.ts | 145 +++++++++++++++--- src/client/languageServer/types.ts | 1 + src/client/languageServer/watcher.ts | 10 ++ 4 files changed, 137 insertions(+), 25 deletions(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index b3a25cef6eaf..6a7cc4b3ca12 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -42,6 +42,12 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { this._disposables.push(config.getSettings()?.onDidChange(() => this._updateHidingMiddleware())); } + // TODO: THIS IS CALLED BEFORE EXPERIMENT STATE IS RECALCULATED. DOES IT MATTER? + // SAME THING COULD HAPPEN WHEN INSTALLING PYLANCE, WHICH MIGHT BE WORSE? + // Scenario: + // Python and Pylance are installed + // Jupyter gets installed + // Code below installs middleware incorrectly private _updateHidingMiddleware() { // Enable notebook support if jupyter support is installed if (this._jupyterDependencyManager) { diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 25b2c2b1310b..a263f30da3cf 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -1,53 +1,148 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable, named } from 'inversify'; -import { extensions } from 'vscode'; +import { inject, injectable } from 'inversify'; import * as semver from 'semver'; -import { IConfigurationService, IOutputChannel } from '../../common/types'; -import { IExtensionSingleActivationService } from '../types'; +import { Disposable, extensions } from 'vscode'; +import { IConfigurationService } from '../../common/types'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; +import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constants'; +import { IExtensionSingleActivationService } from '../types'; +import { traceLog, 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'; @injectable() export class LspNotebooksExperiment implements IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; - private _isInNotebooksExperiment?: boolean; + private _pylanceExtensionChangeHandler: Disposable | undefined; - private _jupyterVersion: string | undefined; + private _jupyterExtensionChangeHandler: Disposable | undefined; - private _pylanceVersion: string | undefined; + private _isInNotebooksExperiment = false; constructor( + @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel, - ) {} + @inject(IJupyterExtensionDependencyManager) + private readonly jupyterDependencyManager: IJupyterExtensionDependencyManager, + ) { + if (!LspNotebooksExperiment._isPylanceInstalled()) { + this._pylanceExtensionChangeHandler = extensions.onDidChange( + this._pylanceExtensionsChangeHandler.bind(this), + ); + } + + if (!this.jupyterDependencyManager.isJupyterExtensionInstalled) { + this._jupyterExtensionChangeHandler = extensions.onDidChange( + this._jupyterExtensionsChangeHandler.bind(this), + ); + } + } public async activate(): Promise { - this._isInNotebooksExperiment = this.configurationService.getSettings().pylanceLspNotebooksEnabled; - this._jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - this._pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; + this._updateExperimentSupport(); + } + + private async reset() { + await this.activate(); + } - if (this._supportsNotebooksExperiment()) { + public isInNotebooksExperiment(): boolean { + return this._isInNotebooksExperiment; + } + + private _updateExperimentSupport(): void { + const wasInExperiment = this.isInNotebooksExperiment(); + + const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled; + + if ( + isInTreatmentGroup && + LspNotebooksExperiment._jupyterSupportsNotebooksExperiment() && + LspNotebooksExperiment._pylanceSupportsNotebooksExperiment() + ) { + this._isInNotebooksExperiment = true; sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); } - this.output.appendLine( - `LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInNotebooksExperiment()}`, - ); + // 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._isInNotebooksExperiment === true) { + const watcher = this.serviceContainer.get(ILanguageServerWatcher); + if (watcher) { + watcher.restartLanguageServers(); + } + } + + traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInNotebooksExperiment()}`); + } + + private static _jupyterSupportsNotebooksExperiment(): boolean { + const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; + return jupyterVersion !== undefined && semver.satisfies(jupyterVersion, '>=2022.4.100'); } - public isInNotebooksExperiment(): boolean | undefined { - return this._isInNotebooksExperiment && this._supportsNotebooksExperiment(); + private static _pylanceSupportsNotebooksExperiment(): boolean { + const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; + return pylanceVersion !== undefined && semver.satisfies(pylanceVersion, '>=2022.5.1-pre.1 || 9999.0.0-dev'); } - private _supportsNotebooksExperiment(): boolean { - return ( - this._jupyterVersion !== undefined && - semver.satisfies(this._jupyterVersion, '>=2022.4.100') && - this._pylanceVersion !== undefined && - semver.satisfies(this._pylanceVersion, '>=2022.4.4-pre.1 || 9999.0.0-dev') + private async _waitForJupyterToRegisterPythonPathFunction(): Promise { + if (!this.isInNotebooksExperiment()) { + return; + } + + const jupyterExtensionIntegration = this.serviceContainer.get( + JupyterExtensionIntegration, ); + + let success = false; + for (let tryCount = 0; tryCount < 20; tryCount += 1) { + const jupyterPythonPathFunction = jupyterExtensionIntegration.getJupyterPythonPathFunction(); + if (jupyterPythonPathFunction) { + traceVerbose(`Jupyter has called registration method...`); + success = true; + break; + } + + traceVerbose(`Waiting for Jupyter to call registration method...`); + await sleep(500); + } + + if (!success) { + traceVerbose(`Timed out waiting for Jupyter to call registration method...`); + } + } + + private static _isPylanceInstalled(): boolean { + return !!extensions.getExtension(PYLANCE_EXTENSION_ID); + } + + private async _pylanceExtensionsChangeHandler(): Promise { + if (LspNotebooksExperiment._isPylanceInstalled() && this._pylanceExtensionChangeHandler) { + this._pylanceExtensionChangeHandler.dispose(); + this._pylanceExtensionChangeHandler = undefined; + + await this.reset(); + } + } + + private async _jupyterExtensionsChangeHandler(): Promise { + if (this.jupyterDependencyManager.isJupyterExtensionInstalled && this._jupyterExtensionChangeHandler) { + this._jupyterExtensionChangeHandler.dispose(); + this._jupyterExtensionChangeHandler = undefined; + + if (LspNotebooksExperiment._jupyterSupportsNotebooksExperiment()) { + await this._waitForJupyterToRegisterPythonPathFunction(); + await this.reset(); + } + } } } diff --git a/src/client/languageServer/types.ts b/src/client/languageServer/types.ts index f0376bf81cbf..6fbeca379a86 100644 --- a/src/client/languageServer/types.ts +++ b/src/client/languageServer/types.ts @@ -14,6 +14,7 @@ export interface ILanguageServerWatcher { readonly languageServerExtensionManager: ILanguageServerExtensionManager | undefined; readonly languageServerType: LanguageServerType; startLanguageServer(languageServerType: LanguageServerType, resource?: Resource): Promise; + restartLanguageServers(): Promise; } export interface ILanguageServerCapabilities extends ILanguageServer { diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index 7e37243621f2..f35e213b00d9 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -186,6 +186,16 @@ export class LanguageServerWatcher return languageServerExtensionManager; } + public async restartLanguageServers(): Promise { + const workspacesUris = this.workspaceService.workspaceFolders?.map((workspace) => workspace.uri) ?? []; + + workspacesUris.forEach(async (resource) => { + const lsResource = this.getWorkspaceUri(resource); + this.stopLanguageServer(resource); + await this.startLanguageServer(this.languageServerType, lsResource); + }); + } + // ILanguageServerCache public async get(resource?: Resource): Promise { From 485db856dfae05088e5182b44995b894f64483a9 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 11:06:40 -0700 Subject: [PATCH 23/41] Post rebase fixes --- src/client/activation/node/languageClientMiddleware.ts | 10 ++++++++-- src/client/common/utils/localize.ts | 5 ----- src/test/languageServer/watcher.unit.test.ts | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 6a7cc4b3ca12..d292ff652709 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -3,9 +3,9 @@ import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; import { Uri } from 'vscode'; +import { localize } from 'vscode-nls'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; import { IConfigurationService, IDisposableRegistry, IExtensions } from '../../common/types'; -import { Interpreters } from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { traceLog } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; @@ -83,7 +83,13 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { const result = await this._jupyterPythonPathFunction(uri); if (result) { - traceLog(Interpreters.pythonInterpreterPathFromJupyter().format(result)); + traceLog( + localize( + 'Interpreters.pythonInterpreterPathFromJupyter', + 'Jupyter provided interpreter path override: {0}', + result, + ), + ); } return result; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index cb1eae04327d..c58011adf004 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -271,11 +271,6 @@ export namespace Interpreters { 'Interpreters.selectInterpreterTip', 'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar', ); - export const pythonInterpreterPath = localize('Interpreters.pythonInterpreterPath', 'Python interpreter path: {0}'); - export const pythonInterpreterPathFromJupyter = localize( - 'Interpreters.pythonInterpreterPathFromJupyter', - 'Jupyter provided interpreter path override: {0}', - ); } export namespace InterpreterQuickPickList { diff --git a/src/test/languageServer/watcher.unit.test.ts b/src/test/languageServer/watcher.unit.test.ts index f8fa8b2e7b7e..18620a57fc6c 100644 --- a/src/test/languageServer/watcher.unit.test.ts +++ b/src/test/languageServer/watcher.unit.test.ts @@ -914,6 +914,7 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, [] as Disposable[], ); @@ -993,6 +994,7 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, [] as Disposable[], ); @@ -1075,6 +1077,7 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, [] as Disposable[], ); @@ -1157,6 +1160,7 @@ suite('Language server watcher', () => { }, } as unknown) as IExtensions, {} as IApplicationShell, + {} as LspNotebooksExperiment, [] as Disposable[], ); From 837566836af92bf8c98b39f40463b9a9532ad3eb Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 11:41:33 -0700 Subject: [PATCH 24/41] Ensure that isInNotebooksExperiment is updated before running createHidingMiddleware logic --- .../node/languageClientMiddleware.ts | 77 ++++++++++--------- .../activation/node/lspNotebooksExperiment.ts | 44 +++++------ 2 files changed, 57 insertions(+), 64 deletions(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index d292ff652709..675e00609311 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -5,7 +5,7 @@ import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; import { Uri } from 'vscode'; import { localize } from 'vscode-nls'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; -import { IConfigurationService, IDisposableRegistry, IExtensions } from '../../common/types'; +import { IDisposableRegistry, IExtensions } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { traceLog } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; @@ -16,10 +16,6 @@ import { LanguageServerType } from '../types'; import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { - private readonly _jupyterDependencyManager: IJupyterExtensionDependencyManager; - - private readonly _disposables: IDisposableRegistry; - private readonly _lspNotebooksExperiment: LspNotebooksExperiment; private _jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; @@ -27,44 +23,49 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { super(serviceContainer, LanguageServerType.Node, sendTelemetryEvent, serverVersion); - this._jupyterDependencyManager = serviceContainer.get( - IJupyterExtensionDependencyManager, - ); - this._disposables = serviceContainer.get(IDisposableRegistry) || []; this._lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); + const jupyterDependencyManager = serviceContainer.get( + IJupyterExtensionDependencyManager, + ); + const disposables = serviceContainer.get(IDisposableRegistry) || []; const extensions = serviceContainer.get(IExtensions); - const config = serviceContainer.get(IConfigurationService); - this._updateHidingMiddleware(); - - this._disposables.push(extensions?.onDidChange(() => this._updateHidingMiddleware())); - this._disposables.push(config.getSettings()?.onDidChange(() => this._updateHidingMiddleware())); - } - - // TODO: THIS IS CALLED BEFORE EXPERIMENT STATE IS RECALCULATED. DOES IT MATTER? - // SAME THING COULD HAPPEN WHEN INSTALLING PYLANCE, WHICH MIGHT BE WORSE? - // Scenario: - // Python and Pylance are installed - // Jupyter gets installed - // Code below installs middleware incorrectly - private _updateHidingMiddleware() { // Enable notebook support if jupyter support is installed - if (this._jupyterDependencyManager) { - if ( - this.notebookAddon && - (!this._jupyterDependencyManager.isJupyterExtensionInstalled || - this._lspNotebooksExperiment.isInNotebooksExperiment() === true) - ) { - this.notebookAddon = undefined; - } else if ( - !this.notebookAddon && - this._jupyterDependencyManager.isJupyterExtensionInstalled && - this._lspNotebooksExperiment.isInNotebooksExperiment() !== true - ) { - this.notebookAddon = createHidingMiddleware(); - } + if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) { + this.notebookAddon = createHidingMiddleware(); } + + // TODO: THIS IS CALLED BEFORE EXPERIMENT STATE IS RECALCULATED. DOES IT MATTER? + // SAME THING COULD HAPPEN WHEN INSTALLING PYLANCE, WHICH MIGHT BE WORSE? + // Scenario: + // Python and Pylance are installed + // Jupyter gets installed + // Code below installs middleware incorrectly + + disposables.push( + extensions?.onDidChange(async () => { + if (jupyterDependencyManager) { + if (jupyterDependencyManager.isJupyterExtensionInstalled) { + await this._lspNotebooksExperiment.onJupyterInstalled(); + } + + if ( + this.notebookAddon && + (!jupyterDependencyManager.isJupyterExtensionInstalled || + this._lspNotebooksExperiment.isInNotebooksExperiment()) + ) { + this.notebookAddon = undefined; + } else if ( + !this.notebookAddon && + jupyterDependencyManager.isJupyterExtensionInstalled && + !this._lspNotebooksExperiment.isInNotebooksExperiment() + ) { + this.notebookAddon = createHidingMiddleware(); + } + } + }), + ); } public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { @@ -75,7 +76,7 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { if ( uri === undefined || this._jupyterPythonPathFunction === undefined || - this._lspNotebooksExperiment.isInNotebooksExperiment() !== true + !this._lspNotebooksExperiment.isInNotebooksExperiment() ) { return undefined; } diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index a263f30da3cf..9f742dc37731 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -21,15 +21,14 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private _pylanceExtensionChangeHandler: Disposable | undefined; - private _jupyterExtensionChangeHandler: Disposable | undefined; + private _isJupyterInstalled = false; private _isInNotebooksExperiment = false; constructor( @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, - @inject(IJupyterExtensionDependencyManager) - private readonly jupyterDependencyManager: IJupyterExtensionDependencyManager, + @inject(IJupyterExtensionDependencyManager) jupyterDependencyManager: IJupyterExtensionDependencyManager, ) { if (!LspNotebooksExperiment._isPylanceInstalled()) { this._pylanceExtensionChangeHandler = extensions.onDidChange( @@ -37,19 +36,24 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService ); } - if (!this.jupyterDependencyManager.isJupyterExtensionInstalled) { - this._jupyterExtensionChangeHandler = extensions.onDidChange( - this._jupyterExtensionsChangeHandler.bind(this), - ); - } + this._isJupyterInstalled = jupyterDependencyManager.isJupyterExtensionInstalled; } public async activate(): Promise { this._updateExperimentSupport(); } - private async reset() { - await this.activate(); + public async onJupyterInstalled(): Promise { + if (this._isJupyterInstalled) { + return; + } + + if (LspNotebooksExperiment._jupyterSupportsNotebooksExperiment()) { + await this._waitForJupyterToRegisterPythonPathFunction(); + this._updateExperimentSupport(); + } + + this._isJupyterInstalled = true; } public isInNotebooksExperiment(): boolean { @@ -57,7 +61,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } private _updateExperimentSupport(): void { - const wasInExperiment = this.isInNotebooksExperiment(); + const wasInExperiment = this._isInNotebooksExperiment; const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled; @@ -81,7 +85,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } } - traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInNotebooksExperiment()}`); + traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this._isInNotebooksExperiment}`); } private static _jupyterSupportsNotebooksExperiment(): boolean { @@ -95,7 +99,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } private async _waitForJupyterToRegisterPythonPathFunction(): Promise { - if (!this.isInNotebooksExperiment()) { + if (!this._isInNotebooksExperiment) { return; } @@ -130,19 +134,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService this._pylanceExtensionChangeHandler.dispose(); this._pylanceExtensionChangeHandler = undefined; - await this.reset(); - } - } - - private async _jupyterExtensionsChangeHandler(): Promise { - if (this.jupyterDependencyManager.isJupyterExtensionInstalled && this._jupyterExtensionChangeHandler) { - this._jupyterExtensionChangeHandler.dispose(); - this._jupyterExtensionChangeHandler = undefined; - - if (LspNotebooksExperiment._jupyterSupportsNotebooksExperiment()) { - await this._waitForJupyterToRegisterPythonPathFunction(); - await this.reset(); - } + this._updateExperimentSupport(); } } } From 4173428ec81e6edcc33b3839ab6a12f7ef9c1787 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 12:02:54 -0700 Subject: [PATCH 25/41] Cleanup remove private _ prefix, ! for undefined checks --- .../activation/jedi/languageServerProxy.ts | 2 +- src/client/activation/jedi/manager.ts | 2 +- .../languageClientMiddlewareBase.ts | 2 +- src/client/activation/node/analysisOptions.ts | 4 +- .../node/languageClientMiddleware.ts | 29 +++----- .../activation/node/lspNotebooksExperiment.ts | 67 +++++++++---------- src/client/activation/node/manager.ts | 2 +- .../node/analysisOptions.unit.test.ts | 4 +- 8 files changed, 49 insertions(+), 63 deletions(-) diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index dd24cc97f9c8..ffed40231796 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -14,11 +14,11 @@ import { IInterpreterPathService, Resource } from '../../common/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; +import { JediLanguageClientMiddleware } from './languageClientMiddleware'; import { ProgressReporting } from '../progress'; import { ILanguageClientFactory, ILanguageServerProxy } from '../types'; import { killPid } from '../../common/process/rawProcessApis'; import { traceDecoratorError, traceDecoratorVerbose, traceError } from '../../logging'; -import { JediLanguageClientMiddleware } from './languageClientMiddleware'; export class JediLanguageServerProxy implements ILanguageServerProxy { public languageClient: LanguageClient | undefined; diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index 2b1600483524..fa3fbc8b7505 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -14,9 +14,9 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { Commands } from '../commands'; +import { JediLanguageClientMiddleware } from './languageClientMiddleware'; import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose, traceVerbose } from '../../logging'; -import { JediLanguageClientMiddleware } from './languageClientMiddleware'; export class JediLanguageServerManager implements ILanguageServerManager { private resource!: Resource; diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 3aac748a6986..446068c4a841 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -89,7 +89,7 @@ export class LanguageClientMiddlewareBase implements Middleware { settingDict.pythonPath = await this.getPythonPathOverride(uri); - if (settingDict.pythonPath === undefined) { + if (!settingDict.pythonPath) { settingDict.pythonPath = configService.getSettings(uri).pythonPath; } diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index a8141590f6b0..3ad46bed234a 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -25,14 +25,14 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return ({ experimentationSupport: true, trustedWorkspaceSupport: true, - lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment() === true, + lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), } as unknown) as LanguageClientOptions; } protected getDocumentFilters(_workspaceFolder?: WorkspaceFolder): DocumentFilter[] { const filters = super.getDocumentFilters(_workspaceFolder); - if (this.lspNotebooksExperiment.isInNotebooksExperiment() === true) { + if (this.lspNotebooksExperiment.isInNotebooksExperiment()) { return [ { language: 'python' }, { diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 675e00609311..ebea6d42a27c 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -16,14 +16,14 @@ import { LanguageServerType } from '../types'; import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { - private readonly _lspNotebooksExperiment: LspNotebooksExperiment; + private readonly lspNotebooksExperiment: LspNotebooksExperiment; - private _jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; + private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { super(serviceContainer, LanguageServerType.Node, sendTelemetryEvent, serverVersion); - this._lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); + this.lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); const jupyterDependencyManager = serviceContainer.get( IJupyterExtensionDependencyManager, @@ -36,30 +36,23 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { this.notebookAddon = createHidingMiddleware(); } - // TODO: THIS IS CALLED BEFORE EXPERIMENT STATE IS RECALCULATED. DOES IT MATTER? - // SAME THING COULD HAPPEN WHEN INSTALLING PYLANCE, WHICH MIGHT BE WORSE? - // Scenario: - // Python and Pylance are installed - // Jupyter gets installed - // Code below installs middleware incorrectly - disposables.push( extensions?.onDidChange(async () => { if (jupyterDependencyManager) { if (jupyterDependencyManager.isJupyterExtensionInstalled) { - await this._lspNotebooksExperiment.onJupyterInstalled(); + await this.lspNotebooksExperiment.onJupyterInstalled(); } if ( this.notebookAddon && (!jupyterDependencyManager.isJupyterExtensionInstalled || - this._lspNotebooksExperiment.isInNotebooksExperiment()) + this.lspNotebooksExperiment.isInNotebooksExperiment()) ) { this.notebookAddon = undefined; } else if ( !this.notebookAddon && jupyterDependencyManager.isJupyterExtensionInstalled && - !this._lspNotebooksExperiment.isInNotebooksExperiment() + !this.lspNotebooksExperiment.isInNotebooksExperiment() ) { this.notebookAddon = createHidingMiddleware(); } @@ -69,19 +62,15 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { } public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { - this._jupyterPythonPathFunction = func; + this.jupyterPythonPathFunction = func; } protected async getPythonPathOverride(uri: Uri | undefined): Promise { - if ( - uri === undefined || - this._jupyterPythonPathFunction === undefined || - !this._lspNotebooksExperiment.isInNotebooksExperiment() - ) { + if (!uri || !this.jupyterPythonPathFunction || !this.lspNotebooksExperiment.isInNotebooksExperiment()) { return undefined; } - const result = await this._jupyterPythonPathFunction(uri); + const result = await this.jupyterPythonPathFunction(uri); if (result) { traceLog( diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 9f742dc37731..42b0583a8cfe 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -19,58 +19,55 @@ import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; export class LspNotebooksExperiment implements IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; - private _pylanceExtensionChangeHandler: Disposable | undefined; + private pylanceExtensionChangeHandler: Disposable | undefined; - private _isJupyterInstalled = false; + private isJupyterInstalled = false; - private _isInNotebooksExperiment = false; + private isInExperiment = false; constructor( @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IJupyterExtensionDependencyManager) jupyterDependencyManager: IJupyterExtensionDependencyManager, ) { - if (!LspNotebooksExperiment._isPylanceInstalled()) { - this._pylanceExtensionChangeHandler = extensions.onDidChange( - this._pylanceExtensionsChangeHandler.bind(this), - ); + if (!LspNotebooksExperiment.isPylanceInstalled()) { + this.pylanceExtensionChangeHandler = extensions.onDidChange(this.pylanceExtensionsChangeHandler.bind(this)); } - this._isJupyterInstalled = jupyterDependencyManager.isJupyterExtensionInstalled; + this.isJupyterInstalled = jupyterDependencyManager.isJupyterExtensionInstalled; } public async activate(): Promise { - this._updateExperimentSupport(); + this.updateExperimentSupport(); } public async onJupyterInstalled(): Promise { - if (this._isJupyterInstalled) { + if (this.isJupyterInstalled) { return; } - if (LspNotebooksExperiment._jupyterSupportsNotebooksExperiment()) { - await this._waitForJupyterToRegisterPythonPathFunction(); - this._updateExperimentSupport(); + if (LspNotebooksExperiment.jupyterSupportsNotebooksExperiment()) { + await this.waitForJupyterToRegisterPythonPathFunction(); + this.updateExperimentSupport(); } - this._isJupyterInstalled = true; + this.isJupyterInstalled = true; } public isInNotebooksExperiment(): boolean { - return this._isInNotebooksExperiment; + return this.isInExperiment; } - private _updateExperimentSupport(): void { - const wasInExperiment = this._isInNotebooksExperiment; - + private updateExperimentSupport(): void { + const wasInExperiment = this.isInExperiment; const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled; if ( isInTreatmentGroup && - LspNotebooksExperiment._jupyterSupportsNotebooksExperiment() && - LspNotebooksExperiment._pylanceSupportsNotebooksExperiment() + LspNotebooksExperiment.jupyterSupportsNotebooksExperiment() && + LspNotebooksExperiment.pylanceSupportsNotebooksExperiment() ) { - this._isInNotebooksExperiment = true; + this.isInExperiment = true; sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); } @@ -78,28 +75,28 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService // 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._isInNotebooksExperiment === true) { + if (wasInExperiment === false && this.isInExperiment === true) { const watcher = this.serviceContainer.get(ILanguageServerWatcher); if (watcher) { watcher.restartLanguageServers(); } } - traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this._isInNotebooksExperiment}`); + traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInExperiment}`); } - private static _jupyterSupportsNotebooksExperiment(): boolean { + private static jupyterSupportsNotebooksExperiment(): boolean { const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - return jupyterVersion !== undefined && semver.satisfies(jupyterVersion, '>=2022.4.100'); + return jupyterVersion && semver.satisfies(jupyterVersion, '>=2022.4.100'); } - private static _pylanceSupportsNotebooksExperiment(): boolean { + private static pylanceSupportsNotebooksExperiment(): boolean { const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; - return pylanceVersion !== undefined && semver.satisfies(pylanceVersion, '>=2022.5.1-pre.1 || 9999.0.0-dev'); + return pylanceVersion && semver.satisfies(pylanceVersion, '>=2022.5.1-pre.1 || 9999.0.0-dev'); } - private async _waitForJupyterToRegisterPythonPathFunction(): Promise { - if (!this._isInNotebooksExperiment) { + private async waitForJupyterToRegisterPythonPathFunction(): Promise { + if (!this.isInExperiment) { return; } @@ -125,16 +122,16 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } } - private static _isPylanceInstalled(): boolean { + private static isPylanceInstalled(): boolean { return !!extensions.getExtension(PYLANCE_EXTENSION_ID); } - private async _pylanceExtensionsChangeHandler(): Promise { - if (LspNotebooksExperiment._isPylanceInstalled() && this._pylanceExtensionChangeHandler) { - this._pylanceExtensionChangeHandler.dispose(); - this._pylanceExtensionChangeHandler = undefined; + private async pylanceExtensionsChangeHandler(): Promise { + if (LspNotebooksExperiment.isPylanceInstalled() && this.pylanceExtensionChangeHandler) { + this.pylanceExtensionChangeHandler.dispose(); + this.pylanceExtensionChangeHandler = undefined; - this._updateExperimentSupport(); + this.updateExperimentSupport(); } } } diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index 9de400396932..dd0108bd6f32 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -10,11 +10,11 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { Commands } from '../commands'; +import { NodeLanguageClientMiddleware } from './languageClientMiddleware'; import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; -import { NodeLanguageClientMiddleware } from './languageClientMiddleware'; export class NodeLanguageServerManager implements ILanguageServerManager { private resource!: Resource; diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index f608e9bec3da..0518fac170e9 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -55,10 +55,10 @@ suite('Pylance Language Server - Analysis Options', () => { expect(filter).to.be.equal(PYTHON); }); - test('Document filter matches all python language schemes when in virtual workspace', async () => { + test('Document filter matches all python language schemes when in virtual workspace', () => { workspace.reset(); workspace.setup((w) => w.isVirtualWorkspace).returns(() => true); - const filter = await analysisOptions.getDocumentFilters(); + const filter = analysisOptions.getDocumentFilters(); assert.deepEqual(filter, [{ language: PYTHON_LANGUAGE }]); }); From bbb49b25beef8eeaf114224611ee160b2093e989 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 14:07:59 -0700 Subject: [PATCH 26/41] Remove need to register func on middleware --- .../node/languageClientMiddleware.ts | 19 +++++++++++-------- .../activation/node/lspNotebooksExperiment.ts | 4 ---- src/client/activation/node/manager.ts | 9 --------- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index ebea6d42a27c..8453e4dfd7c9 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -7,6 +7,7 @@ import { localize } from 'vscode-nls'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; import { IDisposableRegistry, IExtensions } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; +import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; import { traceLog } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; @@ -18,8 +19,6 @@ import { LspNotebooksExperiment } from './lspNotebooksExperiment'; export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { private readonly lspNotebooksExperiment: LspNotebooksExperiment; - private jupyterPythonPathFunction: ((uri: Uri) => Promise) | undefined = undefined; - public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { super(serviceContainer, LanguageServerType.Node, sendTelemetryEvent, serverVersion); @@ -61,16 +60,20 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { ); } - public registerJupyterPythonPathFunction(func: (uri: Uri) => Promise): void { - this.jupyterPythonPathFunction = func; - } - protected async getPythonPathOverride(uri: Uri | undefined): Promise { - if (!uri || !this.jupyterPythonPathFunction || !this.lspNotebooksExperiment.isInNotebooksExperiment()) { + if (!uri || !this.lspNotebooksExperiment.isInNotebooksExperiment()) { + return undefined; + } + + const jupyterExtensionIntegration = this.serviceContainer?.get( + JupyterExtensionIntegration, + ); + const jupyterPythonPathFunction = jupyterExtensionIntegration?.getJupyterPythonPathFunction(); + if (!jupyterPythonPathFunction) { return undefined; } - const result = await this.jupyterPythonPathFunction(uri); + const result = await jupyterPythonPathFunction(uri); if (result) { traceLog( diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 42b0583a8cfe..b6a0e9c5ad00 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -96,10 +96,6 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } private async waitForJupyterToRegisterPythonPathFunction(): Promise { - if (!this.isInExperiment) { - return; - } - const jupyterExtensionIntegration = this.serviceContainer.get( JupyterExtensionIntegration, ); diff --git a/src/client/activation/node/manager.ts b/src/client/activation/node/manager.ts index dd0108bd6f32..63c866748ce6 100644 --- a/src/client/activation/node/manager.ts +++ b/src/client/activation/node/manager.ts @@ -14,7 +14,6 @@ import { NodeLanguageClientMiddleware } from './languageClientMiddleware'; import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types'; import { traceDecoratorError, traceDecoratorVerbose } from '../../logging'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; -import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; export class NodeLanguageServerManager implements ILanguageServerManager { private resource!: Resource; @@ -121,14 +120,6 @@ export class NodeLanguageServerManager implements ILanguageServerManager { this.middleware = new NodeLanguageClientMiddleware(this.serviceContainer, this.lsVersion); options.middleware = this.middleware; - const jupyterExtensionIntegration = this.serviceContainer.get( - JupyterExtensionIntegration, - ); - const jupyterPythonPathFunction = jupyterExtensionIntegration.getJupyterPythonPathFunction(); - if (jupyterPythonPathFunction) { - this.middleware.registerJupyterPythonPathFunction(jupyterPythonPathFunction); - } - // Make sure the middleware is connected if we restart and we we're already connected. if (this.connected) { this.middleware.connect(); From 83421a6e514f8930aeb0d64cbaf0bc35995d8511 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 14:08:42 -0700 Subject: [PATCH 27/41] Allow any -dev pylance build --- src/client/activation/node/lspNotebooksExperiment.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index b6a0e9c5ad00..e16b169741d9 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -92,7 +92,10 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private static pylanceSupportsNotebooksExperiment(): boolean { const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; - return pylanceVersion && semver.satisfies(pylanceVersion, '>=2022.5.1-pre.1 || 9999.0.0-dev'); + return ( + pylanceVersion && + (semver.gte(pylanceVersion, '2022.5.1-pre.1') || semver.prerelease(pylanceVersion)?.includes('dev')) + ); } private async waitForJupyterToRegisterPythonPathFunction(): Promise { From e12df2745675ae448573d82ba5063cf1b9a646f2 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 14:12:17 -0700 Subject: [PATCH 28/41] Adjusted logging --- src/client/activation/node/lspNotebooksExperiment.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index e16b169741d9..167c09bb2b14 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -82,7 +82,9 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } } - traceLog(`LspNotebooksExperiment: activate: isInNotebooksExperiment = ${this.isInExperiment}`); + if (this.isInExperiment) { + traceLog('Pylance LSP Notebooks experiment is enabled.'); + } } private static jupyterSupportsNotebooksExperiment(): boolean { @@ -107,17 +109,16 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService for (let tryCount = 0; tryCount < 20; tryCount += 1) { const jupyterPythonPathFunction = jupyterExtensionIntegration.getJupyterPythonPathFunction(); if (jupyterPythonPathFunction) { - traceVerbose(`Jupyter has called registration method...`); + traceVerbose(`Jupyter called registerJupyterPythonPathFunction`); success = true; break; } - traceVerbose(`Waiting for Jupyter to call registration method...`); await sleep(500); } if (!success) { - traceVerbose(`Timed out waiting for Jupyter to call registration method...`); + traceVerbose(`Timed out waiting for Jupyter to call registerJupyterPythonPathFunction`); } } From 4191e8d3ee822aa0b2a48b2a696277e1619cee26 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 15:20:48 -0700 Subject: [PATCH 29/41] Log specific reason that experiment is not enabled; 'was' needs to start off undefined --- .../activation/node/lspNotebooksExperiment.ts | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 167c09bb2b14..5bb94255ab88 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -23,7 +23,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private isJupyterInstalled = false; - private isInExperiment = false; + private isInExperiment: boolean | undefined; constructor( @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, @@ -55,19 +55,30 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService } public isInNotebooksExperiment(): boolean { - return this.isInExperiment; + return this.isInExperiment ?? false; } private updateExperimentSupport(): void { const wasInExperiment = this.isInExperiment; const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled; - if ( - isInTreatmentGroup && - LspNotebooksExperiment.jupyterSupportsNotebooksExperiment() && - LspNotebooksExperiment.pylanceSupportsNotebooksExperiment() - ) { + this.isInExperiment = false; + if (!isInTreatmentGroup) { + traceLog(`LSP Notebooks experiment is disabled -- not in treatment group`); + } 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 { this.isInExperiment = true; + traceLog(`LSP Notebooks experiment is enabled`); + } + + if (this.isInExperiment) { sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS); } @@ -81,10 +92,6 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService watcher.restartLanguageServers(); } } - - if (this.isInExperiment) { - traceLog('Pylance LSP Notebooks experiment is enabled.'); - } } private static jupyterSupportsNotebooksExperiment(): boolean { @@ -126,6 +133,10 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService 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(); From 1c4121a74bfa7dad5689add4b4d67324eae8dffb Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 15:31:01 -0700 Subject: [PATCH 30/41] Add back experiment check lost during refactoring --- src/client/activation/node/languageClientMiddleware.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 8453e4dfd7c9..3ae3f01a1971 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -31,7 +31,11 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { const extensions = serviceContainer.get(IExtensions); // Enable notebook support if jupyter support is installed - if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) { + if ( + jupyterDependencyManager && + jupyterDependencyManager.isJupyterExtensionInstalled && + !this.lspNotebooksExperiment.isInNotebooksExperiment() + ) { this.notebookAddon = createHidingMiddleware(); } From aa9d869823ab93f83e1c53156a7f736e1c369c01 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 16:42:33 -0700 Subject: [PATCH 31/41] localize is failing on kernel switch; reverting to static string --- src/client/activation/node/languageClientMiddleware.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 3ae3f01a1971..2e27caaaf5e0 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -3,7 +3,6 @@ import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; import { Uri } from 'vscode'; -import { localize } from 'vscode-nls'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; import { IDisposableRegistry, IExtensions } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; @@ -80,13 +79,7 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { const result = await jupyterPythonPathFunction(uri); if (result) { - traceLog( - localize( - 'Interpreters.pythonInterpreterPathFromJupyter', - 'Jupyter provided interpreter path override: {0}', - result, - ), - ); + traceLog(`Jupyter provided interpreter path override: ${result}`); } return result; From e7ff8410faddc84651e4c152f093fa4170baae03 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 18:33:50 -0700 Subject: [PATCH 32/41] Fix Jedi middleware logic --- .../jedi/languageClientMiddleware.ts | 9 ++-- .../activation/languageClientMiddleware.ts | 51 ++++++++++++++++++ .../node/languageClientMiddleware.ts | 54 +++++-------------- 3 files changed, 69 insertions(+), 45 deletions(-) create mode 100644 src/client/activation/languageClientMiddleware.ts diff --git a/src/client/activation/jedi/languageClientMiddleware.ts b/src/client/activation/jedi/languageClientMiddleware.ts index 6e1de4bfe3ff..656c47309bb9 100644 --- a/src/client/activation/jedi/languageClientMiddleware.ts +++ b/src/client/activation/jedi/languageClientMiddleware.ts @@ -2,13 +2,12 @@ // Licensed under the MIT License. import { IServiceContainer } from '../../ioc/types'; -import { sendTelemetryEvent } from '../../telemetry'; - -import { LanguageClientMiddlewareBase } from '../languageClientMiddlewareBase'; +import { LanguageClientMiddleware } from '../languageClientMiddleware'; import { LanguageServerType } from '../types'; -export class JediLanguageClientMiddleware extends LanguageClientMiddlewareBase { +export class JediLanguageClientMiddleware extends LanguageClientMiddleware { public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { - super(serviceContainer, LanguageServerType.Jedi, sendTelemetryEvent, serverVersion); + super(serviceContainer, LanguageServerType.Jedi, serverVersion); + this.setupHidingMiddleware(serviceContainer); } } diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts new file mode 100644 index 000000000000..715e364c8332 --- /dev/null +++ b/src/client/activation/languageClientMiddleware.ts @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { IJupyterExtensionDependencyManager } from '../common/application/types'; +import { IDisposableRegistry, IExtensions } from '../common/types'; +import { IServiceContainer } from '../ioc/types'; +import { sendTelemetryEvent } from '../telemetry'; + +import { LanguageClientMiddlewareBase } from './languageClientMiddlewareBase'; +import { LanguageServerType } from './types'; + +import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; + +export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { + public constructor(serviceContainer: IServiceContainer, serverType: LanguageServerType, serverVersion?: string) { + super(serviceContainer, serverType, sendTelemetryEvent, serverVersion); + } + + protected setupHidingMiddleware(serviceContainer: IServiceContainer) { + const jupyterDependencyManager = serviceContainer.get( + IJupyterExtensionDependencyManager, + ); + const disposables = serviceContainer.get(IDisposableRegistry) || []; + const extensions = serviceContainer.get(IExtensions); + + // Enable notebook support if jupyter support is installed + if (this.shouldCreateHidingMiddleware(jupyterDependencyManager)) { + this.notebookAddon = createHidingMiddleware(); + } + + disposables.push( + extensions?.onDidChange(async () => { + await this.onExtensionChange(jupyterDependencyManager); + }), + ); + } + + protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { + return jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled; + } + + protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise { + if (jupyterDependencyManager) { + if (this.notebookAddon && !this.shouldCreateHidingMiddleware(jupyterDependencyManager)) { + this.notebookAddon = undefined; + } else if (!this.notebookAddon && this.shouldCreateHidingMiddleware(jupyterDependencyManager)) { + this.notebookAddon = createHidingMiddleware(); + } + } + } +} diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index 2e27caaaf5e0..5c124ef461ba 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -1,66 +1,40 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware'; import { Uri } from 'vscode'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; -import { IDisposableRegistry, IExtensions } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; import { traceLog } from '../../logging'; -import { sendTelemetryEvent } from '../../telemetry'; +import { LanguageClientMiddleware } from '../languageClientMiddleware'; -import { LanguageClientMiddlewareBase } from '../languageClientMiddlewareBase'; import { LanguageServerType } from '../types'; import { LspNotebooksExperiment } from './lspNotebooksExperiment'; -export class NodeLanguageClientMiddleware extends LanguageClientMiddlewareBase { +export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { private readonly lspNotebooksExperiment: LspNotebooksExperiment; public constructor(serviceContainer: IServiceContainer, serverVersion?: string) { - super(serviceContainer, LanguageServerType.Node, sendTelemetryEvent, serverVersion); + super(serviceContainer, LanguageServerType.Node, serverVersion); this.lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); + this.setupHidingMiddleware(serviceContainer); + } - const jupyterDependencyManager = serviceContainer.get( - IJupyterExtensionDependencyManager, + protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { + return ( + super.shouldCreateHidingMiddleware(jupyterDependencyManager) && + !this.lspNotebooksExperiment.isInNotebooksExperiment() ); - const disposables = serviceContainer.get(IDisposableRegistry) || []; - const extensions = serviceContainer.get(IExtensions); + } - // Enable notebook support if jupyter support is installed - if ( - jupyterDependencyManager && - jupyterDependencyManager.isJupyterExtensionInstalled && - !this.lspNotebooksExperiment.isInNotebooksExperiment() - ) { - this.notebookAddon = createHidingMiddleware(); + protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise { + if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) { + await this.lspNotebooksExperiment.onJupyterInstalled(); } - disposables.push( - extensions?.onDidChange(async () => { - if (jupyterDependencyManager) { - if (jupyterDependencyManager.isJupyterExtensionInstalled) { - await this.lspNotebooksExperiment.onJupyterInstalled(); - } - - if ( - this.notebookAddon && - (!jupyterDependencyManager.isJupyterExtensionInstalled || - this.lspNotebooksExperiment.isInNotebooksExperiment()) - ) { - this.notebookAddon = undefined; - } else if ( - !this.notebookAddon && - jupyterDependencyManager.isJupyterExtensionInstalled && - !this.lspNotebooksExperiment.isInNotebooksExperiment() - ) { - this.notebookAddon = createHidingMiddleware(); - } - } - }), - ); + super.onExtensionChange(jupyterDependencyManager); } protected async getPythonPathOverride(uri: Uri | undefined): Promise { From 60366e062523cc9a6b362c77c39526e4765d0fd5 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 20:05:32 -0700 Subject: [PATCH 33/41] Current Jupyter releases don't support experiment --- src/client/activation/node/lspNotebooksExperiment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 5bb94255ab88..8cf76baa3b8f 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -96,7 +96,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private static jupyterSupportsNotebooksExperiment(): boolean { const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - return jupyterVersion && semver.satisfies(jupyterVersion, '>=2022.4.100'); + return jupyterVersion && (semver.gte(jupyterVersion, '2022.5.0') || semver.eq(jupyterVersion, '2022.4.100')); } private static pylanceSupportsNotebooksExperiment(): boolean { From af38e1ec617331692c1e56320a53d290d86344eb Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 20:12:09 -0700 Subject: [PATCH 34/41] Added doc comment on setupHidingMiddleware --- src/client/activation/languageClientMiddleware.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/client/activation/languageClientMiddleware.ts b/src/client/activation/languageClientMiddleware.ts index 715e364c8332..110d7461c615 100644 --- a/src/client/activation/languageClientMiddleware.ts +++ b/src/client/activation/languageClientMiddleware.ts @@ -16,6 +16,13 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase { super(serviceContainer, serverType, sendTelemetryEvent, serverVersion); } + /** + * Creates the HidingMiddleware if needed and sets up code to do so if needed after + * Jupyter is installed. + * + * This method should be called from the constructor of derived classes. It is separated + * from the constructor to allow derived classes to initialize before it is called. + */ protected setupHidingMiddleware(serviceContainer: IServiceContainer) { const jupyterDependencyManager = serviceContainer.get( IJupyterExtensionDependencyManager, From b06e082dd720ebd112de4c0fba5cde98aac9a273 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 6 May 2022 21:38:51 -0700 Subject: [PATCH 35/41] Don't enable experiment when using Jedi --- src/client/activation/node/lspNotebooksExperiment.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 8cf76baa3b8f..35318accafa1 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -7,7 +7,7 @@ import { IConfigurationService } from '../../common/types'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constants'; -import { IExtensionSingleActivationService } from '../types'; +import { IExtensionSingleActivationService, LanguageServerType } from '../types'; import { traceLog, traceVerbose } from '../../logging'; import { IJupyterExtensionDependencyManager } from '../../common/application/types'; import { ILanguageServerWatcher } from '../../languageServer/types'; @@ -61,9 +61,12 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private updateExperimentSupport(): void { const wasInExperiment = this.isInExperiment; const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled; + const languageServerType = this.configurationService.getSettings().languageServer; this.isInExperiment = false; - if (!isInTreatmentGroup) { + if (languageServerType !== LanguageServerType.Node) { + traceLog(`LSP Notebooks experiment is disabled -- not using Pylance`); + } else if (!isInTreatmentGroup) { traceLog(`LSP Notebooks experiment is disabled -- not in treatment group`); } else if (!LspNotebooksExperiment.isJupyterInstalled()) { traceLog(`LSP Notebooks experiment is disabled -- Jupyter disabled or not installed`); From 77c04c5404cff3e95548affe5478cb9af296c589 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 9 May 2022 18:54:51 -0700 Subject: [PATCH 36/41] Remove document filter, we'll rely on selector in Pylance --- src/client/activation/node/analysisOptions.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 3ad46bed234a..49573315e1ef 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,9 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { WorkspaceFolder } from 'vscode'; import { LanguageClientOptions } from 'vscode-languageclient'; -import { DocumentFilter } from 'vscode-languageserver-protocol'; import { IWorkspaceService } from '../../common/application/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; @@ -28,20 +26,4 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), } as unknown) as LanguageClientOptions; } - - protected getDocumentFilters(_workspaceFolder?: WorkspaceFolder): DocumentFilter[] { - const filters = super.getDocumentFilters(_workspaceFolder); - - if (this.lspNotebooksExperiment.isInNotebooksExperiment()) { - return [ - { language: 'python' }, - { - notebook: { notebookType: 'jupyter-notebook', pattern: '**/*.ipynb' }, - language: 'python', - }, - ]; - } - - return filters; - } } From cc2ef358db218692802dfaf3c53f42fba551b37a Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 9 May 2022 22:32:57 -0700 Subject: [PATCH 37/41] Apply suggestions from code review Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/client/languageServer/watcher.ts | 8 +++----- src/client/telemetry/index.ts | 4 ++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index f35e213b00d9..3188c077cdd6 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -187,12 +187,10 @@ export class LanguageServerWatcher } public async restartLanguageServers(): Promise { - const workspacesUris = this.workspaceService.workspaceFolders?.map((workspace) => workspace.uri) ?? []; - - workspacesUris.forEach(async (resource) => { - const lsResource = this.getWorkspaceUri(resource); + this.workspaceLanguageServers.forEach(async (_, resourceString) => { + const resource = Uri.parse(resourceString); this.stopLanguageServer(resource); - await this.startLanguageServer(this.languageServerType, lsResource); + await this.startLanguageServer(this.languageServerType, resource); }); } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c16b3f19bf32..70127c684721 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1413,6 +1413,10 @@ export interface IEventNamePropertyMapping { * Telemetry event sent when the installed versions of Python, Jupyter, and Pylance are all capable * of supporting the LSP notebooks experiment. This does not indicate that the experiment is enabled. */ + + /* __GDPR__ + "python_experiments_lsp_notebooks" : { "owner": "luabud" } + */ [EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS]: unknown; /** * Telemetry event sent once on session start with details on which experiments are opted into and opted out from. From 4b24ad7f1ecb2bfdb920625ad43307c7fc184f26 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 10 May 2022 11:16:44 -0700 Subject: [PATCH 38/41] Updates after rebasing on top of LSP upgrade --- src/client/activation/languageClientMiddlewareBase.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 446068c4a841..2e240231fa64 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -87,11 +87,8 @@ export class LanguageClientMiddlewareBase implements Middleware { i ] as LSPObject & { pythonPath: string; _envPYTHONPATH: string }; - settingDict.pythonPath = await this.getPythonPathOverride(uri); - - if (!settingDict.pythonPath) { - settingDict.pythonPath = configService.getSettings(uri).pythonPath; - } + settingDict.pythonPath = + (await this.getPythonPathOverride(uri)) ?? configService.getSettings(uri).pythonPath; const env = await envService.getEnvironmentVariables(uri); const envPYTHONPATH = env.PYTHONPATH; From eac854e72618ba2104599a48cf3c74f5671857af Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 16 May 2022 18:47:57 -0700 Subject: [PATCH 39/41] Post-rebase fixes --- src/client/activation/jedi/languageServerProxy.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index ffed40231796..fb60b539161a 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -56,11 +56,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { interpreter: PythonEnvironment | undefined, options: LanguageClientOptions, ): Promise { - if (this.languageServerTask) { - await this.languageServerTask; - return; - } - this.lsVersion = (options.middleware ? (options.middleware).serverVersion : undefined) ?? '0.19.3'; @@ -97,9 +92,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { const d = this.disposables.shift()!; d.dispose(); } - - this.languageServerTask = this.languageClient.start(); - await this.languageServerTask; } // eslint-disable-next-line class-methods-use-this From 3189554adc703b52bc97a369631f43ebe170f21f Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 16 May 2022 18:53:08 -0700 Subject: [PATCH 40/41] Update Pylance and Jupyter version thresholds --- src/client/activation/node/lspNotebooksExperiment.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 35318accafa1..601c8b7b3dd9 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -99,14 +99,17 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService private static jupyterSupportsNotebooksExperiment(): boolean { const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; - return jupyterVersion && (semver.gte(jupyterVersion, '2022.5.0') || semver.eq(jupyterVersion, '2022.4.100')); + return ( + jupyterVersion && + (semver.gt(jupyterVersion, '2022.5.1001382326') || semver.eq(jupyterVersion, '2022.4.100')) + ); } private static pylanceSupportsNotebooksExperiment(): boolean { const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; return ( pylanceVersion && - (semver.gte(pylanceVersion, '2022.5.1-pre.1') || semver.prerelease(pylanceVersion)?.includes('dev')) + (semver.gte(pylanceVersion, '2022.5.3-pre.1') || semver.prerelease(pylanceVersion)?.includes('dev')) ); } From bd703020ae4612c682ee941e44b82b7786c08f13 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 17 May 2022 09:29:31 -0700 Subject: [PATCH 41/41] Update Jupyter version threshold --- src/client/activation/node/lspNotebooksExperiment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/node/lspNotebooksExperiment.ts b/src/client/activation/node/lspNotebooksExperiment.ts index 601c8b7b3dd9..a94536cad1d8 100644 --- a/src/client/activation/node/lspNotebooksExperiment.ts +++ b/src/client/activation/node/lspNotebooksExperiment.ts @@ -101,7 +101,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version; return ( jupyterVersion && - (semver.gt(jupyterVersion, '2022.5.1001382326') || semver.eq(jupyterVersion, '2022.4.100')) + (semver.gt(jupyterVersion, '2022.5.1001391015') || semver.eq(jupyterVersion, '2022.4.100')) ); }