From 1451840fcb5223441f463b79a16cd3fd3e16659b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 15:36:50 -0800 Subject: [PATCH 01/21] Changes --- .../languageServer/languageServer.ts | 23 +++++++++++++------ src/client/common/configSettings.ts | 6 ++--- src/client/interpreter/autoSelection/index.ts | 4 +--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index 9dc1d119ffb3..ee839285f881 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -6,8 +6,8 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { - CancellationToken, CompletionContext, OutputChannel, Position, - TextDocument, Uri + CancellationToken, CompletionContext, ConfigurationChangeEvent, OutputChannel, + Position, TextDocument, Uri } from 'vscode'; import { Disposable, LanguageClient, LanguageClientOptions, @@ -16,8 +16,6 @@ import { import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; -import { PythonSettings } from '../../common/configSettings'; -// tslint:disable-next-line:ordered-imports import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; import { Logger } from '../../common/logger'; import { IFileSystem, IPlatformService } from '../../common/platform/types'; @@ -27,8 +25,10 @@ import { IOutputChannel, IPathUtils, IPythonExtensionBanner, IPythonSettings } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; +import { debounce } from '../../common/utils/decorators'; import { StopWatch } from '../../common/utils/stopWatch'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerSymbolProvider } from '../../providers/symbolProvider'; import { sendTelemetryEvent } from '../../telemetry'; @@ -111,8 +111,12 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { deprecationManager.registerDeprecation(buildSymbolsCmdDeprecatedInfo); this.surveyBanner = services.get(IPythonExtensionBanner, BANNER_NAME_LS_SURVEY); + let disposable = this.workspace.onDidChangeConfiguration(this.onSettingsChangedHandler, this); + this.disposables.push(disposable); - (this.configuration.getSettings() as PythonSettings).addListener('change', this.onSettingsChanged.bind(this)); + const interpreterService = services.get(IInterpreterService); + disposable = interpreterService.onDidChangeInterpreter(() => this.onSettingsChangedHandler(), this); + this.disposables.push(disposable); } public async activate(): Promise { @@ -141,7 +145,6 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { for (const d of this.disposables) { d.dispose(); } - (this.configuration.getSettings() as PythonSettings).removeListener('change', this.onSettingsChanged.bind(this)); } private async startLanguageServer(clientOptions: LanguageClientOptions): Promise { @@ -339,7 +342,13 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { ? settings.analysis.typeshedPaths : [path.join(this.context.extensionPath, this.languageServerFolder, 'Typeshed')]; } - + @debounce(1000) + private async onSettingsChangedHandler(e?: ConfigurationChangeEvent): Promise { + if (e && !e.affectsConfiguration('python', this.root)) { + return; + } + this.onSettingsChanged(); + } private async onSettingsChanged(): Promise { const ids = new InterpreterDataService(this.context, this.services); const idata = await ids.getInterpreterData(); diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index b10c7ad0c686..a437a7445893 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -26,10 +26,10 @@ import { } from './types'; import { SystemVariables } from './variables/systemVariables'; -// tslint:disable-next-line:no-require-imports no-var-requires +// tslint:disable:no-require-imports no-var-requires const untildify = require('untildify'); +const _debounce = require('lodash/debounce') as typeof import('lodash/debounce'); -// tslint:disable-next-line:completed-docs export class PythonSettings extends EventEmitter implements IPythonSettings { private static pythonSettings: Map = new Map(); public downloadLanguageServer = true; @@ -374,7 +374,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { // If workspace config changes, then we could have a cascading effect of on change events. // Let's defer the change notification. - setTimeout(() => this.emit('change'), 1); + _debounce(() => this.emit('change'), 1); }; this.disposables.push(this.InterpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this))); this.disposables.push(this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 43fbffa515e7..20c98bbc5c75 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -62,10 +62,10 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio winRegInterpreter.setNextRule(systemInterpreter); } public async autoSelectInterpreter(resource: Resource): Promise { - Promise.all(this.rules.map(item => item.autoSelectInterpreter(undefined))).ignoreErrors(); await this.initializeStore(); await this.userDefinedInterpreter.autoSelectInterpreter(resource, this); this.didAutoSelectedInterpreterEmitter.fire(); + Promise.all(this.rules.map(item => item.autoSelectInterpreter(undefined))).ignoreErrors(); } public get onDidChangeAutoSelectedInterpreter(): Event { return this.didAutoSelectedInterpreterEmitter.event; @@ -113,8 +113,6 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio } this.autoSelectedInterpreterByWorkspace.set(workspaceFolderPath, interpreter); } - - this.didAutoSelectedInterpreterEmitter.fire(); } protected async initializeStore() { if (this.globallyPreferredInterpreter) { From 29cbedd2c535103e22193d0cfa6f2115dea155f1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sun, 13 Jan 2019 20:23:23 -0800 Subject: [PATCH 02/21] Add tests --- gulpfile.js | 1 - package-lock.json | 123 +++--- package.json | 1 + src/client/activation/activationService.ts | 2 +- src/client/activation/downloader.ts | 43 +- src/client/activation/hashVerifier.ts | 28 -- .../activation/interpreterDataService.ts | 223 +++++----- src/client/activation/jedi.ts | 6 +- .../activation/languageServer/activator.ts | 49 +++ .../languageServer/analysisOptions.ts | 204 +++++++++ .../languageServer/languageClientFactory.ts | 78 ++++ .../languageServer/languageServer.ts | 396 ++---------------- .../languageServer/languageServerHashes.ts | 10 - .../activation/languageServer/manager.ts | 51 +++ src/client/activation/platformData.ts | 39 +- src/client/activation/serviceRegistry.ts | 2 +- src/client/activation/types.ts | 82 +++- src/client/common/types.ts | 15 +- .../datascience/dataScienceSurveyBanner.ts | 9 - .../datascience/jupyter/jupyterServer.ts | 4 +- src/client/datascience/types.ts | 4 +- .../languageServerSurveyBanner.ts | 8 - .../proposeLanguageServerBanner.ts | 9 - .../activation/activationService.unit.test.ts | 4 +- src/test/activation/downloader.unit.test.ts | 20 +- .../languageServer/activator.unit.test.ts | 136 ++++++ .../analysisOptions.unit.test.ts | 208 +++++++++ .../languageClientFactory.unit.test.ts | 138 ++++++ .../languageServer.unit.test.ts | 123 +++--- .../languageServer/manager.unit.test.ts | 135 ++++++ src/test/activation/platformData.unit.test.ts | 24 +- .../autoSelection/index.unit.test.ts | 18 +- 32 files changed, 1424 insertions(+), 769 deletions(-) delete mode 100644 src/client/activation/hashVerifier.ts create mode 100644 src/client/activation/languageServer/activator.ts create mode 100644 src/client/activation/languageServer/analysisOptions.ts create mode 100644 src/client/activation/languageServer/languageClientFactory.ts delete mode 100644 src/client/activation/languageServer/languageServerHashes.ts create mode 100644 src/client/activation/languageServer/manager.ts create mode 100644 src/test/activation/languageServer/activator.unit.test.ts create mode 100644 src/test/activation/languageServer/analysisOptions.unit.test.ts create mode 100644 src/test/activation/languageServer/languageClientFactory.unit.test.ts create mode 100644 src/test/activation/languageServer/manager.unit.test.ts diff --git a/gulpfile.js b/gulpfile.js index 21e45228c4fb..b5eee666090d 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -704,7 +704,6 @@ function getFilesToProcess(fileList) { * @param {hygieneOptions} options */ function getFileListToProcess(options) { - return []; const mode = options ? options.mode : 'all'; const gulpSrcOptions = { base: '.' }; diff --git a/package-lock.json b/package-lock.json index 302ad6ddb3cb..a1fcc739a083 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3973,6 +3973,12 @@ "integrity": "sha1-3dgA2gxmEnOTzKWVDqloo6rxJTs=", "dev": true }, + "compare-module-exports": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/compare-module-exports/-/compare-module-exports-2.1.0.tgz", + "integrity": "sha512-3Lc0sTIuX1jmY2K2RrXRJOND6KsRTX2D4v3+eu1PDptsuJZVK4LZc852eZa9I+avj0NrUKlTNgqvccNOH6mbGg==", + "dev": true + }, "component-emitter": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/component-emitter/-/component-emitter-1.2.1.tgz", @@ -8751,23 +8757,6 @@ "sshpk": "^1.7.0" } }, - "httpplease": { - "version": "0.16.4", - "resolved": "https://registry.npmjs.org/httpplease/-/httpplease-0.16.4.tgz", - "integrity": "sha1-04Lr4jDvUHkIC06f/r8xap51wNo=", - "requires": { - "urllite": "~0.5.0", - "xmlhttprequest": "*", - "xtend": "~3.0.0" - }, - "dependencies": { - "xtend": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/xtend/-/xtend-3.0.0.tgz", - "integrity": "sha1-XM50B7r2Qsunvs2laBEcST9ZZlo=" - } - } - }, "https-browserify": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/https-browserify/-/https-browserify-1.0.0.tgz", @@ -9724,7 +9713,8 @@ "js-tokens": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-3.0.2.tgz", - "integrity": "sha1-mGbfOVECEw449/mWvOtlRDIJwls=" + "integrity": "sha1-mGbfOVECEw449/mWvOtlRDIJwls=", + "dev": true }, "js-yaml": { "version": "3.11.0", @@ -10405,6 +10395,12 @@ "integrity": "sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM=", "dev": true }, + "lodash.some": { + "version": "4.6.0", + "resolved": "https://registry.npmjs.org/lodash.some/-/lodash.some-4.6.0.tgz", + "integrity": "sha1-G7nzFO9ri63tE7VJFpsqlF62jk0=", + "dev": true + }, "lodash.sortby": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/lodash.sortby/-/lodash.sortby-4.7.0.tgz", @@ -10510,6 +10506,7 @@ "version": "1.4.0", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", "integrity": "sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==", + "dev": true, "requires": { "js-tokens": "^3.0.0 || ^4.0.0" } @@ -10805,11 +10802,6 @@ "dom-walk": "^0.1.0" } }, - "mini-svg-data-uri": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/mini-svg-data-uri/-/mini-svg-data-uri-1.0.2.tgz", - "integrity": "sha512-3bDQR0/DIws7pkqi/dhtmv5BGgTT2HPRzq9fos3Jz4Xc9bVnn5eC6jBb4mK25Jdt8UclKeRhateLLTz9J2Wwug==" - }, "minimalistic-assert": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz", @@ -12515,7 +12507,8 @@ "object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", - "integrity": "sha1-IQmtx5ZYh8/AXLvUQsrIv7s2CGM=" + "integrity": "sha1-IQmtx5ZYh8/AXLvUQsrIv7s2CGM=", + "dev": true }, "object-copy": { "version": "0.1.0", @@ -13491,6 +13484,7 @@ "version": "15.6.2", "resolved": "https://registry.npmjs.org/prop-types/-/prop-types-15.6.2.tgz", "integrity": "sha512-3pboPvLiWD7dkI3qf3KbUe6hKFKa52w+AE0VCqECtf+QHAKgOL37tTaNCnuX1nAAQ4ZhyP+kYVKf8rLmJ/feDQ==", + "dev": true, "requires": { "loose-envify": "^1.3.1", "object-assign": "^4.1.1" @@ -13881,15 +13875,6 @@ "shallowequal": "^1.0.2" } }, - "react-inlinesvg": { - "version": "0.8.3", - "resolved": "https://registry.npmjs.org/react-inlinesvg/-/react-inlinesvg-0.8.3.tgz", - "integrity": "sha512-CQnNFbZtgPC8FyqHJLnurPkljX5z2cKhUUhBqJ1I6BugQMPEwBDxKgTQNAWzhdbYTtXF7sHTzTD79XW5XS2sLw==", - "requires": { - "httpplease": "^0.16.4", - "once": "^1.4.0" - } - }, "react-is": { "version": "16.5.2", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.5.2.tgz", @@ -14487,6 +14472,43 @@ "integrity": "sha1-Jkgr9JFcftn4MAu1y+xI/U/1vGI=", "dev": true }, + "rewiremock": { + "version": "3.13.0", + "resolved": "https://registry.npmjs.org/rewiremock/-/rewiremock-3.13.0.tgz", + "integrity": "sha512-1MkO4mX4j31GilbMsqdgLNXjmrHo9EUKQFCa82rLye8ltOHnJe0rRaHUSKz2yUClr8l0Qnj1ZTjZHmp6vNTrzQ==", + "dev": true, + "requires": { + "babel-runtime": "^6.26.0", + "compare-module-exports": "^2.1.0", + "lodash.some": "^4.6.0", + "lodash.template": "^4.4.0", + "node-libs-browser": "^2.1.0", + "path-parse": "^1.0.5", + "wipe-node-cache": "^2.1.0", + "wipe-webpack-cache": "^2.1.0" + }, + "dependencies": { + "lodash.template": { + "version": "4.4.0", + "resolved": "https://registry.npmjs.org/lodash.template/-/lodash.template-4.4.0.tgz", + "integrity": "sha1-5zoDhcg1VZF0bgILmWecaQ5o+6A=", + "dev": true, + "requires": { + "lodash._reinterpolate": "~3.0.0", + "lodash.templatesettings": "^4.0.0" + } + }, + "lodash.templatesettings": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/lodash.templatesettings/-/lodash.templatesettings-4.1.0.tgz", + "integrity": "sha1-K01OlbpEDZFf8IvImeRVNmZxMxY=", + "dev": true, + "requires": { + "lodash._reinterpolate": "~3.0.0" + } + } + } + }, "right-align": { "version": "0.1.3", "resolved": "https://registry.npmjs.org/right-align/-/right-align-0.1.3.tgz", @@ -15200,8 +15222,7 @@ "stack-trace": { "version": "0.0.10", "resolved": "https://registry.npmjs.org/stack-trace/-/stack-trace-0.0.10.tgz", - "integrity": "sha1-VHxws0fo0ytOEI6hoqFZ5f3eGcA=", - "dev": true + "integrity": "sha1-VHxws0fo0ytOEI6hoqFZ5f3eGcA=" }, "stat-mode": { "version": "0.2.2", @@ -15584,6 +15605,7 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/svg-inline-react/-/svg-inline-react-3.1.0.tgz", "integrity": "sha512-c39AIRQOUXLMD8fQ2rHmK1GOSO3tVuZk61bAXqIT05uhhm3z4VtQFITQSwyhL0WA2uxoJAIhPd2YV0CYQOolSA==", + "dev": true, "requires": { "prop-types": "^15.5.0" } @@ -16908,14 +16930,6 @@ "integrity": "sha1-iS/pWWCAXoVRnxzUOJ8stMu3ZS8=", "dev": true }, - "urllite": { - "version": "0.5.0", - "resolved": "https://registry.npmjs.org/urllite/-/urllite-0.5.0.tgz", - "integrity": "sha1-G3u5yj+w25Ug3hE0ZrvPfMNBRRo=", - "requires": { - "xtend": "~4.0.0" - } - }, "use": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/use/-/use-3.1.0.tgz", @@ -17946,6 +17960,21 @@ "resolved": "https://registry.npmjs.org/winreg/-/winreg-1.2.4.tgz", "integrity": "sha1-ugZWKbepJRMOFXeRCM9UCZDpjRs=" }, + "wipe-node-cache": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/wipe-node-cache/-/wipe-node-cache-2.1.0.tgz", + "integrity": "sha512-Vdash0WV9Di/GeYW9FJrAZcPjGK4dO7M/Be/sJybguEgcM7As0uwLyvewZYqdlepoh7Rj4ZJKEdo8uX83PeNIw==", + "dev": true + }, + "wipe-webpack-cache": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/wipe-webpack-cache/-/wipe-webpack-cache-2.1.0.tgz", + "integrity": "sha512-OXzQMGpA7MnQQ8AG+uMl5mWR2ezy6fw1+DMHY+wzYP1qkF1jrek87psLBmhZEj+er4efO/GD4R8jXWFierobaA==", + "dev": true, + "requires": { + "wipe-node-cache": "^2.1.0" + } + }, "wordwrap": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-1.0.0.tgz", @@ -18054,11 +18083,6 @@ "integrity": "sha512-tGkGJkN8XqCod7OT+EvGYK5Z4SfDQGD30zAa58OcnAa0RRWgzUEK72tkXhsX1FZd+rgnhRxFtmO+ihkp8LHSkw==", "dev": true }, - "xmlhttprequest": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/xmlhttprequest/-/xmlhttprequest-1.8.0.tgz", - "integrity": "sha1-Z/4HXFwk/vOfnWX197f+dRcZaPw=" - }, "xregexp": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/xregexp/-/xregexp-4.0.0.tgz", @@ -18068,7 +18092,8 @@ "xtend": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/xtend/-/xtend-4.0.1.tgz", - "integrity": "sha1-pcbVMr5lbiPbgg77lDofBJmNY68=" + "integrity": "sha1-pcbVMr5lbiPbgg77lDofBJmNY68=", + "dev": true }, "y18n": { "version": "4.0.0", diff --git a/package.json b/package.json index 30c2095b5463..6f5a31147664 100644 --- a/package.json +++ b/package.json @@ -1963,6 +1963,7 @@ "relative": "^3.0.2", "remap-istanbul": "^0.10.1", "retyped-diff-match-patch-tsd-ambient": "^1.0.0-0", + "rewiremock": "^3.13.0", "shortid": "^2.2.8", "source-map-support": "^0.5.9", "style-loader": "^0.23.1", diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 5100bb44e3f4..45dbfe816cd7 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -75,7 +75,7 @@ export class ExtensionActivationService implements IExtensionActivationService, public dispose() { if (this.currentActivator) { - this.currentActivator.activator.deactivate().ignoreErrors(); + this.currentActivator.activator.dispose().ignoreErrors(); } } diff --git a/src/client/activation/downloader.ts b/src/client/activation/downloader.ts index 32c49369ba6f..5b4141f4fab4 100644 --- a/src/client/activation/downloader.ts +++ b/src/client/activation/downloader.ts @@ -3,43 +3,38 @@ 'use strict'; +import { inject, named } from 'inversify'; import * as path from 'path'; import { ProgressLocation, window } from 'vscode'; import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; -import { IExtensionContext, IOutputChannel } from '../common/types'; +import { IOutputChannel } from '../common/types'; import { createDeferred } from '../common/utils/async'; import { StopWatch } from '../common/utils/stopWatch'; -import { IServiceContainer } from '../ioc/types'; import { sendTelemetryEvent } from '../telemetry'; import { PYTHON_LANGUAGE_SERVER_DOWNLOADED, PYTHON_LANGUAGE_SERVER_EXTRACTED } from '../telemetry/constants'; -import { PlatformData } from './platformData'; +import { IPlatformData } from './platformData'; import { IHttpClient, ILanguageServerDownloader, ILanguageServerFolderService } from './types'; const downloadFileExtension = '.nupkg'; export class LanguageServerDownloader implements ILanguageServerDownloader { - private readonly output: IOutputChannel; - private readonly fs: IFileSystem; constructor( - private readonly platformData: PlatformData, - private readonly engineFolder: string, - private readonly serviceContainer: IServiceContainer + @inject(IPlatformData) private readonly platformData: IPlatformData, + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel, + @inject(IHttpClient) private readonly httpClient: IHttpClient, + @inject(ILanguageServerFolderService) private readonly lsFolderService: ILanguageServerFolderService, + @inject(IFileSystem) private readonly fs: IFileSystem ) { - this.output = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - this.fs = this.serviceContainer.get(IFileSystem); - } public async getDownloadInfo() { - const lsFolderService = this.serviceContainer.get(ILanguageServerFolderService); - return lsFolderService.getLatestLanguageServerVersion().then(item => item!); + return this.lsFolderService.getLatestLanguageServerVersion().then(item => item!); } - - public async downloadLanguageServer(context: IExtensionContext): Promise { + public async downloadLanguageServer(destinationFolder: string): Promise { const downloadInfo = await this.getDownloadInfo(); const downloadUri = downloadInfo.uri; const lsVersion = downloadInfo.version.raw; @@ -64,7 +59,7 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { timer.reset(); try { - await this.unpackArchive(context.extensionPath, localTempFilePath); + await this.unpackArchive(destinationFolder, localTempFilePath); } catch (err) { this.output.appendLine('extraction failed.'); this.output.appendLine(err); @@ -80,7 +75,7 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { } } - private async downloadFile(uri: string, title: string): Promise { + protected async downloadFile(uri: string, title: string): Promise { this.output.append(`Downloading ${uri}... `); const tempFile = await this.fs.createTemporaryFile(downloadFileExtension); @@ -96,8 +91,7 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { await window.withProgress({ location: ProgressLocation.Window }, async (progress) => { - const httpClient = this.serviceContainer.get(IHttpClient); - const req = await httpClient.downloadFile(uri); + const req = await this.httpClient.downloadFile(uri); const requestProgress = await import('request-progress'); requestProgress(req) .on('progress', (state) => { @@ -123,10 +117,9 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { return tempFile.filePath; } - private async unpackArchive(extensionPath: string, tempFilePath: string): Promise { + protected async unpackArchive(destinationFolder: string, tempFilePath: string): Promise { this.output.append('Unpacking archive... '); - const installFolder = path.join(extensionPath, this.engineFolder); const deferred = createDeferred(); const title = 'Extracting files... '; @@ -144,10 +137,10 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { let extractedFiles = 0; zip.on('ready', async () => { totalFiles = zip.entriesCount; - if (!await this.fs.directoryExists(installFolder)) { - await this.fs.createDirectory(installFolder); + if (!await this.fs.directoryExists(destinationFolder)) { + await this.fs.createDirectory(destinationFolder); } - zip.extract(null, installFolder, (err) => { + zip.extract(null, destinationFolder, (err) => { if (err) { deferred.reject(err); } else { @@ -165,7 +158,7 @@ export class LanguageServerDownloader implements ILanguageServerDownloader { }); // Set file to executable (nothing happens in Windows, as chmod has no definition there) - const executablePath = path.join(installFolder, this.platformData.getEngineExecutableName()); + const executablePath = path.join(destinationFolder, this.platformData.engineExecutableName); await this.fs.chmod(executablePath, '0764'); // -rwxrw-r-- this.output.appendLine('done.'); diff --git a/src/client/activation/hashVerifier.ts b/src/client/activation/hashVerifier.ts deleted file mode 100644 index e0b405ff5877..000000000000 --- a/src/client/activation/hashVerifier.ts +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { createHash } from 'crypto'; -import * as fs from 'fs'; -import { createDeferred } from '../common/utils/async'; - -export class HashVerifier { - public async verifyHash(filePath: string, platformString: string, expectedDigest: string): Promise { - const readStream = fs.createReadStream(filePath); - const deferred = createDeferred(); - const hash = createHash('sha512'); - hash.setEncoding('hex'); - readStream - .on('end', () => { - hash.end(); - deferred.resolve(); - }) - .on('error', (err) => { - deferred.reject(`Unable to calculate file hash. Error ${err}`); - }); - - readStream.pipe(hash); - await deferred.promise; - const actual = hash.read() as string; - return expectedDigest === platformString ? true : actual.toLowerCase() === expectedDigest.toLowerCase(); - } -} diff --git a/src/client/activation/interpreterDataService.ts b/src/client/activation/interpreterDataService.ts index 1e082bb171d5..9d5933d8b322 100644 --- a/src/client/activation/interpreterDataService.ts +++ b/src/client/activation/interpreterDataService.ts @@ -3,140 +3,141 @@ import { createHash } from 'crypto'; import * as fs from 'fs'; +import { inject } from 'inversify'; import * as path from 'path'; -import { ExtensionContext, Uri } from 'vscode'; import { IApplicationShell } from '../common/application/types'; import '../common/extensions'; import { IPlatformService } from '../common/platform/types'; import { IPythonExecutionFactory, IPythonExecutionService } from '../common/process/types'; +import { IExtensionContext, Resource } from '../common/types'; import { createDeferred } from '../common/utils/async'; import { IServiceContainer } from '../ioc/types'; +import { IInterpreterDataService, InterpreterData } from './types'; const DataVersion = 1; - -export class InterpreterData { - constructor( - public readonly dataVersion: number, - // tslint:disable-next-line:no-shadowed-variable - public readonly path: string, - public readonly version: string, - public readonly searchPaths: string, - public readonly hash: string - ) { } +class InterpreterDataCls { + constructor( + public readonly dataVersion: number, + // tslint:disable-next-line:no-shadowed-variable + public readonly path: string, + public readonly version: string, + public readonly searchPaths: string, + public readonly hash: string + ) { } } -export class InterpreterDataService { - constructor( - private readonly context: ExtensionContext, - private readonly serviceContainer: IServiceContainer) { } +export class InterpreterDataService implements IInterpreterDataService { + constructor( + @inject(IExtensionContext) private readonly context: IExtensionContext, + @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { } - public async getInterpreterData(resource?: Uri): Promise { - const executionFactory = this.serviceContainer.get(IPythonExecutionFactory); - const execService = await executionFactory.create({ resource }); + public async getInterpreterData(resource: Resource): Promise { + const executionFactory = this.serviceContainer.get(IPythonExecutionFactory); + const execService = await executionFactory.create({ resource }); - const interpreterPath = await execService.getExecutablePath(); - if (interpreterPath.length === 0) { - return; - } + const interpreterPath = await execService.getExecutablePath(); + if (interpreterPath.length === 0) { + return; + } - const cacheKey = `InterpreterData-${interpreterPath}`; - let interpreterData = this.context.globalState.get(cacheKey); - let interpreterChanged = false; - if (interpreterData) { - // Check if interpreter executable changed - if (interpreterData.dataVersion !== DataVersion) { - interpreterChanged = true; - } else { - const currentHash = await this.getInterpreterHash(interpreterPath); - interpreterChanged = currentHash !== interpreterData.hash; - } - } + const cacheKey = `InterpreterData-${interpreterPath}`; + let interpreterData = this.context.globalState.get(cacheKey); + let interpreterChanged = false; + if (interpreterData) { + // Check if interpreter executable changed + if (interpreterData.dataVersion !== DataVersion) { + interpreterChanged = true; + } else { + const currentHash = await this.getInterpreterHash(interpreterPath); + interpreterChanged = currentHash !== interpreterData.hash; + } + } - if (interpreterChanged || !interpreterData) { - interpreterData = await this.getInterpreterDataFromPython(execService, interpreterPath); - this.context.globalState.update(interpreterPath, interpreterData); - } else { - // Make sure we verify that search paths did not change. This must be done - // completely async so we don't delay Python language server startup. - this.verifySearchPaths(interpreterData.searchPaths, interpreterPath, execService); + if (interpreterChanged || !interpreterData) { + interpreterData = await this.getInterpreterDataFromPython(execService, interpreterPath); + this.context.globalState.update(interpreterPath, interpreterData); + } else { + // Make sure we verify that search paths did not change. This must be done + // completely async so we don't delay Python language server startup. + this.verifySearchPaths(interpreterData.searchPaths, interpreterPath, execService); + } + return interpreterData; } - return interpreterData; - } - public getInterpreterHash(interpreterPath: string): Promise { - const platform = this.serviceContainer.get(IPlatformService); - const pythonExecutable = path.join(path.dirname(interpreterPath), platform.isWindows ? 'python.exe' : 'python'); - // Hash mod time and creation time - const deferred = createDeferred(); - fs.lstat(pythonExecutable, (err, stats) => { - if (err) { - deferred.resolve(''); - } else { - const actual = createHash('sha512').update(`${stats.ctime}-${stats.mtime}`).digest('hex'); - deferred.resolve(actual); - } - }); - return deferred.promise; - } - - private async getInterpreterDataFromPython(execService: IPythonExecutionService, interpreterPath: string): Promise { - const result = await execService.exec(['-c', 'import sys; print(sys.version_info)'], {}); - // sys.version_info(major=3, minor=6, micro=6, releaselevel='final', serial=0) - if (!result.stdout) { - throw Error('Unable to determine Python interpreter version and system prefix.'); - } - const output = result.stdout.splitLines({ removeEmptyEntries: true, trim: true }); - if (!output || output.length < 1) { - throw Error('Unable to parse version and and system prefix from the Python interpreter output.'); - } - const majorMatches = output[0].match(/major=(\d*?),/); - const minorMatches = output[0].match(/minor=(\d*?),/); - if (!majorMatches || majorMatches.length < 2 || !minorMatches || minorMatches.length < 2) { - throw Error('Unable to parse interpreter version.'); + public getInterpreterHash(interpreterPath: string): Promise { + const platform = this.serviceContainer.get(IPlatformService); + const pythonExecutable = path.join(path.dirname(interpreterPath), platform.isWindows ? 'python.exe' : 'python'); + // Hash mod time and creation time + const deferred = createDeferred(); + fs.lstat(pythonExecutable, (err, stats) => { + if (err) { + deferred.resolve(''); + } else { + const actual = createHash('sha512').update(`${stats.ctime}-${stats.mtime}`).digest('hex'); + deferred.resolve(actual); + } + }); + return deferred.promise; } - const hash = await this.getInterpreterHash(interpreterPath); - const searchPaths = await this.getSearchPaths(execService); - return new InterpreterData(DataVersion, interpreterPath, `${majorMatches[1]}.${minorMatches[1]}`, searchPaths, hash); - } - private async getSearchPaths(execService: IPythonExecutionService): Promise { - const result = await execService.exec(['-c', 'import sys; import os; print(sys.path + os.getenv("PYTHONPATH", "").split(os.pathsep));'], {}); - if (!result.stdout) { - throw Error('Unable to determine Python interpreter search paths.'); + private async getInterpreterDataFromPython(execService: IPythonExecutionService, interpreterPath: string): Promise { + const result = await execService.exec(['-c', 'import sys; print(sys.version_info)'], {}); + // sys.version_info(major=3, minor=6, micro=6, releaselevel='final', serial=0) + if (!result.stdout) { + throw Error('Unable to determine Python interpreter version and system prefix.'); + } + const output = result.stdout.splitLines({ removeEmptyEntries: true, trim: true }); + if (!output || output.length < 1) { + throw Error('Unable to parse version and and system prefix from the Python interpreter output.'); + } + const majorMatches = output[0].match(/major=(\d*?),/); + const minorMatches = output[0].match(/minor=(\d*?),/); + if (!majorMatches || majorMatches.length < 2 || !minorMatches || minorMatches.length < 2) { + throw Error('Unable to parse interpreter version.'); + } + const hash = await this.getInterpreterHash(interpreterPath); + const searchPaths = await this.getSearchPaths(execService); + return new InterpreterDataCls(DataVersion, interpreterPath, `${majorMatches[1]}.${minorMatches[1]}`, searchPaths, hash); } - // tslint:disable-next-line:no-unnecessary-local-variable - const paths = result.stdout.split(',') - .filter(p => this.isValidPath(p)) - .map(p => this.pathCleanup(p)); - return paths.join(';'); // PTVS uses ; on all platforms - } - private pathCleanup(s: string): string { - s = s.trim(); - if (s[0] === '\'') { - s = s.substr(1); - } - if (s[s.length - 1] === ']') { - s = s.substr(0, s.length - 1); + private async getSearchPaths(execService: IPythonExecutionService): Promise { + const result = await execService.exec(['-c', 'import sys; import os; print(sys.path + os.getenv("PYTHONPATH", "").split(os.pathsep));'], {}); + if (!result.stdout) { + throw Error('Unable to determine Python interpreter search paths.'); + } + // tslint:disable-next-line:no-unnecessary-local-variable + const paths = result.stdout.split(',') + .filter(p => this.isValidPath(p)) + .map(p => this.pathCleanup(p)); + return paths.join(';'); // PTVS uses ; on all platforms } - if (s[s.length - 1] === '\'') { - s = s.substr(0, s.length - 1); + + private pathCleanup(s: string): string { + s = s.trim(); + if (s[0] === '\'') { + s = s.substr(1); + } + if (s[s.length - 1] === ']') { + s = s.substr(0, s.length - 1); + } + if (s[s.length - 1] === '\'') { + s = s.substr(0, s.length - 1); + } + return s; } - return s; - } - private isValidPath(s: string): boolean { - return s.length > 0 && s[0] !== '['; - } + private isValidPath(s: string): boolean { + return s.length > 0 && s[0] !== '['; + } - private verifySearchPaths(currentPaths: string, interpreterPath: string, execService: IPythonExecutionService): void { - this.getSearchPaths(execService) - .then(async paths => { - if (paths !== currentPaths) { - this.context.globalState.update(interpreterPath, undefined); - const appShell = this.serviceContainer.get(IApplicationShell); - await appShell.showWarningMessage('Search paths have changed for this Python interpreter. Please reload the extension to ensure that the IntelliSense works correctly.'); - } - }).ignoreErrors(); - } + private verifySearchPaths(currentPaths: string, interpreterPath: string, execService: IPythonExecutionService): void { + this.getSearchPaths(execService) + .then(async paths => { + if (paths !== currentPaths) { + this.context.globalState.update(interpreterPath, undefined); + const appShell = this.serviceContainer.get(IApplicationShell); + await appShell.showWarningMessage('Search paths have changed for this Python interpreter. Please reload the extension to ensure that the IntelliSense works correctly.'); + } + }).ignoreErrors(); + } } diff --git a/src/client/activation/jedi.ts b/src/client/activation/jedi.ts index f12debb4b0c4..6b809c6ae3d7 100644 --- a/src/client/activation/jedi.ts +++ b/src/client/activation/jedi.ts @@ -33,7 +33,7 @@ export class JediExtensionActivator implements IExtensionActivator { this.documentSelector = PYTHON; } - public async activate(): Promise { + public async activate(): Promise { const context = this.context; const jediFactory = this.jediFactory = new JediFactory(context.asAbsolutePath('.'), this.serviceManager); @@ -76,11 +76,9 @@ export class JediExtensionActivator implements IExtensionActivator { testManagementService.activate() .then(() => testManagementService.activateCodeLenses(symbolProvider)) .catch(ex => this.serviceManager.get(ILogger).logError('Failed to activate Unit Tests', ex)); - - return true; } - public async deactivate(): Promise { + public async dispose(): Promise { if (this.jediFactory) { this.jediFactory.dispose(); } diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts new file mode 100644 index 000000000000..4267f6e86fd9 --- /dev/null +++ b/src/client/activation/languageServer/activator.ts @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import { IWorkspaceService } from '../../common/application/types'; +import { IFileSystem } from '../../common/platform/types'; +import { IConfigurationService, Resource } from '../../common/types'; +import { EXTENSION_ROOT_DIR } from '../../constants'; +import { IExtensionActivator, ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager } from '../types'; + +/** + * Starts the language server managers per workspaces (currently one for first workspace). + * + * @export + * @class LanguageServerExtensionActivator + * @implements {IExtensionActivator} + */ +@injectable() +export class LanguageServerExtensionActivator implements IExtensionActivator { + constructor(@inject(ILanguageServerManager) private readonly manager: ILanguageServerManager, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, + @inject(IFileSystem) private readonly fs: IFileSystem, + @inject(ILanguageServerDownloader) private readonly lsDownloader: ILanguageServerDownloader, + @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService, + @inject(IConfigurationService) private readonly configurationService: IConfigurationService) { } + public async activate(): Promise { + const mainWorkspaceUri = this.workspace.hasWorkspaceFolders ? this.workspace.workspaceFolders![0].uri : undefined; + await this.ensureLanguageServerIsAvailable(mainWorkspaceUri); + await this.manager.start(mainWorkspaceUri); + } + public async dispose(): Promise { + this.manager.dispose(); + } + protected async ensureLanguageServerIsAvailable(resource: Resource) { + const settings = this.configurationService.getSettings(resource); + if (!settings.downloadLanguageServer) { + return; + } + const languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(); + const languageServerFolderPath = path.join(EXTENSION_ROOT_DIR, languageServerFolder); + const mscorlib = path.join(languageServerFolderPath, 'mscorlib.dll'); + if (!await this.fs.fileExists(mscorlib)) { + await this.lsDownloader.downloadLanguageServer(languageServerFolderPath); + } + } +} diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts new file mode 100644 index 000000000000..5b918d48e1da --- /dev/null +++ b/src/client/activation/languageServer/analysisOptions.ts @@ -0,0 +1,204 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable, named } from 'inversify'; +import * as path from 'path'; +import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposable, Event, EventEmitter, OutputChannel, Position, TextDocument } from 'vscode'; +import { LanguageClientOptions, ProvideCompletionItemsSignature } from 'vscode-languageclient'; +import { IWorkspaceService } from '../../common/application/types'; +import { isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; +import { traceError } from '../../common/logger'; +import { IConfigurationService, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../common/types'; +import { debounce } from '../../common/utils/decorators'; +import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { IInterpreterService } from '../../interpreter/contracts'; +import { IInterpreterDataService, ILanguageServerAnalysisOptions, ILanguageServerFolderService, InterpreterData } from '../types'; + +@injectable() +export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOptions { + private excludedFiles: string[] = []; + private typeshedPaths: string[] = []; + private disposables: Disposable[] = []; + private interpreterHash: string = ''; + private languageServerFolder: string = ''; + private resource: Resource; + private readonly didChange = new EventEmitter(); + constructor(@inject(IExtensionContext) private readonly context: IExtensionContext, + @inject(IEnvironmentVariablesProvider) private readonly envVarsProvider: IEnvironmentVariablesProvider, + @inject(IConfigurationService) private readonly configuration: IConfigurationService, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, + @inject(IPythonExtensionBanner) private readonly surveyBanner: IPythonExtensionBanner, + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, + @inject(IInterpreterDataService) private readonly interpreterDataService: IInterpreterDataService, + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: OutputChannel, + @inject(IPathUtils) private readonly pathUtils: IPathUtils, + @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService) { + + } + public async initialize(resource: Resource) { + this.resource = resource; + this.languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(); + + let disposable = this.workspace.onDidChangeConfiguration(this.onSettingsChangedHandler, this); + this.disposables.push(disposable); + + disposable = this.interpreterService.onDidChangeInterpreter(() => this.onSettingsChangedHandler(), this); + this.disposables.push(disposable); + } + public get onDidChange(): Event { + return this.didChange.event; + } + public dispose(): void { + this.disposables.forEach(d => d.dispose()); + this.didChange.dispose(); + } + public async getAnalysisOptions(): Promise { + const properties = new Map(); + let interpreterData: InterpreterData | undefined; + let pythonPath = ''; + + try { + interpreterData = await this.interpreterDataService.getInterpreterData(this.resource); + } catch (ex) { + traceError('Unable to determine path to the Python interpreter. IntelliSense will be limited.', ex); + } + + this.interpreterHash = interpreterData ? interpreterData.hash : ''; + if (interpreterData) { + pythonPath = path.dirname(interpreterData.path); + // tslint:disable-next-line:no-string-literal + properties['InterpreterPath'] = interpreterData.path; + // tslint:disable-next-line:no-string-literal + properties['Version'] = interpreterData.version; + } + + // tslint:disable-next-line:no-string-literal + properties['DatabasePath'] = path.join(this.context.extensionPath, this.languageServerFolder); + + let searchPaths = interpreterData ? interpreterData.searchPaths.split(path.delimiter) : []; + const settings = this.configuration.getSettings(); + if (settings.autoComplete) { + const extraPaths = settings.autoComplete.extraPaths; + if (extraPaths && extraPaths.length > 0) { + searchPaths.push(...extraPaths); + } + } + const vars = await this.envVarsProvider.getEnvironmentVariables(); + if (vars.PYTHONPATH && vars.PYTHONPATH.length > 0) { + const paths = vars.PYTHONPATH.split(this.pathUtils.delimiter).filter(item => item.trim().length > 0); + searchPaths.push(...paths); + } + // Make sure paths do not contain multiple slashes so file URIs + // in VS Code (Node.js) and in the language server (.NET) match. + // Note: for the language server paths separator is always ; + searchPaths.push(pythonPath); + searchPaths = searchPaths.map(p => path.normalize(p)); + + this.excludedFiles = this.getExcludedFiles(); + this.typeshedPaths = this.getTypeshedPaths(); + + // Options to control the language client + return { + // Register the server for Python documents + documentSelector: PYTHON, + synchronize: { + configurationSection: PYTHON_LANGUAGE + }, + outputChannel: this.output, + initializationOptions: { + interpreter: { + properties + }, + displayOptions: { + preferredFormat: 'markdown', + trimDocumentationLines: false, + maxDocumentationLineLength: 0, + trimDocumentationText: false, + maxDocumentationTextLength: 0 + }, + searchPaths, + typeStubSearchPaths: this.typeshedPaths, + excludeFiles: this.excludedFiles, + testEnvironment: isTestExecution(), + analysisUpdates: true, + traceLogging: true, // Max level, let LS decide through settings actual level of logging. + asyncStartup: true + }, + middleware: { + provideCompletionItem: (document: TextDocument, position: Position, context: CompletionContext, token: CancellationToken, next: ProvideCompletionItemsSignature) => { + this.surveyBanner.showBanner().ignoreErrors(); + return next(document, position, context, token); + } + } + }; + } + protected getExcludedFiles(): string[] { + const list: string[] = ['**/Lib/**', '**/site-packages/**']; + this.getVsCodeExcludeSection('search.exclude', list); + this.getVsCodeExcludeSection('files.exclude', list); + this.getVsCodeExcludeSection('files.watcherExclude', list); + this.getPythonExcludeSection(list); + return list; + } + + protected getVsCodeExcludeSection(setting: string, list: string[]): void { + const states = this.workspace.getConfiguration(setting); + if (states) { + Object.keys(states) + .filter(k => (k.indexOf('*') >= 0 || k.indexOf('/') >= 0) && states[k]) + .forEach(p => list.push(p)); + } + } + protected getPythonExcludeSection(list: string[]): void { + const pythonSettings = this.configuration.getSettings(this.resource); + const paths = pythonSettings && pythonSettings.linting ? pythonSettings.linting.ignorePatterns : undefined; + if (paths && Array.isArray(paths)) { + paths + .filter(p => p && p.length > 0) + .forEach(p => list.push(p)); + } + } + protected getTypeshedPaths(): string[] { + const settings = this.configuration.getSettings(this.resource); + return settings.analysis.typeshedPaths && settings.analysis.typeshedPaths.length > 0 + ? settings.analysis.typeshedPaths + : [path.join(this.context.extensionPath, this.languageServerFolder, 'Typeshed')]; + } + protected async onSettingsChangedHandler(e?: ConfigurationChangeEvent): Promise { + if (e && !e.affectsConfiguration('python', this.resource)) { + return; + } + this.onSettingsChanged().catch(ex => traceError('Failed to detect changes', ex)); + } + @debounce(1000) + protected async onSettingsChanged(): Promise { + const idata = await this.interpreterDataService.getInterpreterData(this.resource); + if (!idata || idata.hash !== this.interpreterHash) { + this.interpreterHash = idata ? idata.hash : ''; + this.didChange.fire(); + return; + } + + const excludedFiles = this.getExcludedFiles(); + await this.notifyIfValuesHaveChanged(this.excludedFiles, excludedFiles); + + const typeshedPaths = this.getTypeshedPaths(); + await this.notifyIfValuesHaveChanged(this.typeshedPaths, typeshedPaths); + } + + protected async notifyIfValuesHaveChanged(oldArray: string[], newArray: string[]): Promise { + if (newArray.length !== oldArray.length) { + this.didChange.fire(); + return; + } + + for (let i = 0; i < oldArray.length; i += 1) { + if (oldArray[i] !== newArray[i]) { + this.didChange.fire(); + return; + } + } + } +} diff --git a/src/client/activation/languageServer/languageClientFactory.ts b/src/client/activation/languageServer/languageClientFactory.ts new file mode 100644 index 000000000000..83d47d0d3299 --- /dev/null +++ b/src/client/activation/languageServer/languageClientFactory.ts @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable, named } from 'inversify'; +import * as path from 'path'; +import { LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient'; +import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants'; +import { IConfigurationService, Resource } from '../../common/types'; +import { IPlatformData } from '../platformData'; +import { ILanguageClientFactory, ILanguageServerFolderService, LanguageClientFactory } from '../types'; + +// tslint:disable:no-require-imports no-require-imports no-var-requires max-classes-per-file + +const dotNetCommand = 'dotnet'; +const languageClientName = 'Python Tools'; + +@injectable() +export class BaseLanguageClientFactory implements ILanguageClientFactory { + constructor(@inject(ILanguageClientFactory) @named(LanguageClientFactory.downloaded) private readonly downloadedFactory: ILanguageClientFactory, + @inject(ILanguageClientFactory) @named(LanguageClientFactory.simple) private readonly simpleFactory: ILanguageClientFactory, + @inject(IConfigurationService) private readonly configurationService: IConfigurationService) { } + public async createLanguageClient(resource: Resource, clientOptions: LanguageClientOptions): Promise { + const settings = this.configurationService.getSettings(resource); + const factory = settings.downloadLanguageServer ? this.downloadedFactory : this.simpleFactory; + return factory.createLanguageClient(resource, clientOptions); + } +} + +/** + * Creates a langauge client for use by users of the extension. + * + * @export + * @class DownloadedLanguageClientFactory + * @implements {ILanguageClientFactory} + */ +@injectable() +export class DownloadedLanguageClientFactory implements ILanguageClientFactory { + constructor(@inject(IPlatformData) private readonly platformData: IPlatformData, + @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService) { } + public async createLanguageClient(_resource: Resource, clientOptions: LanguageClientOptions): Promise { + const languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(); + const serverModule = path.join(EXTENSION_ROOT_DIR, languageServerFolder, this.platformData.engineExecutableName); + + const options = { stdio: 'pipe' }; + const serverOptions: ServerOptions = { + run: { command: serverModule, rgs: [], options: options }, + debug: { command: serverModule, args: ['--debug'], options } + }; + const vscodeLanaguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); + return new vscodeLanaguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); + } +} + +/** + * Creates a language client factory primarily used for LS development purposes. + * + * @export + * @class SimpleLanguageClientFactory + * @implements {ILanguageClientFactory} + */ +@injectable() +export class SimpleLanguageClientFactory implements ILanguageClientFactory { + constructor(@inject(IPlatformData) private readonly platformData: IPlatformData, + @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService) { } + public async createLanguageClient(_resource: Resource, clientOptions: LanguageClientOptions): Promise { + const languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(); + const commandOptions = { stdio: 'pipe' }; + const serverModule = path.join(EXTENSION_ROOT_DIR, languageServerFolder, this.platformData.engineDllName); + const serverOptions: ServerOptions = { + run: { command: dotNetCommand, args: [serverModule], options: commandOptions }, + debug: { command: dotNetCommand, args: [serverModule, '--debug'], options: commandOptions } + }; + const vscodeLanaguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); + return new vscodeLanaguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); + } +} diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index ee839285f881..9963dbc0b648 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -4,387 +4,69 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import { - CancellationToken, CompletionContext, ConfigurationChangeEvent, OutputChannel, - Position, TextDocument, Uri -} from 'vscode'; -import { - Disposable, LanguageClient, LanguageClientOptions, - ProvideCompletionItemsSignature, ServerOptions -} from 'vscode-languageclient'; -import { - IApplicationShell, ICommandManager, IWorkspaceService -} from '../../common/application/types'; -import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; -import { Logger } from '../../common/logger'; -import { IFileSystem, IPlatformService } from '../../common/platform/types'; -import { - BANNER_NAME_LS_SURVEY, DeprecatedFeatureInfo, IConfigurationService, - IDisposableRegistry, IExtensionContext, IFeatureDeprecationManager, ILogger, - IOutputChannel, IPathUtils, IPythonExtensionBanner, IPythonSettings -} from '../../common/types'; -import { createDeferred, Deferred } from '../../common/utils/async'; -import { debounce } from '../../common/utils/decorators'; -import { StopWatch } from '../../common/utils/stopWatch'; -import { IEnvironmentVariablesProvider } from '../../common/variables/types'; -import { IInterpreterService } from '../../interpreter/contracts'; -import { IServiceContainer } from '../../ioc/types'; -import { LanguageServerSymbolProvider } from '../../providers/symbolProvider'; -import { sendTelemetryEvent } from '../../telemetry'; -import { - PYTHON_LANGUAGE_SERVER_ENABLED, - PYTHON_LANGUAGE_SERVER_ERROR, - PYTHON_LANGUAGE_SERVER_TELEMETRY -} from '../../telemetry/constants'; -import { IUnitTestManagementService } from '../../unittests/types'; -import { LanguageServerDownloader } from '../downloader'; -import { InterpreterData, InterpreterDataService } from '../interpreterDataService'; -import { PlatformData } from '../platformData'; +import { Disposable, LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; +import { traceDecorators, traceError } from '../../common/logger'; +import { Resource } from '../../common/types'; +import { createDeferred, Deferred, sleep } from '../../common/utils/async'; +import { noop } from '../../common/utils/misc'; +import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; +import { PYTHON_LANGUAGE_SERVER_ENABLED, PYTHON_LANGUAGE_SERVER_TELEMETRY } from '../../telemetry/constants'; import { ProgressReporting } from '../progress'; -import { IExtensionActivator, ILanguageServerFolderService } from '../types'; - -const PYTHON = 'python'; -const dotNetCommand = 'dotnet'; -const languageClientName = 'Python Tools'; -const loadExtensionCommand = 'python._loadLanguageServerExtension'; -const buildSymbolsCmdDeprecatedInfo: DeprecatedFeatureInfo = { - doNotDisplayPromptStateKey: 'SHOW_DEPRECATED_FEATURE_PROMPT_BUILD_WORKSPACE_SYMBOLS', - message: 'The command \'Python: Build Workspace Symbols\' is deprecated when using the Python Language Server. The Python Language Server builds symbols in the workspace in the background.', - moreInfoUrl: 'https://github.com/Microsoft/vscode-python/issues/2267#issuecomment-408996859', - commands: ['python.buildWorkspaceSymbols'] -}; +import { ILanaguageServer, ILanguageClientFactory } from '../types'; @injectable() -export class LanguageServerExtensionActivator implements IExtensionActivator { - private readonly configuration: IConfigurationService; - private readonly appShell: IApplicationShell; - private readonly output: OutputChannel; - private readonly fs: IFileSystem; - private readonly sw = new StopWatch(); - private readonly platformData: PlatformData; +export class LanguageServer implements ILanaguageServer { private readonly startupCompleted: Deferred; private readonly disposables: Disposable[] = []; - private readonly context: IExtensionContext; - private readonly workspace: IWorkspaceService; - private readonly root: Uri | undefined; - - private languageClient: LanguageClient | undefined; - private interpreterHash: string = ''; - private excludedFiles: string[] = []; - private typeshedPaths: string[] = []; - private loadExtensionArgs: {} | undefined; - private surveyBanner: IPythonExtensionBanner; - private languageServerFolder!: string; - private languageServerFolderService: ILanguageServerFolderService; - constructor(@inject(IServiceContainer) private readonly services: IServiceContainer) { - this.context = this.services.get(IExtensionContext); - this.configuration = this.services.get(IConfigurationService); - this.appShell = this.services.get(IApplicationShell); - this.output = this.services.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - this.fs = this.services.get(IFileSystem); - this.platformData = new PlatformData(services.get(IPlatformService), this.fs); - this.workspace = this.services.get(IWorkspaceService); - this.languageServerFolderService = this.services.get(ILanguageServerFolderService); - const deprecationManager: IFeatureDeprecationManager = - this.services.get(IFeatureDeprecationManager); + private languageClient?: LanguageClient; - // Currently only a single root. Multi-root support is future. - this.root = this.workspace && this.workspace.hasWorkspaceFolders - ? this.workspace.workspaceFolders![0]!.uri : undefined; + constructor(@inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory) { this.startupCompleted = createDeferred(); - const commandManager = this.services.get(ICommandManager); - - this.disposables.push(commandManager.registerCommand(loadExtensionCommand, - async (args) => { - if (this.languageClient) { - await this.startupCompleted.promise; - this.languageClient.sendRequest('python/loadExtension', args); - } else { - this.loadExtensionArgs = args; - } - } - )); - - deprecationManager.registerDeprecation(buildSymbolsCmdDeprecatedInfo); - - this.surveyBanner = services.get(IPythonExtensionBanner, BANNER_NAME_LS_SURVEY); - let disposable = this.workspace.onDidChangeConfiguration(this.onSettingsChangedHandler, this); - this.disposables.push(disposable); - - const interpreterService = services.get(IInterpreterService); - disposable = interpreterService.onDidChangeInterpreter(() => this.onSettingsChangedHandler(), this); - this.disposables.push(disposable); - } - - public async activate(): Promise { - this.sw.reset(); - this.languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(); - const clientOptions = await this.getAnalysisOptions(); - if (!clientOptions) { - return false; - } - - this.startupCompleted.promise.then(() => { - const testManagementService = this.services.get(IUnitTestManagementService); - testManagementService.activate() - .then(() => testManagementService.activateCodeLenses(new LanguageServerSymbolProvider(this.languageClient!))) - .catch(ex => this.services.get(ILogger).logError('Failed to activate Unit Tests', ex)); - }).ignoreErrors(); - - return this.startLanguageServer(clientOptions); } - public async deactivate(): Promise { + public dispose() { if (this.languageClient) { - // Do not await on this - this.languageClient.stop(); + // Do not await on this. + this.languageClient.stop() + .then(noop, ex => traceError('Stopping language client failed', ex)); + this.languageClient = undefined; } - for (const d of this.disposables) { + while (this.disposables.length > 0) { + const d = this.disposables.shift()!; d.dispose(); } } - private async startLanguageServer(clientOptions: LanguageClientOptions): Promise { - // Determine if we are running MSIL/Universal via dotnet or self-contained app. - sendTelemetryEvent(PYTHON_LANGUAGE_SERVER_ENABLED); - - const settings = this.configuration.getSettings(); - if (!settings.downloadLanguageServer) { - // Depends on .NET Runtime or SDK. Typically development-only case. - this.languageClient = await this.createSimpleLanguageClient(clientOptions); - await this.startLanguageClient(); - return true; - } - - const mscorlib = path.join(this.context.extensionPath, this.languageServerFolder, 'mscorlib.dll'); - if (!await this.fs.fileExists(mscorlib)) { - const downloader = new LanguageServerDownloader(this.platformData, this.languageServerFolder, this.services); - await downloader.downloadLanguageServer(this.context); - } - - const serverModule = path.join(this.context.extensionPath, this.languageServerFolder, this.platformData.getEngineExecutableName()); - this.languageClient = await this.createSelfContainedLanguageClient(serverModule, clientOptions); - try { - await this.startLanguageClient(); - this.languageClient.onTelemetry(telemetryEvent => { - const eventName = telemetryEvent.EventName ? telemetryEvent.EventName : PYTHON_LANGUAGE_SERVER_TELEMETRY; - sendTelemetryEvent(eventName, telemetryEvent.Measurements, telemetryEvent.Properties); - }); - return true; - } catch (ex) { - this.appShell.showErrorMessage(`Language server failed to start. Error ${ex}`); - sendTelemetryEvent(PYTHON_LANGUAGE_SERVER_ERROR, undefined, { error: 'Failed to start (platform)' }); - return false; - } - } - - private async startLanguageClient(): Promise { - this.context.subscriptions.push(this.languageClient!.start()); + @traceDecorators.error('Failed to start language server') + @captureTelemetry(PYTHON_LANGUAGE_SERVER_ENABLED) + public async start(resource: Resource, options: LanguageClientOptions): Promise { + this.languageClient = await this.factory.createLanguageClient(resource, options); + this.disposables.push(this.languageClient!.start()); await this.serverReady(); - const disposables = this.services.get(IDisposableRegistry); const progressReporting = new ProgressReporting(this.languageClient!); - disposables.push(progressReporting); - } - - private async serverReady(): Promise { - while (!this.languageClient!.initializeResult) { - await new Promise(resolve => setTimeout(resolve, 100)); - } - if (this.loadExtensionArgs) { - this.languageClient!.sendRequest('python/loadExtension', this.loadExtensionArgs); - } - - this.startupCompleted.resolve(); - } - - private async createSimpleLanguageClient(clientOptions: LanguageClientOptions): Promise { - const commandOptions = { stdio: 'pipe' }; - const serverModule = path.join(this.context.extensionPath, this.languageServerFolder, this.platformData.getEngineDllName()); - const serverOptions: ServerOptions = { - run: { command: dotNetCommand, args: [serverModule], options: commandOptions }, - debug: { command: dotNetCommand, args: [serverModule, '--debug'], options: commandOptions } - }; - const vscodeLanaguageClient = await import('vscode-languageclient'); - return new vscodeLanaguageClient.LanguageClient(PYTHON, languageClientName, serverOptions, clientOptions); - } - - private async createSelfContainedLanguageClient(serverModule: string, clientOptions: LanguageClientOptions): Promise { - const options = { stdio: 'pipe' }; - const serverOptions: ServerOptions = { - run: { command: serverModule, rgs: [], options: options }, - debug: { command: serverModule, args: ['--debug'], options } - }; - const vscodeLanaguageClient = await import('vscode-languageclient'); - return new vscodeLanaguageClient.LanguageClient(PYTHON, languageClientName, serverOptions, clientOptions); - } - - // tslint:disable-next-line:member-ordering - public async getAnalysisOptions(): Promise { - const properties = new Map(); - let interpreterData: InterpreterData | undefined; - let pythonPath = ''; - - try { - const interpreterDataService = new InterpreterDataService(this.context, this.services); - interpreterData = await interpreterDataService.getInterpreterData(); - } catch (ex) { - Logger.error('Unable to determine path to the Python interpreter. IntelliSense will be limited.', ex); - } - - this.interpreterHash = interpreterData ? interpreterData.hash : ''; - if (interpreterData) { - pythonPath = path.dirname(interpreterData.path); - // tslint:disable-next-line:no-string-literal - properties['InterpreterPath'] = interpreterData.path; - // tslint:disable-next-line:no-string-literal - properties['Version'] = interpreterData.version; - } - - // tslint:disable-next-line:no-string-literal - properties['DatabasePath'] = path.join(this.context.extensionPath, this.languageServerFolder); - - let searchPaths = interpreterData ? interpreterData.searchPaths.split(path.delimiter) : []; - const settings = this.configuration.getSettings(); - if (settings.autoComplete) { - const extraPaths = settings.autoComplete.extraPaths; - if (extraPaths && extraPaths.length > 0) { - searchPaths.push(...extraPaths); - } - } - const envVarsProvider = this.services.get(IEnvironmentVariablesProvider); - const vars = await envVarsProvider.getEnvironmentVariables(); - if (vars.PYTHONPATH && vars.PYTHONPATH.length > 0) { - const pathUtils = this.services.get(IPathUtils); - const paths = vars.PYTHONPATH.split(pathUtils.delimiter).filter(item => item.trim().length > 0); - searchPaths.push(...paths); - } - // Make sure paths do not contain multiple slashes so file URIs - // in VS Code (Node.js) and in the language server (.NET) match. - // Note: for the language server paths separator is always ; - searchPaths.push(pythonPath); - searchPaths = searchPaths.map(p => path.normalize(p)); - - const selector = [{ language: PYTHON, scheme: 'file' }]; - this.excludedFiles = this.getExcludedFiles(); - this.typeshedPaths = this.getTypeshedPaths(settings); - - // Options to control the language client - return { - // Register the server for Python documents - documentSelector: selector, - synchronize: { - configurationSection: PYTHON - }, - outputChannel: this.output, - initializationOptions: { - interpreter: { - properties - }, - displayOptions: { - preferredFormat: 'markdown', - trimDocumentationLines: false, - maxDocumentationLineLength: 0, - trimDocumentationText: false, - maxDocumentationTextLength: 0 - }, - searchPaths, - typeStubSearchPaths: this.typeshedPaths, - excludeFiles: this.excludedFiles, - testEnvironment: isTestExecution(), - analysisUpdates: true, - traceLogging: true, // Max level, let LS decide through settings actual level of logging. - asyncStartup: true - }, - middleware: { - provideCompletionItem: (document: TextDocument, position: Position, context: CompletionContext, token: CancellationToken, next: ProvideCompletionItemsSignature) => { - if (this.surveyBanner) { - this.surveyBanner.showBanner().ignoreErrors(); - } - return next(document, position, context, token); - } - } - }; - } - - private getExcludedFiles(): string[] { - const list: string[] = ['**/Lib/**', '**/site-packages/**']; - this.getVsCodeExcludeSection('search.exclude', list); - this.getVsCodeExcludeSection('files.exclude', list); - this.getVsCodeExcludeSection('files.watcherExclude', list); - this.getPythonExcludeSection(list); - return list; + this.disposables.push(progressReporting); + this.languageClient.onTelemetry(telemetryEvent => { + const eventName = telemetryEvent.EventName || PYTHON_LANGUAGE_SERVER_TELEMETRY; + sendTelemetryEvent(eventName, telemetryEvent.Measurements, telemetryEvent.Properties); + }); } - - private getVsCodeExcludeSection(setting: string, list: string[]): void { - const states = this.workspace.getConfiguration(setting, this.root); - if (states) { - Object.keys(states) - .filter(k => (k.indexOf('*') >= 0 || k.indexOf('/') >= 0) && states[k]) - .forEach(p => list.push(p)); + public loadExtension(args?: {}) { + if (!this.languageClient) { + throw new Error('Activation not completed or not invoked'); } - } - - private getPythonExcludeSection(list: string[]): void { - const pythonSettings = this.configuration.getSettings(this.root); - const paths = pythonSettings && pythonSettings.linting ? pythonSettings.linting.ignorePatterns : undefined; - if (paths && Array.isArray(paths)) { - paths - .filter(p => p && p.length > 0) - .forEach(p => list.push(p)); + if (!this.startupCompleted.completed) { + throw new Error('Activation not completed'); } + this.languageClient.sendRequest('python/loadExtension', args) + .then(noop, ex => traceError('Request python/loadExtension failed', ex)); } - private getTypeshedPaths(settings: IPythonSettings): string[] { - return settings.analysis.typeshedPaths && settings.analysis.typeshedPaths.length > 0 - ? settings.analysis.typeshedPaths - : [path.join(this.context.extensionPath, this.languageServerFolder, 'Typeshed')]; - } - @debounce(1000) - private async onSettingsChangedHandler(e?: ConfigurationChangeEvent): Promise { - if (e && !e.affectsConfiguration('python', this.root)) { - return; - } - this.onSettingsChanged(); - } - private async onSettingsChanged(): Promise { - const ids = new InterpreterDataService(this.context, this.services); - const idata = await ids.getInterpreterData(); - if (!idata || idata.hash !== this.interpreterHash) { - this.interpreterHash = idata ? idata.hash : ''; - await this.restartLanguageServer(); - return; - } - - const excludedFiles = this.getExcludedFiles(); - await this.restartLanguageServerIfArrayChanged(this.excludedFiles, excludedFiles); - - const settings = this.configuration.getSettings(); - const typeshedPaths = this.getTypeshedPaths(settings); - await this.restartLanguageServerIfArrayChanged(this.typeshedPaths, typeshedPaths); - } - - private async restartLanguageServerIfArrayChanged(oldArray: string[], newArray: string[]): Promise { - if (newArray.length !== oldArray.length) { - await this.restartLanguageServer(); - return; - } - - for (let i = 0; i < oldArray.length; i += 1) { - if (oldArray[i] !== newArray[i]) { - await this.restartLanguageServer(); - return; - } - } - } - - private async restartLanguageServer(): Promise { - if (!this.context) { - return; + private async serverReady(): Promise { + while (this.languageClient && !this.languageClient!.initializeResult) { + await sleep(100); } - await this.deactivate(); - await this.activate(); + this.startupCompleted.resolve(); } } diff --git a/src/client/activation/languageServer/languageServerHashes.ts b/src/client/activation/languageServer/languageServerHashes.ts deleted file mode 100644 index d8d856064363..000000000000 --- a/src/client/activation/languageServer/languageServerHashes.ts +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// This file will be replaced by a generated one during the release build -// with actual hashes of the uploaded packages. -// Values are for test purposes only -export const language_server_win_x86_sha512 = 'win-x86'; -export const language_server_win_x64_sha512 = 'win-x64'; -export const language_server_osx_x64_sha512 = 'osx-x64'; -export const language_server_linux_x64_sha512 = 'linux-x64'; diff --git a/src/client/activation/languageServer/manager.ts b/src/client/activation/languageServer/manager.ts new file mode 100644 index 000000000000..d383cbaaf329 --- /dev/null +++ b/src/client/activation/languageServer/manager.ts @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject } from 'inversify'; +import { traceDecorators } from '../../common/logger'; +import { IDisposable, Resource } from '../../common/types'; +import { debounce } from '../../common/utils/decorators'; +import { IServiceContainer } from '../../ioc/types'; +import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../types'; + +export class LanguageServerManager implements ILanguageServerManager { + private lanaguageServer?: ILanaguageServer; + private resource!: Resource; + private disposables: IDisposable[] = []; + constructor( + @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, + @inject(ILanguageServerAnalysisOptions) private readonly analysisOptions: ILanguageServerAnalysisOptions) { } + public dispose() { + if (this.lanaguageServer) { + this.lanaguageServer.dispose(); + } + } + @traceDecorators.error('Failed to start Language Server') + public async start(resource: Resource): Promise { + if (this.lanaguageServer) { + throw new Error('Language Server already started'); + } + this.resource = resource; + this.analysisOptions.onDidChange(this.restartLanguageServer, this, this.disposables); + + await this.analysisOptions.initialize(resource); + await this.startLanguageServer(); + } + @traceDecorators.error('Failed to restart Language Server') + @traceDecorators.verbose('Restarting Langauge Server') + @debounce(1000) + protected async restartLanguageServer(): Promise { + if (this.lanaguageServer) { + this.lanaguageServer.dispose(); + } + await this.startLanguageServer(); + } + @traceDecorators.verbose('Starting Langauge Server') + protected async startLanguageServer(): Promise { + this.lanaguageServer = this.serviceContainer.get(ILanaguageServer); + const options = await this.analysisOptions!.getAnalysisOptions(); + await this.lanaguageServer.start(this.resource, options); + } +} diff --git a/src/client/activation/platformData.ts b/src/client/activation/platformData.ts index fc0f01b2b2de..faf1ebb23735 100644 --- a/src/client/activation/platformData.ts +++ b/src/client/activation/platformData.ts @@ -1,13 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { IFileSystem, IPlatformService } from '../common/platform/types'; -import { - language_server_linux_x64_sha512, - language_server_osx_x64_sha512, - language_server_win_x64_sha512, - language_server_win_x86_sha512 -} from './languageServer/languageServerHashes'; +import { inject, injectable } from 'inversify'; +import { IPlatformService } from '../common/platform/types'; export enum PlatformName { Windows32Bit = 'win-x86', @@ -22,9 +17,16 @@ export enum PlatformLSExecutables { Linux = 'Microsoft.Python.LanguageServer' } -export class PlatformData { - constructor(private platform: IPlatformService, fs: IFileSystem) { } - public getPlatformName(): PlatformName { +export const IPlatformData = Symbol('IPlatformData'); +export interface IPlatformData { + readonly platformName: PlatformName; + readonly engineDllName: string; + readonly engineExecutableName: string; +} +@injectable() +export class PlatformData implements IPlatformData { + constructor(@inject(IPlatformService) private readonly platform: IPlatformService) { } + public get platformName(): PlatformName { if (this.platform.isWindows) { return this.platform.is64bit ? PlatformName.Windows64Bit : PlatformName.Windows32Bit; } @@ -40,11 +42,11 @@ export class PlatformData { throw new Error('Unknown OS platform.'); } - public getEngineDllName(): string { + public get engineDllName(): string { return 'Microsoft.Python.LanguageServer.dll'; } - public getEngineExecutableName(): string { + public get engineExecutableName(): string { if (this.platform.isWindows) { return PlatformLSExecutables.Windows; } else if (this.platform.isLinux) { @@ -55,17 +57,4 @@ export class PlatformData { return 'unknown-platform'; } } - - public async getExpectedHash(): Promise { - if (this.platform.isWindows) { - return this.platform.is64bit ? language_server_win_x64_sha512 : language_server_win_x86_sha512; - } - if (this.platform.isMac) { - return language_server_osx_x64_sha512; - } - if (this.platform.isLinux && this.platform.is64bit) { - return language_server_linux_x64_sha512; - } - throw new Error('Unknown platform.'); - } } diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index 6108c85cd43d..486ebf6a82c5 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -12,7 +12,7 @@ import { ProposeLanguageServerBanner } from '../languageServices/proposeLanguage import { ExtensionActivationService } from './activationService'; import { DownloadBetaChannelRule, DownloadDailyChannelRule, DownloadStableChannelRule } from './downloadChannelRules'; import { JediExtensionActivator } from './jedi'; -import { LanguageServerExtensionActivator } from './languageServer/languageServer'; +import { LanguageServerExtensionActivator } from './languageServer/activator'; import { LanguageServerCompatibilityService } from './languageServer/languageServerCompatibilityService'; import { LanguageServerFolderService } from './languageServer/languageServerFolderService'; import { BetaLanguageServerPackageRepository, DailyLanguageServerPackageRepository, LanguageServerDownloadChannel, StableLanguageServerPackageRepository } from './languageServer/languageServerPackageRepository'; diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 98ff23dad689..891cca8d5af3 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -5,60 +5,104 @@ import { Request as RequestResult } from 'request'; import { SemVer } from 'semver'; +import { Event } from 'vscode'; +import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; import { NugetPackage } from '../common/nuget/types'; -import { IExtensionContext, LanguageServerDownloadChannels } from '../common/types'; +import { IAsyncDisposable, IDisposable, LanguageServerDownloadChannels, Resource } from '../common/types'; export const IExtensionActivationService = Symbol('IExtensionActivationService'); export interface IExtensionActivationService { - activate(): Promise; + activate(): Promise; } export enum ExtensionActivators { - Jedi = 'Jedi', - DotNet = 'DotNet' + Jedi = 'Jedi', + DotNet = 'DotNet' } export const IExtensionActivator = Symbol('IExtensionActivator'); -export interface IExtensionActivator { - activate(): Promise; - deactivate(): Promise; +export interface IExtensionActivator extends IAsyncDisposable { + activate(): Promise; } export const IHttpClient = Symbol('IHttpClient'); export interface IHttpClient { - downloadFile(uri: string): Promise; - getJSON(uri: string): Promise; + downloadFile(uri: string): Promise; + getJSON(uri: string): Promise; } export type FolderVersionPair = { path: string; version: SemVer }; export const ILanguageServerFolderService = Symbol('ILanguageServerFolderService'); export interface ILanguageServerFolderService { - getLanguageServerFolderName(): Promise; - getLatestLanguageServerVersion(): Promise; - getCurrentLanguageServerDirectory(): Promise; + getLanguageServerFolderName(): Promise; + getLatestLanguageServerVersion(): Promise; + getCurrentLanguageServerDirectory(): Promise; } export const ILanguageServerDownloader = Symbol('ILanguageServerDownloader'); export interface ILanguageServerDownloader { - getDownloadInfo(): Promise; - downloadLanguageServer(context: IExtensionContext): Promise; + getDownloadInfo(): Promise; + downloadLanguageServer(destinationFolder: string): Promise; } export const ILanguageServerPackageService = Symbol('ILanguageServerPackageService'); export interface ILanguageServerPackageService { - getNugetPackageName(): string; - getLatestNugetPackageVersion(): Promise; - getLanguageServerDownloadChannel(): LanguageServerDownloadChannels; + getNugetPackageName(): string; + getLatestNugetPackageVersion(): Promise; + getLanguageServerDownloadChannel(): LanguageServerDownloadChannels; } export const MajorLanguageServerVersion = Symbol('MajorLanguageServerVersion'); export const IDownloadChannelRule = Symbol('IDownloadChannelRule'); export interface IDownloadChannelRule { - shouldLookForNewLanguageServer(currentFolder?: FolderVersionPair): Promise; + shouldLookForNewLanguageServer(currentFolder?: FolderVersionPair): Promise; } export const ILanguageServerCompatibilityService = Symbol('ILanguageServerCompatibilityService'); export interface ILanguageServerCompatibilityService { - isSupported(): Promise; + isSupported(): Promise; +} +export enum LanguageClientFactory { + base = 'base', + simple = 'simple', + downloaded = 'downloaded' +} +export const ILanguageClientFactory = Symbol('ILanguageClientFactory'); +export interface ILanguageClientFactory { + createLanguageClient(resource: Resource, clientOptions: LanguageClientOptions): Promise; +} +export const ILanguageServerAnalysisOptions = Symbol('ILanguageServerAnalysisOptions'); +export interface ILanguageServerAnalysisOptions extends IDisposable { + readonly onDidChange: Event; + initialize(resource: Resource): Promise; + getAnalysisOptions(): Promise; +} +export const ILanguageServerManager = Symbol('ILanguageServerManager'); +export interface ILanguageServerManager extends IDisposable { + start(resource: Resource): Promise; +} +export const ILanaguageServer = Symbol('ILanaguageServer'); +export interface ILanaguageServer extends IDisposable { + start(resource: Resource, options: LanguageClientOptions): Promise; + /** + * Sends a request to LS so as to load other extensions. + * This is used as a plugin loader mechanism. + * Anyone (such as intellicode) wanting to interact with LS, needs to send this request to LS. + * @param {{}} [args] + * @memberof ILanaguageServer + */ + loadExtension(args?: {}): void; +} +export type InterpreterData = { + readonly dataVersion: number; + readonly path: string; + readonly version: string; + readonly searchPaths: string; + readonly hash: string; +}; + +export const IInterpreterDataService = Symbol('InterpreterDataService'); +export interface IInterpreterDataService { + getInterpreterData(resource: Resource): Promise; } diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 3d8ad02b97a1..451be549fce6 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -339,9 +339,7 @@ export interface IBrowserService { export const IPythonExtensionBanner = Symbol('IPythonExtensionBanner'); export interface IPythonExtensionBanner { - enabled: boolean; - shownCount: Promise; - optionLabels: string[]; + readonly enabled: boolean; showBanner(): Promise; } export const BANNER_NAME_LS_SURVEY: string = 'LSSurveyBanner'; @@ -370,16 +368,17 @@ export interface IFeatureDeprecationManager extends Disposable { export const IEditorUtils = Symbol('IEditorUtils'); export interface IEditorUtils { - // getTextEditor(uri: Uri): Promise<{ editor: TextEditor; dispose?(): void }>; getWorkspaceEditsFromPatch(originalContents: string, patch: string, uri: Uri): WorkspaceEdit; } export interface IDisposable { - dispose(): Promise | undefined | void; + dispose(): void | undefined; +} +export interface IAsyncDisposable { + dispose(): Promise; } export const IAsyncDisposableRegistry = Symbol('IAsyncDisposableRegistry'); -export interface IAsyncDisposableRegistry { - dispose(): Promise; - push(disposable: IDisposable); +export interface IAsyncDisposableRegistry extends IAsyncDisposable { + push(disposable: IDisposable | IAsyncDisposable); } diff --git a/src/client/datascience/dataScienceSurveyBanner.ts b/src/client/datascience/dataScienceSurveyBanner.ts index 2ccca9ae9b49..96ae9379723f 100644 --- a/src/client/datascience/dataScienceSurveyBanner.ts +++ b/src/client/datascience/dataScienceSurveyBanner.ts @@ -48,15 +48,6 @@ export class DataScienceSurveyBanner implements IPythonExtensionBanner { } this.isInitialized = true; } - - public get optionLabels(): string[] { - return this.bannerLabels; - } - - public get shownCount(): Promise { - return this.getPythonDSCommandCounter(); - } - public get enabled(): boolean { return this.persistentState.createGlobalPersistentState(DSSurveyStateKeys.ShowBanner, true).value; } diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts index 9435784b5e21..4162455e2e25 100644 --- a/src/client/datascience/jupyter/jupyterServer.ts +++ b/src/client/datascience/jupyter/jupyterServer.ts @@ -15,7 +15,7 @@ import { CancellationToken } from 'vscode-jsonrpc'; import { IWorkspaceService } from '../../common/application/types'; import { CancellationError } from '../../common/cancellation'; -import { IAsyncDisposableRegistry, IDisposable, IDisposableRegistry, ILogger } from '../../common/types'; +import { IAsyncDisposable, IAsyncDisposableRegistry, IDisposableRegistry, ILogger } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -108,7 +108,7 @@ class CellSubscriber { // https://www.npmjs.com/package/@jupyterlab/services @injectable() -export class JupyterServer implements INotebookServer, IDisposable { +export class JupyterServer implements INotebookServer, IAsyncDisposable { private session: IJupyterSession | undefined; private connInfo: IConnection | undefined; private workingDir: string | undefined; diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index ba60905249f2..2436c82097e7 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -8,7 +8,7 @@ import { Observable } from 'rxjs/Observable'; import { CancellationToken, CodeLens, CodeLensProvider, Disposable, Event, Range, TextDocument, TextEditor } from 'vscode'; import { ICommandManager } from '../common/application/types'; -import { IDisposable } from '../common/types'; +import { IAsyncDisposable, IDisposable } from '../common/types'; import { PythonInterpreter } from '../interpreter/contracts'; // Main interface @@ -62,7 +62,7 @@ export interface IJupyterExecution { } export const IJupyterSession = Symbol('IJupyterSession'); -export interface IJupyterSession extends IDisposable { +export interface IJupyterSession extends IAsyncDisposable { onRestarted: Event; restart() : Promise; interrupt() : Promise; diff --git a/src/client/languageServices/languageServerSurveyBanner.ts b/src/client/languageServices/languageServerSurveyBanner.ts index d2156a71e21c..1b4a8060502f 100644 --- a/src/client/languageServices/languageServerSurveyBanner.ts +++ b/src/client/languageServices/languageServerSurveyBanner.ts @@ -62,14 +62,6 @@ export class LanguageServerSurveyBanner implements IPythonExtensionBanner { } } - public get optionLabels(): string[] { - return this.bannerLabels; - } - - public get shownCount(): Promise { - return this.getPythonLSLaunchCounter(); - } - public get enabled(): boolean { return this.persistentState.createGlobalPersistentState(LSSurveyStateKeys.ShowBanner, true).value; } diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index c58c4697a114..e881bbe628ec 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -65,15 +65,6 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { return; } } - - public get shownCount(): Promise { - return Promise.resolve(-1); // we don't count this popup banner! - } - - public get optionLabels(): string[] { - return this.bannerLabels; - } - public get enabled(): boolean { return this.persistentState.createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, true).value; } diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index cff248783b3d..bd700cab0f30 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -73,7 +73,7 @@ suite('Activation - ActivationService', () => { async function testActivation(activationService: IExtensionActivationService, activator: TypeMoq.IMock, lsSupported: boolean = true) { activator - .setup(a => a.activate()).returns(() => Promise.resolve(true)) + .setup(a => a.activate()).returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); let activatorName = ExtensionActivators.Jedi; if (lsSupported && !jediIsEnabled) { @@ -132,7 +132,7 @@ suite('Activation - ActivationService', () => { await testActivation(activationService, activator); activator - .setup(a => a.deactivate()).returns(() => Promise.resolve()) + .setup(a => a.dispose()).returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); activationService.dispose(); diff --git a/src/test/activation/downloader.unit.test.ts b/src/test/activation/downloader.unit.test.ts index 684ddcd61592..62174115cbc8 100644 --- a/src/test/activation/downloader.unit.test.ts +++ b/src/test/activation/downloader.unit.test.ts @@ -8,30 +8,16 @@ import { expect } from 'chai'; import * as TypeMoq from 'typemoq'; import { LanguageServerDownloader } from '../../client/activation/downloader'; -import { PlatformData } from '../../client/activation/platformData'; import { ILanguageServerFolderService } from '../../client/activation/types'; -import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants'; -import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; -import { IOutputChannel } from '../../client/common/types'; -import { IServiceContainer } from '../../client/ioc/types'; suite('Activation - Downloader', () => { let languageServerDownloader: LanguageServerDownloader; - let platformService: TypeMoq.IMock; - let container: TypeMoq.IMock; let folderService: TypeMoq.IMock; setup(() => { - container = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); folderService = TypeMoq.Mock.ofType(); - const fs = TypeMoq.Mock.ofType(); - const output = TypeMoq.Mock.ofType(); - const platformData: PlatformData = new PlatformData(platformService.object, fs.object); - container.setup(a => a.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isValue(STANDARD_OUTPUT_CHANNEL))).returns(() => output.object); - container.setup(a => a.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fs.object); - container.setup(a => a.get(TypeMoq.It.isValue(ILanguageServerFolderService))).returns(() => folderService.object); - - languageServerDownloader = new LanguageServerDownloader(platformData, '', container.object); + languageServerDownloader = new LanguageServerDownloader(undefined as any, + undefined as any, undefined as any, + folderService.object, undefined as any); }); test('Get download uri', async () => { diff --git a/src/test/activation/languageServer/activator.unit.test.ts b/src/test/activation/languageServer/activator.unit.test.ts new file mode 100644 index 000000000000..b3576789dbaa --- /dev/null +++ b/src/test/activation/languageServer/activator.unit.test.ts @@ -0,0 +1,136 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as path from 'path'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { LanguageServerDownloader } from '../../../client/activation/downloader'; +import { LanguageServerExtensionActivator } from '../../../client/activation/languageServer/activator'; +import { LanguageServerFolderService } from '../../../client/activation/languageServer/languageServerFolderService'; +import { LanguageServerManager } from '../../../client/activation/languageServer/manager'; +import { IExtensionActivator, ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager } from '../../../client/activation/types'; +import { IWorkspaceService } from '../../../client/common/application/types'; +import { WorkspaceService } from '../../../client/common/application/workspace'; +import { PythonSettings } from '../../../client/common/configSettings'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { IFileSystem } from '../../../client/common/platform/types'; +import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; +import { createDeferred } from '../../../client/common/utils/async'; +import { EXTENSION_ROOT_DIR } from '../../../client/constants'; +import { sleep } from '../../core'; + +// tslint:disable:max-func-body-length + +suite('xLanguage Server - Activator', () => { + let activator: IExtensionActivator; + let workspaceService: IWorkspaceService; + let manager: ILanguageServerManager; + let fs: IFileSystem; + let lsDownloader: ILanguageServerDownloader; + let lsFolderService: ILanguageServerFolderService; + let configuration: IConfigurationService; + let settings: IPythonSettings; + setup(() => { + manager = mock(LanguageServerManager); + workspaceService = mock(WorkspaceService); + fs = mock(FileSystem); + lsDownloader = mock(LanguageServerDownloader); + lsFolderService = mock(LanguageServerFolderService); + configuration = mock(ConfigurationService); + settings = mock(PythonSettings); + when(configuration.getSettings(anything())).thenReturn(instance(settings)); + activator = new LanguageServerExtensionActivator(instance(manager), + instance(workspaceService), instance(fs), + instance(lsDownloader), instance(lsFolderService), + instance(configuration)); + }); + test('Manager must be started without any workspace', async () => { + when(workspaceService.hasWorkspaceFolders).thenReturn(false); + when(manager.start(undefined)).thenResolve(); + when(settings.downloadLanguageServer).thenReturn(false); + + await activator.activate(); + + verify(manager.start(undefined)).once(); + verify(workspaceService.hasWorkspaceFolders).once(); + }); + test('Do not download LS if not required', async () => { + when(workspaceService.hasWorkspaceFolders).thenReturn(false); + when(manager.start(undefined)).thenResolve(); + when(settings.downloadLanguageServer).thenReturn(false); + + await activator.activate(); + + verify(manager.start(undefined)).once(); + verify(workspaceService.hasWorkspaceFolders).once(); + verify(lsFolderService.getLanguageServerFolderName()).never(); + verify(lsDownloader.downloadLanguageServer(anything())).never(); + }); + test('Do not download LS if not required', async () => { + const languageServerFolder = 'Some folder name'; + const languageServerFolderPath = path.join(EXTENSION_ROOT_DIR, languageServerFolder); + const mscorlib = path.join(languageServerFolderPath, 'mscorlib.dll'); + + when(workspaceService.hasWorkspaceFolders).thenReturn(false); + when(manager.start(undefined)).thenResolve(); + when(settings.downloadLanguageServer).thenReturn(true); + when(lsFolderService.getLanguageServerFolderName()).thenResolve(languageServerFolder); + when(fs.fileExists(mscorlib)).thenResolve(true); + + await activator.activate(); + + verify(manager.start(undefined)).once(); + verify(workspaceService.hasWorkspaceFolders).once(); + verify(lsFolderService.getLanguageServerFolderName()).once(); + verify(lsDownloader.downloadLanguageServer(anything())).never(); + }); + test('Start language server after downloading', async () => { + const deferred = createDeferred(); + const languageServerFolder = 'Some folder name'; + const languageServerFolderPath = path.join(EXTENSION_ROOT_DIR, languageServerFolder); + const mscorlib = path.join(languageServerFolderPath, 'mscorlib.dll'); + + when(workspaceService.hasWorkspaceFolders).thenReturn(false); + when(manager.start(undefined)).thenResolve(); + when(settings.downloadLanguageServer).thenReturn(true); + when(lsFolderService.getLanguageServerFolderName()).thenResolve(languageServerFolder); + when(fs.fileExists(mscorlib)).thenResolve(false); + when(lsDownloader.downloadLanguageServer(languageServerFolderPath)).thenReturn(deferred.promise); + + const promise = activator.activate(); + await sleep(1); + verify(workspaceService.hasWorkspaceFolders).once(); + verify(lsFolderService.getLanguageServerFolderName()).once(); + verify(lsDownloader.downloadLanguageServer(anything())).once(); + + verify(manager.start(undefined)).never(); + + deferred.resolve(); + await sleep(1); + verify(manager.start(undefined)).once(); + + await promise; + }); + test('Manager must be started with resource for first available workspace', async () => { + const uri = Uri.file(__filename); + when(workspaceService.hasWorkspaceFolders).thenReturn(true); + when(workspaceService.workspaceFolders).thenReturn([{ index: 0, name: '', uri }]); + when(manager.start(uri)).thenResolve(); + when(settings.downloadLanguageServer).thenReturn(false); + + await activator.activate(); + + verify(manager.start(uri)).once(); + verify(workspaceService.hasWorkspaceFolders).once(); + verify(workspaceService.workspaceFolders).once(); + }); + + test('Manager must be disposed', async () => { + await activator.dispose(); + + verify(manager.dispose()).once(); + }); +}); diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts new file mode 100644 index 000000000000..d591fd245ea6 --- /dev/null +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -0,0 +1,208 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import { instance, mock, verify, when } from 'ts-mockito'; +import * as typemoq from 'typemoq'; +import { ConfigurationChangeEvent, Uri } from 'vscode'; +import { InterpreterDataService } from '../../../client/activation/interpreterDataService'; +import { LanguageServerAnalysisOptions } from '../../../client/activation/languageServer/analysisOptions'; +import { LanguageServerFolderService } from '../../../client/activation/languageServer/languageServerFolderService'; +import { IInterpreterDataService, ILanguageServerFolderService } 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 { PathUtils } from '../../../client/common/platform/pathUtils'; +import { IConfigurationService, IDisposable, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner } from '../../../client/common/types'; +import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; +import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; +import { IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; +import { ProposeLanguageServerBanner } from '../../../client/languageServices/proposeLanguageServerBanner'; +import { sleep } from '../../core'; + +// tslint:disable:no-unnecessary-override no-any chai-vague-errors no-unused-expression max-func-body-length + +suite('Language Server - Analysis Options', () => { + class TestClass extends LanguageServerAnalysisOptions { + public getExcludedFiles(): string[] { + return super.getExcludedFiles(); + } + public getVsCodeExcludeSection(setting: string, list: string[]): void { + return super.getVsCodeExcludeSection(setting, list); + } + public getPythonExcludeSection(list: string[]): void { + return super.getPythonExcludeSection(list); + } + public getTypeshedPaths(): string[] { + return super.getTypeshedPaths(); + } + public async onSettingsChanged(): Promise { + return super.onSettingsChanged(); + } + public async notifyIfValuesHaveChanged(oldArray: string[], newArray: string[]): Promise { + return super.notifyIfValuesHaveChanged(oldArray, newArray); + } + } + let analysisOptions: TestClass; + let context: typemoq.IMock; + let envVarsProvider: IEnvironmentVariablesProvider; + let configurationService: IConfigurationService; + let workspace: IWorkspaceService; + let surveyBanner: IPythonExtensionBanner; + let interpreterService: IInterpreterService; + let outputChannel: IOutputChannel; + let pathUtils: IPathUtils; + let lsFolderService: ILanguageServerFolderService; + let interpreterDataService: IInterpreterDataService; + setup(() => { + context = typemoq.Mock.ofType(); + envVarsProvider = mock(EnvironmentVariablesProvider); + configurationService = mock(ConfigurationService); + workspace = mock(WorkspaceService); + surveyBanner = mock(ProposeLanguageServerBanner); + interpreterService = mock(InterpreterService); + outputChannel = typemoq.Mock.ofType().object; + pathUtils = mock(PathUtils); + interpreterDataService = mock(InterpreterDataService); + lsFolderService = mock(LanguageServerFolderService); + analysisOptions = new TestClass(context.object, instance(envVarsProvider), + instance(configurationService), + instance(workspace), instance(surveyBanner), + instance(interpreterService), instance(interpreterDataService), outputChannel, + instance(pathUtils), instance(lsFolderService)); + }); + test('Initialize will add event handlers and will dispose them when running dispose', async () => { + const disposable1 = typemoq.Mock.ofType(); + const disposable2 = typemoq.Mock.ofType(); + when(workspace.onDidChangeConfiguration).thenReturn(() => disposable1.object); + when(interpreterService.onDidChangeInterpreter).thenReturn(() => disposable2.object); + + await analysisOptions.initialize(undefined); + + verify(workspace.onDidChangeConfiguration).once(); + verify(interpreterService.onDidChangeInterpreter).once(); + + disposable1.setup(d => d.dispose()).verifiable(typemoq.Times.once()); + disposable2.setup(d => d.dispose()).verifiable(typemoq.Times.once()); + + analysisOptions.dispose(); + + disposable1.verifyAll(); + disposable2.verifyAll(); + }); + test('Changes to settings or interpreter will be debounced', async () => { + const disposable1 = typemoq.Mock.ofType(); + const disposable2 = typemoq.Mock.ofType(); + let configChangedHandler!: Function; + let interpreterChangedHandler!: Function; + when(workspace.onDidChangeConfiguration).thenReturn(cb => { configChangedHandler = cb; return disposable1.object; }); + when(interpreterService.onDidChangeInterpreter).thenReturn(cb => { interpreterChangedHandler = cb; return disposable2.object; }); + let settingsChangedInvokedCount = 0; + when(interpreterDataService.getInterpreterData(undefined)) + .thenCall(() => settingsChangedInvokedCount += 1) + .thenResolve(); + + await analysisOptions.initialize(undefined); + expect(configChangedHandler).to.not.be.undefined; + expect(interpreterChangedHandler).to.not.be.undefined; + + for (let i = 0; i < 100; i += 1) { + configChangedHandler.call(analysisOptions); + interpreterChangedHandler.call(analysisOptions); + } + expect(settingsChangedInvokedCount).to.be.equal(0); + + await sleep(1); + + expect(settingsChangedInvokedCount).to.be.equal(1); + }); + test('If there are no changes then no events will be fired', async () => { + when(interpreterDataService.getInterpreterData(undefined)) + .thenResolve({ hash: '' } as any); + analysisOptions.getExcludedFiles = () => []; + analysisOptions.getTypeshedPaths = () => []; + + let eventFired = false; + analysisOptions.onDidChange(() => eventFired = true); + + await analysisOptions.onSettingsChanged(); + await sleep(1); + + expect(eventFired).to.be.equal(false); + }); + test('Event must be fired if excluded files are different', async () => { + when(interpreterDataService.getInterpreterData(undefined)) + .thenResolve(); + analysisOptions.getExcludedFiles = () => ['1']; + analysisOptions.getTypeshedPaths = () => []; + + let eventFired = false; + analysisOptions.onDidChange(() => eventFired = true); + + await analysisOptions.onSettingsChanged(); + await sleep(1); + + expect(eventFired).to.be.equal(true); + }); + test('Event must be fired if typeshed files are different', async () => { + when(interpreterDataService.getInterpreterData(undefined)) + .thenResolve(); + analysisOptions.getExcludedFiles = () => []; + analysisOptions.getTypeshedPaths = () => ['1']; + + let eventFired = false; + analysisOptions.onDidChange(() => eventFired = true); + + await analysisOptions.onSettingsChanged(); + await sleep(1); + + expect(eventFired).to.be.equal(true); + }); + test('Event must be fired if interpreter info is different', async () => { + when(interpreterDataService.getInterpreterData({ hash: '1234' } as any)) + .thenResolve(); + + let eventFired = false; + analysisOptions.onDidChange(() => eventFired = true); + + await analysisOptions.onSettingsChanged(); + await sleep(1); + + expect(eventFired).to.be.equal(true); + }); + test('Changes to settings will be filtered to current resoruce', async () => { + const uri = Uri.file(__filename); + const disposable1 = typemoq.Mock.ofType(); + const disposable2 = typemoq.Mock.ofType(); + let configChangedHandler!: Function; + let interpreterChangedHandler!: Function; + when(workspace.onDidChangeConfiguration).thenReturn(cb => { configChangedHandler = cb; return disposable1.object; }); + when(interpreterService.onDidChangeInterpreter).thenReturn(cb => { interpreterChangedHandler = cb; return disposable2.object; }); + let settingsChangedInvokedCount = 0; + when(interpreterDataService.getInterpreterData(uri)).thenResolve(); + + analysisOptions.onDidChange(() => settingsChangedInvokedCount += 1); + await analysisOptions.initialize(uri); + expect(configChangedHandler).to.not.be.undefined; + expect(interpreterChangedHandler).to.not.be.undefined; + + settingsChangedInvokedCount = 0; + for (let i = 0; i < 100; i += 1) { + const event = typemoq.Mock.ofType(); + event.setup(e => e.affectsConfiguration(typemoq.It.isValue('python'), typemoq.It.isValue(uri))) + .verifiable(typemoq.Times.once()); + configChangedHandler.call(analysisOptions, event.object); + interpreterChangedHandler.call(analysisOptions); + + event.verifyAll(); + } + expect(settingsChangedInvokedCount).to.be.equal(0); + + await sleep(1); + + expect(settingsChangedInvokedCount).to.be.equal(1); + }); +}); diff --git a/src/test/activation/languageServer/languageClientFactory.unit.test.ts b/src/test/activation/languageServer/languageClientFactory.unit.test.ts new file mode 100644 index 000000000000..133992583ac8 --- /dev/null +++ b/src/test/activation/languageServer/languageClientFactory.unit.test.ts @@ -0,0 +1,138 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +//tslint:disable:no-require-imports no-require-imports no-var-requires no-any no-unnecessary-class max-func-body-length match-default-export-name + +import { expect } from 'chai'; +import * as path from 'path'; +import rewiremock from 'rewiremock'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import * as typemoq from 'typemoq'; +import { Uri } from 'vscode'; +import { LanguageClientOptions, ServerOptions } from 'vscode-languageclient'; +import { BaseLanguageClientFactory, DownloadedLanguageClientFactory, SimpleLanguageClientFactory } from '../../../client/activation/languageServer/languageClientFactory'; +import { LanguageServerFolderService } from '../../../client/activation/languageServer/languageServerFolderService'; +import { PlatformData } from '../../../client/activation/platformData'; +import { PythonSettings } from '../../../client/common/configSettings'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; +import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; + +const dotNetCommand = 'dotnet'; +const languageClientName = 'Python Tools'; + +suite('Language Server - LanguageClient Factory', () => { + let configurationService: IConfigurationService; + let settings: IPythonSettings; + setup(() => { + configurationService = mock(ConfigurationService); + settings = mock(PythonSettings); + when(configurationService.getSettings(anything())).thenReturn(instance(settings)); + }); + teardown(() => { + rewiremock.disable(); + }); + + test('Download factory is used when required to download the LS', async () => { + const downloadFactory = mock(DownloadedLanguageClientFactory); + const simpleFactory = mock(SimpleLanguageClientFactory); + const factory = new BaseLanguageClientFactory(instance(downloadFactory), instance(simpleFactory), instance(configurationService)); + const uri = Uri.file(__filename); + const options = typemoq.Mock.ofType().object; + when(settings.downloadLanguageServer).thenReturn(true); + + await factory.createLanguageClient(uri, options); + + verify(configurationService.getSettings(uri)).once(); + verify(downloadFactory.createLanguageClient(uri, options)).once(); + verify(simpleFactory.createLanguageClient(uri, options)).never(); + }); + test('Simple factory is used when not required to download the LS', async () => { + const downloadFactory = mock(DownloadedLanguageClientFactory); + const simpleFactory = mock(SimpleLanguageClientFactory); + const factory = new BaseLanguageClientFactory(instance(downloadFactory), instance(simpleFactory), instance(configurationService)); + const uri = Uri.file(__filename); + const options = typemoq.Mock.ofType().object; + when(settings.downloadLanguageServer).thenReturn(false); + + await factory.createLanguageClient(uri, options); + + verify(configurationService.getSettings(uri)).once(); + verify(downloadFactory.createLanguageClient(uri, options)).never(); + verify(simpleFactory.createLanguageClient(uri, options)).once(); + }); + test('Download factory will make use of the language server folder name and client will be created', async () => { + const platformData = mock(PlatformData); + const lsFolderService = mock(LanguageServerFolderService); + const factory = new DownloadedLanguageClientFactory(instance(platformData), instance(lsFolderService)); + const uri = Uri.file(__filename); + const options = typemoq.Mock.ofType().object; + const languageServerFolder = 'some folder name'; + const engineDllName = 'xyz.dll'; + when(lsFolderService.getLanguageServerFolderName()).thenResolve(languageServerFolder); + when(platformData.engineExecutableName).thenReturn(engineDllName); + + const serverModule = path.join(EXTENSION_ROOT_DIR, languageServerFolder, engineDllName); + const expectedServerOptions = { + run: { command: serverModule, rgs: [], options: { stdio: 'pipe' } }, + debug: { command: serverModule, args: ['--debug'], options: { stdio: 'pipe' } } + }; + rewiremock.enable(); + + class MockClass { + constructor(language: string, name: string, serverOptions: ServerOptions, clientOptions: LanguageClientOptions) { + expect(language).to.be.equal('python'); + expect(name).to.be.equal(languageClientName); + expect(clientOptions).to.be.deep.equal(options); + expect(serverOptions).to.be.deep.equal(expectedServerOptions); + } + } + rewiremock('vscode-languageclient').with({ LanguageClient: MockClass }); + + const client = await factory.createLanguageClient(uri, options); + + verify(lsFolderService.getLanguageServerFolderName()).once(); + verify(platformData.engineExecutableName).atLeast(1); + verify(platformData.engineDllName).never(); + verify(platformData.platformName).never(); + expect(client).to.be.instanceOf(MockClass); + }); + test('Simple factory will make use of the language server folder name and client will be created', async () => { + const platformData = mock(PlatformData); + const lsFolderService = mock(LanguageServerFolderService); + const factory = new SimpleLanguageClientFactory(instance(platformData), instance(lsFolderService)); + const uri = Uri.file(__filename); + const options = typemoq.Mock.ofType().object; + const languageServerFolder = 'some folder name'; + const engineDllName = 'xyz.dll'; + when(lsFolderService.getLanguageServerFolderName()).thenResolve(languageServerFolder); + when(platformData.engineDllName).thenReturn(engineDllName); + + const serverModule = path.join(EXTENSION_ROOT_DIR, languageServerFolder, engineDllName); + const expectedServerOptions = { + run: { command: dotNetCommand, args: [serverModule], options: { stdio: 'pipe' } }, + debug: { command: dotNetCommand, args: [serverModule, '--debug'], options: { stdio: 'pipe' } } + }; + rewiremock.enable(); + + class MockClass { + constructor(language: string, name: string, serverOptions: ServerOptions, clientOptions: LanguageClientOptions) { + expect(language).to.be.equal('python'); + expect(name).to.be.equal(languageClientName); + expect(clientOptions).to.be.deep.equal(options); + expect(serverOptions).to.be.deep.equal(expectedServerOptions); + } + } + rewiremock('vscode-languageclient').with({ LanguageClient: MockClass }); + + const client = await factory.createLanguageClient(uri, options); + + verify(lsFolderService.getLanguageServerFolderName()).once(); + verify(platformData.engineExecutableName).never(); + verify(platformData.engineDllName).once(); + verify(platformData.platformName).never(); + expect(client).to.be.instanceOf(MockClass); + }); +}); diff --git a/src/test/activation/languageServer/languageServer.unit.test.ts b/src/test/activation/languageServer/languageServer.unit.test.ts index 492ccfc9e4da..168bc4a1310c 100644 --- a/src/test/activation/languageServer/languageServer.unit.test.ts +++ b/src/test/activation/languageServer/languageServer.unit.test.ts @@ -3,75 +3,76 @@ 'use strict'; -// tslint:disable:max-func-body-length - import { expect } from 'chai'; -import * as path from 'path'; -import * as TypeMoq from 'typemoq'; -import { LanguageServerExtensionActivator } from '../../../client/activation/languageServer/languageServer'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../client/common/application/types'; -import { IPlatformService } from '../../../client/common/platform/types'; -import { IConfigurationService, IDisposableRegistry, IExtensionContext, IFeatureDeprecationManager, IOutputChannel, IPathUtils, IPythonSettings } from '../../../client/common/types'; -import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; -import { IServiceContainer } from '../../../client/ioc/types'; +import { instance, mock, when } from 'ts-mockito'; +import * as typemoq from 'typemoq'; +import { Uri } from 'vscode'; +import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; +import { BaseLanguageClientFactory } from '../../../client/activation/languageServer/languageClientFactory'; +import { LanguageServer } from '../../../client/activation/languageServer/languageServer'; +import { ILanaguageServer, ILanguageClientFactory } from '../../../client/activation/types'; +import { IDisposable } from '../../../client/common/types'; +import { sleep } from '../../../client/common/utils/async'; + +//tslint:disable:no-require-imports no-require-imports no-var-requires no-any no-unnecessary-class max-func-body-length -suite('Language Server', () => { - let serviceContainer: TypeMoq.IMock; - let pythonSettings: TypeMoq.IMock; - let appShell: TypeMoq.IMock; - let cmdManager: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let languageServer: LanguageServerExtensionActivator; - let extensionContext: TypeMoq.IMock; +suite('Language Server - LanguageServer', () => { + let clientFactory: ILanguageClientFactory; + let server: ILanaguageServer; + let client: typemoq.IMock; setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - extensionContext = TypeMoq.Mock.ofType(); - appShell = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - cmdManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - const configService = TypeMoq.Mock.ofType(); - pythonSettings = TypeMoq.Mock.ofType(); + client = typemoq.Mock.ofType(); + clientFactory = mock(BaseLanguageClientFactory); + server = new LanguageServer(instance(clientFactory)); + }); + teardown(() => { + client.setup(c => c.stop()).returns(() => Promise.resolve()); + server.dispose(); + }); + test('Loading extension will throw an error if not activated', () => { + expect(() => server.loadExtension()).throws(); + }); + test('Send telemetry when LS has started and disposes appropriately', async () => { + const loadExtensionArgs = { x: 1 }; + const uri = Uri.file(__filename); + const options = typemoq.Mock.ofType().object; + client.setup(c => (c as any).then).returns(() => undefined); + when(clientFactory.createLanguageClient(uri, options)).thenResolve(client.object); + const startDisposable = typemoq.Mock.ofType(); + client.setup(c => c.stop()).returns(() => Promise.resolve()); + client + .setup(c => c.start()).returns(() => startDisposable.object) + .verifiable(typemoq.Times.once()); + client + .setup(c => c.sendRequest(typemoq.It.isValue('python/loadExtension'), typemoq.It.isValue(loadExtensionArgs))) + .returns(() => Promise.resolve(undefined) as any); - workspaceService.setup(w => w.hasWorkspaceFolders).returns(() => false); - workspaceService.setup(w => w.workspaceFolders).returns(() => []); - configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); + expect(() => server.loadExtension(loadExtensionArgs)).throws(); + client.verify(c => c.sendRequest(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + client + .setup(c => c.initializeResult).returns(() => false as any) + .verifiable(typemoq.Times.once()); - const output = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())).returns(() => output.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService))).returns(() => workspaceService.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IConfigurationService))).returns(() => configService.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICommandManager))).returns(() => cmdManager.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformService.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IExtensionContext))).returns(() => extensionContext.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFeatureDeprecationManager))).returns(() => TypeMoq.Mock.ofType().object); + const promise = server.start(uri, options); - languageServer = new LanguageServerExtensionActivator(serviceContainer.object); - }); + // Even though server has started request should not yet be sent out. + // Not untill language client has initialized. + expect(() => server.loadExtension(loadExtensionArgs)).throws(); + client.verify(c => c.sendRequest(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); - test('Must get PYTHONPATH from env vars provider', async () => { - const pathDelimiter = 'x'; - const pythonPathVar = ['A', 'B', '1']; - const envVarsProvider = TypeMoq.Mock.ofType(); - const pathUtils = TypeMoq.Mock.ofType(); - extensionContext.setup(e => e.extensionPath).returns(() => path.join('a', 'b', 'c')); - pathUtils.setup(p => p.delimiter).returns(() => pathDelimiter); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IEnvironmentVariablesProvider))).returns(() => envVarsProvider.object); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object); - envVarsProvider - .setup(p => p.getEnvironmentVariables()) - .returns(() => { return Promise.resolve({ PYTHONPATH: pythonPathVar.join(pathDelimiter) }); }) - .verifiable(TypeMoq.Times.once()); + // // Initialize language client and verify that the request was sent out. + client + .setup(c => c.initializeResult).returns(() => true as any) + .verifiable(typemoq.Times.once()); + await sleep(120); + expect(() => server.loadExtension(loadExtensionArgs)).to.not.throw(); + client.verify(c => c.sendRequest(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.once()); + client.verify(c => c.stop(), typemoq.Times.never()); - // tslint:disable-next-line:no-any - (languageServer as any).languageServerFolder = ''; - const options = await languageServer.getAnalysisOptions(); + await promise; + server.dispose(); - expect(options!).not.to.equal(undefined, 'options cannot be undefined'); - expect(options!.initializationOptions).not.to.equal(undefined, 'initializationOptions cannot be undefined'); - expect(options!.initializationOptions!.searchPaths).to.include.members(pythonPathVar); + client.verify(c => c.stop(), typemoq.Times.once()); + startDisposable.verify(d => d.dispose(), typemoq.Times.once()); }); }); diff --git a/src/test/activation/languageServer/manager.unit.test.ts b/src/test/activation/languageServer/manager.unit.test.ts new file mode 100644 index 000000000000..c737903db4cd --- /dev/null +++ b/src/test/activation/languageServer/manager.unit.test.ts @@ -0,0 +1,135 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect, use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; +import { instance, mock, verify, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { LanguageClientOptions } from 'vscode-languageclient'; +import { LanguageServerAnalysisOptions } from '../../../client/activation/languageServer/analysisOptions'; +import { LanguageServer } from '../../../client/activation/languageServer/languageServer'; +import { LanguageServerManager } from '../../../client/activation/languageServer/manager'; +import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../../../client/activation/types'; +import { ServiceContainer } from '../../../client/ioc/container'; +import { IServiceContainer } from '../../../client/ioc/types'; +import { sleep } from '../../core'; + +use(chaiAsPromised); + +// tslint:disable:max-func-body-length no-any chai-vague-errors no-unused-expression + +suite('Language Server - Manager', () => { + let manager: ILanguageServerManager; + let serviceContainer: IServiceContainer; + let analysisOptions: ILanguageServerAnalysisOptions; + let languageServer: ILanaguageServer; + let onChangeHandler: Function; + const languageClientOptions = { x: 1 } as any as LanguageClientOptions; + setup(() => { + serviceContainer = mock(ServiceContainer); + analysisOptions = mock(LanguageServerAnalysisOptions); + languageServer = mock(LanguageServer); + manager = new LanguageServerManager(instance(serviceContainer), instance(analysisOptions)); + }); + + [undefined, Uri.file(__filename)].forEach(resource => { + async function startLanguageServer() { + let handlerRegistered = false; + const changeFn = (handler: Function) => { handlerRegistered = true; onChangeHandler = handler; }; + when(analysisOptions.initialize(resource)).thenResolve(); + when(analysisOptions.getAnalysisOptions()).thenResolve(languageClientOptions); + when(analysisOptions.onDidChange).thenReturn(changeFn as any); + when(serviceContainer.get(ILanaguageServer)).thenReturn(instance(languageServer)); + when(languageServer.start(resource, languageClientOptions)).thenResolve(); + + await manager.start(resource); + + verify(analysisOptions.initialize(resource)).once(); + verify(analysisOptions.getAnalysisOptions()).once(); + verify(serviceContainer.get(ILanaguageServer)).once(); + verify(languageServer.start(resource, languageClientOptions)).once(); + expect(handlerRegistered).to.be.true; + verify(languageServer.dispose()).never(); + } + test('Start must register handlers and initialize analysis options', async () => { + await startLanguageServer(); + + manager.dispose(); + + verify(languageServer.dispose()).once(); + }); + test('Attempting to start LS will throw an exception', async () => { + await startLanguageServer(); + + await expect(manager.start(resource)).to.eventually.be.rejectedWith('Language Server already started'); + }); + test('Changes in analysis options must restart LS', async () => { + await startLanguageServer(); + + await onChangeHandler.call(manager); + await sleep(1); + + verify(languageServer.dispose()).once(); + + verify(analysisOptions.getAnalysisOptions()).twice(); + verify(serviceContainer.get(ILanaguageServer)).twice(); + verify(languageServer.start(resource, languageClientOptions)).twice(); + }); + test('Changes in analysis options must throttled when restarting LS', async () => { + await startLanguageServer(); + + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await Promise.all([onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager)]); + await sleep(1); + + verify(languageServer.dispose()).once(); + + verify(analysisOptions.getAnalysisOptions()).twice(); + verify(serviceContainer.get(ILanaguageServer)).twice(); + verify(languageServer.start(resource, languageClientOptions)).twice(); + }); + test('Multiple changes in analysis options must restart LS twice', async () => { + await startLanguageServer(); + + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await Promise.all([onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager)]); + await sleep(1); + + verify(languageServer.dispose()).once(); + + verify(analysisOptions.getAnalysisOptions()).twice(); + verify(serviceContainer.get(ILanaguageServer)).twice(); + verify(languageServer.start(resource, languageClientOptions)).twice(); + + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await onChangeHandler.call(manager); + await Promise.all([onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager), + onChangeHandler.call(manager)]); + await sleep(1); + + verify(languageServer.dispose()).twice(); + + verify(analysisOptions.getAnalysisOptions()).thrice(); + verify(serviceContainer.get(ILanaguageServer)).thrice(); + verify(languageServer.start(resource, languageClientOptions)).thrice(); + }); + }); +}); diff --git a/src/test/activation/platformData.unit.test.ts b/src/test/activation/platformData.unit.test.ts index 0044c27b0182..0acf76e9b7cb 100644 --- a/src/test/activation/platformData.unit.test.ts +++ b/src/test/activation/platformData.unit.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import * as TypeMoq from 'typemoq'; import { PlatformData, PlatformLSExecutables } from '../../client/activation/platformData'; -import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; +import { IPlatformService } from '../../client/common/platform/types'; const testDataWinMac = [ { isWindows: true, is64Bit: true, expectedName: 'win-x64' }, @@ -38,14 +38,10 @@ suite('Activation - platform data', () => { platformService.setup(x => x.isMac).returns(() => !t.isWindows); platformService.setup(x => x.is64bit).returns(() => t.is64Bit); - const fs = TypeMoq.Mock.ofType(); - const pd = new PlatformData(platformService.object, fs.object); + const pd = new PlatformData(platformService.object); - const actual = await pd.getPlatformName(); + const actual = pd.platformName; assert.equal(actual, t.expectedName, `${actual} does not match ${t.expectedName}`); - - const actualHash = await pd.getExpectedHash(); - assert.equal(actualHash, t.expectedName, `${actual} hash not match ${t.expectedName}`); } }); test('Name and hash (Linux)', async () => { @@ -56,15 +52,10 @@ suite('Activation - platform data', () => { platformService.setup(x => x.isLinux).returns(() => true); platformService.setup(x => x.is64bit).returns(() => true); - const fs = TypeMoq.Mock.ofType(); - fs.setup(x => x.readFile(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(`NAME="name"\nID=${t.name}\nID_LIKE=debian`)); - const pd = new PlatformData(platformService.object, fs.object); + const pd = new PlatformData(platformService.object); - const actual = await pd.getPlatformName(); + const actual = pd.platformName; assert.equal(actual, t.expectedName, `${actual} does not match ${t.expectedName}`); - - const actualHash = await pd.getExpectedHash(); - assert.equal(actual, t.expectedName, `${actual} hash not match ${t.expectedName}`); } }); test('Module name', async () => { @@ -74,10 +65,9 @@ suite('Activation - platform data', () => { platformService.setup(x => x.isLinux).returns(() => t.isLinux); platformService.setup(x => x.isMac).returns(() => t.isMac); - const fs = TypeMoq.Mock.ofType(); - const pd = new PlatformData(platformService.object, fs.object); + const pd = new PlatformData(platformService.object); - const actual = pd.getEngineExecutableName(); + const actual = pd.engineExecutableName; assert.equal(actual, t.expectedName, `${actual} does not match ${t.expectedName}`); } }); diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index dbfe0eec6b52..146a0c2c0700 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -87,9 +87,13 @@ suite('Interpreters - Auto Selection', () => { verify(systemInterpreter.setNextRule(anything())).never(); }); test('Run rules in background', async () => { + let eventFired = false; + autoSelectionService.onDidChangeAutoSelectedInterpreter(() => eventFired = true); autoSelectionService.initializeStore = () => Promise.resolve(); await autoSelectionService.autoSelectInterpreter(undefined); + expect(eventFired).to.deep.equal(true, 'event not fired'); + const allRules = [userDefinedInterpreter, winRegInterpreter, currentPathInterpreter, systemInterpreter, workspaceInterpreter, cachedPaths]; for (const service of allRules) { verify(service.autoSelectInterpreter(undefined)).once(); @@ -100,16 +104,24 @@ suite('Interpreters - Auto Selection', () => { verify(userDefinedInterpreter.autoSelectInterpreter(anything(), autoSelectionService)).once(); }); test('Run userDefineInterpreter as the first rule', async () => { + let eventFired = false; + autoSelectionService.onDidChangeAutoSelectedInterpreter(() => eventFired = true); autoSelectionService.initializeStore = () => Promise.resolve(); + await autoSelectionService.autoSelectInterpreter(undefined); + expect(eventFired).to.deep.equal(true, 'event not fired'); verify(userDefinedInterpreter.autoSelectInterpreter(undefined, autoSelectionService)).once(); }); test('Initialize the store', async () => { let initialize = false; + let eventFired = false; + autoSelectionService.onDidChangeAutoSelectedInterpreter(() => eventFired = true); autoSelectionService.initializeStore = async () => initialize = true as any; + await autoSelectionService.autoSelectInterpreter(undefined); + expect(eventFired).to.deep.equal(true, 'event not fired'); expect(initialize).to.be.equal(true, 'Not invoked'); }); test('Initializing the store would be executed once', async () => { @@ -162,7 +174,7 @@ suite('Interpreters - Auto Selection', () => { verify(state.updateValue(interpreterInfo)).once(); expect(selectedInterpreter).to.deep.equal(interpreterInfo); - expect(eventFired).to.deep.equal(true, 'event not fired'); + expect(eventFired).to.deep.equal(false, 'event fired'); }); test('Do not store global interpreter info in state store when resource is undefined and version is lower than one already in state', async () => { let eventFired = false; @@ -198,7 +210,7 @@ suite('Interpreters - Auto Selection', () => { verify(state.updateValue(anything())).once(); expect(selectedInterpreter).to.deep.equal(interpreterInfo); - expect(eventFired).to.deep.equal(true, 'event fired'); + expect(eventFired).to.deep.equal(false, 'event fired'); }); test('Store global interpreter info in state store', async () => { const pythonPath = 'Hello World'; @@ -227,7 +239,7 @@ suite('Interpreters - Auto Selection', () => { verify(state.updateValue(interpreterInfo)).never(); expect(selectedInterpreter).to.deep.equal(interpreterInfo); - expect(eventFired).to.deep.equal(true, 'event not fired'); + expect(eventFired).to.deep.equal(false, 'event fired'); }); test('Store workspace interpreter info in state store', async () => { const pythonPath = 'Hello World'; From 92748143cb5f1d24c3f4c2a28f36282034064f4c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 08:43:30 -0800 Subject: [PATCH 03/21] Added telemetry --- package-lock.json | 6 ++ package.json | 1 + .../activation/languageServer/activator.ts | 3 + .../languageServer/analysisOptions.ts | 4 +- .../languageServer/languageServer.ts | 7 ++- .../activation/languageServer/manager.ts | 3 + src/client/common/logger.ts | 3 + src/client/extension.ts | 34 +---------- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 58 +++++++++++++++++-- .../languageServer/activator.unit.test.ts | 2 +- 11 files changed, 80 insertions(+), 42 deletions(-) diff --git a/package-lock.json b/package-lock.json index a1fcc739a083..165e6681945f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1635,6 +1635,12 @@ "integrity": "sha512-Tt7w/ylBS/OEAlSCwzB0Db1KbxnkycP/1UkQpbvKFYoUuRn4uYsC3xh5TRPrOjTy0i8TIkSz1JdNL4GPVdf3KQ==", "dev": true }, + "@types/stack-trace": { + "version": "0.0.29", + "resolved": "https://registry.npmjs.org/@types/stack-trace/-/stack-trace-0.0.29.tgz", + "integrity": "sha512-TgfOX+mGY/NyNxJLIbDWrO9DjGoVSW9+aB8H2yy1fy32jsvxijhmyJI9fDFgvz3YP4lvJaq9DzdR/M1bOgVc9g==", + "dev": true + }, "@types/tapable": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/@types/tapable/-/tapable-1.0.4.tgz", diff --git a/package.json b/package.json index 6f5a31147664..0fe5ea17be91 100644 --- a/package.json +++ b/package.json @@ -1904,6 +1904,7 @@ "@types/semver": "^5.5.0", "@types/shortid": "^0.0.29", "@types/sinon": "^4.3.0", + "@types/stack-trace": "0.0.29", "@types/temp": "^0.8.32", "@types/tmp": "0.0.33", "@types/untildify": "^3.0.0", diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts index 4267f6e86fd9..3625e63ab5ac 100644 --- a/src/client/activation/languageServer/activator.ts +++ b/src/client/activation/languageServer/activator.ts @@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; +import { traceDecorators } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; import { IConfigurationService, Resource } from '../../common/types'; import { EXTENSION_ROOT_DIR } from '../../constants'; @@ -26,6 +27,7 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { @inject(ILanguageServerDownloader) private readonly lsDownloader: ILanguageServerDownloader, @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService) { } + @traceDecorators.error('Failed to activate language server') public async activate(): Promise { const mainWorkspaceUri = this.workspace.hasWorkspaceFolders ? this.workspace.workspaceFolders![0].uri : undefined; await this.ensureLanguageServerIsAvailable(mainWorkspaceUri); @@ -34,6 +36,7 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { public async dispose(): Promise { this.manager.dispose(); } + @traceDecorators.error('Failed to ensure language server is available') protected async ensureLanguageServerIsAvailable(resource: Resource) { const settings = this.configurationService.getSettings(resource); if (!settings.downloadLanguageServer) { diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts index 5b918d48e1da..3107e3ba45c8 100644 --- a/src/client/activation/languageServer/analysisOptions.ts +++ b/src/client/activation/languageServer/analysisOptions.ts @@ -9,7 +9,7 @@ import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposa import { LanguageClientOptions, ProvideCompletionItemsSignature } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; import { isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; -import { traceError } from '../../common/logger'; +import { traceDecorators, traceError } from '../../common/logger'; import { IConfigurationService, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../common/types'; import { debounce } from '../../common/utils/decorators'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; @@ -54,6 +54,7 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt this.disposables.forEach(d => d.dispose()); this.didChange.dispose(); } + @traceDecorators.error('Failed to get analysis options') public async getAnalysisOptions(): Promise { const properties = new Map(); let interpreterData: InterpreterData | undefined; @@ -172,6 +173,7 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt } this.onSettingsChanged().catch(ex => traceError('Failed to detect changes', ex)); } + @traceDecorators.verbose('Changes in python settings detected in analysis options') @debounce(1000) protected async onSettingsChanged(): Promise { const idata = await this.interpreterDataService.getInterpreterData(this.resource); diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index 9963dbc0b648..bc2d12de35ec 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -10,7 +10,7 @@ import { Resource } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { noop } from '../../common/utils/misc'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; -import { PYTHON_LANGUAGE_SERVER_ENABLED, PYTHON_LANGUAGE_SERVER_TELEMETRY } from '../../telemetry/constants'; +import { PYTHON_LANGUAGE_SERVER_ENABLED, PYTHON_LANGUAGE_SERVER_READY, PYTHON_LANGUAGE_SERVER_TELEMETRY } from '../../telemetry/constants'; import { ProgressReporting } from '../progress'; import { ILanaguageServer, ILanguageClientFactory } from '../types'; @@ -40,7 +40,7 @@ export class LanguageServer implements ILanaguageServer { } @traceDecorators.error('Failed to start language server') - @captureTelemetry(PYTHON_LANGUAGE_SERVER_ENABLED) + @captureTelemetry(PYTHON_LANGUAGE_SERVER_ENABLED, undefined, true) public async start(resource: Resource, options: LanguageClientOptions): Promise { this.languageClient = await this.factory.createLanguageClient(resource, options); this.disposables.push(this.languageClient!.start()); @@ -52,6 +52,7 @@ export class LanguageServer implements ILanaguageServer { sendTelemetryEvent(eventName, telemetryEvent.Measurements, telemetryEvent.Properties); }); } + @traceDecorators.error('Failed to load Language Server extension') public loadExtension(args?: {}) { if (!this.languageClient) { throw new Error('Activation not completed or not invoked'); @@ -62,7 +63,7 @@ export class LanguageServer implements ILanaguageServer { this.languageClient.sendRequest('python/loadExtension', args) .then(noop, ex => traceError('Request python/loadExtension failed', ex)); } - + @captureTelemetry(PYTHON_LANGUAGE_SERVER_READY, undefined, true) private async serverReady(): Promise { while (this.languageClient && !this.languageClient!.initializeResult) { await sleep(100); diff --git a/src/client/activation/languageServer/manager.ts b/src/client/activation/languageServer/manager.ts index d383cbaaf329..2dbfd74b4d70 100644 --- a/src/client/activation/languageServer/manager.ts +++ b/src/client/activation/languageServer/manager.ts @@ -8,6 +8,8 @@ import { traceDecorators } from '../../common/logger'; import { IDisposable, Resource } from '../../common/types'; import { debounce } from '../../common/utils/decorators'; import { IServiceContainer } from '../../ioc/types'; +import { captureTelemetry } from '../../telemetry'; +import { PYTHON_LANGUAGE_SERVER_STARTUP } from '../../telemetry/constants'; import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../types'; export class LanguageServerManager implements ILanguageServerManager { @@ -42,6 +44,7 @@ export class LanguageServerManager implements ILanguageServerManager { } await this.startLanguageServer(); } + @captureTelemetry(PYTHON_LANGUAGE_SERVER_STARTUP, undefined, true) @traceDecorators.verbose('Starting Langauge Server') protected async startLanguageServer(): Promise { this.lanaguageServer = this.serviceContainer.get(ILanaguageServer); diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index 3843da84a42c..b52e9a933f65 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -1,6 +1,7 @@ // tslint:disable:no-console import { injectable } from 'inversify'; +import { sendTelemetryEvent } from '../telemetry'; import { skipIfTest } from './helpers'; import { ILogger, LogLevel } from './types'; @@ -137,6 +138,8 @@ function trace(message: string, options: LogOptions = LogOptions.None, logLevel? } if (ex) { new Logger().logError(messagesToLog.join(', '), ex); + // tslint:disable-next-line:no-any + sendTelemetryEvent('ERROR', undefined, undefined, ex); } else { new Logger().logInformation(messagesToLog.join(', ')); } diff --git a/src/client/extension.ts b/src/client/extension.ts index bb9d72540dbb..359e58f54da0 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -10,8 +10,6 @@ import { StopWatch } from './common/utils/stopWatch'; // Do not move this line of code (used to measure extension load times). const stopWatch = new StopWatch(); import { Container } from 'inversify'; -import { basename as pathBasename, sep as pathSep } from 'path'; -import * as stackTrace from 'stack-trace'; import { CodeActionKind, debug, @@ -58,7 +56,6 @@ import { import { createDeferred } from './common/utils/async'; import { Common } from './common/utils/localize'; import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; -import { EXTENSION_ROOT_DIR } from './constants'; import { registerTypes as dataScienceRegisterTypes } from './datascience/serviceRegistry'; import { IDataScience } from './datascience/types'; import { DebuggerTypeName } from './debugger/constants'; @@ -402,34 +399,6 @@ function notifyUser(msg: string) { } } -function sanitizeFilename(filename: string): string { - if (filename.startsWith(EXTENSION_ROOT_DIR + pathSep)) { - filename = `${filename.substring(EXTENSION_ROOT_DIR.length)}`; - } else { - // We don't really care about files outside our extension. - filename = `${pathSep}${pathBasename(filename)}`; - } - return filename; -} - -function getStackTrace(ex: Error): string { - // We aren't showing the error message (ex.message) since it might - // contain PII. - let trace = ''; - for (const frame of stackTrace.parse(ex)) { - let filename = frame.getFileName(); - if (filename) { - filename = sanitizeFilename(filename); - const lineno = frame.getLineNumber(); - const colno = frame.getColumnNumber(); - trace += `\n\tat ${filename}:${lineno}:${colno}`; - } else { - trace += '\n\tat '; - } - } - return trace.trim(); -} - async function sendErrorTelemetry(ex: Error) { try { // tslint:disable-next-line:no-any @@ -441,8 +410,7 @@ async function sendErrorTelemetry(ex: Error) { // ignore } } - props.stackTrace = getStackTrace(ex); - sendTelemetryEvent(EDITOR_LOAD, durations, props); + sendTelemetryEvent(EDITOR_LOAD, durations, props, ex); } catch (exc2) { traceError('sendErrorTelemetry() failed.', exc2); } diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index d05e97805348..61b61107a785 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -45,6 +45,7 @@ export const PYTHON_LANGUAGE_SERVER_EXTRACTED = 'PYTHON_LANGUAGE_SERVER.EXTRACTE export const PYTHON_LANGUAGE_SERVER_DOWNLOADED = 'PYTHON_LANGUAGE_SERVER.DOWNLOADED'; export const PYTHON_LANGUAGE_SERVER_ERROR = 'PYTHON_LANGUAGE_SERVER.ERROR'; export const PYTHON_LANGUAGE_SERVER_STARTUP = 'PYTHON_LANGUAGE_SERVER.STARTUP'; +export const PYTHON_LANGUAGE_SERVER_READY = 'PYTHON_LANGUAGE_SERVER.READY'; export const PYTHON_LANGUAGE_SERVER_PLATFORM_NOT_SUPPORTED = 'PYTHON_LANGUAGE_SERVER.PLATFORM_NOT_SUPPORTED'; export const PYTHON_LANGUAGE_SERVER_PLATFORM_SUPPORTED = 'PYTHON_LANGUAGE_SERVER.PLATFORM_SUPPORTED'; export const PYTHON_LANGUAGE_SERVER_TELEMETRY = 'PYTHON_LANGUAGE_SERVER.EVENT'; diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 58ea3e6a212f..92f23a30818c 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -3,8 +3,10 @@ // tslint:disable:no-reference no-any import-name /// +import { basename as pathBasename, sep as pathSep } from 'path'; +import * as stackTrace from 'stack-trace'; import TelemetryReporter from 'vscode-extension-telemetry'; -import { isTestExecution, PVSC_EXTENSION_ID } from '../common/constants'; +import { EXTENSION_ROOT_DIR, isTestExecution, PVSC_EXTENSION_ID } from '../common/constants'; import { StopWatch } from '../common/utils/stopWatch'; import { TelemetryProperties } from './types'; @@ -45,7 +47,7 @@ function getTelemetryReporter() { return telemetryReporter = new reporter(extensionId, extensionVersion, aiKey); } -export function sendTelemetryEvent(eventName: string, durationMs?: { [key: string]: number } | number, properties?: TelemetryProperties) { +export function sendTelemetryEvent(eventName: string, durationMs?: { [key: string]: number } | number, properties?: TelemetryProperties, ex?: Error) { if (isTestExecution() || !isTelemetrySupported()) { return; } @@ -65,6 +67,10 @@ export function sendTelemetryEvent(eventName: string, durationMs?: { [key: strin (customProperties as any)[prop] = typeof data[prop] === 'string' ? data[prop] : data[prop].toString(); }); } + if (ex && eventName !== 'ERROR') { + customProperties.stackTrace = getStackTrace(ex); + reporter.sendTelemetryEvent(eventName, properties ? customProperties : undefined, measures); + } reporter.sendTelemetryEvent(eventName, properties ? customProperties : undefined, measures); } @@ -104,7 +110,7 @@ export function captureTelemetry( // tslint:disable-next-line:no-any properties = properties || {}; (properties as any).failed = true; - sendTelemetryEvent(failureEventName ? failureEventName : eventName, stopWatch.elapsedTime, properties); + sendTelemetryEvent(failureEventName ? failureEventName : eventName, stopWatch.elapsedTime, properties, ex); }); } else { sendTelemetryEvent(eventName, stopWatch.elapsedTime, properties); @@ -131,10 +137,54 @@ export function sendTelemetryWhenDone(eventName: string, promise: Promise | // tslint:disable-next-line:promise-function-async }, ex => { // tslint:disable-next-line:no-non-null-assertion - sendTelemetryEvent(eventName, stopWatch!.elapsedTime, properties); + sendTelemetryEvent(eventName, stopWatch!.elapsedTime, properties, ex); return Promise.reject(ex); }); } else { throw new Error('Method is neither a Promise nor a Theneable'); } } + +function sanitizeFilename(filename: string): string { + if (filename.startsWith(EXTENSION_ROOT_DIR)) { + filename = `${filename.substring(EXTENSION_ROOT_DIR.length)}`; + } else { + // We don't really care about files outside our extension. + filename = `${pathSep}${pathBasename(filename)}`; + } + return filename; +} + +function getStackTrace(ex: Error): string { + // We aren't showing the error message (ex.message) since it might + // contain PII. + let trace = ''; + for (const frame of stackTrace.parse(ex)) { + let filename = frame.getFileName(); + if (filename) { + filename = sanitizeFilename(filename); + const lineno = frame.getLineNumber(); + const colno = frame.getColumnNumber(); + trace += `\n\tat ${getCallsite(frame)} ${filename}:${lineno}:${colno}`; + } else { + trace += '\n\tat '; + } + } + return trace.trim(); +} + +function getCallsite(frame: stackTrace.StackFrame) { + const parts: string[] = []; + if (typeof frame.getTypeName() === 'string' && frame.getTypeName().length > 0) { + parts.push(frame.getTypeName()); + } + if (typeof frame.getMethodName() === 'string' && frame.getMethodName().length > 0) { + parts.push(frame.getMethodName()); + } + if (typeof frame.getFunctionName() === 'string' && frame.getFunctionName().length > 0) { + if (parts.length !== 2 || parts.join('.') !== frame.getFunctionName()) { + parts.push(frame.getFunctionName()); + } + } + return parts.join('.'); +} diff --git a/src/test/activation/languageServer/activator.unit.test.ts b/src/test/activation/languageServer/activator.unit.test.ts index b3576789dbaa..ba31a5fcf458 100644 --- a/src/test/activation/languageServer/activator.unit.test.ts +++ b/src/test/activation/languageServer/activator.unit.test.ts @@ -24,7 +24,7 @@ import { sleep } from '../../core'; // tslint:disable:max-func-body-length -suite('xLanguage Server - Activator', () => { +suite('Language Server - Activator', () => { let activator: IExtensionActivator; let workspaceService: IWorkspaceService; let manager: ILanguageServerManager; From bf1e00d369ad5923f73964582b6e8ec12ec6a58f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 10:05:42 -0800 Subject: [PATCH 04/21] Change coverage info and add news entry --- .github/codecov.yml | 5 +++++ news/2 Fixes/3346.md | 1 + 2 files changed, 6 insertions(+) create mode 100644 news/2 Fixes/3346.md diff --git a/.github/codecov.yml b/.github/codecov.yml index f16d332d04b0..f6edc9849e2e 100644 --- a/.github/codecov.yml +++ b/.github/codecov.yml @@ -2,3 +2,8 @@ coverage: precision: 0 round: up range: "70...90" + status: + project: off +comment: + layout: "diff, flags" + behavior: default diff --git a/news/2 Fixes/3346.md b/news/2 Fixes/3346.md new file mode 100644 index 000000000000..b4b57ef8a4af --- /dev/null +++ b/news/2 Fixes/3346.md @@ -0,0 +1 @@ +Ensure extension does not start multiple language servers. From 14a3c3e5b95dd8e7742f79c759931a8e08d287f4 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 12:17:22 -0800 Subject: [PATCH 05/21] Added more tests --- src/client/activation/jedi.ts | 2 +- .../activation/languageServer/activator.ts | 2 +- .../activation/languageServer/manager.ts | 20 ++++++ src/client/activation/types.ts | 4 +- src/client/common/asyncDisposableRegistry.ts | 5 +- .../languageServer/activator.unit.test.ts | 7 ++- .../languageServer.unit.test.ts | 4 +- .../languageServer/manager.unit.test.ts | 62 +++++++++++++++++-- 8 files changed, 91 insertions(+), 15 deletions(-) diff --git a/src/client/activation/jedi.ts b/src/client/activation/jedi.ts index 6b809c6ae3d7..e986e95180d8 100644 --- a/src/client/activation/jedi.ts +++ b/src/client/activation/jedi.ts @@ -78,7 +78,7 @@ export class JediExtensionActivator implements IExtensionActivator { .catch(ex => this.serviceManager.get(ILogger).logError('Failed to activate Unit Tests', ex)); } - public async dispose(): Promise { + public dispose(): void { if (this.jediFactory) { this.jediFactory.dispose(); } diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts index 3625e63ab5ac..a6cc3904bcbc 100644 --- a/src/client/activation/languageServer/activator.ts +++ b/src/client/activation/languageServer/activator.ts @@ -33,7 +33,7 @@ export class LanguageServerExtensionActivator implements IExtensionActivator { await this.ensureLanguageServerIsAvailable(mainWorkspaceUri); await this.manager.start(mainWorkspaceUri); } - public async dispose(): Promise { + public dispose(): void { this.manager.dispose(); } @traceDecorators.error('Failed to ensure language server is available') diff --git a/src/client/activation/languageServer/manager.ts b/src/client/activation/languageServer/manager.ts index 2dbfd74b4d70..cb06fc344db4 100644 --- a/src/client/activation/languageServer/manager.ts +++ b/src/client/activation/languageServer/manager.ts @@ -4,6 +4,7 @@ 'use strict'; import { inject } from 'inversify'; +import { ICommandManager } from '../../common/application/types'; import { traceDecorators } from '../../common/logger'; import { IDisposable, Resource } from '../../common/types'; import { debounce } from '../../common/utils/decorators'; @@ -12,29 +13,47 @@ import { captureTelemetry } from '../../telemetry'; import { PYTHON_LANGUAGE_SERVER_STARTUP } from '../../telemetry/constants'; import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../types'; +const loadExtensionCommand = 'python._loadLanguageServerExtension'; + export class LanguageServerManager implements ILanguageServerManager { + protected static loadExtensionArgs?: {}; private lanaguageServer?: ILanaguageServer; private resource!: Resource; private disposables: IDisposable[] = []; constructor( @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, + @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(ILanguageServerAnalysisOptions) private readonly analysisOptions: ILanguageServerAnalysisOptions) { } public dispose() { if (this.lanaguageServer) { this.lanaguageServer.dispose(); } + this.disposables.forEach(d => d.dispose()); } @traceDecorators.error('Failed to start Language Server') public async start(resource: Resource): Promise { if (this.lanaguageServer) { throw new Error('Language Server already started'); } + this.registerCommandHandler(); this.resource = resource; this.analysisOptions.onDidChange(this.restartLanguageServer, this, this.disposables); await this.analysisOptions.initialize(resource); await this.startLanguageServer(); } + protected registerCommandHandler() { + const disposable = this.commandManager.registerCommand(loadExtensionCommand, args => { + LanguageServerManager.loadExtensionArgs = args; + this.loadExtensionIfNecessary(); + }); + this.disposables.push(disposable); + } + protected loadExtensionIfNecessary() { + if (this.lanaguageServer && LanguageServerManager.loadExtensionArgs) { + this.lanaguageServer.loadExtension(LanguageServerManager.loadExtensionArgs); + } + } @traceDecorators.error('Failed to restart Language Server') @traceDecorators.verbose('Restarting Langauge Server') @debounce(1000) @@ -50,5 +69,6 @@ export class LanguageServerManager implements ILanguageServerManager { this.lanaguageServer = this.serviceContainer.get(ILanaguageServer); const options = await this.analysisOptions!.getAnalysisOptions(); await this.lanaguageServer.start(this.resource, options); + this.loadExtensionIfNecessary(); } } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 5ca28f5a7733..eec359c7ea47 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -8,7 +8,7 @@ import { SemVer } from 'semver'; import { Event } from 'vscode'; import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; import { NugetPackage } from '../common/nuget/types'; -import { IAsyncDisposable, IDisposable, LanguageServerDownloadChannels, Resource } from '../common/types'; +import { IDisposable, LanguageServerDownloadChannels, Resource } from '../common/types'; export const IExtensionActivationService = Symbol('IExtensionActivationService'); export interface IExtensionActivationService { @@ -21,7 +21,7 @@ export enum ExtensionActivators { } export const IExtensionActivator = Symbol('IExtensionActivator'); -export interface IExtensionActivator extends IAsyncDisposable { +export interface IExtensionActivator extends IDisposable { activate(): Promise; } diff --git a/src/client/common/asyncDisposableRegistry.ts b/src/client/common/asyncDisposableRegistry.ts index 01daa9c3ec95..1aa60fa2feae 100644 --- a/src/client/common/asyncDisposableRegistry.ts +++ b/src/client/common/asyncDisposableRegistry.ts @@ -4,18 +4,17 @@ import { injectable } from 'inversify'; import { IAsyncDisposable, IAsyncDisposableRegistry, IDisposable } from './types'; -type Disposable = IDisposable | IAsyncDisposable; // List of disposables that need to run a promise. @injectable() export class AsyncDisposableRegistry implements IAsyncDisposableRegistry { - private list: Disposable[] = []; + private list: (IDisposable | IAsyncDisposable)[] = []; public async dispose(): Promise { const promises = this.list.map(l => l.dispose()); await Promise.all(promises); } - public push(disposable: IDisposable | IAsyncDisposable | undefined) { + public push(disposable?: IDisposable | IAsyncDisposable) { if (disposable) { this.list.push(disposable); } diff --git a/src/test/activation/languageServer/activator.unit.test.ts b/src/test/activation/languageServer/activator.unit.test.ts index ba31a5fcf458..d4461b7f54c7 100644 --- a/src/test/activation/languageServer/activator.unit.test.ts +++ b/src/test/activation/languageServer/activator.unit.test.ts @@ -57,6 +57,11 @@ suite('Language Server - Activator', () => { verify(manager.start(undefined)).once(); verify(workspaceService.hasWorkspaceFolders).once(); }); + test('Manager must be disposed', async () => { + activator.dispose(); + + verify(manager.dispose()).once(); + }); test('Do not download LS if not required', async () => { when(workspaceService.hasWorkspaceFolders).thenReturn(false); when(manager.start(undefined)).thenResolve(); @@ -129,7 +134,7 @@ suite('Language Server - Activator', () => { }); test('Manager must be disposed', async () => { - await activator.dispose(); + activator.dispose(); verify(manager.dispose()).once(); }); diff --git a/src/test/activation/languageServer/languageServer.unit.test.ts b/src/test/activation/languageServer/languageServer.unit.test.ts index 168bc4a1310c..95143827bfe1 100644 --- a/src/test/activation/languageServer/languageServer.unit.test.ts +++ b/src/test/activation/languageServer/languageServer.unit.test.ts @@ -47,7 +47,7 @@ suite('Language Server - LanguageServer', () => { .setup(c => c.sendRequest(typemoq.It.isValue('python/loadExtension'), typemoq.It.isValue(loadExtensionArgs))) .returns(() => Promise.resolve(undefined) as any); - expect(() => server.loadExtension(loadExtensionArgs)).throws(); + expect(() => server.loadExtension(loadExtensionArgs)).throws('Activation not completed or not invoked'); client.verify(c => c.sendRequest(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); client .setup(c => c.initializeResult).returns(() => false as any) @@ -57,7 +57,7 @@ suite('Language Server - LanguageServer', () => { // Even though server has started request should not yet be sent out. // Not untill language client has initialized. - expect(() => server.loadExtension(loadExtensionArgs)).throws(); + expect(() => server.loadExtension(loadExtensionArgs)).throws('Activation not completed'); client.verify(c => c.sendRequest(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); // // Initialize language client and verify that the request was sent out. diff --git a/src/test/activation/languageServer/manager.unit.test.ts b/src/test/activation/languageServer/manager.unit.test.ts index c737903db4cd..ba31249904dd 100644 --- a/src/test/activation/languageServer/manager.unit.test.ts +++ b/src/test/activation/languageServer/manager.unit.test.ts @@ -5,13 +5,17 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { instance, mock, verify, when } from 'ts-mockito'; +import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; +import * as typemoq from 'typemoq'; import { Uri } from 'vscode'; import { LanguageClientOptions } from 'vscode-languageclient'; import { LanguageServerAnalysisOptions } from '../../../client/activation/languageServer/analysisOptions'; import { LanguageServer } from '../../../client/activation/languageServer/languageServer'; import { LanguageServerManager } from '../../../client/activation/languageServer/manager'; -import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../../../client/activation/types'; +import { ILanaguageServer, ILanguageServerAnalysisOptions } from '../../../client/activation/types'; +import { CommandManager } from '../../../client/common/application/commandManager'; +import { ICommandManager } from '../../../client/common/application/types'; +import { IDisposable } from '../../../client/common/types'; import { ServiceContainer } from '../../../client/ioc/container'; import { IServiceContainer } from '../../../client/ioc/types'; import { sleep } from '../../core'; @@ -20,22 +24,42 @@ use(chaiAsPromised); // tslint:disable:max-func-body-length no-any chai-vague-errors no-unused-expression -suite('Language Server - Manager', () => { - let manager: ILanguageServerManager; +const loadExtensionCommand = 'python._loadLanguageServerExtension'; + +suite('xLanguage Server - Manager', () => { + class LanguageServerManagerTest extends LanguageServerManager { + public static initializeExtensionArgs(args: {}) { + LanguageServerManager.loadExtensionArgs = args; + } + public clearLoadExtensionArgs() { + LanguageServerManager.loadExtensionArgs = undefined; + } + } + let manager: LanguageServerManagerTest; let serviceContainer: IServiceContainer; let analysisOptions: ILanguageServerAnalysisOptions; let languageServer: ILanaguageServer; + let commandManager: ICommandManager; let onChangeHandler: Function; const languageClientOptions = { x: 1 } as any as LanguageClientOptions; + let commandRegistrationDisposable: typemoq.IMock; setup(() => { serviceContainer = mock(ServiceContainer); analysisOptions = mock(LanguageServerAnalysisOptions); languageServer = mock(LanguageServer); - manager = new LanguageServerManager(instance(serviceContainer), instance(analysisOptions)); + commandManager = mock(CommandManager); + commandRegistrationDisposable = typemoq.Mock.ofType(); + manager = new LanguageServerManagerTest(instance(serviceContainer), + instance(commandManager), + instance(analysisOptions)); + manager.clearLoadExtensionArgs(); }); [undefined, Uri.file(__filename)].forEach(resource => { async function startLanguageServer() { + when(commandManager.registerCommand(loadExtensionCommand, anything())) + .thenReturn(commandRegistrationDisposable.object); + let handlerRegistered = false; const changeFn = (handler: Function) => { handlerRegistered = true; onChangeHandler = handler; }; when(analysisOptions.initialize(resource)).thenResolve(); @@ -52,6 +76,8 @@ suite('Language Server - Manager', () => { verify(languageServer.start(resource, languageClientOptions)).once(); expect(handlerRegistered).to.be.true; verify(languageServer.dispose()).never(); + verify(commandManager.registerCommand(loadExtensionCommand, anything())).once(); + commandRegistrationDisposable.verify(d => d.dispose(), typemoq.Times.never()); } test('Start must register handlers and initialize analysis options', async () => { await startLanguageServer(); @@ -131,5 +157,31 @@ suite('Language Server - Manager', () => { verify(serviceContainer.get(ILanaguageServer)).thrice(); verify(languageServer.start(resource, languageClientOptions)).thrice(); }); + test('Must register command handler', async () => { + await startLanguageServer(); + manager.dispose(); + + commandRegistrationDisposable.verify(d => d.dispose(), typemoq.Times.once()); + }); + test('Must load extension when command is sent', async () => { + const args = { x: 1 }; + await startLanguageServer(); + + verify(languageServer.loadExtension(args)).never(); + + const cb = capture(commandManager.registerCommand).first()[1] as Function; + cb.call(manager, args); + + verify(languageServer.loadExtension(args)).once(); + commandRegistrationDisposable.verify(d => d.dispose(), typemoq.Times.never()); + }); + test('Must load extension when command was been sent before starting LS', async () => { + const args = { x: 1 }; + LanguageServerManagerTest.initializeExtensionArgs(args); + + await startLanguageServer(); + + verify(languageServer.loadExtension(args)).once(); + }); }); }); From 1f44e7b3061c88704a7c68086896cf5f8e36527b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 13:21:31 -0800 Subject: [PATCH 06/21] Make pipenvPath resource specific --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 3a8e8e768dbb..905ce0cedd68 100644 --- a/package.json +++ b/package.json @@ -1525,7 +1525,7 @@ "type": "string", "default": "pipenv", "description": "Path to the pipenv executable to use for activation.", - "scope": "window" + "scope": "resource" }, "python.sortImports.args": { "type": "array", @@ -1811,7 +1811,7 @@ "compile-webviews-verbose": "npx webpack --config webpack.datascience-ui.config.js", "postinstall": "node ./node_modules/vscode/bin/install", "test": "node ./out/test/standardTest.js && node ./out/test/multiRootTest.js", - "test:unittests": "mocha --require source-map-support/register --opts ./build/.mocha.unittests.opts", + "test:unittests": "mocha --require source-map-support/register --opts ./build/.mocha.unittests.opts --grep='xLang'", "test:unittests:cover": "nyc --nycrc-path ./build/.nycrc npm run test:unittests", "test:functional": "mocha --require source-map-support/register --opts ./build/.mocha.functional.opts", "test:functional:cover": "nyc --nycrc-path ./build/.nycrc npm run test:functional", From 34d02ec7aa8d9baea449081e0113b33fa5d09038 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 13:22:28 -0800 Subject: [PATCH 07/21] Fix error --- src/client/activation/activationService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 767fc7584d3c..d9a7bf023b8c 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -89,7 +89,7 @@ export class ExtensionActivationService implements IExtensionActivationService, public dispose() { if (this.currentActivator) { - this.currentActivator.activator.dispose().ignoreErrors(); + this.currentActivator.activator.dispose(); } } From 25b88a5b5ac78f7057ec068324639091b49613d3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 14:15:47 -0800 Subject: [PATCH 08/21] Tests for telemetry --- .vscode/settings.json | 5 +- package.json | 2 +- src/client/telemetry/index.ts | 16 +- .../languageServer/manager.unit.test.ts | 2 +- src/test/telemetry/index.unit.test.ts | 213 ++++++++++++++++++ 5 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 src/test/telemetry/index.unit.test.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index d928b65c7554..a5f575069106 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -22,5 +22,8 @@ "python.linting.enabled": false, "python.unitTest.promptToConfigure": false, "python.workspaceSymbols.enabled": false, - "python.formatting.provider": "none" + "python.formatting.provider": "none", + "typescript.preferences.quoteStyle": "single", + "javascript.preferences.quoteStyle": "single", + "typescriptHero.imports.stringQuoteStyle": "'" } diff --git a/package.json b/package.json index 905ce0cedd68..aeb91398304a 100644 --- a/package.json +++ b/package.json @@ -1811,7 +1811,7 @@ "compile-webviews-verbose": "npx webpack --config webpack.datascience-ui.config.js", "postinstall": "node ./node_modules/vscode/bin/install", "test": "node ./out/test/standardTest.js && node ./out/test/multiRootTest.js", - "test:unittests": "mocha --require source-map-support/register --opts ./build/.mocha.unittests.opts --grep='xLang'", + "test:unittests": "mocha --require source-map-support/register --opts ./build/.mocha.unittests.opts", "test:unittests:cover": "nyc --nycrc-path ./build/.nycrc npm run test:unittests", "test:functional": "mocha --require source-map-support/register --opts ./build/.mocha.functional.opts", "test:functional:cover": "nyc --nycrc-path ./build/.nycrc npm run test:functional", diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 92f23a30818c..5215fcb3cc30 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -29,7 +29,7 @@ function isTelemetrySupported(): boolean { } let telemetryReporter: TelemetryReporter; function getTelemetryReporter() { - if (telemetryReporter) { + if (!isTestExecution() && telemetryReporter) { return telemetryReporter; } const extensionId = PVSC_EXTENSION_ID; @@ -67,8 +67,10 @@ export function sendTelemetryEvent(eventName: string, durationMs?: { [key: strin (customProperties as any)[prop] = typeof data[prop] === 'string' ? data[prop] : data[prop].toString(); }); } - if (ex && eventName !== 'ERROR') { + if (ex) { customProperties.stackTrace = getStackTrace(ex); + } + if (ex && eventName !== 'ERROR') { reporter.sendTelemetryEvent(eventName, properties ? customProperties : undefined, measures); } reporter.sendTelemetryEvent(eventName, properties ? customProperties : undefined, measures); @@ -155,6 +157,14 @@ function sanitizeFilename(filename: string): string { return filename; } +function sanitizeName(name: string): string { + if (name.indexOf('/') === -1 && name.indexOf('\\') === -1) { + return name; + } else { + return ''; + } +} + function getStackTrace(ex: Error): string { // We aren't showing the error message (ex.message) since it might // contain PII. @@ -186,5 +196,5 @@ function getCallsite(frame: stackTrace.StackFrame) { parts.push(frame.getFunctionName()); } } - return parts.join('.'); + return parts.map(sanitizeName).join('.'); } diff --git a/src/test/activation/languageServer/manager.unit.test.ts b/src/test/activation/languageServer/manager.unit.test.ts index ba31249904dd..da4fac9daddf 100644 --- a/src/test/activation/languageServer/manager.unit.test.ts +++ b/src/test/activation/languageServer/manager.unit.test.ts @@ -26,7 +26,7 @@ use(chaiAsPromised); const loadExtensionCommand = 'python._loadLanguageServerExtension'; -suite('xLanguage Server - Manager', () => { +suite('Language Server - Manager', () => { class LanguageServerManagerTest extends LanguageServerManager { public static initializeExtensionArgs(args: {}) { LanguageServerManager.loadExtensionArgs = args; diff --git a/src/test/telemetry/index.unit.test.ts b/src/test/telemetry/index.unit.test.ts new file mode 100644 index 000000000000..bf794c151e4f --- /dev/null +++ b/src/test/telemetry/index.unit.test.ts @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +//tslint:disable:max-func-body-length match-default-export-name + +import { expect } from 'chai'; +import rewiremock from 'rewiremock'; +import { EXTENSION_ROOT_DIR } from '../../client/constants'; +import { sendTelemetryEvent } from '../../client/telemetry'; + +suite('Telemetry', () => { + const oldValueOfVSC_PYTHON_UNIT_TEST = process.env.VSC_PYTHON_UNIT_TEST; + const oldValueOfVSC_PYTHON_CI_TEST = process.env.VSC_PYTHON_CI_TEST; + setup(() => { + process.env.VSC_PYTHON_UNIT_TEST = undefined; + process.env.VSC_PYTHON_CI_TEST = undefined; + }); + teardown(() => { + process.env.VSC_PYTHON_UNIT_TEST = oldValueOfVSC_PYTHON_UNIT_TEST; + process.env.VSC_PYTHON_CI_TEST = oldValueOfVSC_PYTHON_CI_TEST; + rewiremock.disable(); + }); + + class Reporter { + public static eventName: string; + public static properties: { [key: string]: string }; + public static measures: {}; + public sendTelemetryEvent(eventName: string, properties?: {}, measures?: {}) { + Reporter.eventName = eventName; + Reporter.properties = properties!; + Reporter.measures = measures!; + } + } + test('Send Telemetry', () => { + rewiremock.enable(); + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measuers = { start: 123, end: 987 }; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName, measuers, properties as any); + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.deep.equal(measuers); + expect(Reporter.properties).to.deep.equal(properties); + }); + test('Send Telemetry', () => { + rewiremock.enable(); + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName); + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.equal(undefined, 'Measures should be empty'); + expect(Reporter.properties).to.equal(undefined, 'Properties should be empty'); + }); + test('Send Error Telemetry', () => { + rewiremock.enable(); + const error = new Error('Boo'); + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measuers = { start: 123, end: 987 }; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName, measuers, properties as any, error); + + const stackTrace = Reporter.properties.stackTrace; + delete Reporter.properties.stackTrace; + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.deep.equal(measuers); + expect(Reporter.properties).to.deep.equal(properties); + expect(stackTrace).to.be.length.greaterThan(1); + }); + test('Send Error Telemetry', () => { + rewiremock.enable(); + const error = new Error('Boo'); + error.stack = `Error: Boo +at Context.test (${EXTENSION_ROOT_DIR}/src/test/telemetry/index.unit.test.ts:50:23) +at callFn (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runnable.js:372:21) +at Test.Runnable.run (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runnable.js:364:7) +at Runner.runTest (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:455:10) +at ${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:573:12 +at next (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:369:14) +at ${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:379:7 +at next (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:303:14) +at ${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:342:7 +at done (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runnable.js:319:5) +at callFn (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runnable.js:395:7) +at Hook.Runnable.run (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runnable.js:364:7) +at next (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:317:10) +at Immediate. (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:347:5) +at runCallback (timers.js:789:20) +at tryOnImmediate (timers.js:751:5) +at processImmediate [as _immediateCallback] (timers.js:722:5)`; + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measuers = { start: 123, end: 987 }; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName, measuers, properties as any, error); + + const stackTrace = Reporter.properties.stackTrace; + delete Reporter.properties.stackTrace; + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.deep.equal(measuers); + expect(Reporter.properties).to.deep.equal(properties); + expect(stackTrace).to.be.length.greaterThan(1); + + // tslint:disable-next-line:no-multiline-string + const expectedStack = `at Context.test /src/test/telemetry/index.unit.test.ts:50:23 +\tat callFn /node_modules/mocha/lib/runnable.js:372:21 +\tat Test.Runnable.run /node_modules/mocha/lib/runnable.js:364:7 +\tat Runner.runTest /node_modules/mocha/lib/runner.js:455:10 +\tat /node_modules/mocha/lib/runner.js:573:12 +\tat next /node_modules/mocha/lib/runner.js:369:14 +\tat /node_modules/mocha/lib/runner.js:379:7 +\tat next /node_modules/mocha/lib/runner.js:303:14 +\tat /node_modules/mocha/lib/runner.js:342:7 +\tat done /node_modules/mocha/lib/runnable.js:319:5 +\tat callFn /node_modules/mocha/lib/runnable.js:395:7 +\tat Hook.Runnable.run /node_modules/mocha/lib/runnable.js:364:7 +\tat next /node_modules/mocha/lib/runner.js:317:10 +\tat Immediate /node_modules/mocha/lib/runner.js:347:5 +\tat runCallback /timers.js:789:20 +\tat tryOnImmediate /timers.js:751:5 +\tat processImmediate [as _immediateCallback] /timers.js:722:5`; + + expect(stackTrace).to.be.equal(expectedStack); + }); + test('Ensure non extension file paths are stripped from stack trace', () => { + rewiremock.enable(); + const error = new Error('Boo'); + error.stack = `Error: Boo +at Context.test (${EXTENSION_ROOT_DIR}/src/test/telemetry/index.unit.test.ts:50:23) +at callFn (c:/one/two/user/node_modules/mocha/lib/runnable.js:372:21) +at Test.Runnable.run (/usr/Paul/Homer/desktop/node_modules/mocha/lib/runnable.js:364:7) +at Runner.runTest (\\wow\wee/node_modules/mocha/lib/runner.js:455:10) +at Immediate. (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:347:5)`; + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measuers = { start: 123, end: 987 }; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName, measuers, properties as any, error); + + const stackTrace = Reporter.properties.stackTrace; + delete Reporter.properties.stackTrace; + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.deep.equal(measuers); + expect(Reporter.properties).to.deep.equal(properties); + expect(stackTrace).to.be.length.greaterThan(1); + + // tslint:disable-next-line:no-multiline-string + const expectedStack = `at Context.test /src/test/telemetry/index.unit.test.ts:50:23 +\tat callFn /runnable.js:372:21 +\tat Test.Runnable.run /runnable.js:364:7 +\tat Runner.runTest /runner.js:455:10 +\tat Immediate /node_modules/mocha/lib/runner.js:347:5`; + + expect(stackTrace).to.be.equal(expectedStack); + }); + test('Ensure non function names containing file names (unlikely, but for sake of completeness) are stripped from stack trace', () => { + rewiremock.enable(); + const error = new Error('Boo'); + error.stack = `Error: Boo +at Context.test (${EXTENSION_ROOT_DIR}/src/test/telemetry/index.unit.test.ts:50:23) +at callFn (c:/one/two/user/node_modules/mocha/lib/runnable.js:372:21) +at Test./usr/Paul/Homer/desktop/node_modules/mocha/lib/runnable.run (/usr/Paul/Homer/desktop/node_modules/mocha/lib/runnable.js:364:7) +at Runner.runTest (\\wow\wee/node_modules/mocha/lib/runner.js:455:10) +at Immediate. (${EXTENSION_ROOT_DIR}/node_modules/mocha/lib/runner.js:347:5)`; + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measuers = { start: 123, end: 987 }; + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName, measuers, properties as any, error); + + const stackTrace = Reporter.properties.stackTrace; + delete Reporter.properties.stackTrace; + + expect(Reporter.eventName).to.equal(eventName); + expect(Reporter.measures).to.deep.equal(measuers); + expect(Reporter.properties).to.deep.equal(properties); + expect(stackTrace).to.be.length.greaterThan(1); + + // tslint:disable-next-line:no-multiline-string + const expectedStack = `at Context.test /src/test/telemetry/index.unit.test.ts:50:23 +\tat callFn /runnable.js:372:21 +\tat .run /runnable.js:364:7 +\tat Runner.runTest /runner.js:455:10 +\tat Immediate /node_modules/mocha/lib/runner.js:347:5`; + + expect(stackTrace).to.be.equal(expectedStack); + }); +}); From 8e5f45f755f4c9ebba0d5dff190ee874061fffd2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 15:43:47 -0800 Subject: [PATCH 09/21] Fix typo --- .../activation/languageServer/languageClientFactory.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/activation/languageServer/languageClientFactory.ts b/src/client/activation/languageServer/languageClientFactory.ts index 83d47d0d3299..b2c0e3c3420d 100644 --- a/src/client/activation/languageServer/languageClientFactory.ts +++ b/src/client/activation/languageServer/languageClientFactory.ts @@ -48,8 +48,8 @@ export class DownloadedLanguageClientFactory implements ILanguageClientFactory { run: { command: serverModule, rgs: [], options: options }, debug: { command: serverModule, args: ['--debug'], options } }; - const vscodeLanaguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); - return new vscodeLanaguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); + const vscodeLanguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); + return new vscodeLanguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); } } @@ -72,7 +72,7 @@ export class SimpleLanguageClientFactory implements ILanguageClientFactory { run: { command: dotNetCommand, args: [serverModule], options: commandOptions }, debug: { command: dotNetCommand, args: [serverModule, '--debug'], options: commandOptions } }; - const vscodeLanaguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); - return new vscodeLanaguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); + const vscodeLanguageClient = require('vscode-languageclient') as typeof import('vscode-languageclient'); + return new vscodeLanguageClient.LanguageClient(PYTHON_LANGUAGE, languageClientName, serverOptions, clientOptions); } } From 9c3ca37f56150e03e2eec0035fa871a7cec9210a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 15:45:06 -0800 Subject: [PATCH 10/21] Address code review comments --- src/client/common/logger.ts | 1 - src/test/telemetry/index.unit.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index b52e9a933f65..08ba9ff3eaeb 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -138,7 +138,6 @@ function trace(message: string, options: LogOptions = LogOptions.None, logLevel? } if (ex) { new Logger().logError(messagesToLog.join(', '), ex); - // tslint:disable-next-line:no-any sendTelemetryEvent('ERROR', undefined, undefined, ex); } else { new Logger().logInformation(messagesToLog.join(', ')); diff --git a/src/test/telemetry/index.unit.test.ts b/src/test/telemetry/index.unit.test.ts index bf794c151e4f..327597403b2d 100644 --- a/src/test/telemetry/index.unit.test.ts +++ b/src/test/telemetry/index.unit.test.ts @@ -54,7 +54,6 @@ suite('Telemetry', () => { const eventName = 'Testing'; - // tslint:disable-next-line:no-any sendTelemetryEvent(eventName); expect(Reporter.eventName).to.equal(eventName); From 94f3001b17e5e4c0a6eeb287fa5fdeb87127cafa Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 15:52:26 -0800 Subject: [PATCH 11/21] Fix json --- package-lock.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package-lock.json b/package-lock.json index 4743634f1533..d4956cb7c708 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1665,6 +1665,7 @@ "resolved": "https://registry.npmjs.org/@types/stack-trace/-/stack-trace-0.0.29.tgz", "integrity": "sha512-TgfOX+mGY/NyNxJLIbDWrO9DjGoVSW9+aB8H2yy1fy32jsvxijhmyJI9fDFgvz3YP4lvJaq9DzdR/M1bOgVc9g==", "dev": true + }, "@types/strip-json-comments": { "version": "0.0.30", "resolved": "https://registry.npmjs.org/@types/strip-json-comments/-/strip-json-comments-0.0.30.tgz", From fb896578ded94c2e135d346efca3b40b2914ba1e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 15:58:07 -0800 Subject: [PATCH 12/21] Add missing dependencies --- .../activation/languageServer/languageServer.ts | 4 ++-- src/client/activation/serviceRegistry.ts | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index bc2d12de35ec..e4287092155c 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -12,10 +12,10 @@ import { noop } from '../../common/utils/misc'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { PYTHON_LANGUAGE_SERVER_ENABLED, PYTHON_LANGUAGE_SERVER_READY, PYTHON_LANGUAGE_SERVER_TELEMETRY } from '../../telemetry/constants'; import { ProgressReporting } from '../progress'; -import { ILanaguageServer, ILanguageClientFactory } from '../types'; +import { ILanaguageServer as ILanguageServer, ILanguageClientFactory } from '../types'; @injectable() -export class LanguageServer implements ILanaguageServer { +export class LanguageServer implements ILanguageServer { private readonly startupCompleted: Deferred; private readonly disposables: Disposable[] = []; diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index 486ebf6a82c5..eb949ff5e85f 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -13,11 +13,15 @@ import { ExtensionActivationService } from './activationService'; import { DownloadBetaChannelRule, DownloadDailyChannelRule, DownloadStableChannelRule } from './downloadChannelRules'; import { JediExtensionActivator } from './jedi'; import { LanguageServerExtensionActivator } from './languageServer/activator'; +import { LanguageServerAnalysisOptions } from './languageServer/analysisOptions'; +import { BaseLanguageClientFactory, DownloadedLanguageClientFactory, SimpleLanguageClientFactory } from './languageServer/languageClientFactory'; +import { LanguageServer } from './languageServer/languageServer'; import { LanguageServerCompatibilityService } from './languageServer/languageServerCompatibilityService'; import { LanguageServerFolderService } from './languageServer/languageServerFolderService'; import { BetaLanguageServerPackageRepository, DailyLanguageServerPackageRepository, LanguageServerDownloadChannel, StableLanguageServerPackageRepository } from './languageServer/languageServerPackageRepository'; import { LanguageServerPackageService } from './languageServer/languageServerPackageService'; -import { ExtensionActivators, IDownloadChannelRule, IExtensionActivationService, IExtensionActivator, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerFolderService, ILanguageServerPackageService } from './types'; +import { LanguageServerManager } from './languageServer/manager'; +import { ExtensionActivators, IDownloadChannelRule, IExtensionActivationService, IExtensionActivator, ILanaguageServer, ILanguageClientFactory, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, LanguageClientFactory } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IExtensionActivationService, ExtensionActivationService); @@ -35,4 +39,10 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDownloadChannelRule, DownloadBetaChannelRule, LanguageServerDownloadChannel.beta); serviceManager.addSingleton(IDownloadChannelRule, DownloadStableChannelRule, LanguageServerDownloadChannel.stable); serviceManager.addSingleton(ILanagueServerCompatibilityService, LanguageServerCompatibilityService); + serviceManager.addSingleton(ILanguageClientFactory, BaseLanguageClientFactory, LanguageClientFactory.base); + serviceManager.addSingleton(ILanguageClientFactory, DownloadedLanguageClientFactory, LanguageClientFactory.downloaded); + serviceManager.addSingleton(ILanguageClientFactory, SimpleLanguageClientFactory, LanguageClientFactory.simple); + serviceManager.add(ILanguageServerAnalysisOptions, LanguageServerAnalysisOptions); + serviceManager.add(ILanaguageServer, LanguageServer); + serviceManager.add(ILanguageServerManager, LanguageServerManager); } From 260c22c440cd4a87bf7abb0b2e0609d03fea3439 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 16:10:30 -0800 Subject: [PATCH 13/21] Fixed code review comments and svc registrations --- src/client/activation/downloader.ts | 6 ++--- .../activation/interpreterDataService.ts | 3 ++- .../languageServer/analysisOptions.ts | 4 +-- .../languageServer/languageClientFactory.ts | 3 +-- .../languageServer/languageServer.ts | 6 ++--- .../activation/languageServer/manager.ts | 27 ++++++++++--------- src/client/activation/platformData.ts | 7 +---- src/client/activation/serviceRegistry.ts | 8 +++++- src/client/activation/types.ts | 6 +++++ src/test/activation/downloader.unit.test.ts | 3 +-- 10 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/client/activation/downloader.ts b/src/client/activation/downloader.ts index 112ea8862488..4a2ebb1828c9 100644 --- a/src/client/activation/downloader.ts +++ b/src/client/activation/downloader.ts @@ -3,7 +3,7 @@ 'use strict'; -import { inject, named } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import * as path from 'path'; import { ProgressLocation, window } from 'vscode'; import { IApplicationShell } from '../common/application/types'; @@ -19,11 +19,11 @@ import { PYTHON_LANGUAGE_SERVER_ERROR, PYTHON_LANGUAGE_SERVER_EXTRACTED } from '../telemetry/constants'; -import { IPlatformData } from './platformData'; -import { IHttpClient, ILanguageServerDownloader, ILanguageServerFolderService } from './types'; +import { IHttpClient, ILanguageServerDownloader, ILanguageServerFolderService, IPlatformData } from './types'; const downloadFileExtension = '.nupkg'; +@injectable() export class LanguageServerDownloader implements ILanguageServerDownloader { constructor( @inject(IPlatformData) private readonly platformData: IPlatformData, diff --git a/src/client/activation/interpreterDataService.ts b/src/client/activation/interpreterDataService.ts index 9d5933d8b322..723f2857e2bc 100644 --- a/src/client/activation/interpreterDataService.ts +++ b/src/client/activation/interpreterDataService.ts @@ -3,7 +3,7 @@ import { createHash } from 'crypto'; import * as fs from 'fs'; -import { inject } from 'inversify'; +import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IApplicationShell } from '../common/application/types'; import '../common/extensions'; @@ -26,6 +26,7 @@ class InterpreterDataCls { ) { } } +@injectable() export class InterpreterDataService implements IInterpreterDataService { constructor( @inject(IExtensionContext) private readonly context: IExtensionContext, diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts index 3107e3ba45c8..827bd1ac48d6 100644 --- a/src/client/activation/languageServer/analysisOptions.ts +++ b/src/client/activation/languageServer/analysisOptions.ts @@ -10,7 +10,7 @@ import { LanguageClientOptions, ProvideCompletionItemsSignature } from 'vscode-l import { IWorkspaceService } from '../../common/application/types'; import { isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; import { traceDecorators, traceError } from '../../common/logger'; -import { IConfigurationService, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../common/types'; +import { BANNER_NAME_PROPOSE_LS, IConfigurationService, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../common/types'; import { debounce } from '../../common/utils/decorators'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { IInterpreterService } from '../../interpreter/contracts'; @@ -29,7 +29,7 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt @inject(IEnvironmentVariablesProvider) private readonly envVarsProvider: IEnvironmentVariablesProvider, @inject(IConfigurationService) private readonly configuration: IConfigurationService, @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, - @inject(IPythonExtensionBanner) private readonly surveyBanner: IPythonExtensionBanner, + @inject(IPythonExtensionBanner) @named(BANNER_NAME_PROPOSE_LS) private readonly surveyBanner: IPythonExtensionBanner, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IInterpreterDataService) private readonly interpreterDataService: IInterpreterDataService, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: OutputChannel, diff --git a/src/client/activation/languageServer/languageClientFactory.ts b/src/client/activation/languageServer/languageClientFactory.ts index b2c0e3c3420d..d9d74598d92f 100644 --- a/src/client/activation/languageServer/languageClientFactory.ts +++ b/src/client/activation/languageServer/languageClientFactory.ts @@ -8,8 +8,7 @@ import * as path from 'path'; import { LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient'; import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants'; import { IConfigurationService, Resource } from '../../common/types'; -import { IPlatformData } from '../platformData'; -import { ILanguageClientFactory, ILanguageServerFolderService, LanguageClientFactory } from '../types'; +import { ILanguageClientFactory, ILanguageServerFolderService, IPlatformData, LanguageClientFactory } from '../types'; // tslint:disable:no-require-imports no-require-imports no-var-requires max-classes-per-file diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index e4287092155c..11b16bdfd402 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -3,7 +3,7 @@ 'use strict'; -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { Disposable, LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; import { traceDecorators, traceError } from '../../common/logger'; import { Resource } from '../../common/types'; @@ -12,7 +12,7 @@ import { noop } from '../../common/utils/misc'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { PYTHON_LANGUAGE_SERVER_ENABLED, PYTHON_LANGUAGE_SERVER_READY, PYTHON_LANGUAGE_SERVER_TELEMETRY } from '../../telemetry/constants'; import { ProgressReporting } from '../progress'; -import { ILanaguageServer as ILanguageServer, ILanguageClientFactory } from '../types'; +import { ILanaguageServer as ILanguageServer, ILanguageClientFactory, LanguageClientFactory } from '../types'; @injectable() export class LanguageServer implements ILanguageServer { @@ -21,7 +21,7 @@ export class LanguageServer implements ILanguageServer { private languageClient?: LanguageClient; - constructor(@inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory) { + constructor(@inject(ILanguageClientFactory) @named(LanguageClientFactory.base) private readonly factory: ILanguageClientFactory) { this.startupCompleted = createDeferred(); } diff --git a/src/client/activation/languageServer/manager.ts b/src/client/activation/languageServer/manager.ts index cb06fc344db4..a8e53f9818d9 100644 --- a/src/client/activation/languageServer/manager.ts +++ b/src/client/activation/languageServer/manager.ts @@ -3,7 +3,7 @@ 'use strict'; -import { inject } from 'inversify'; +import { inject, injectable } from 'inversify'; import { ICommandManager } from '../../common/application/types'; import { traceDecorators } from '../../common/logger'; import { IDisposable, Resource } from '../../common/types'; @@ -15,9 +15,10 @@ import { ILanaguageServer, ILanguageServerAnalysisOptions, ILanguageServerManage const loadExtensionCommand = 'python._loadLanguageServerExtension'; +@injectable() export class LanguageServerManager implements ILanguageServerManager { protected static loadExtensionArgs?: {}; - private lanaguageServer?: ILanaguageServer; + private languageServer?: ILanaguageServer; private resource!: Resource; private disposables: IDisposable[] = []; constructor( @@ -25,14 +26,14 @@ export class LanguageServerManager implements ILanguageServerManager { @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(ILanguageServerAnalysisOptions) private readonly analysisOptions: ILanguageServerAnalysisOptions) { } public dispose() { - if (this.lanaguageServer) { - this.lanaguageServer.dispose(); + if (this.languageServer) { + this.languageServer.dispose(); } this.disposables.forEach(d => d.dispose()); } @traceDecorators.error('Failed to start Language Server') public async start(resource: Resource): Promise { - if (this.lanaguageServer) { + if (this.languageServer) { throw new Error('Language Server already started'); } this.registerCommandHandler(); @@ -50,25 +51,25 @@ export class LanguageServerManager implements ILanguageServerManager { this.disposables.push(disposable); } protected loadExtensionIfNecessary() { - if (this.lanaguageServer && LanguageServerManager.loadExtensionArgs) { - this.lanaguageServer.loadExtension(LanguageServerManager.loadExtensionArgs); + if (this.languageServer && LanguageServerManager.loadExtensionArgs) { + this.languageServer.loadExtension(LanguageServerManager.loadExtensionArgs); } } @traceDecorators.error('Failed to restart Language Server') - @traceDecorators.verbose('Restarting Langauge Server') + @traceDecorators.verbose('Restarting Language Server') @debounce(1000) protected async restartLanguageServer(): Promise { - if (this.lanaguageServer) { - this.lanaguageServer.dispose(); + if (this.languageServer) { + this.languageServer.dispose(); } await this.startLanguageServer(); } @captureTelemetry(PYTHON_LANGUAGE_SERVER_STARTUP, undefined, true) - @traceDecorators.verbose('Starting Langauge Server') + @traceDecorators.verbose('Starting Language Server') protected async startLanguageServer(): Promise { - this.lanaguageServer = this.serviceContainer.get(ILanaguageServer); + this.languageServer = this.serviceContainer.get(ILanaguageServer); const options = await this.analysisOptions!.getAnalysisOptions(); - await this.lanaguageServer.start(this.resource, options); + await this.languageServer.start(this.resource, options); this.loadExtensionIfNecessary(); } } diff --git a/src/client/activation/platformData.ts b/src/client/activation/platformData.ts index faf1ebb23735..c49ba4442793 100644 --- a/src/client/activation/platformData.ts +++ b/src/client/activation/platformData.ts @@ -3,6 +3,7 @@ import { inject, injectable } from 'inversify'; import { IPlatformService } from '../common/platform/types'; +import { IPlatformData } from './types'; export enum PlatformName { Windows32Bit = 'win-x86', @@ -17,12 +18,6 @@ export enum PlatformLSExecutables { Linux = 'Microsoft.Python.LanguageServer' } -export const IPlatformData = Symbol('IPlatformData'); -export interface IPlatformData { - readonly platformName: PlatformName; - readonly engineDllName: string; - readonly engineExecutableName: string; -} @injectable() export class PlatformData implements IPlatformData { constructor(@inject(IPlatformService) private readonly platform: IPlatformService) { } diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index eb949ff5e85f..96689743f581 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -11,6 +11,8 @@ import { LanguageServerSurveyBanner } from '../languageServices/languageServerSu import { ProposeLanguageServerBanner } from '../languageServices/proposeLanguageServerBanner'; import { ExtensionActivationService } from './activationService'; import { DownloadBetaChannelRule, DownloadDailyChannelRule, DownloadStableChannelRule } from './downloadChannelRules'; +import { LanguageServerDownloader } from './downloader'; +import { InterpreterDataService } from './interpreterDataService'; import { JediExtensionActivator } from './jedi'; import { LanguageServerExtensionActivator } from './languageServer/activator'; import { LanguageServerAnalysisOptions } from './languageServer/analysisOptions'; @@ -21,7 +23,8 @@ import { LanguageServerFolderService } from './languageServer/languageServerFold import { BetaLanguageServerPackageRepository, DailyLanguageServerPackageRepository, LanguageServerDownloadChannel, StableLanguageServerPackageRepository } from './languageServer/languageServerPackageRepository'; import { LanguageServerPackageService } from './languageServer/languageServerPackageService'; import { LanguageServerManager } from './languageServer/manager'; -import { ExtensionActivators, IDownloadChannelRule, IExtensionActivationService, IExtensionActivator, ILanaguageServer, ILanguageClientFactory, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, LanguageClientFactory } from './types'; +import { PlatformData } from './platformData'; +import { ExtensionActivators, IDownloadChannelRule, IExtensionActivationService, IExtensionActivator, IInterpreterDataService, ILanaguageServer, ILanguageClientFactory, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, IPlatformData, LanguageClientFactory } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IExtensionActivationService, ExtensionActivationService); @@ -42,6 +45,9 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ILanguageClientFactory, BaseLanguageClientFactory, LanguageClientFactory.base); serviceManager.addSingleton(ILanguageClientFactory, DownloadedLanguageClientFactory, LanguageClientFactory.downloaded); serviceManager.addSingleton(ILanguageClientFactory, SimpleLanguageClientFactory, LanguageClientFactory.simple); + serviceManager.addSingleton(IInterpreterDataService, InterpreterDataService); + serviceManager.addSingleton(ILanguageServerDownloader, LanguageServerDownloader); + serviceManager.addSingleton(IPlatformData, PlatformData); serviceManager.add(ILanguageServerAnalysisOptions, LanguageServerAnalysisOptions); serviceManager.add(ILanaguageServer, LanguageServer); serviceManager.add(ILanguageServerManager, LanguageServerManager); diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index eec359c7ea47..905921216088 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -113,3 +113,9 @@ export enum PlatformName { Mac64Bit = 'osx-x64', Linux64Bit = 'linux-x64' } +export const IPlatformData = Symbol('IPlatformData'); +export interface IPlatformData { + readonly platformName: PlatformName; + readonly engineDllName: string; + readonly engineExecutableName: string; +} diff --git a/src/test/activation/downloader.unit.test.ts b/src/test/activation/downloader.unit.test.ts index 7d501c5685c3..ca0bd168f3e8 100644 --- a/src/test/activation/downloader.unit.test.ts +++ b/src/test/activation/downloader.unit.test.ts @@ -9,8 +9,7 @@ import { expect } from 'chai'; import { SemVer } from 'semver'; import * as TypeMoq from 'typemoq'; import { LanguageServerDownloader } from '../../client/activation/downloader'; -import { IPlatformData } from '../../client/activation/platformData'; -import { ILanguageServerFolderService } from '../../client/activation/types'; +import { ILanguageServerFolderService, IPlatformData } from '../../client/activation/types'; import { IApplicationShell } from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; import { IOutputChannel } from '../../client/common/types'; From 1d6a919381ea3a54c86ee7ed7f93e8a8ad7ed696 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 16:28:01 -0800 Subject: [PATCH 14/21] Add logging --- src/client/activation/languageServer/languageServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index 11b16bdfd402..eb93d8cf355a 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -5,7 +5,7 @@ import { inject, injectable, named } from 'inversify'; import { Disposable, LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; -import { traceDecorators, traceError } from '../../common/logger'; +import { traceDecorators, traceError, traceVerbose } from '../../common/logger'; import { Resource } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { noop } from '../../common/utils/misc'; @@ -25,7 +25,7 @@ export class LanguageServer implements ILanguageServer { this.startupCompleted = createDeferred(); } - + @traceDecorators.verbose('Stopping Language Server') public dispose() { if (this.languageClient) { // Do not await on this. From ccc43e4c8b3eca05a7623b3d1db4b774520f17e3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 16:28:27 -0800 Subject: [PATCH 15/21] Add more logging --- src/client/activation/languageServer/languageServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/languageServer/languageServer.ts b/src/client/activation/languageServer/languageServer.ts index eb93d8cf355a..ba231cc820eb 100644 --- a/src/client/activation/languageServer/languageServer.ts +++ b/src/client/activation/languageServer/languageServer.ts @@ -5,7 +5,7 @@ import { inject, injectable, named } from 'inversify'; import { Disposable, LanguageClient, LanguageClientOptions } from 'vscode-languageclient'; -import { traceDecorators, traceError, traceVerbose } from '../../common/logger'; +import { traceDecorators, traceError } from '../../common/logger'; import { Resource } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { noop } from '../../common/utils/misc'; From 8b4d1d54aef718d752ff7a7fd204b08347654449 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Jan 2019 20:00:54 -0800 Subject: [PATCH 16/21] Expose an event to notify changes to settings --- news/3 Code Health/642.md | 1 + src/client/common/configSettings.ts | 13 +- src/client/common/types.ts | 3 +- src/client/datascience/datascience.ts | 36 ++-- src/client/datascience/history.ts | 126 +++++++------- .../datascience/dataScienceIocContainer.ts | 8 +- src/test/datascience/mockJupyterManager.ts | 3 +- src/test/mocks/vsc/extHostedTypes.ts | 164 ++++++++++++++---- 8 files changed, 230 insertions(+), 124 deletions(-) create mode 100644 news/3 Code Health/642.md diff --git a/news/3 Code Health/642.md b/news/3 Code Health/642.md new file mode 100644 index 000000000000..8786ca0bf04f --- /dev/null +++ b/news/3 Code Health/642.md @@ -0,0 +1 @@ +Expose an event to notify changes to settings instead of casting settings to concrete class. diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 3a9865aa8427..458a7c4a66e3 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -1,9 +1,8 @@ 'use strict'; import * as child_process from 'child_process'; -import { EventEmitter } from 'events'; import * as path from 'path'; -import { ConfigurationChangeEvent, ConfigurationTarget, DiagnosticSeverity, Disposable, Uri, WorkspaceConfiguration } from 'vscode'; +import { ConfigurationChangeEvent, ConfigurationTarget, DiagnosticSeverity, Disposable, Event, EventEmitter, Uri, WorkspaceConfiguration } from 'vscode'; import '../common/extensions'; import { IInterpreterAutoSeletionProxyService } from '../interpreter/autoSelection/types'; import { sendTelemetryEvent } from '../telemetry'; @@ -30,7 +29,8 @@ import { SystemVariables } from './variables/systemVariables'; const untildify = require('untildify'); const _debounce = require('lodash/debounce') as typeof import('lodash/debounce'); -export class PythonSettings extends EventEmitter implements IPythonSettings { +// tslint:disable-next-line:completed-docs +export class PythonSettings implements IPythonSettings { private static pythonSettings: Map = new Map(); public downloadLanguageServer = true; public jediEnabled = true; @@ -55,15 +55,18 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { public autoUpdateLanguageServer: boolean = true; public datascience!: IDataScienceSettings; + protected readonly changed = new EventEmitter(); private workspaceRoot: Uri; private disposables: Disposable[] = []; // tslint:disable-next-line:variable-name private _pythonPath = ''; private readonly workspace: IWorkspaceService; + public get onDidChange(): Event { + return this.changed.event; + } constructor(workspaceFolder: Uri | undefined, private readonly InterpreterAutoSelectionService: IInterpreterAutoSeletionProxyService, workspace?: IWorkspaceService) { - super(); this.workspace = workspace || new WorkspaceService(); this.workspaceRoot = workspaceFolder ? workspaceFolder : Uri.file(__dirname); this.initialize(); @@ -377,7 +380,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { // If workspace config changes, then we could have a cascading effect of on change events. // Let's defer the change notification. - _debounce(() => this.emit('change'), 1); + _debounce(() => this.changed.fire(), 1); }; this.disposables.push(this.InterpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this))); this.disposables.push(this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { diff --git a/src/client/common/types.ts b/src/client/common/types.ts index b90d9eec5ff5..7563697d85ef 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -3,7 +3,7 @@ 'use strict'; import { Socket } from 'net'; -import { ConfigurationTarget, DiagnosticSeverity, Disposable, Extension, ExtensionContext, OutputChannel, Uri, WorkspaceEdit } from 'vscode'; +import { ConfigurationTarget, DiagnosticSeverity, Disposable, Event, Extension, ExtensionContext, OutputChannel, Uri, WorkspaceEdit } from 'vscode'; import { EnvironmentVariables } from './variables/types'; export const IOutputChannel = Symbol('IOutputChannel'); export interface IOutputChannel extends OutputChannel { } @@ -160,6 +160,7 @@ export interface IPythonSettings { readonly analysis: IAnalysisSettings; readonly autoUpdateLanguageServer: boolean; readonly datascience: IDataScienceSettings; + readonly onDidChange: Event; } export interface ISortImportSettings { readonly path: string; diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index 06a6396c8d70..71154ef9486f 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -8,12 +8,12 @@ import { URL } from 'url'; import * as vscode from 'vscode'; import { IApplicationShell, ICommandManager, IDocumentManager } from '../common/application/types'; -import { PythonSettings } from '../common/configSettings'; import { PYTHON, PYTHON_LANGUAGE } from '../common/constants'; import { ContextKey } from '../common/contextKey'; import { BANNER_NAME_DS_SURVEY, IConfigurationService, + IDisposable, IDisposableRegistry, IExtensionContext, IPythonExtensionBanner @@ -30,6 +30,7 @@ export class DataScience implements IDataScience { public isDisposed: boolean = false; private readonly commandListeners: IDataScienceCommandListener[]; private readonly dataScienceSurveyBanner: IPythonExtensionBanner; + private changeHandler: IDisposable | undefined; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(ICommandManager) private commandManager: ICommandManager, @inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry, @@ -38,8 +39,8 @@ export class DataScience implements IDataScience { @inject(IConfigurationService) private configuration: IConfigurationService, @inject(IDocumentManager) private documentManager: IDocumentManager, @inject(IApplicationShell) private appShell: IApplicationShell) { - this.commandListeners = this.serviceContainer.getAll(IDataScienceCommandListener); - this.dataScienceSurveyBanner = this.serviceContainer.get(IPythonExtensionBanner, BANNER_NAME_DS_SURVEY); + this.commandListeners = this.serviceContainer.getAll(IDataScienceCommandListener); + this.dataScienceSurveyBanner = this.serviceContainer.get(IPythonExtensionBanner, BANNER_NAME_DS_SURVEY); } public async activate(): Promise { @@ -53,8 +54,8 @@ export class DataScience implements IDataScience { // Set our initial settings and sign up for changes this.onSettingsChanged(); - (this.configuration.getSettings() as PythonSettings).addListener('change', this.onSettingsChanged); - this.disposableRegistry.push(this); + const disposable = this.configuration.getSettings().onDidChange(this.onSettingsChanged.bind(this)); + this.disposableRegistry.push(disposable); // Listen for active editor changes so we can detect have code cells or not this.disposableRegistry.push(this.documentManager.onDidChangeActiveTextEditor(() => this.onChangedActiveTextEditor())); @@ -62,9 +63,9 @@ export class DataScience implements IDataScience { } public async dispose() { - if (!this.isDisposed) { - this.isDisposed = true; - (this.configuration.getSettings() as PythonSettings).removeListener('change', this.onSettingsChanged); + if (this.changeHandler) { + this.changeHandler.dispose(); + this.changeHandler = undefined; } } @@ -121,13 +122,13 @@ export class DataScience implements IDataScience { switch (selection) { case localize.DataScience.jupyterSelectURILaunchLocal(): return this.setJupyterURIToLocal(); - break; + break; case localize.DataScience.jupyterSelectURISpecifyURI(): return this.selectJupyterLaunchURI(); - break; + break; default: // If user cancels quick pick we will get undefined as the selection and fall through here - break; + break; } } @@ -139,8 +140,10 @@ export class DataScience implements IDataScience { @captureTelemetry(Telemetry.SetJupyterURIToUserSpecified) private async selectJupyterLaunchURI(): Promise { // First get the proposed URI from the user - const userURI = await this.appShell.showInputBox({prompt: localize.DataScience.jupyterSelectURIPrompt(), - placeHolder: 'https://hostname:8080/?token=849d61a414abafab97bc4aab1f3547755ddc232c2b8cb7fe', validateInput: this.validateURI, ignoreFocusOut: true}); + const userURI = await this.appShell.showInputBox({ + prompt: localize.DataScience.jupyterSelectURIPrompt(), + placeHolder: 'https://hostname:8080/?token=849d61a414abafab97bc4aab1f3547755ddc232c2b8cb7fe', validateInput: this.validateURI, ignoreFocusOut: true + }); if (userURI) { await this.configuration.updateSetting('dataScience.jupyterServerURI', userURI, undefined, vscode.ConfigurationTarget.Workspace); @@ -149,8 +152,8 @@ export class DataScience implements IDataScience { private validateURI = (testURI: string): string | undefined | null => { try { - // tslint:disable-next-line:no-unused-expression - new URL(testURI); + // tslint:disable-next-line:no-unused-expression + new URL(testURI); } catch { return localize.DataScience.jupyterSelectURIInvalidURI(); } @@ -169,8 +172,7 @@ export class DataScience implements IDataScience { // Get our matching code watcher for the active document private getCurrentCodeWatcher(): ICodeWatcher | undefined { const activeEditor = vscode.window.activeTextEditor; - if (!activeEditor || !activeEditor.document) - { + if (!activeEditor || !activeEditor.document) { return undefined; } diff --git a/src/client/datascience/history.ts b/src/client/datascience/history.ts index b7a0c957398b..fb4534263f2b 100644 --- a/src/client/datascience/history.ts +++ b/src/client/datascience/history.ts @@ -21,11 +21,10 @@ import { IWorkspaceService } from '../common/application/types'; import { CancellationError } from '../common/cancellation'; -import { PythonSettings } from '../common/configSettings'; import { EXTENSION_ROOT_DIR } from '../common/constants'; import { ContextKey } from '../common/contextKey'; import { IFileSystem } from '../common/platform/types'; -import { IConfigurationService, IDisposableRegistry, ILogger } from '../common/types'; +import { IConfigurationService, IDisposable, IDisposableRegistry, ILogger } from '../common/types'; import { createDeferred } from '../common/utils/async'; import * as localize from '../common/utils/localize'; import { IInterpreterService } from '../interpreter/contracts'; @@ -54,18 +53,19 @@ export enum SysInfoReason { @injectable() export class History implements IWebPanelMessageListener, IHistory { - private disposed : boolean = false; - private webPanel : IWebPanel | undefined; + private disposed: boolean = false; + private webPanel: IWebPanel | undefined; private loadPromise: Promise; - private interpreterChangedDisposable : Disposable; - private closedEvent : EventEmitter; + private interpreterChangedDisposable: Disposable; + private closedEvent: EventEmitter; private unfinishedCells: ICell[] = []; private restartingKernel: boolean = false; private potentiallyUnfinishedStatus: Disposable[] = []; private addedSysInfo: boolean = false; private ignoreCount: number = 0; - private waitingForExportCells : boolean = false; + private waitingForExportCells: boolean = false; private jupyterServer: INotebookServer | undefined; + private changeHandler: IDisposable | undefined; constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, @@ -73,9 +73,9 @@ export class History implements IWebPanelMessageListener, IHistory { @inject(IInterpreterService) private interpreterService: IInterpreterService, @inject(IWebPanelProvider) private provider: IWebPanelProvider, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, - @inject(ICodeCssGenerator) private cssGenerator : ICodeCssGenerator, - @inject(ILogger) private logger : ILogger, - @inject(IStatusProvider) private statusProvider : IStatusProvider, + @inject(ICodeCssGenerator) private cssGenerator: ICodeCssGenerator, + @inject(ILogger) private logger: ILogger, + @inject(IStatusProvider) private statusProvider: IStatusProvider, @inject(IJupyterExecution) private jupyterExecution: IJupyterExecution, @inject(IFileSystem) private fileSystem: IFileSystem, @inject(IConfigurationService) private configuration: IConfigurationService, @@ -85,7 +85,7 @@ export class History implements IWebPanelMessageListener, IHistory { // Sign up for configuration changes this.interpreterChangedDisposable = this.interpreterService.onDidChangeInterpreter(this.onInterpreterChanged); - (this.configuration.getSettings() as PythonSettings).addListener('change', this.onSettingsChanged); + this.changeHandler = this.configuration.getSettings().onDidChange(this.onSettingsChanged.bind(this)); // Create our event emitter this.closedEvent = new EventEmitter(); @@ -95,7 +95,7 @@ export class History implements IWebPanelMessageListener, IHistory { this.loadPromise = this.load(); } - public async show() : Promise { + public async show(): Promise { if (!this.disposed) { // Make sure we're loaded first await this.loadPromise; @@ -107,11 +107,11 @@ export class History implements IWebPanelMessageListener, IHistory { } } - public get closed() : Event { + public get closed(): Event { return this.closedEvent.event; } - public async addCode(code: string, file: string, line: number, editor?: TextEditor) : Promise { + public async addCode(code: string, file: string, line: number, editor?: TextEditor): Promise { // Start a status item const status = this.setStatus(localize.DataScience.executingCode()); @@ -179,7 +179,7 @@ export class History implements IWebPanelMessageListener, IHistory { // tslint:disable-next-line: no-any no-empty public postMessage(type: string, payload?: any) { if (this.webPanel) { - this.webPanel.postMessage({type: type, payload: payload}); + this.webPanel.postMessage({ type: type, payload: payload }); } } @@ -243,17 +243,20 @@ export class History implements IWebPanelMessageListener, IHistory { } } - public async dispose() { + public async dispose() { if (!this.disposed) { this.disposed = true; this.interpreterChangedDisposable.dispose(); - (this.configuration.getSettings() as PythonSettings).removeListener('change', this.onSettingsChanged); this.closedEvent.fire(this); if (this.jupyterServer) { await this.jupyterServer.shutdown(); } this.updateContexts(); } + if (this.changeHandler) { + this.changeHandler.dispose(); + this.changeHandler = undefined; + } } @captureTelemetry(Telemetry.Undo) @@ -345,7 +348,7 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private async restartKernelInternal() : Promise { + private async restartKernelInternal(): Promise { this.restartingKernel = true; // First we need to finish all outstanding cells. @@ -416,13 +419,13 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private setStatus = (message: string) : Disposable => { + private setStatus = (message: string): Disposable => { const result = this.statusProvider.set(message); this.potentiallyUnfinishedStatus.push(result); return result; } - private logTelemetry = (event : string) => { + private logTelemetry = (event: string) => { sendTelemetryEvent(event); } @@ -434,13 +437,13 @@ export class History implements IWebPanelMessageListener, IHistory { copy.data.execution_count = count - this.ignoreCount; } if (this.webPanel) { - this.webPanel.postMessage({type: message, payload: copy}); + this.webPanel.postMessage({ type: message, payload: copy }); } } - private onAddCodeEvent = (cells : ICell[], editor?: TextEditor) => { + private onAddCodeEvent = (cells: ICell[], editor?: TextEditor) => { // Send each cell to the other side - cells.forEach((cell : ICell) => { + cells.forEach((cell: ICell) => { if (this.webPanel) { switch (cell.state) { case CellState.init: @@ -459,7 +462,7 @@ export class History implements IWebPanelMessageListener, IHistory { case CellState.error: case CellState.finished: // Tell the react controls we're done - this.sendCell(cell, HistoryMessages.FinishCell); + this.sendCell(cell, HistoryMessages.FinishCell); // Remove from the list of unfinished cells this.unfinishedCells = this.unfinishedCells.filter(c => c.id !== cell.id); @@ -488,7 +491,7 @@ export class History implements IWebPanelMessageListener, IHistory { const dsSettings = JSON.stringify(this.configuration.getSettings().datascience); if (this.webPanel) { - this.webPanel.postMessage({type: HistoryMessages.UpdateSettings, payload: dsSettings}); + this.webPanel.postMessage({ type: HistoryMessages.UpdateSettings, payload: dsSettings }); } } @@ -511,10 +514,10 @@ export class History implements IWebPanelMessageListener, IHistory { } private async gotoCodeInternal(file: string, line: number) { - let editor : TextEditor | undefined; + let editor: TextEditor | undefined; if (await fs.pathExists(file)) { - editor = await this.documentManager.showTextDocument(Uri.file(file), {viewColumn: ViewColumn.One}); + editor = await this.documentManager.showTextDocument(Uri.file(file), { viewColumn: ViewColumn.One }); } else { // File URI isn't going to work. Look through the active text documents editor = this.documentManager.visibleTextEditors.find(te => te.document.fileName === file); @@ -532,7 +535,7 @@ export class History implements IWebPanelMessageListener, IHistory { @captureTelemetry(Telemetry.ExportNotebook, {}, false) // tslint:disable-next-line: no-any no-empty - private export (payload: any) { + private export(payload: any) { if (payload.contents) { // Should be an array of cells const cells = payload.contents as ICell[]; @@ -556,7 +559,7 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private exportToFile = async (cells: ICell[], file : string) => { + private exportToFile = async (cells: ICell[], file: string) => { // Take the list of cells, convert them to a notebook json format and write to disk if (this.jupyterServer) { let directoryChange; @@ -569,8 +572,8 @@ export class History implements IWebPanelMessageListener, IHistory { try { // tslint:disable-next-line: no-any - await this.fileSystem.writeFile(file, JSON.stringify(notebook), {encoding: 'utf8', flag: 'w'}); - this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(file), localize.DataScience.exportOpenQuestion()).then((str : string | undefined) => { + await this.fileSystem.writeFile(file, JSON.stringify(notebook), { encoding: 'utf8', flag: 'w' }); + this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(file), localize.DataScience.exportOpenQuestion()).then((str: string | undefined) => { if (str && this.jupyterServer) { // If the user wants to, open the notebook they just generated. this.jupyterExecution.spawnNotebook(file).ignoreErrors(); @@ -583,12 +586,12 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private loadJupyterServer = async (restart?: boolean) : Promise => { + private loadJupyterServer = async (restart?: boolean): Promise => { // Startup our jupyter server const settings = this.configuration.getSettings(); let serverURI: string | undefined = settings.datascience.jupyterServerURI; let workingDir: string | undefined; - const useDefaultConfig : boolean | undefined = settings.datascience.useDefaultConfigForJupyter; + const useDefaultConfig: boolean | undefined = settings.datascience.useDefaultConfigForJupyter; const status = this.setStatus(localize.DataScience.connectingToJupyter()); try { // For the local case pass in our URI as undefined, that way connect doesn't have to check the setting @@ -611,8 +614,7 @@ export class History implements IWebPanelMessageListener, IHistory { } // Calculate the working directory that we should move into when starting up our Jupyter server locally - private calculateWorkingDirectory = async (): Promise => - { + private calculateWorkingDirectory = async (): Promise => { let workingDir: string | undefined; // For a local launch calculate the working directory that we should switch into const settings = this.configuration.getSettings(); @@ -631,21 +633,21 @@ export class History implements IWebPanelMessageListener, IHistory { workingDir = workspaceFolderPath; } } else { - // fileRoot is a relative path, combine it with the workspace folder - const combinedPath = path.join(workspaceFolderPath, fileRoot); - if (await this.fileSystem.directoryExists(combinedPath)) { - // combined path exists, use it - workingDir = combinedPath; - } else { - // Combined path doesn't exist, use workspace - workingDir = workspaceFolderPath; - } + // fileRoot is a relative path, combine it with the workspace folder + const combinedPath = path.join(workspaceFolderPath, fileRoot); + if (await this.fileSystem.directoryExists(combinedPath)) { + // combined path exists, use it + workingDir = combinedPath; + } else { + // Combined path doesn't exist, use workspace + workingDir = workspaceFolderPath; + } } } return workingDir; } - private extractStreamOutput(cell: ICell) : string { + private extractStreamOutput(cell: ICell): string { let result = ''; if (cell.state === CellState.error || cell.state === CellState.finished) { const outputs = cell.data.outputs as nbformat.IOutput[]; @@ -666,7 +668,7 @@ export class History implements IWebPanelMessageListener, IHistory { return result; } - private generateSysInfoCell = async (reason: SysInfoReason) : Promise => { + private generateSysInfoCell = async (reason: SysInfoReason): Promise => { // Execute the code 'import sys\r\nsys.version' and 'import sys\r\nsys.executable' to get our // version and executable if (this.jupyterServer) { @@ -715,22 +717,22 @@ export class History implements IWebPanelMessageListener, IHistory { private async generateSysInfoMessage(reason: SysInfoReason): Promise { switch (reason) { case SysInfoReason.Start: - // Message depends upon if ipykernel is supported or not. - if (!(await this.jupyterExecution.isKernelCreateSupported())) { - return localize.DataScience.pythonVersionHeaderNoPyKernel(); - } - return localize.DataScience.pythonVersionHeader(); - break; + // Message depends upon if ipykernel is supported or not. + if (!(await this.jupyterExecution.isKernelCreateSupported())) { + return localize.DataScience.pythonVersionHeaderNoPyKernel(); + } + return localize.DataScience.pythonVersionHeader(); + break; case SysInfoReason.Restart: - return localize.DataScience.pythonRestartHeader(); - break; + return localize.DataScience.pythonRestartHeader(); + break; case SysInfoReason.Interrupt: - return localize.DataScience.pythonInterruptFailedHeader(); - break; + return localize.DataScience.pythonInterruptFailedHeader(); + break; default: - this.logger.logError('Invalid SysInfoReason'); - return ''; - break; + this.logger.logError('Invalid SysInfoReason'); + return ''; + break; } } @@ -745,7 +747,7 @@ export class History implements IWebPanelMessageListener, IHistory { return `${localize.DataScience.sysInfoURILabel()}${urlString}`; } - private addSysInfo = async (reason: SysInfoReason) : Promise => { + private addSysInfo = async (reason: SysInfoReason): Promise => { if (!this.addedSysInfo || reason === SysInfoReason.Interrupt || reason === SysInfoReason.Restart) { this.addedSysInfo = true; this.ignoreCount = 0; @@ -758,7 +760,7 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private loadWebPanel = async () : Promise => { + private loadWebPanel = async (): Promise => { // Create our web panel (it's the UI that shows up for the history) // Figure out the name of our main bundle. Should be in our output directory @@ -772,7 +774,7 @@ export class History implements IWebPanelMessageListener, IHistory { this.webPanel = this.provider.create(this, localize.DataScience.historyTitle(), mainScriptPath, css); } - private load = async () : Promise => { + private load = async (): Promise => { const status = this.setStatus(localize.DataScience.startingJupyter()); // Check to see if we support ipykernel or not diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 648660fbecfe..4446fa313beb 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -150,7 +150,11 @@ import { MockJupyterManager } from './mockJupyterManager'; export class DataScienceIocContainer extends UnitTestIocContainer { - private pythonSettings: PythonSettings = new PythonSettings(undefined, new MockAutoSelectionService()); + private pythonSettings = new class extends PythonSettings { + public fireChangeEvent() { + this.changed.fire(); + } + }(undefined, new MockAutoSelectionService()); private commandManager: MockCommandManager = new MockCommandManager(); private setContexts: { [name: string]: boolean } = {}; private contextSetEvent: EventEmitter<{ name: string; value: boolean }> = new EventEmitter<{ name: string; value: boolean }>(); @@ -391,7 +395,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } public forceSettingsChanged() { - this.pythonSettings.emit('change'); + this.pythonSettings.fireChangeEvent(); } public get mockJupyter(): MockJupyterManager | undefined { diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts index fe4db14d4325..a6c9ff4b8a39 100644 --- a/src/test/datascience/mockJupyterManager.ts +++ b/src/test/datascience/mockJupyterManager.ts @@ -12,7 +12,6 @@ import { EventEmitter } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import { Cancellation } from '../../client/common/cancellation'; -import { PythonSettings } from '../../client/common/configSettings'; import { ExecutionResult, IProcessServiceFactory, IPythonExecutionFactory, Output } from '../../client/common/process/types'; import { IAsyncDisposableRegistry, IConfigurationService } from '../../client/common/types'; import { EXTENSION_ROOT_DIR } from '../../client/constants'; @@ -77,7 +76,7 @@ export class MockJupyterManager implements IJupyterSessionManager { // Listen to configuration changes like the real interpreter service does so that we fire our settings changed event const configService = serviceManager.get(IConfigurationService); if (configService && configService !== null) { - (configService.getSettings() as PythonSettings).addListener('change', this.onConfigChanged); + configService.getSettings().onDidChange(this.onConfigChanged.bind(this)); } // Stick our services into the service manager diff --git a/src/test/mocks/vsc/extHostedTypes.ts b/src/test/mocks/vsc/extHostedTypes.ts index c8f5a97814a5..b5c00bb05a1f 100644 --- a/src/test/mocks/vsc/extHostedTypes.ts +++ b/src/test/mocks/vsc/extHostedTypes.ts @@ -4,6 +4,8 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; +// @ts-ignore + // import * as crypto from 'crypto'; // tslint:disable:all @@ -35,6 +37,7 @@ export namespace vscMockExtHostedTypes { disposable.dispose(); } } + // @ts-ignore disposables = undefined; } }); @@ -48,7 +51,9 @@ export namespace vscMockExtHostedTypes { dispose(): any { if (typeof this._callOnDispose === 'function') { + // @ts-ignore this._callOnDispose(); + // @ts-ignore this._callOnDispose = undefined; } } @@ -57,22 +62,27 @@ export namespace vscMockExtHostedTypes { export class Position { static Min(...positions: Position[]): Position { + // @ts-ignore let result = positions.pop(); for (let p of positions) { + // @ts-ignore if (p.isBefore(result)) { result = p; } } + // @ts-ignore return result; } static Max(...positions: Position[]): Position { let result = positions.pop(); for (let p of positions) { + // @ts-ignore if (p.isAfter(result)) { result = p; } } + // @ts-ignore return result; } @@ -161,8 +171,9 @@ export namespace vscMockExtHostedTypes { } } } - + // @ts-ignore translate(change: { lineDelta?: number; characterDelta?: number; }): Position; + // @ts-ignore translate(lineDelta?: number, characterDelta?: number): Position; translate(lineDeltaOrChange: number | { lineDelta?: number; characterDelta?: number; }, characterDelta: number = 0): Position { @@ -185,8 +196,9 @@ export namespace vscMockExtHostedTypes { } return new Position(this.line + lineDelta, this.character + characterDelta); } - + // @ts-ignore with(change: { line?: number; character?: number; }): Position; + // @ts-ignore with(line?: number, character?: number): Position; with(lineOrChange: number | { line?: number; character?: number; }, character: number = this.character): Position { @@ -254,8 +266,9 @@ export namespace vscMockExtHostedTypes { start = startLineOrStart; end = startColumnOrEnd; } - + // @ts-ignore if (!start || !end) { + // @ts-ignore throw new Error('Invalid arguments'); } @@ -296,6 +309,7 @@ export namespace vscMockExtHostedTypes { // this happens when there is no overlap: // |-----| // |----| + // @ts-ignore return undefined; } return new Range(start, end); @@ -319,9 +333,11 @@ export namespace vscMockExtHostedTypes { get isSingleLine(): boolean { return this._start.line === this._end.line; } - + // @ts-ignore with(change: { start?: Position, end?: Position }): Range; + // @ts-ignore with(start?: Position, end?: Position): Range; + // @ts-ignore with(startOrChange: Position | { start?: Position, end?: Position }, end: Position = this.end): Range { if (startOrChange === null || end === null) { @@ -391,8 +407,9 @@ export namespace vscMockExtHostedTypes { anchor = anchorLineOrAnchor; active = anchorColumnOrActive; } - + // @ts-ignore if (!anchor || !active) { + // @ts-ignore throw new Error('Invalid arguments'); } @@ -447,13 +464,16 @@ export namespace vscMockExtHostedTypes { } static setEndOfLine(eol: EndOfLine): TextEdit { + // @ts-ignore let ret = new TextEdit(undefined, undefined); ret.newEol = eol; return ret; } - + // @ts-ignore protected _range: Range; + // @ts-ignore protected _newText: string; + // @ts-ignore protected _newEol: EndOfLine; get range(): Range { @@ -566,6 +586,7 @@ export namespace vscMockExtHostedTypes { this._textEdits.set(uri.toString(), data); } if (!edits) { + // @ts-ignore data.edits = undefined; } else { data.edits = edits.slice(0); @@ -574,8 +595,10 @@ export namespace vscMockExtHostedTypes { get(uri: vscUri.URI): TextEdit[] { if (!this._textEdits.has(uri.toString())) { + // @ts-ignore return undefined; } + // @ts-ignore const { edits } = this._textEdits.get(uri.toString()); return edits ? edits.slice() : undefined; } @@ -719,6 +742,7 @@ export namespace vscMockExtHostedTypes { } uri: vscUri.URI; + // @ts-ignore range: Range; constructor(uri: vscUri.URI, rangeOrPosition: Range | Position) { @@ -765,12 +789,17 @@ export namespace vscMockExtHostedTypes { } export class Diagnostic { - + // @ts-ignore range: Range; + // @ts-ignore message: string; + // @ts-ignore source: string; + // @ts-ignore code: string | number; + // @ts-ignore severity: DiagnosticSeverity; + // @ts-ignore relatedInformation: DiagnosticRelatedInformation[]; customTags?: DiagnosticTag[]; @@ -810,6 +839,7 @@ export namespace vscMockExtHostedTypes { } else { this.contents = [contents]; } + // @ts-ignore this.range = range; } } @@ -870,7 +900,9 @@ export namespace vscMockExtHostedTypes { export class SymbolInformation { name: string; + // @ts-ignore location: Location; + // @ts-ignore kind: SymbolKind; containerName: string; @@ -879,6 +911,7 @@ export namespace vscMockExtHostedTypes { constructor(name: string, kind: SymbolKind, rangeOrContainer: string | Range, locationOrUri?: Location | vscUri.URI, containerName?: string) { this.name = name; this.kind = kind; + // @ts-ignore this.containerName = containerName; if (typeof rangeOrContainer === 'string') { @@ -888,6 +921,7 @@ export namespace vscMockExtHostedTypes { if (locationOrUri instanceof Location) { this.location = locationOrUri; } else if (rangeOrContainer instanceof Range) { + // @ts-ignore this.location = new Location(locationOrUri, rangeOrContainer); } } @@ -971,6 +1005,7 @@ export namespace vscMockExtHostedTypes { constructor(range: Range, command?: vscode.Command) { this.range = range; + // @ts-ignore this.command = command; } @@ -1034,9 +1069,11 @@ export namespace vscMockExtHostedTypes { } export class SignatureHelp { - + // @ts-ignore signatures: SignatureInformation[]; + // @ts-ignore activeSignature: number; + // @ts-ignore activeParameter: number; constructor() { @@ -1084,21 +1121,33 @@ export namespace vscMockExtHostedTypes { } export class CompletionItem { - + // @ts-ignore label: string; + // @ts-ignore kind: CompletionItemKind; + // @ts-ignore detail: string; + // @ts-ignore documentation: string | MarkdownString; + // @ts-ignore sortText: string; + // @ts-ignore filterText: string; + // @ts-ignore insertText: string | SnippetString; + // @ts-ignore range: Range; + // @ts-ignore textEdit: TextEdit; + // @ts-ignore additionalTextEdits: TextEdit[]; + // @ts-ignore command: vscode.Command; constructor(label: string, kind?: CompletionItemKind) { + // @ts-ignore this.label = label; + // @ts-ignore this.kind = kind; } @@ -1340,9 +1389,11 @@ export namespace vscMockExtHostedTypes { } export class ProcessExecution implements vscode.ProcessExecution { - + // @ts-ignore private _process: string; + // @ts-ignore private _args: string[]; + // @ts-ignore private _options: vscode.ProcessExecutionOptions; constructor(process: string, options?: vscode.ProcessExecutionOptions); @@ -1355,12 +1406,15 @@ export namespace vscMockExtHostedTypes { if (varg1 !== void 0) { if (Array.isArray(varg1)) { this._args = varg1; + // @ts-ignore this._options = varg2; } else { this._options = varg1; } } + // @ts-ignore if (this._args === void 0) { + // @ts-ignore this._args = []; } } @@ -1413,9 +1467,11 @@ export namespace vscMockExtHostedTypes { } export class ShellExecution implements vscode.ShellExecution { - + // @ts-ignore private _commandLine: string; + // @ts-ignore private _command: string | vscode.ShellQuotedString; + // @ts-ignore private _args: (string | vscode.ShellQuotedString)[]; private _options: vscode.ShellExecutionOptions; @@ -1431,12 +1487,14 @@ export namespace vscMockExtHostedTypes { } this._command = arg0; this._args = arg1 as (string | vscode.ShellQuotedString)[]; + // @ts-ignore this._options = arg2; } else { if (typeof arg0 !== 'string') { throw illegalArgument('commandLine'); } this._commandLine = arg0; + // @ts-ignore this._options = arg1; } } @@ -1511,18 +1569,26 @@ export namespace vscMockExtHostedTypes { export class Task implements vscode.Task { - private __id: string; + private static ProcessType: string = 'process'; + private static ShellType: string = 'shell'; + private static EmptyType: string = '$empty'; + private __id: string | undefined; + // @ts-ignore private _definition: vscode.TaskDefinition; - private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder; + // @ts-ignore + private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined; + // @ts-ignore private _name: string; - private _execution: ProcessExecution | ShellExecution; + private _execution: ProcessExecution | ShellExecution | undefined; private _problemMatchers: string[]; private _hasDefinedMatchers: boolean; private _isBackground: boolean; + // @ts-ignore private _source: string; - private _group: TaskGroup; + private _group: TaskGroup | undefined; private _presentationOptions: vscode.TaskPresentationOptions; + private _runOptions: vscode.RunOptions; constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); @@ -1558,33 +1624,43 @@ export namespace vscMockExtHostedTypes { this._hasDefinedMatchers = false; } this._isBackground = false; + this._presentationOptions = Object.create(null); + this._runOptions = Object.create(null); } - get _id(): string { + get _id(): string | undefined { return this.__id; } - set _id(value: string) { + set _id(value: string | undefined) { this.__id = value; } private clear(): void { - if (this.__id === void 0) { + if (this.__id === undefined) { return; } this.__id = undefined; this._scope = undefined; - this._definition = undefined; + this.computeDefinitionBasedOnExecution(); + } + + private computeDefinitionBasedOnExecution(): void { if (this._execution instanceof ProcessExecution) { this._definition = { - type: 'process', + type: Task.ProcessType, id: this._execution.computeId() }; } else if (this._execution instanceof ShellExecution) { this._definition = { - type: 'shell', + type: Task.ShellType, id: this._execution.computeId() }; + } else { + this._definition = { + type: Task.EmptyType, + id: new Date().getTime().toString() + }; } } @@ -1593,14 +1669,14 @@ export namespace vscMockExtHostedTypes { } set definition(value: vscode.TaskDefinition) { - if (value === void 0 || value === null) { + if (value === undefined || value === null) { throw illegalArgument('Kind can\'t be undefined or null'); } this.clear(); this._definition = value; } - get scope(): vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder { + get scope(): vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined { return this._scope; } @@ -1621,16 +1697,20 @@ export namespace vscMockExtHostedTypes { this._name = value; } - get execution(): ProcessExecution | ShellExecution { + get execution(): ProcessExecution | ShellExecution | undefined { return this._execution; } - set execution(value: ProcessExecution | ShellExecution) { + set execution(value: ProcessExecution | ShellExecution | undefined) { if (value === null) { value = undefined; } this.clear(); this._execution = value; + let type = this._definition.type; + if (Task.EmptyType === type || Task.ProcessType === type || Task.ShellType === type) { + this.computeDefinitionBasedOnExecution(); + } } get problemMatchers(): string[] { @@ -1639,13 +1719,15 @@ export namespace vscMockExtHostedTypes { set problemMatchers(value: string[]) { if (!Array.isArray(value)) { + this.clear(); this._problemMatchers = []; this._hasDefinedMatchers = false; return; + } else { + this.clear(); + this._problemMatchers = value; + this._hasDefinedMatchers = true; } - this.clear(); - this._problemMatchers = value; - this._hasDefinedMatchers = true; } get hasDefinedMatchers(): boolean { @@ -1676,14 +1758,13 @@ export namespace vscMockExtHostedTypes { this._source = value; } - get group(): TaskGroup { + get group(): TaskGroup | undefined { return this._group; } - set group(value: TaskGroup) { - if (value === void 0 || value === null) { - this._group = undefined; - return; + set group(value: TaskGroup | undefined) { + if (value === null) { + value = undefined; } this.clear(); this._group = value; @@ -1694,12 +1775,24 @@ export namespace vscMockExtHostedTypes { } set presentationOptions(value: vscode.TaskPresentationOptions) { - if (value === null) { - value = undefined; + if (value === null || value === undefined) { + value = Object.create(null); } this.clear(); this._presentationOptions = value; } + + get runOptions(): vscode.RunOptions { + return this._runOptions; + } + + set runOptions(value: vscode.RunOptions) { + if (value === null || value === undefined) { + value = Object.create(null); + } + this.clear(); + this._runOptions = value; + } } @@ -1838,6 +1931,7 @@ export namespace vscMockExtHostedTypes { constructor(command: string, args?: string[]) { this.command = command; + // @ts-ignore this.args = args; } } From 6e6ffa9256cf95cf154140162ad631f2f05b0972 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Jan 2019 17:38:22 -0800 Subject: [PATCH 17/21] Revert changes --- src/client/datascience/types.ts | 250 ++++++++++++-------------------- 1 file changed, 93 insertions(+), 157 deletions(-) diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 4c520b1cb298..4b127c51b03d 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -1,8 +1,3 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; @@ -10,16 +5,7 @@ import { nbformat } from '@jupyterlab/coreutils'; import { Kernel, KernelMessage } from '@jupyterlab/services/lib/kernel'; import { JSONObject } from '@phosphor/coreutils'; import { Observable } from 'rxjs/Observable'; -import { - CancellationToken, - CodeLens, - CodeLensProvider, - Disposable, - Event, - Range, - TextDocument, - TextEditor -} from 'vscode'; +import { CancellationToken, CodeLens, CodeLensProvider, Disposable, Event, Range, TextDocument, TextEditor } from 'vscode'; import { ICommandManager } from '../common/application/types'; import { IAsyncDisposable } from '../common/types'; @@ -28,227 +14,177 @@ import { PythonInterpreter } from '../interpreter/contracts'; // Main interface export const IDataScience = Symbol('IDataScience'); export interface IDataScience extends Disposable { - activate(): Promise; + activate(): Promise; } -export const IDataScienceCommandListener = Symbol( - 'IDataScienceCommandListener' -); +export const IDataScienceCommandListener = Symbol('IDataScienceCommandListener'); export interface IDataScienceCommandListener { - register(commandManager: ICommandManager); + register(commandManager: ICommandManager); } // Connection information for talking to a jupyter notebook process export interface IConnection extends Disposable { - baseUrl: string; - token: string; - localLaunch: boolean; + baseUrl: string; + token: string; + localLaunch: boolean; } export enum InterruptResult { - Success = 0, - TimedOut = 1, - Restarted = 2 + Success = 0, + TimedOut = 1, + Restarted = 2 } // Talks to a jupyter ipython kernel to retrieve data for cells export const INotebookServer = Symbol('INotebookServer'); export interface INotebookServer extends Disposable { - onStatusChanged: Event; - connect( - conninfo: IConnection, - kernelSpec: IJupyterKernelSpec, - cancelToken?: CancellationToken, - workingDir?: string - ): Promise; - executeObservable( - code: string, - file: string, - line: number - ): Observable; - execute( - code: string, - file: string, - line: number, - cancelToken?: CancellationToken - ): Promise; - restartKernel(): Promise; - waitForIdle(): Promise; - shutdown(): Promise; - interruptKernel(timeoutInMs: number): Promise; - setInitialDirectory(directory: string): Promise; - getConnectionInfo(): IConnection | undefined; + onStatusChanged: Event; + connect(conninfo: IConnection, kernelSpec: IJupyterKernelSpec, cancelToken?: CancellationToken, workingDir?: string) : Promise; + executeObservable(code: string, file: string, line: number) : Observable; + execute(code: string, file: string, line: number, cancelToken?: CancellationToken) : Promise; + restartKernel() : Promise; + waitForIdle() : Promise; + shutdown() : Promise; + interruptKernel(timeoutInMs: number) : Promise; + setInitialDirectory(directory: string): Promise; + getConnectionInfo(): IConnection | undefined; } export const IJupyterExecution = Symbol('IJupyterExecution'); export interface IJupyterExecution { - isNotebookSupported(cancelToken?: CancellationToken): Promise; - isImportSupported(cancelToken?: CancellationToken): Promise; - isKernelCreateSupported(cancelToken?: CancellationToken): Promise; - connectToNotebookServer( - uri: string | undefined, - useDefaultConfig: boolean, - cancelToken?: CancellationToken, - workingDir?: string - ): Promise; - spawnNotebook(file: string): Promise; - importNotebook(file: string, template: string): Promise; - getUsableJupyterPython( - cancelToken?: CancellationToken - ): Promise; + isNotebookSupported(cancelToken?: CancellationToken) : Promise; + isImportSupported(cancelToken?: CancellationToken) : Promise; + isKernelCreateSupported(cancelToken?: CancellationToken): Promise; + connectToNotebookServer(uri: string | undefined, useDefaultConfig: boolean, cancelToken?: CancellationToken, workingDir?: string) : Promise; + spawnNotebook(file: string) : Promise; + importNotebook(file: string, template: string) : Promise; + getUsableJupyterPython(cancelToken?: CancellationToken) : Promise; } export const IJupyterSession = Symbol('IJupyterSession'); export interface IJupyterSession extends IAsyncDisposable { - onRestarted: Event; - restart(): Promise; - interrupt(): Promise; - waitForIdle(): Promise; - requestExecute( - content: KernelMessage.IExecuteRequest, - disposeOnDone?: boolean, - metadata?: JSONObject - ): Kernel.IFuture | undefined; + onRestarted: Event; + restart() : Promise; + interrupt() : Promise; + waitForIdle() : Promise; + requestExecute(content: KernelMessage.IExecuteRequest, disposeOnDone?: boolean, metadata?: JSONObject) : Kernel.IFuture | undefined; } export const IJupyterSessionManager = Symbol('IJupyterSessionManager'); export interface IJupyterSessionManager { - startNew( - connInfo: IConnection, - kernelSpec: IJupyterKernelSpec, - cancelToken?: CancellationToken - ): Promise; - getActiveKernelSpecs(connInfo: IConnection): Promise; + startNew(connInfo: IConnection, kernelSpec: IJupyterKernelSpec, cancelToken?: CancellationToken) : Promise; + getActiveKernelSpecs(connInfo: IConnection) : Promise; } export interface IJupyterKernelSpec extends IAsyncDisposable { - name: string | undefined; - language: string | undefined; - path: string | undefined; + name: string | undefined; + language: string | undefined; + path: string | undefined; } export const INotebookImporter = Symbol('INotebookImporter'); export interface INotebookImporter extends Disposable { - importFromFile(file: string): Promise; + importFromFile(file: string) : Promise; } export const INotebookExporter = Symbol('INotebookExporter'); export interface INotebookExporter extends Disposable { - translateToNotebook( - cells: ICell[], - directoryChange?: string - ): Promise; + translateToNotebook(cells: ICell[], directoryChange?: string) : Promise; } export const IHistoryProvider = Symbol('IHistoryProvider'); export interface IHistoryProvider { - getActive(): IHistory | undefined; + getActive() : IHistory | undefined; - getOrCreateActive(): IHistory; + getOrCreateActive(): IHistory; } export const IHistory = Symbol('IHistory'); export interface IHistory extends Disposable { - closed: Event; - show(): Promise; - addCode( - code: string, - file: string, - line: number, - editor?: TextEditor - ): Promise; - // tslint:disable-next-line:no-any - postMessage(type: string, payload?: any); - undoCells(); - redoCells(); - removeAllCells(); - interruptKernel(); - restartKernel(); - expandAllCells(); - collapseAllCells(); - exportCells(); + closed: Event; + show() : Promise; + addCode(code: string, file: string, line: number, editor?: TextEditor) : Promise; + // tslint:disable-next-line:no-any + postMessage(type: string, payload?: any); + undoCells(); + redoCells(); + removeAllCells(); + interruptKernel(); + restartKernel(); + expandAllCells(); + collapseAllCells(); + exportCells(); } // Wraps the vscode API in order to send messages back and forth from a webview export const IPostOffice = Symbol('IPostOffice'); export interface IPostOffice { - // tslint:disable-next-line:no-any - post(message: string, params: any[] | undefined); - // tslint:disable-next-line:no-any - listen(message: string, listener: (args: any[] | undefined) => void); + // tslint:disable-next-line:no-any + post(message: string, params: any[] | undefined); + // tslint:disable-next-line:no-any + listen(message: string, listener: (args: any[] | undefined) => void); } // Wraps the vscode CodeLensProvider base class -export const IDataScienceCodeLensProvider = Symbol( - 'IDataScienceCodeLensProvider' -); +export const IDataScienceCodeLensProvider = Symbol('IDataScienceCodeLensProvider'); export interface IDataScienceCodeLensProvider extends CodeLensProvider { - getCodeWatcher(document: TextDocument): ICodeWatcher | undefined; + getCodeWatcher(document: TextDocument) : ICodeWatcher | undefined; } // Wraps the Code Watcher API export const ICodeWatcher = Symbol('ICodeWatcher'); export interface ICodeWatcher { - addFile(document: TextDocument); - getFileName(): string; - getVersion(): number; - getCodeLenses(): CodeLens[]; - runAllCells(); - runCell(range: Range); - runCurrentCell(); - runCurrentCellAndAdvance(); + addFile(document: TextDocument); + getFileName() : string; + getVersion() : number; + getCodeLenses() : CodeLens[]; + runAllCells(); + runCell(range: Range); + runCurrentCell(); + runCurrentCellAndAdvance(); } export enum CellState { - init = 0, - executing = 1, - finished = 2, - error = 3 + init = 0, + executing = 1, + finished = 2, + error = 3 } // Basic structure for a cell from a notebook export interface ICell { - id: string; - file: string; - line: number; - state: CellState; - data: - | nbformat.ICodeCell - | nbformat.IRawCell - | nbformat.IMarkdownCell - | ISysInfo; + id: string; + file: string; + line: number; + state: CellState; + data: nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell | ISysInfo; } export interface IHistoryInfo { - cellCount: number; - undoCount: number; - redoCount: number; + cellCount: number; + undoCount: number; + redoCount: number; } export interface ISysInfo extends nbformat.IBaseCell { - cell_type: 'sys_info'; - version: string; - notebook_version: string; - path: string; - message: string; - connection: string; + cell_type: 'sys_info'; + version: string; + notebook_version: string; + path: string; + message: string; + connection: string; } export const ICodeCssGenerator = Symbol('ICodeCssGenerator'); export interface ICodeCssGenerator { - generateThemeCss(): Promise; + generateThemeCss() : Promise; } export const IStatusProvider = Symbol('IStatusProvider'); export interface IStatusProvider { - // call this function to set the new status on the active - // history window. Dispose of the returned object when done. - set(message: string, timeout?: number): Disposable; - - // call this function to wait for a promise while displaying status - waitWithStatus( - promise: () => Promise, - message: string, - timeout?: number, - canceled?: () => void - ): Promise; + // call this function to set the new status on the active + // history window. Dispose of the returned object when done. + set(message: string, timeout?: number) : Disposable; + + // call this function to wait for a promise while displaying status + waitWithStatus(promise: () => Promise, message: string, timeout?: number, canceled?: () => void) : Promise; } From 0c2e946dd6556f80e87cca1bd66e4cdd82961165 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Jan 2019 19:15:44 -0800 Subject: [PATCH 18/21] Revert changes --- src/test/mocks/vsc/extHostedTypes.ts | 164 ++++++--------------------- 1 file changed, 35 insertions(+), 129 deletions(-) diff --git a/src/test/mocks/vsc/extHostedTypes.ts b/src/test/mocks/vsc/extHostedTypes.ts index b5c00bb05a1f..c8f5a97814a5 100644 --- a/src/test/mocks/vsc/extHostedTypes.ts +++ b/src/test/mocks/vsc/extHostedTypes.ts @@ -4,8 +4,6 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -// @ts-ignore - // import * as crypto from 'crypto'; // tslint:disable:all @@ -37,7 +35,6 @@ export namespace vscMockExtHostedTypes { disposable.dispose(); } } - // @ts-ignore disposables = undefined; } }); @@ -51,9 +48,7 @@ export namespace vscMockExtHostedTypes { dispose(): any { if (typeof this._callOnDispose === 'function') { - // @ts-ignore this._callOnDispose(); - // @ts-ignore this._callOnDispose = undefined; } } @@ -62,27 +57,22 @@ export namespace vscMockExtHostedTypes { export class Position { static Min(...positions: Position[]): Position { - // @ts-ignore let result = positions.pop(); for (let p of positions) { - // @ts-ignore if (p.isBefore(result)) { result = p; } } - // @ts-ignore return result; } static Max(...positions: Position[]): Position { let result = positions.pop(); for (let p of positions) { - // @ts-ignore if (p.isAfter(result)) { result = p; } } - // @ts-ignore return result; } @@ -171,9 +161,8 @@ export namespace vscMockExtHostedTypes { } } } - // @ts-ignore + translate(change: { lineDelta?: number; characterDelta?: number; }): Position; - // @ts-ignore translate(lineDelta?: number, characterDelta?: number): Position; translate(lineDeltaOrChange: number | { lineDelta?: number; characterDelta?: number; }, characterDelta: number = 0): Position { @@ -196,9 +185,8 @@ export namespace vscMockExtHostedTypes { } return new Position(this.line + lineDelta, this.character + characterDelta); } - // @ts-ignore + with(change: { line?: number; character?: number; }): Position; - // @ts-ignore with(line?: number, character?: number): Position; with(lineOrChange: number | { line?: number; character?: number; }, character: number = this.character): Position { @@ -266,9 +254,8 @@ export namespace vscMockExtHostedTypes { start = startLineOrStart; end = startColumnOrEnd; } - // @ts-ignore + if (!start || !end) { - // @ts-ignore throw new Error('Invalid arguments'); } @@ -309,7 +296,6 @@ export namespace vscMockExtHostedTypes { // this happens when there is no overlap: // |-----| // |----| - // @ts-ignore return undefined; } return new Range(start, end); @@ -333,11 +319,9 @@ export namespace vscMockExtHostedTypes { get isSingleLine(): boolean { return this._start.line === this._end.line; } - // @ts-ignore + with(change: { start?: Position, end?: Position }): Range; - // @ts-ignore with(start?: Position, end?: Position): Range; - // @ts-ignore with(startOrChange: Position | { start?: Position, end?: Position }, end: Position = this.end): Range { if (startOrChange === null || end === null) { @@ -407,9 +391,8 @@ export namespace vscMockExtHostedTypes { anchor = anchorLineOrAnchor; active = anchorColumnOrActive; } - // @ts-ignore + if (!anchor || !active) { - // @ts-ignore throw new Error('Invalid arguments'); } @@ -464,16 +447,13 @@ export namespace vscMockExtHostedTypes { } static setEndOfLine(eol: EndOfLine): TextEdit { - // @ts-ignore let ret = new TextEdit(undefined, undefined); ret.newEol = eol; return ret; } - // @ts-ignore + protected _range: Range; - // @ts-ignore protected _newText: string; - // @ts-ignore protected _newEol: EndOfLine; get range(): Range { @@ -586,7 +566,6 @@ export namespace vscMockExtHostedTypes { this._textEdits.set(uri.toString(), data); } if (!edits) { - // @ts-ignore data.edits = undefined; } else { data.edits = edits.slice(0); @@ -595,10 +574,8 @@ export namespace vscMockExtHostedTypes { get(uri: vscUri.URI): TextEdit[] { if (!this._textEdits.has(uri.toString())) { - // @ts-ignore return undefined; } - // @ts-ignore const { edits } = this._textEdits.get(uri.toString()); return edits ? edits.slice() : undefined; } @@ -742,7 +719,6 @@ export namespace vscMockExtHostedTypes { } uri: vscUri.URI; - // @ts-ignore range: Range; constructor(uri: vscUri.URI, rangeOrPosition: Range | Position) { @@ -789,17 +765,12 @@ export namespace vscMockExtHostedTypes { } export class Diagnostic { - // @ts-ignore + range: Range; - // @ts-ignore message: string; - // @ts-ignore source: string; - // @ts-ignore code: string | number; - // @ts-ignore severity: DiagnosticSeverity; - // @ts-ignore relatedInformation: DiagnosticRelatedInformation[]; customTags?: DiagnosticTag[]; @@ -839,7 +810,6 @@ export namespace vscMockExtHostedTypes { } else { this.contents = [contents]; } - // @ts-ignore this.range = range; } } @@ -900,9 +870,7 @@ export namespace vscMockExtHostedTypes { export class SymbolInformation { name: string; - // @ts-ignore location: Location; - // @ts-ignore kind: SymbolKind; containerName: string; @@ -911,7 +879,6 @@ export namespace vscMockExtHostedTypes { constructor(name: string, kind: SymbolKind, rangeOrContainer: string | Range, locationOrUri?: Location | vscUri.URI, containerName?: string) { this.name = name; this.kind = kind; - // @ts-ignore this.containerName = containerName; if (typeof rangeOrContainer === 'string') { @@ -921,7 +888,6 @@ export namespace vscMockExtHostedTypes { if (locationOrUri instanceof Location) { this.location = locationOrUri; } else if (rangeOrContainer instanceof Range) { - // @ts-ignore this.location = new Location(locationOrUri, rangeOrContainer); } } @@ -1005,7 +971,6 @@ export namespace vscMockExtHostedTypes { constructor(range: Range, command?: vscode.Command) { this.range = range; - // @ts-ignore this.command = command; } @@ -1069,11 +1034,9 @@ export namespace vscMockExtHostedTypes { } export class SignatureHelp { - // @ts-ignore + signatures: SignatureInformation[]; - // @ts-ignore activeSignature: number; - // @ts-ignore activeParameter: number; constructor() { @@ -1121,33 +1084,21 @@ export namespace vscMockExtHostedTypes { } export class CompletionItem { - // @ts-ignore + label: string; - // @ts-ignore kind: CompletionItemKind; - // @ts-ignore detail: string; - // @ts-ignore documentation: string | MarkdownString; - // @ts-ignore sortText: string; - // @ts-ignore filterText: string; - // @ts-ignore insertText: string | SnippetString; - // @ts-ignore range: Range; - // @ts-ignore textEdit: TextEdit; - // @ts-ignore additionalTextEdits: TextEdit[]; - // @ts-ignore command: vscode.Command; constructor(label: string, kind?: CompletionItemKind) { - // @ts-ignore this.label = label; - // @ts-ignore this.kind = kind; } @@ -1389,11 +1340,9 @@ export namespace vscMockExtHostedTypes { } export class ProcessExecution implements vscode.ProcessExecution { - // @ts-ignore + private _process: string; - // @ts-ignore private _args: string[]; - // @ts-ignore private _options: vscode.ProcessExecutionOptions; constructor(process: string, options?: vscode.ProcessExecutionOptions); @@ -1406,15 +1355,12 @@ export namespace vscMockExtHostedTypes { if (varg1 !== void 0) { if (Array.isArray(varg1)) { this._args = varg1; - // @ts-ignore this._options = varg2; } else { this._options = varg1; } } - // @ts-ignore if (this._args === void 0) { - // @ts-ignore this._args = []; } } @@ -1467,11 +1413,9 @@ export namespace vscMockExtHostedTypes { } export class ShellExecution implements vscode.ShellExecution { - // @ts-ignore + private _commandLine: string; - // @ts-ignore private _command: string | vscode.ShellQuotedString; - // @ts-ignore private _args: (string | vscode.ShellQuotedString)[]; private _options: vscode.ShellExecutionOptions; @@ -1487,14 +1431,12 @@ export namespace vscMockExtHostedTypes { } this._command = arg0; this._args = arg1 as (string | vscode.ShellQuotedString)[]; - // @ts-ignore this._options = arg2; } else { if (typeof arg0 !== 'string') { throw illegalArgument('commandLine'); } this._commandLine = arg0; - // @ts-ignore this._options = arg1; } } @@ -1569,26 +1511,18 @@ export namespace vscMockExtHostedTypes { export class Task implements vscode.Task { - private static ProcessType: string = 'process'; - private static ShellType: string = 'shell'; - private static EmptyType: string = '$empty'; + private __id: string; - private __id: string | undefined; - // @ts-ignore private _definition: vscode.TaskDefinition; - // @ts-ignore - private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined; - // @ts-ignore + private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder; private _name: string; - private _execution: ProcessExecution | ShellExecution | undefined; + private _execution: ProcessExecution | ShellExecution; private _problemMatchers: string[]; private _hasDefinedMatchers: boolean; private _isBackground: boolean; - // @ts-ignore private _source: string; - private _group: TaskGroup | undefined; + private _group: TaskGroup; private _presentationOptions: vscode.TaskPresentationOptions; - private _runOptions: vscode.RunOptions; constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); @@ -1624,43 +1558,33 @@ export namespace vscMockExtHostedTypes { this._hasDefinedMatchers = false; } this._isBackground = false; - this._presentationOptions = Object.create(null); - this._runOptions = Object.create(null); } - get _id(): string | undefined { + get _id(): string { return this.__id; } - set _id(value: string | undefined) { + set _id(value: string) { this.__id = value; } private clear(): void { - if (this.__id === undefined) { + if (this.__id === void 0) { return; } this.__id = undefined; this._scope = undefined; - this.computeDefinitionBasedOnExecution(); - } - - private computeDefinitionBasedOnExecution(): void { + this._definition = undefined; if (this._execution instanceof ProcessExecution) { this._definition = { - type: Task.ProcessType, + type: 'process', id: this._execution.computeId() }; } else if (this._execution instanceof ShellExecution) { this._definition = { - type: Task.ShellType, + type: 'shell', id: this._execution.computeId() }; - } else { - this._definition = { - type: Task.EmptyType, - id: new Date().getTime().toString() - }; } } @@ -1669,14 +1593,14 @@ export namespace vscMockExtHostedTypes { } set definition(value: vscode.TaskDefinition) { - if (value === undefined || value === null) { + if (value === void 0 || value === null) { throw illegalArgument('Kind can\'t be undefined or null'); } this.clear(); this._definition = value; } - get scope(): vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined { + get scope(): vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder { return this._scope; } @@ -1697,20 +1621,16 @@ export namespace vscMockExtHostedTypes { this._name = value; } - get execution(): ProcessExecution | ShellExecution | undefined { + get execution(): ProcessExecution | ShellExecution { return this._execution; } - set execution(value: ProcessExecution | ShellExecution | undefined) { + set execution(value: ProcessExecution | ShellExecution) { if (value === null) { value = undefined; } this.clear(); this._execution = value; - let type = this._definition.type; - if (Task.EmptyType === type || Task.ProcessType === type || Task.ShellType === type) { - this.computeDefinitionBasedOnExecution(); - } } get problemMatchers(): string[] { @@ -1719,15 +1639,13 @@ export namespace vscMockExtHostedTypes { set problemMatchers(value: string[]) { if (!Array.isArray(value)) { - this.clear(); this._problemMatchers = []; this._hasDefinedMatchers = false; return; - } else { - this.clear(); - this._problemMatchers = value; - this._hasDefinedMatchers = true; } + this.clear(); + this._problemMatchers = value; + this._hasDefinedMatchers = true; } get hasDefinedMatchers(): boolean { @@ -1758,13 +1676,14 @@ export namespace vscMockExtHostedTypes { this._source = value; } - get group(): TaskGroup | undefined { + get group(): TaskGroup { return this._group; } - set group(value: TaskGroup | undefined) { - if (value === null) { - value = undefined; + set group(value: TaskGroup) { + if (value === void 0 || value === null) { + this._group = undefined; + return; } this.clear(); this._group = value; @@ -1775,24 +1694,12 @@ export namespace vscMockExtHostedTypes { } set presentationOptions(value: vscode.TaskPresentationOptions) { - if (value === null || value === undefined) { - value = Object.create(null); + if (value === null) { + value = undefined; } this.clear(); this._presentationOptions = value; } - - get runOptions(): vscode.RunOptions { - return this._runOptions; - } - - set runOptions(value: vscode.RunOptions) { - if (value === null || value === undefined) { - value = Object.create(null); - } - this.clear(); - this._runOptions = value; - } } @@ -1931,7 +1838,6 @@ export namespace vscMockExtHostedTypes { constructor(command: string, args?: string[]) { this.command = command; - // @ts-ignore this.args = args; } } From b3b2fef18069fca130ded72ef535c0f3bc81bd0c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 17 Jan 2019 13:06:58 -0800 Subject: [PATCH 19/21] Fix code review comments --- src/client/datascience/datascience.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index 71154ef9486f..9442a101d69e 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -54,8 +54,8 @@ export class DataScience implements IDataScience { // Set our initial settings and sign up for changes this.onSettingsChanged(); - const disposable = this.configuration.getSettings().onDidChange(this.onSettingsChanged.bind(this)); - this.disposableRegistry.push(disposable); + this.changeHandler = this.configuration.getSettings().onDidChange(this.onSettingsChanged.bind(this)); + this.disposableRegistry.push(this); // Listen for active editor changes so we can detect have code cells or not this.disposableRegistry.push(this.documentManager.onDidChangeActiveTextEditor(() => this.onChangedActiveTextEditor())); From 213c1dcb045a0b955288eb305aaf855c3a5e6a70 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 17 Jan 2019 14:46:59 -0800 Subject: [PATCH 20/21] Resolve code review --- src/client/datascience/jupyter/jupyterExecution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index bef1289c029e..1ab69a4ed7e0 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -151,7 +151,7 @@ export class JupyterExecution implements IJupyterExecution, Disposable { // Try to connect to our jupyter process const result = this.serviceContainer.get(INotebookServer); - await result.connect(connection, kernelSpec!, cancelToken, workingDir); + await result.connect(connection, kernelSpec, cancelToken, workingDir); sendTelemetryEvent(uri ? Telemetry.ConnectRemoteJupyter : Telemetry.ConnectLocalJupyter); return result; } catch (err) { From 5ae6002894fba42211477d4cf856436cee9cc984 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 17 Jan 2019 14:47:50 -0800 Subject: [PATCH 21/21] Fix merge issue --- src/client/common/configSettings.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index d0ba98f85973..db0f5e9c55f2 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -396,7 +396,7 @@ export class PythonSettings implements IPythonSettings { } @debounce(1) protected debounceChangeNotification() { - this.changed.fire() + this.changed.fire(); } }