diff --git a/.eslintignore b/.eslintignore index 49fd184755a5..9c9908ede444 100644 --- a/.eslintignore +++ b/.eslintignore @@ -382,7 +382,6 @@ src/client/providers/docStringFoldingProvider.ts src/client/providers/linterProvider.ts src/client/providers/simpleRefactorProvider.ts src/client/providers/completionProvider.ts -src/client/providers/jediProxy.ts src/client/providers/definitionProvider.ts src/client/providers/referenceProvider.ts src/client/providers/terminalProvider.ts diff --git a/package.json b/package.json index 444949114355..a144154582e8 100644 --- a/package.json +++ b/package.json @@ -1044,7 +1044,6 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "tryPylance", - "jediLSP", "debuggerDataViewer", "pythonSendEntireLineToREPL", "pythonTensorboardExperiment", @@ -1074,7 +1073,6 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "tryPylance", - "jediLSP", "debuggerDataViewer", "pythonSendEntireLineToREPL", "pythonTensorboardExperiment", diff --git a/src/client/activation/jedi/analysisOptions.ts b/src/client/activation/jedi/analysisOptions.ts index f2ba1555fcd8..6eb2e296c31c 100644 --- a/src/client/activation/jedi/analysisOptions.ts +++ b/src/client/activation/jedi/analysisOptions.ts @@ -1,8 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import { WorkspaceFolder } from 'vscode'; +import { IWorkspaceService } from '../../common/application/types'; +import { IConfigurationService, Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { PythonEnvironment } from '../../pythonEnvironments/info'; import { LanguageServerAnalysisOptionsWithEnv } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; @@ -10,15 +15,47 @@ import { ILanguageServerOutputChannel } from '../types'; @injectable() export class JediLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsWithEnv { - // eslint-disable-next-line @typescript-eslint/no-useless-constructor + private resource: Resource | undefined; + constructor( @inject(IEnvironmentVariablesProvider) envVarsProvider: IEnvironmentVariablesProvider, @inject(ILanguageServerOutputChannel) lsOutputChannel: ILanguageServerOutputChannel, + @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, ) { super(envVarsProvider, lsOutputChannel); + this.resource = undefined; + } + + public async initialize(resource: Resource, interpreter: PythonEnvironment | undefined) { + this.resource = resource; + return super.initialize(resource, interpreter); + } + + protected getWorkspaceFolder(): WorkspaceFolder | undefined { + return this.workspace.getWorkspaceFolder(this.resource); } protected async getInitializationOptions() { + const pythonSettings = this.configurationService.getSettings(this.resource); + const workspacePath = this.getWorkspaceFolder()?.uri.fsPath; + const extraPaths = pythonSettings.autoComplete + ? pythonSettings.autoComplete.extraPaths.map((extraPath) => { + if (path.isAbsolute(extraPath)) { + return extraPath; + } + return workspacePath ? path.join(workspacePath, extraPath) : ''; + }) + : []; + + if (workspacePath) { + extraPaths.unshift(workspacePath); + } + + const distinctExtraPaths = extraPaths + .filter((value) => value.length > 0) + .filter((value, index, self) => self.indexOf(value) === index); + return { markupKindPreferred: 'markdown', completion: { @@ -31,6 +68,9 @@ export class JediLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt didSave: true, didChange: true, }, + workspace: { + extraPaths: distinctExtraPaths, + }, }; } } diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index e3564a8b5e79..b76418e75940 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -1,9 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +/* eslint-disable max-classes-per-file, @typescript-eslint/no-explicit-any */ + import { ChildProcess } from 'child_process'; import * as path from 'path'; -// @ts-ignore +// @ts-ignore pidusage does not work in strict mode import * as pidusage from 'pidusage'; import { CancellationToken, CancellationTokenSource, CompletionItemKind, Disposable, SymbolKind, Uri } from 'vscode'; import '../common/extensions'; @@ -21,7 +23,7 @@ import { IServiceContainer } from '../ioc/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; -import { traceError, traceWarning } from './../common/logger'; +import { traceError, traceWarning } from '../common/logger'; const pythonVSCodeTypeMappings = new Map(); pythonVSCodeTypeMappings.set('none', CompletionItemKind.Value); @@ -153,23 +155,41 @@ type JediProxyPayload = { export class JediProxy implements Disposable { private proc?: ChildProcess; + private pythonSettings: IPythonSettings; - private cmdId: number = 0; + + private cmdId = 0; + private lastKnownPythonInterpreter: string; + private previousData = ''; + private commands = new Map>(); + private commandQueue: number[] = []; + private spawnRetryAttempts = 0; + private additionalAutoCompletePaths: string[] = []; + private workspacePath: string; + private languageServerStarted!: Deferred; + private initialized: Deferred; + private environmentVariablesProvider!: IEnvironmentVariablesProvider; - private ignoreJediMemoryFootprint: boolean = false; + + private ignoreJediMemoryFootprint = false; + private pidUsageFailures = { timer: new StopWatch(), counter: 0 }; + private lastCmdIdProcessed?: number; + private lastCmdIdProcessedForPidUsage?: number; + private readonly disposables: Disposable[] = []; + private timer?: NodeJS.Timer | number; public constructor( @@ -188,11 +208,11 @@ export class JediProxy implements Disposable { this.checkJediMemoryFootprint().ignoreErrors(); } - private static getProperty(o: object, name: string): T { - return (o as any)[name]; + private static getProperty(o: Record, name: string): T { + return o[name]; } - public dispose() { + public dispose(): void { while (this.disposables.length > 0) { const disposable = this.disposables.pop(); if (disposable) { @@ -200,6 +220,7 @@ export class JediProxy implements Disposable { } } if (this.timer) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any clearTimeout(this.timer as any); } this.killProcess(); @@ -227,7 +248,7 @@ export class JediProxy implements Disposable { this.commandQueue.push(executionCmd.id); } catch (ex) { traceError(ex); - //If 'This socket is closed.' that means process didn't start at all (at least not properly). + // If 'This socket is closed.' that means process didn't start at all (at least not properly). if (ex.message === 'This socket is closed.') { this.killProcess(); } else { @@ -247,6 +268,7 @@ export class JediProxy implements Disposable { this.handleError('spawnProcess', ex); }); } + private shouldCheckJediMemoryFootprint() { if (this.ignoreJediMemoryFootprint || this.pythonSettings.jediMemoryLimit === -1) { return false; @@ -262,6 +284,7 @@ export class JediProxy implements Disposable { } return true; } + private async checkJediMemoryFootprint() { // Check memory footprint periodically. Do not check on every request due to // the performance impact. See https://github.com/soyuka/pidusage - on Windows @@ -276,6 +299,7 @@ export class JediProxy implements Disposable { } this.timer = setTimeout(() => this.checkJediMemoryFootprint(), 15 * 1000); } + private async checkJediMemoryFootprintImpl(): Promise { if (!this.proc || this.proc.killed) { return; @@ -321,24 +345,27 @@ export class JediProxy implements Disposable { deferred.resolve(); }); + // eslint-disable-next-line consistent-return return deferred.promise; } // @debounce(1500) @swallowExceptions('JediProxy') private async environmentVariablesChangeHandler() { - const newAutoComletePaths = await this.buildAutoCompletePaths(); - if (this.additionalAutoCompletePaths.join(',') !== newAutoComletePaths.join(',')) { - this.additionalAutoCompletePaths = newAutoComletePaths; + const newAutoCompletePaths = await this.buildAutoCompletePaths(); + if (this.additionalAutoCompletePaths.join(',') !== newAutoCompletePaths.join(',')) { + this.additionalAutoCompletePaths = newAutoCompletePaths; this.restartLanguageServer().ignoreErrors(); } } + @swallowExceptions('JediProxy') private async startLanguageServer(): Promise { const newAutoComletePaths = await this.buildAutoCompletePaths(); this.additionalAutoCompletePaths = newAutoComletePaths; return this.restartLanguageServer(); } + private restartLanguageServer(): Promise { this.killProcess(); this.clearPendingRequests(); @@ -360,11 +387,14 @@ export class JediProxy implements Disposable { if (this.proc) { this.proc.kill(); } - } catch (ex) {} + } catch (ex) { + // intentionally left blank + } this.proc = undefined; } - private handleError(source: string, errorMessage: string) { + // eslint-disable-next-line class-methods-use-this + private handleError(source: string, errorMessage: string): void { traceError(`${source} jediProxy`, `Error (${source}) ${errorMessage}`); } @@ -412,7 +442,8 @@ export class JediProxy implements Disposable { const data = output.out; // Possible there was an exception in parsing the data returned, // so append the data and then parse it. - const dataStr = (this.previousData = `${this.previousData}${data}`); + this.previousData = `${this.previousData}${data}`; + const dataStr = this.previousData; let responses: any[]; try { @@ -444,7 +475,7 @@ export class JediProxy implements Disposable { return; } this.lastCmdIdProcessed = cmd.id; - if (JediProxy.getProperty(response, 'arguments')) { + if (JediProxy.getProperty(response, 'arguments')) { this.commandQueue.splice(this.commandQueue.indexOf(cmd.id), 1); return; } @@ -473,9 +504,10 @@ export class JediProxy implements Disposable { (error) => this.handleError('subscription.error', `${error}`), ); } + private getCommandHandler( command: CommandType, - ): undefined | ((command: IExecutionCommand, response: object) => void) { + ): undefined | ((command: IExecutionCommand, response: Record) => void) { switch (command) { case CommandType.Completions: return this.onCompletion; @@ -490,10 +522,11 @@ export class JediProxy implements Disposable { case CommandType.Arguments: return this.onArguments; default: - return; } + return undefined; } - private onCompletion(command: IExecutionCommand, response: object): void { + + private onCompletion(command: IExecutionCommand, response: Record): void { let results = JediProxy.getProperty(response, 'results'); results = Array.isArray(results) ? results : []; results.forEach((item) => { @@ -509,7 +542,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, completionResult); } - private onDefinition(command: IExecutionCommand, response: object): void { + private onDefinition(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); const defResult: IDefinitionResult = { requestId: command.id, @@ -537,7 +570,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResult); } - private onHover(command: IExecutionCommand, response: object): void { + private onHover(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); const defResult: IHoverResult = { requestId: command.id, @@ -554,7 +587,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResult); } - private onSymbols(command: IExecutionCommand, response: object): void { + private onSymbols(command: IExecutionCommand, response: Record): void { let defs = JediProxy.getProperty(response, 'results'); defs = Array.isArray(defs) ? defs : []; const defResults: ISymbolResult = { @@ -581,7 +614,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResults); } - private onUsages(command: IExecutionCommand, response: object): void { + private onUsages(command: IExecutionCommand, response: Record): void { let defs = JediProxy.getProperty(response, 'results'); defs = Array.isArray(defs) ? defs : []; const refResult: IReferenceResult = { @@ -599,7 +632,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, refResult); } - private onArguments(command: IExecutionCommand, response: object): void { + private onArguments(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); this.safeResolve(command, { @@ -617,6 +650,7 @@ export class JediProxy implements Disposable { try { this.safeResolve(cmd1, undefined); } catch (ex) { + // Intentionally left blank } finally { this.commands.delete(id); } @@ -663,6 +697,7 @@ export class JediProxy implements Disposable { return ''; } } + private async buildAutoCompletePaths(): Promise { const filePathPromises = [ // Sysprefix. @@ -703,6 +738,7 @@ export class JediProxy implements Disposable { return []; } } + private getEnvironmentVariablesProvider() { if (!this.environmentVariablesProvider) { this.environmentVariablesProvider = this.serviceContainer.get( @@ -714,6 +750,7 @@ export class JediProxy implements Disposable { } return this.environmentVariablesProvider; } + private getConfig(): JediProxyConfig { // Add support for paths relative to workspace. const extraPaths = this.pythonSettings.autoComplete @@ -747,6 +784,7 @@ export class JediProxy implements Disposable { }; } + // eslint-disable-next-line class-methods-use-this private safeResolve( command: IExecutionCommand | undefined | null, result: ICommandResult | PromiseLike | undefined, @@ -827,6 +865,7 @@ export interface IAutoCompleteItem { kind: SymbolKind; text: string; description: string; + // eslint-disable-next-line camelcase raw_docstring: string; rightLabel: string; } @@ -865,12 +904,13 @@ export class JediProxyHandler implements Disposable { return this.jediProxy; } - public dispose() { + public dispose(): void { if (this.jediProxy) { this.jediProxy.dispose(); } } + // eslint-disable-next-line @typescript-eslint/no-unused-vars public sendCommand(cmd: ICommand, _token?: CancellationToken): Promise { const executionCmd = >cmd; executionCmd.id = executionCmd.id || this.jediProxy.getNextCommandId(); diff --git a/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts b/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts new file mode 100644 index 000000000000..e7a1f5011edb --- /dev/null +++ b/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as path from 'path'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { EventEmitter, Uri, WorkspaceFolder } from 'vscode'; +import { JediLanguageServerAnalysisOptions } from '../../../client/activation/jedi/analysisOptions'; +import { ILanguageServerAnalysisOptions, ILanguageServerOutputChannel } from '../../../client/activation/types'; +import { IWorkspaceService } from '../../../client/common/application/types'; +import { WorkspaceService } from '../../../client/common/application/workspace'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { IConfigurationService } from '../../../client/common/types'; +import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; + +suite('Jedi LSP - analysis Options', () => { + const workspacePath = path.join('this', 'is', 'fake', 'workspace', 'path'); + const expectedWorkspacePath = path.sep + workspacePath; + + let envVarsProvider: IEnvironmentVariablesProvider; + let lsOutputChannel: ILanguageServerOutputChannel; + let configurationService: IConfigurationService; + let workspaceService: IWorkspaceService; + + let analysisOptions: ILanguageServerAnalysisOptions; + + class MockWorkspaceFolder implements WorkspaceFolder { + public uri: Uri; + + public name: string; + + public ownedResources = new Set(); + + constructor(folder: string, public index: number = 0) { + this.uri = Uri.file(folder); + this.name = folder; + } + } + + setup(() => { + envVarsProvider = mock(IEnvironmentVariablesProvider); + lsOutputChannel = mock(ILanguageServerOutputChannel); + configurationService = mock(ConfigurationService); + workspaceService = mock(WorkspaceService); + + const onDidChangeEnvVariables = new EventEmitter(); + when(envVarsProvider.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event); + + analysisOptions = new JediLanguageServerAnalysisOptions( + instance(envVarsProvider), + instance(lsOutputChannel), + instance(configurationService), + instance(workspaceService), + ); + }); + test('Without extraPaths provided and no workspace', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + when(configurationService.getSettings(anything())).thenReturn({} as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([]); + }); + + test('Without extraPaths provided', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(new MockWorkspaceFolder(workspacePath)); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + when(configurationService.getSettings(anything())).thenReturn({} as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([expectedWorkspacePath]); + }); + + test('With extraPaths provided', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(new MockWorkspaceFolder(workspacePath)); + when(configurationService.getSettings(anything())).thenReturn({ + // We expect a distinct set of paths back, using __dirname to test absolute path + autoComplete: { extraPaths: [__dirname, 'relative/pathB', 'relative/pathB'] }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([ + expectedWorkspacePath, + __dirname, + path.join(expectedWorkspacePath, 'relative/pathB'), + ]); + }); +});