From 3b763f05442a6736e00c6f5d9be47415f3cc8066 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 7 Apr 2020 13:26:57 -0600 Subject: [PATCH] Consolidate the info for internal scripts to one place. (#10876) For #10681 This makes it much easier to make changes that are specific to internal scripts generally, like send them through a separate script to run them isolated. There should be zero change in behavior. --- src/client/activation/jedi.ts | 6 +- .../common/process/internal/scripts/index.ts | 332 ++++++++++++++++++ .../process/internal/scripts/testing_tools.ts | 57 +++ .../scripts/vscode_datascience_helpers.ts | 83 +++++ .../common/process/pythonEnvironment.ts | 22 +- .../common/terminal/syncTerminalService.ts | 12 +- src/client/interpreter/activation/service.ts | 16 +- .../terminalEnvironmentActivationService.ts | 17 +- .../languageServices/jediProxyFactory.ts | 41 +-- src/client/providers/importSortProvider.ts | 106 ++++-- src/client/providers/jediProxy.ts | 18 +- src/client/providers/renameProvider.ts | 14 +- .../providers/simpleRefactorProvider.ts | 23 +- src/client/refactor/proxy.ts | 32 +- src/client/terminals/codeExecution/helper.ts | 8 +- src/client/testing/common/debugLauncher.ts | 12 +- .../testing/common/services/discovery.ts | 28 +- src/client/testing/common/services/types.ts | 43 +-- .../testing/navigation/symbolProvider.ts | 14 +- src/client/testing/unittest/runner.ts | 8 +- .../providers/importSortProvider.unit.test.ts | 6 +- .../extension.refactor.extract.method.test.ts | 20 +- .../extension.refactor.extract.var.test.ts | 22 +- src/test/refactor/rename.test.ts | 39 +- .../common/services/discovery.unit.test.ts | 7 +- 25 files changed, 719 insertions(+), 267 deletions(-) create mode 100644 src/client/common/process/internal/scripts/index.ts create mode 100644 src/client/common/process/internal/scripts/testing_tools.ts create mode 100644 src/client/common/process/internal/scripts/vscode_datascience_helpers.ts diff --git a/src/client/activation/jedi.ts b/src/client/activation/jedi.ts index a67a3b72a385..7a19a9c5ed08 100644 --- a/src/client/activation/jedi.ts +++ b/src/client/activation/jedi.ts @@ -73,11 +73,7 @@ export class JediExtensionActivator implements ILanguageServerActivator { throw new Error('Jedi already started'); } const context = this.context; - const jediFactory = (this.jediFactory = new JediFactory( - context.asAbsolutePath('.'), - interpreter, - this.serviceManager - )); + const jediFactory = (this.jediFactory = new JediFactory(interpreter, this.serviceManager)); context.subscriptions.push(jediFactory); const serviceContainer = this.serviceManager.get(IServiceContainer); diff --git a/src/client/common/process/internal/scripts/index.ts b/src/client/common/process/internal/scripts/index.ts new file mode 100644 index 000000000000..9629b627f003 --- /dev/null +++ b/src/client/common/process/internal/scripts/index.ts @@ -0,0 +1,332 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { EXTENSION_ROOT_DIR } from '../../../constants'; +import { PythonVersionInfo } from '../../types'; + +// It is simpler to hard-code it instead of using vscode.ExtensionContext.extensionPath. +export const _SCRIPTS_DIR = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); +const SCRIPTS_DIR = _SCRIPTS_DIR; + +// "scripts" contains everything relevant to the scripts found under +// the top-level "pythonFiles" directory. Each of those scripts has +// a function in this module which matches the script's filename. +// Each function provides the commandline arguments that should be +// used when invoking a Python executable, whether through spawn/exec +// or a terminal. +// +// Where relevant (nearly always), the function also returns a "parse" +// function that may be used to deserialize the stdout of the script +// into the corresponding object or objects. "parse()" takes a single +// string as the stdout text and returns the relevant data. +// +// Some of the scripts are located in subdirectories of "pythonFiles". +// For each of those subdirectories there is a sub-module where +// those scripts' functions may be found. +// +// In some cases one or more types related to a script are exported +// from the same module in which the script's function is located. +// These types typically relate to the return type of "parse()". +// +// ignored scripts: +// * install_debugpy.py (used only for extension development) +// * ptvsd_launcher.py (used only for the old debug adapter) + +export * as testing_tools from './testing_tools'; +export * as vscode_datascience_helpers from './vscode_datascience_helpers'; + +//============================ +// interpreterInfo.py + +type PythonEnvInfo = { + versionInfo: PythonVersionInfo; + sysPrefix: string; + sysVersion: string; + is64Bit: boolean; +}; + +export function interpreterInfo(): [string[], (out: string) => PythonEnvInfo] { + const script = path.join(SCRIPTS_DIR, 'interpreterInfo.py'); + const args = [script]; + + function parse(out: string): PythonEnvInfo { + let json: PythonEnvInfo; + try { + json = JSON.parse(out); + } catch (ex) { + throw Error(`python ${args} returned bad JSON (${out}) (${ex})`); + } + return json; + } + + return [args, parse]; +} + +//============================ +// completion.py + +namespace _completion { + export type Response = (_Response1 | _Response2) & { + id: number; + }; + type _Response1 = { + // tslint:disable-next-line:no-any no-banned-terms + arguments: any[]; + }; + type _Response2 = + | CompletionResponse + | HoverResponse + | DefinitionResponse + | ReferenceResponse + | SymbolResponse + | ArgumentsResponse; + + type CompletionResponse = { + results: AutoCompleteItem[]; + }; + type HoverResponse = { + results: HoverItem[]; + }; + type DefinitionResponse = { + results: Definition[]; + }; + type ReferenceResponse = { + results: Reference[]; + }; + type SymbolResponse = { + results: Definition[]; + }; + type ArgumentsResponse = { + results: Signature[]; + }; + + type Signature = { + name: string; + docstring: string; + description: string; + paramindex: number; + params: Argument[]; + }; + type Argument = { + name: string; + value: string; + docstring: string; + description: string; + }; + + type Reference = { + name: string; + fileName: string; + columnIndex: number; + lineIndex: number; + moduleName: string; + }; + + type AutoCompleteItem = { + type: string; + kind: string; + text: string; + description: string; + raw_docstring: string; + rightLabel: string; + }; + + type DefinitionRange = { + startLine: number; + startColumn: number; + endLine: number; + endColumn: number; + }; + type Definition = { + type: string; + kind: string; + text: string; + fileName: string; + container: string; + range: DefinitionRange; + }; + + type HoverItem = { + kind: string; + text: string; + description: string; + docstring: string; + signature: string; + }; +} + +export function completion(jediPath?: string): [string[], (out: string) => _completion.Response[]] { + const script = path.join(SCRIPTS_DIR, 'completion.py'); + const args = [script]; + if (jediPath) { + args.push('custom'); + args.push(jediPath); + } + + function parse(out: string): _completion.Response[] { + return out.splitLines().map((resp) => JSON.parse(resp)); + } + + return [args, parse]; +} + +//============================ +// sortImports.py + +export function sortImports(filename: string, sortArgs?: string[]): [string[], (out: string) => string] { + const script = path.join(SCRIPTS_DIR, 'sortImports.py'); + const args = [script, filename, '--diff']; + if (sortArgs) { + args.push(...sortArgs); + } + + function parse(out: string) { + // It should just be a diff that the extension will use directly. + return out; + } + + return [args, parse]; +} + +//============================ +// refactor.py + +export function refactor(root: string): [string[], (out: string) => object[]] { + const script = path.join(SCRIPTS_DIR, 'refactor.py'); + const args = [script, root]; + + // tslint:disable-next-line:no-suspicious-comment + // TODO: Make the return type more specific, like we did + // with completion(). + function parse(out: string): object[] { + // tslint:disable-next-line:no-suspicious-comment + // TODO: Also handle "STARTED"? + return out + .split(/\r?\n/g) + .filter((line) => line.length > 0) + .map((resp) => JSON.parse(resp)); + } + + return [args, parse]; +} + +//============================ +// normalizeForInterpreter.py + +export function normalizeForInterpreter(code: string): [string[], (out: string) => string] { + const script = path.join(SCRIPTS_DIR, 'normalizeForInterpreter.py'); + const args = [script, code]; + + function parse(out: string) { + // The text will be used as-is. + return out; + } + + return [args, parse]; +} + +//============================ +// symbolProvider.py + +namespace _symbolProvider { + type Position = { + line: number; + character: number; + }; + type RawSymbol = { + // If no namespace then ''. + namespace: string; + name: string; + range: { + start: Position; + end: Position; + }; + }; + export type Symbols = { + classes: RawSymbol[]; + methods: RawSymbol[]; + functions: RawSymbol[]; + }; +} + +export function symbolProvider( + filename: string, + // If "text" is provided then it gets passed to the script as-is. + text?: string +): [string[], (out: string) => _symbolProvider.Symbols] { + const script = path.join(SCRIPTS_DIR, 'symbolProvider.py'); + const args = [script, filename]; + if (text) { + args.push(text); + } + + function parse(out: string): _symbolProvider.Symbols { + return JSON.parse(out); + } + + return [args, parse]; +} + +//============================ +// printEnvVariables.py + +export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessEnv] { + const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgument(); + const args = [script]; + + function parse(out: string): NodeJS.ProcessEnv { + return JSON.parse(out); + } + + return [args, parse]; +} + +//============================ +// printEnvVariablesToFile.py + +export function printEnvVariablesToFile(filename: string): [string[], (out: string) => NodeJS.ProcessEnv] { + const script = path.join(SCRIPTS_DIR, 'printEnvVariablesToFile.py'); + const args = [script, filename.fileToCommandArgument()]; + + function parse(out: string): NodeJS.ProcessEnv { + return JSON.parse(out); + } + + return [args, parse]; +} + +//============================ +// shell_exec.py + +export function shell_exec(command: string, lockfile: string, shellArgs: string[]): string[] { + const script = path.join(SCRIPTS_DIR, 'shell_exec.py'); + // We don't bother with a "parse" function since the output + // could be anything. + return [ + script, + command.fileToCommandArgument(), + // The shell args must come after the command + // but before the lockfile. + ...shellArgs, + lockfile.fileToCommandArgument() + ]; +} + +//============================ +// testlauncher.py + +export function testlauncher(testArgs: string[]): string[] { + const script = path.join(SCRIPTS_DIR, 'testlauncher.py'); + // There is no output to parse, so we do not return a function. + return [script, ...testArgs]; +} + +//============================ +// visualstudio_py_testlauncher.py + +export function visualstudio_py_testlauncher(testArgs: string[]): string[] { + const script = path.join(SCRIPTS_DIR, 'visualstudio_py_testlauncher.py'); + // There is no output to parse, so we do not return a function. + return [script, ...testArgs]; +} diff --git a/src/client/common/process/internal/scripts/testing_tools.ts b/src/client/common/process/internal/scripts/testing_tools.ts new file mode 100644 index 000000000000..febf378abd35 --- /dev/null +++ b/src/client/common/process/internal/scripts/testing_tools.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { _SCRIPTS_DIR } from './index'; + +const SCRIPTS_DIR = path.join(_SCRIPTS_DIR, 'testing_tools'); + +//============================ +// run_adapter.py + +type TestNode = { + id: string; + name: string; + parentid: string; +}; +type TestParent = TestNode & { + kind: 'folder' | 'file' | 'suite' | 'function'; +}; +type TestFSNode = TestParent & { + kind: 'folder' | 'file'; + relpath: string; +}; + +export type TestFolder = TestFSNode & { + kind: 'folder'; +}; +export type TestFile = TestFSNode & { + kind: 'file'; +}; +export type TestSuite = TestParent & { + kind: 'suite'; +}; +// function-as-a-container is for parameterized ("sub") tests. +export type TestFunction = TestParent & { + kind: 'function'; +}; +export type Test = TestNode & { + source: string; +}; +export type DiscoveredTests = { + rootid: string; + root: string; + parents: TestParent[]; + tests: Test[]; +}; + +export function run_adapter(adapterArgs: string[]): [string[], (out: string) => DiscoveredTests[]] { + const script = path.join(SCRIPTS_DIR, 'run_adapter.py'); + const args = [script, ...adapterArgs]; + + function parse(out: string): DiscoveredTests[] { + return JSON.parse(out); + } + + return [args, parse]; +} diff --git a/src/client/common/process/internal/scripts/vscode_datascience_helpers.ts b/src/client/common/process/internal/scripts/vscode_datascience_helpers.ts new file mode 100644 index 000000000000..d1d5a0fd2736 --- /dev/null +++ b/src/client/common/process/internal/scripts/vscode_datascience_helpers.ts @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { _SCRIPTS_DIR } from './index'; + +const SCRIPTS_DIR = path.join(_SCRIPTS_DIR, 'vscode_datascience_helpers'); + +//============================ +// getJupyterVariableDataFrameInfo.py + +export function getJupyterVariableDataFrameInfo(): string[] { + const script = path.join(SCRIPTS_DIR, 'getJupyterVariableDataFrameInfo.py'); + // There is no script-specific output to parse, so we do not return a function. + return [script]; +} + +//============================ +// getJupyterVariableDataFrameRows.py + +export function getJupyterVariableDataFrameRows(): string[] { + const script = path.join(SCRIPTS_DIR, 'getJupyterVariableDataFrameRows.py'); + // There is no script-specific output to parse, so we do not return a function. + return [script]; +} + +//============================ +// getServerInfo.py + +type JupyterServerInfo = { + base_url: string; + notebook_dir: string; + hostname: string; + password: boolean; + pid: number; + port: number; + secure: boolean; + token: string; + url: string; +}; + +export function getServerInfo(): [string[], (out: string) => JupyterServerInfo[]] { + const script = path.join(SCRIPTS_DIR, 'getServerInfo.py'); + const args = [script]; + + function parse(out: string): JupyterServerInfo[] { + return JSON.parse(out.trim()); + } + + return [args, parse]; +} + +//============================ +// getJupyterKernels.py + +export function getJupyterKernels(): string[] { + const script = path.join(SCRIPTS_DIR, 'getJupyterKernels.py'); + // There is no script-specific output to parse, so we do not return a function. + return [script]; +} + +//============================ +// getJupyterKernelspecVersion.py + +export function getJupyterKernelspecVersion(): string[] { + const script = path.join(SCRIPTS_DIR, 'getJupyterKernelspecVersion.py'); + // For now we do not worry about parsing the output here. + return [script]; +} + +//============================ +// jupyter_nbInstalled.py + +export function jupyter_nbInstalled(): [string[], (out: string) => boolean] { + const script = path.join(SCRIPTS_DIR, 'jupyter_nbInstalled.py'); + const args = [script]; + + function parse(out: string): boolean { + return out.toLowerCase().includes('available'); + } + + return [args, parse]; +} diff --git a/src/client/common/process/pythonEnvironment.ts b/src/client/common/process/pythonEnvironment.ts index ae1cc043c28e..8462dbb0a211 100644 --- a/src/client/common/process/pythonEnvironment.ts +++ b/src/client/common/process/pythonEnvironment.ts @@ -1,19 +1,17 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as path from 'path'; import { CondaEnvironmentInfo } from '../../interpreter/contracts'; -import { EXTENSION_ROOT_DIR } from '../constants'; import { traceError, traceInfo } from '../logger'; import { IFileSystem } from '../platform/types'; import { Architecture } from '../utils/platform'; import { parsePythonVersion } from '../utils/version'; +import * as internalScripts from './internal/scripts'; import { ExecutionResult, InterpreterInfomation, IProcessService, PythonExecutionInfo, - PythonVersionInfo, ShellOptions, SpawnOptions } from './types'; @@ -80,9 +78,9 @@ class PythonEnvironment { private async getInterpreterInformationImpl(): Promise { try { - const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'interpreterInfo.py'); - const { command, args } = this.getExecutionInfo([file]); - const argv = [command, ...args]; + const execInfo = this.getExecutionInfo(); + const [args, parse] = internalScripts.interpreterInfo(); + const argv = [...execInfo.python, ...args]; // Concat these together to make a set of quoted strings const quoted = argv.reduce((p, c) => (p ? `${p} "${c}"` : `"${c.replace('\\', '\\\\')}"`), ''); @@ -98,17 +96,7 @@ class PythonEnvironment { traceError(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`); return; } - let json: { - versionInfo: PythonVersionInfo; - sysPrefix: string; - sysVersion: string; - is64Bit: boolean; - }; - try { - json = JSON.parse(result.stdout); - } catch (ex) { - throw Error(`${argv} returned bad JSON (${result.stdout}) (${ex})`); - } + const json = parse(result.stdout); traceInfo(`Found interpreter for ${argv}`); const versionValue = json.versionInfo.length === 4 diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts index 149b631d463a..6ade13eb4131 100644 --- a/src/client/common/terminal/syncTerminalService.ts +++ b/src/client/common/terminal/syncTerminalService.ts @@ -4,19 +4,17 @@ 'use strict'; import { inject } from 'inversify'; -import * as path from 'path'; import { CancellationToken, Disposable, Event } from 'vscode'; -import { EXTENSION_ROOT_DIR } from '../../constants'; import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts'; import { Cancellation } from '../cancellation'; import { traceVerbose } from '../logger'; import { IFileSystem, TemporaryFile } from '../platform/types'; +import * as internalScripts from '../process/internal/scripts'; import { createDeferred, Deferred } from '../utils/async'; import { noop } from '../utils/misc'; import { TerminalService } from './service'; import { ITerminalService } from './types'; -const shellExecFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'shell_exec.py'); enum State { notStarted = 0, started = 1, @@ -139,12 +137,8 @@ export class SynchronousTerminalService implements ITerminalService, Disposable const state = new ExecutionState(lockFile.filePath, this.fs, [command, ...args]); try { const pythonExec = this.pythonInterpreter || (await this.interpreter.getActiveInterpreter(undefined)); - await this.terminalService.sendCommand(pythonExec?.path || 'python', [ - shellExecFile.fileToCommandArgument(), - command.fileToCommandArgument(), - ...args, - lockFile.filePath.fileToCommandArgument() - ]); + const sendArgs = internalScripts.shell_exec(command, lockFile.filePath, args); + await this.terminalService.sendCommand(pythonExec?.path || 'python', sendArgs); const promise = swallowExceptions ? state.completed : state.completed.catch(noop); await Cancellation.race(() => promise, cancel); } finally { diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 9bac975eb3dc..39747bfa0f0d 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -4,12 +4,12 @@ import '../../common/extensions'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; import { PYTHON_WARNINGS } from '../../common/constants'; import { LogOptions, traceDecorators, traceError, traceInfo, traceVerbose } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; +import * as internalScripts from '../../common/process/internal/scripts'; import { ExecutionResult, IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types'; import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; @@ -17,7 +17,6 @@ import { sleep } from '../../common/utils/async'; import { InMemoryCache } from '../../common/utils/cacheUtils'; import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; -import { EXTENSION_ROOT_DIR } from '../../constants'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { IInterpreterService, PythonInterpreter } from '../contracts'; @@ -179,8 +178,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // In order to make sure we know where the environment output is, // put in a dummy echo we can look for - const printEnvPyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariables.py'); - const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && python ${printEnvPyFile.fileToCommandArgument()}`; + const [args, parse] = internalScripts.printEnvVariables(); + args.forEach((arg, i) => { + args[i] = arg.toCommandArgument(); + }); + const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && python ${args.join(' ')}`; traceVerbose(`Activating Environment to capture Environment variables, ${command}`); // Do some wrapping of the call. For two reasons: @@ -218,7 +220,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } } } - const returnedEnv = this.parseEnvironmentOutput(result.stdout); + const returnedEnv = this.parseEnvironmentOutput(result.stdout, parse); // Put back the PYTHONWARNINGS value if (oldWarnings && returnedEnv) { @@ -247,9 +249,9 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } @traceDecorators.error('Failed to parse Environment variables') @traceDecorators.verbose('parseEnvironmentOutput', LogOptions.None) - protected parseEnvironmentOutput(output: string): NodeJS.ProcessEnv | undefined { + protected parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) { output = output.substring(output.indexOf(getEnvironmentPrefix) + getEnvironmentPrefix.length); const js = output.substring(output.indexOf('{')).trim(); - return JSON.parse(js); + return parse(js); } } diff --git a/src/client/interpreter/activation/terminalEnvironmentActivationService.ts b/src/client/interpreter/activation/terminalEnvironmentActivationService.ts index 5666aaa97b4b..557a95fca4e6 100644 --- a/src/client/interpreter/activation/terminalEnvironmentActivationService.ts +++ b/src/client/interpreter/activation/terminalEnvironmentActivationService.ts @@ -4,11 +4,10 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; import { CancellationTokenSource } from 'vscode'; -import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { LogOptions, traceDecorators } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; +import * as internalScripts from '../../common/process/internal/scripts'; import { ITerminalServiceFactory } from '../../common/terminal/types'; import { Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; @@ -17,8 +16,6 @@ import { EventName } from '../../telemetry/constants'; import { PythonInterpreter } from '../contracts'; import { IEnvironmentActivationService } from './types'; -const pyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariablesToFile.py'); - /** * This class will provide the environment variables of an interpreter by activating it in a terminal. * This has the following benefit: @@ -61,19 +58,15 @@ export class TerminalEnvironmentActivationService implements IEnvironmentActivat const command = interpreter?.path || 'python'; const jsonFile = await this.fs.createTemporaryFile('.json'); - try { + const [args, parse] = internalScripts.printEnvVariablesToFile(jsonFile.filePath); + // Pass a cancellation token to ensure we wait until command has completed. // If there are any errors in executing in the terminal, throw them so they get logged and bubbled up. - await terminal.sendCommand( - command, - [pyFile.fileToCommandArgument(), jsonFile.filePath.fileToCommandArgument()], - new CancellationTokenSource().token, - false - ); + await terminal.sendCommand(command, args, new CancellationTokenSource().token, false); const contents = await this.fs.readFile(jsonFile.filePath); - return JSON.parse(contents); + return parse(contents); } finally { // We created a hidden terminal for temp usage, hence dispose when done. terminal.dispose(); diff --git a/src/client/languageServices/jediProxyFactory.ts b/src/client/languageServices/jediProxyFactory.ts index 5207b71d3d1c..72b5ac41a79c 100644 --- a/src/client/languageServices/jediProxyFactory.ts +++ b/src/client/languageServices/jediProxyFactory.ts @@ -9,8 +9,8 @@ export class JediFactory implements Disposable { private jediProxyHandlers: Map>; constructor( - private extensionRootPath: string, private interpreter: PythonInterpreter | undefined, + // This is passed through to JediProxy(). private serviceContainer: IServiceContainer ) { this.disposables = []; @@ -21,29 +21,30 @@ export class JediFactory implements Disposable { this.disposables.forEach((disposable) => disposable.dispose()); this.disposables = []; } + public getJediProxyHandler(resource?: Uri): JediProxyHandler { - const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined; - let workspacePath = workspaceFolder ? workspaceFolder.uri.fsPath : undefined; - if (!workspacePath) { - if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) { - workspacePath = workspace.workspaceFolders[0].uri.fsPath; - } else { - workspacePath = __dirname; + const workspacePath = this.getWorkspacePath(resource); + if (!this.jediProxyHandlers.has(workspacePath)) { + const jediProxy = new JediProxy(workspacePath, this.interpreter, this.serviceContainer); + const jediProxyHandler = new JediProxyHandler(jediProxy); + this.disposables.push(jediProxy, jediProxyHandler); + this.jediProxyHandlers.set(workspacePath, jediProxyHandler); + } + return this.jediProxyHandlers.get(workspacePath)! as JediProxyHandler; + } + + private getWorkspacePath(resource?: Uri): string { + if (resource) { + const workspaceFolder = workspace.getWorkspaceFolder(resource); + if (workspaceFolder) { + return workspaceFolder.uri.fsPath; } } - if (!this.jediProxyHandlers.has(workspacePath!)) { - const jediProxy = new JediProxy( - this.extensionRootPath, - workspacePath!, - this.interpreter, - this.serviceContainer - ); - const jediProxyHandler = new JediProxyHandler(jediProxy); - this.disposables.push(jediProxy, jediProxyHandler); - this.jediProxyHandlers.set(workspacePath!, jediProxyHandler); + if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) { + return workspace.workspaceFolders[0].uri.fsPath; + } else { + return __dirname; } - // tslint:disable-next-line:no-non-null-assertion - return this.jediProxyHandlers.get(workspacePath!)! as JediProxyHandler; } } diff --git a/src/client/providers/importSortProvider.ts b/src/client/providers/importSortProvider.ts index abb9f09e0472..cfb5e60eaea0 100644 --- a/src/client/providers/importSortProvider.ts +++ b/src/client/providers/importSortProvider.ts @@ -1,11 +1,12 @@ import { inject, injectable } from 'inversify'; import { EOL } from 'os'; import * as path from 'path'; -import { CancellationToken, Uri, WorkspaceEdit } from 'vscode'; +import { CancellationToken, TextDocument, Uri, WorkspaceEdit } from 'vscode'; import { IApplicationShell, ICommandManager, IDocumentManager } from '../common/application/types'; -import { Commands, EXTENSION_ROOT_DIR, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../common/constants'; +import { Commands, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import { traceError } from '../common/logger'; import { IFileSystem } from '../common/platform/types'; +import * as internalScripts from '../common/process/internal/scripts'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../common/process/types'; import { IConfigurationService, IDisposableRegistry, IEditorUtils, IOutputChannel } from '../common/types'; import { noop } from '../common/utils/misc'; @@ -14,6 +15,28 @@ import { captureTelemetry } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { ISortImportsEditingProvider } from './types'; +async function withRealFile( + document: TextDocument, + fs: IFileSystem, + useFile: (filename: string) => Promise +): Promise<[string, T]> { + const filename = document.uri.fsPath; + const text = document.getText(); + if (document.isDirty) { + const tmpFile = await fs.createTemporaryFile(path.extname(filename)); + try { + await fs.writeFile(tmpFile.filePath, text); + const result = await useFile(tmpFile.filePath); + return [text, result]; + } finally { + tmpFile.dispose(); + } + } else { + const result = await useFile(filename); + return [text, result]; + } +} + @injectable() export class SortImportsEditingProvider implements ISortImportsEditingProvider { private readonly processServiceFactory: IProcessServiceFactory; @@ -30,6 +53,7 @@ export class SortImportsEditingProvider implements ISortImportsEditingProvider { this.processServiceFactory = serviceContainer.get(IProcessServiceFactory); this.editorUtils = serviceContainer.get(IEditorUtils); } + @captureTelemetry(EventName.FORMAT_SORT_IMPORTS) public async provideDocumentSortImportsEdits( uri: Uri, @@ -42,44 +66,22 @@ export class SortImportsEditingProvider implements ISortImportsEditingProvider { if (document.lineCount <= 1) { return; } + + const execIsort = await this.getExecIsort(document, uri, token); + // isort does have the ability to read from the process input stream and return the formatted code out of the output stream. // However they don't support returning the diff of the formatted text when reading data from the input stream. // Yes getting text formatted that way avoids having to create a temporary file, however the diffing will have // to be done here in node (extension), i.e. extension cpu, i.e. less responsive solution. - const importScript = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'sortImports.py'); - const fsService = this.serviceContainer.get(IFileSystem); - const tmpFile = document.isDirty - ? await fsService.createTemporaryFile(path.extname(document.uri.fsPath)) - : undefined; - if (tmpFile) { - await fsService.writeFile(tmpFile.filePath, document.getText()); - } - const settings = this.configurationService.getSettings(uri); - const isort = settings.sortImports.path; - const filePath = tmpFile ? tmpFile.filePath : document.uri.fsPath; - const args = [filePath, '--diff'].concat(settings.sortImports.args); - let diffPatch: string; - - if (token && token.isCancellationRequested) { - return; - } - try { - if (typeof isort === 'string' && isort.length > 0) { - // Lets just treat this as a standard tool. - const processService = await this.processServiceFactory.create(document.uri); - diffPatch = (await processService.exec(isort, args, { throwOnStdErr: true, token })).stdout; - } else { - const processExeService = await this.pythonExecutionFactory.create({ resource: document.uri }); - diffPatch = (await processExeService.exec([importScript].concat(args), { throwOnStdErr: true, token })) - .stdout; + const fs = this.serviceContainer.get(IFileSystem); + const [text, diffPatch] = await withRealFile(document, fs, async (filename: string) => { + if (token && token.isCancellationRequested) { + return; } - return this.editorUtils.getWorkspaceEditsFromPatch(document.getText(), diffPatch, document.uri); - } finally { - if (tmpFile) { - tmpFile.dispose(); - } - } + return execIsort(filename); + }); + return diffPatch ? this.editorUtils.getWorkspaceEditsFromPatch(text, diffPatch, document.uri) : undefined; } public registerCommands() { @@ -87,6 +89,7 @@ export class SortImportsEditingProvider implements ISortImportsEditingProvider { const disposable = cmdManager.registerCommand(Commands.Sort_Imports, this.sortImports, this); this.serviceContainer.get(IDisposableRegistry).push(disposable); } + public async sortImports(uri?: Uri): Promise { if (!uri) { const activeEditor = this.documentManager.activeTextEditor; @@ -125,4 +128,39 @@ export class SortImportsEditingProvider implements ISortImportsEditingProvider { this.shell.showErrorMessage(message).then(noop, noop); } } + + private async getExecIsort(document: TextDocument, uri: Uri, token?: CancellationToken) { + const settings = this.configurationService.getSettings(uri); + const _isort = settings.sortImports.path; + const isort = typeof _isort === 'string' && _isort.length > 0 ? _isort : undefined; + const isortArgs = settings.sortImports.args; + + if (isort) { + const procService = await this.processServiceFactory.create(document.uri); + // Use isort directly instead of the internal script. + return async (filename: string) => { + const args = getIsortArgs(filename, isortArgs); + const proc = await procService.exec(isort, args, { throwOnStdErr: true, token }); + return proc.stdout; + }; + } else { + const procService = await this.pythonExecutionFactory.create({ resource: document.uri }); + return async (filename: string) => { + const [args, parse] = internalScripts.sortImports(filename, isortArgs); + const proc = await procService.exec(args, { throwOnStdErr: true, token }); + return parse(proc.stdout); + }; + } + } +} + +function getIsortArgs(filename: string, extraArgs?: string[]): string[] { + // We could just adapt internalScripts.sortImports(). However, + // the following is simpler and the alternative doesn't offer + // any signficant benefit. + const args = [filename, '--diff']; + if (extraArgs) { + args.push(...extraArgs); + } + return args; } diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index 10f32d2b4b04..50ee5d00f049 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -11,6 +11,7 @@ import { isTestExecution } from '../common/constants'; import '../common/extensions'; import { IS_WINDOWS } from '../common/platform/constants'; import { IFileSystem } from '../common/platform/types'; +import * as internalScripts from '../common/process/internal/scripts'; import { IPythonExecutionFactory } from '../common/process/types'; import { BANNER_NAME_PROPOSE_LS, @@ -160,7 +161,6 @@ export class JediProxy implements Disposable { private timer?: NodeJS.Timer | number; public constructor( - private extensionRootDir: string, workspacePath: string, interpreter: PythonInterpreter | undefined, private serviceContainer: IServiceContainer @@ -234,7 +234,7 @@ export class JediProxy implements Disposable { // keep track of the directory so we can re-spawn the process. private initialize(): Promise { - return this.spawnProcess(path.join(this.extensionRootDir, 'pythonFiles')).catch((ex) => { + return this.spawnProcess().catch((ex) => { if (this.languageServerStarted) { this.languageServerStarted.reject(ex); } @@ -367,7 +367,7 @@ export class JediProxy implements Disposable { } // tslint:disable-next-line:max-func-body-length - private async spawnProcess(cwd: string) { + private async spawnProcess() { if (this.languageServerStarted && !this.languageServerStarted.completed) { this.languageServerStarted.reject(new Error('Language Server not started.')); } @@ -379,12 +379,8 @@ export class JediProxy implements Disposable { if ((await pythonProcess.getExecutablePath().catch(() => '')).length === 0) { return; } - const args = ['completion.py']; - if (typeof this.pythonSettings.jediPath === 'string' && this.pythonSettings.jediPath.length > 0) { - args.push('custom'); - args.push(this.pythonSettings.jediPath); - } - const result = pythonProcess.execObservable(args, { cwd }); + const [args, parse] = internalScripts.completion(this.pythonSettings.jediPath); + const result = pythonProcess.execObservable(args, {}); this.proc = result.proc; this.languageServerStarted.resolve(); this.proc!.on('end', (end) => { @@ -399,7 +395,7 @@ export class JediProxy implements Disposable { error.message && error.message.indexOf('This socket has been ended by the other party') >= 0 ) { - this.spawnProcess(cwd).catch((ex) => { + this.spawnProcess().catch((ex) => { if (this.languageServerStarted) { this.languageServerStarted.reject(ex); } @@ -419,7 +415,7 @@ export class JediProxy implements Disposable { // tslint:disable-next-line:no-any let responses: any[]; try { - responses = dataStr.splitLines().map((resp) => JSON.parse(resp)); + responses = parse(dataStr); this.previousData = ''; } catch (ex) { // Possible we've only received part of the data, hence don't clear previousData. diff --git a/src/client/providers/renameProvider.ts b/src/client/providers/renameProvider.ts index 500923e0afbf..86037f894c60 100644 --- a/src/client/providers/renameProvider.ts +++ b/src/client/providers/renameProvider.ts @@ -5,15 +5,17 @@ import { ProviderResult, RenameProvider, TextDocument, + Uri, window, workspace, WorkspaceEdit } from 'vscode'; -import { EXTENSION_ROOT_DIR, STANDARD_OUTPUT_CHANNEL } from '../common/constants'; +import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import { getWorkspaceEditsFromPatch } from '../common/editor'; import { traceError } from '../common/logger'; import { IFileSystem } from '../common/platform/types'; -import { IConfigurationService, IInstaller, IOutputChannel, Product } from '../common/types'; +import { IPythonExecutionFactory } from '../common/process/types'; +import { IInstaller, IOutputChannel, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { RefactorProxy } from '../refactor/proxy'; import { captureTelemetry } from '../telemetry'; @@ -25,10 +27,8 @@ type RenameResponse = { export class PythonRenameProvider implements RenameProvider { private readonly outputChannel: OutputChannel; - private readonly configurationService: IConfigurationService; constructor(private serviceContainer: IServiceContainer) { this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - this.configurationService = serviceContainer.get(IConfigurationService); } @captureTelemetry(EventName.REFACTOR_RENAME) public provideRenameEdits( @@ -64,9 +64,11 @@ export class PythonRenameProvider implements RenameProvider { workspaceFolder = workspace.workspaceFolders[0]; } const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; - const pythonSettings = this.configurationService.getSettings(workspaceFolder ? workspaceFolder.uri : undefined); - const proxy = new RefactorProxy(EXTENSION_ROOT_DIR, pythonSettings, workspaceRoot, this.serviceContainer); + const proxy = new RefactorProxy(workspaceRoot, async () => { + const factory = this.serviceContainer.get(IPythonExecutionFactory); + return factory.create({ resource: Uri.file(workspaceRoot) }); + }); return proxy .rename(document, newName, document.uri.fsPath, range) .then((response) => { diff --git a/src/client/providers/simpleRefactorProvider.ts b/src/client/providers/simpleRefactorProvider.ts index 0e0bdb53b714..185c22522539 100644 --- a/src/client/providers/simpleRefactorProvider.ts +++ b/src/client/providers/simpleRefactorProvider.ts @@ -2,7 +2,8 @@ import * as vscode from 'vscode'; import { Commands } from '../common/constants'; import { getTextEditsFromPatch } from '../common/editor'; import { traceError } from '../common/logger'; -import { IConfigurationService, IInstaller, Product } from '../common/types'; +import { IPythonExecutionFactory } from '../common/process/types'; +import { IInstaller, Product } from '../common/types'; import { StopWatch } from '../common/utils/stopWatch'; import { IServiceContainer } from '../ioc/types'; import { RefactorProxy } from '../refactor/proxy'; @@ -24,7 +25,6 @@ export function activateSimplePythonRefactorProvider( let disposable = vscode.commands.registerCommand(Commands.Refactor_Extract_Variable, () => { const stopWatch = new StopWatch(); const promise = extractVariable( - context.extensionPath, vscode.window.activeTextEditor!, vscode.window.activeTextEditor!.selection, outputChannel, @@ -38,7 +38,6 @@ export function activateSimplePythonRefactorProvider( disposable = vscode.commands.registerCommand(Commands.Refactor_Extract_Method, () => { const stopWatch = new StopWatch(); const promise = extractMethod( - context.extensionPath, vscode.window.activeTextEditor!, vscode.window.activeTextEditor!.selection, outputChannel, @@ -52,7 +51,6 @@ export function activateSimplePythonRefactorProvider( // Exported for unit testing export function extractVariable( - extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, outputChannel: vscode.OutputChannel, @@ -68,13 +66,13 @@ export function extractVariable( workspaceFolder = vscode.workspace.workspaceFolders[0]; } const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; - const pythonSettings = serviceContainer - .get(IConfigurationService) - .getSettings(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { const newName = `newvariable${new Date().getMilliseconds().toString()}`; - const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot, serviceContainer); + const proxy = new RefactorProxy(workspaceRoot, async () => { + const factory = serviceContainer.get(IPythonExecutionFactory); + return factory.create({ resource: vscode.Uri.file(workspaceRoot) }); + }); const rename = proxy .extractVariable( textEditor.document, @@ -93,7 +91,6 @@ export function extractVariable( // Exported for unit testing export function extractMethod( - extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, outputChannel: vscode.OutputChannel, @@ -109,13 +106,13 @@ export function extractMethod( workspaceFolder = vscode.workspace.workspaceFolders[0]; } const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; - const pythonSettings = serviceContainer - .get(IConfigurationService) - .getSettings(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { const newName = `newmethod${new Date().getMilliseconds().toString()}`; - const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot, serviceContainer); + const proxy = new RefactorProxy(workspaceRoot, async () => { + const factory = serviceContainer.get(IPythonExecutionFactory); + return factory.create({ resource: vscode.Uri.file(workspaceRoot) }); + }); const rename = proxy .extractMethod( textEditor.document, diff --git a/src/client/refactor/proxy.ts b/src/client/refactor/proxy.ts index 29196a8e3786..94bc5cba1b7a 100644 --- a/src/client/refactor/proxy.ts +++ b/src/client/refactor/proxy.ts @@ -1,20 +1,17 @@ // tslint:disable:no-any no-empty member-ordering prefer-const prefer-template no-var-self import { ChildProcess } from 'child_process'; -import * as path from 'path'; -import { Disposable, Position, Range, TextDocument, TextEditorOptions, Uri, window } from 'vscode'; +import { Disposable, Position, Range, TextDocument, TextEditorOptions, window } from 'vscode'; import '../common/extensions'; import { traceError } from '../common/logger'; import { IS_WINDOWS } from '../common/platform/constants'; -import { IPythonExecutionFactory } from '../common/process/types'; -import { IPythonSettings } from '../common/types'; +import * as internalScripts from '../common/process/internal/scripts'; +import { IPythonExecutionService } from '../common/process/types'; import { createDeferred, Deferred } from '../common/utils/async'; import { getWindowsLineEndingCount } from '../common/utils/text'; -import { IServiceContainer } from '../ioc/types'; export class RefactorProxy extends Disposable { private _process?: ChildProcess; - private _extensionDir: string; private _previousOutData: string = ''; private _previousStdErrData: string = ''; private _startedSuccessfully: boolean = false; @@ -22,13 +19,10 @@ export class RefactorProxy extends Disposable { private _commandReject!: (reason?: any) => void; private initialized!: Deferred; constructor( - extensionDir: string, - _pythonSettings: IPythonSettings, private workspaceRoot: string, - private serviceContainer: IServiceContainer + private getPythonExecutionService: () => Promise ) { super(() => {}); - this._extensionDir = extensionDir; } public dispose() { @@ -132,13 +126,10 @@ export class RefactorProxy extends Disposable { }); } private async initialize(): Promise { - const pythonProc = await this.serviceContainer - .get(IPythonExecutionFactory) - .create({ resource: Uri.file(this.workspaceRoot) }); + const pythonProc = await this.getPythonExecutionService(); this.initialized = createDeferred(); - const args = ['refactor.py', this.workspaceRoot]; - const cwd = path.join(this._extensionDir, 'pythonFiles'); - const result = pythonProc.execObservable(args, { cwd }); + const [args, parse] = internalScripts.refactor(this.workspaceRoot); + const result = pythonProc.execObservable(args, {}); this._process = result.proc; result.out.subscribe( (output) => { @@ -147,7 +138,7 @@ export class RefactorProxy extends Disposable { this._startedSuccessfully = true; return this.initialized.resolve(); } - this.onData(output.out); + this.onData(output.out, parse); } else { this.handleStdError(output.out); } @@ -195,7 +186,7 @@ export class RefactorProxy extends Disposable { } this.initialized.reject(error); } - private onData(data: string) { + private onData(data: string, parse: (out: string) => object[]) { if (!this._commandResolve) { return; } @@ -205,10 +196,7 @@ export class RefactorProxy extends Disposable { let dataStr = (this._previousOutData = this._previousOutData + data + ''); let response: any; try { - response = dataStr - .split(/\r?\n/g) - .filter((line) => line.length > 0) - .map((resp) => JSON.parse(resp)); + response = parse(dataStr); this._previousOutData = ''; } catch (ex) { // Possible we've only received part of the data, hence don't clear previousData diff --git a/src/client/terminals/codeExecution/helper.ts b/src/client/terminals/codeExecution/helper.ts index ca2802b4517f..87af541f7547 100644 --- a/src/client/terminals/codeExecution/helper.ts +++ b/src/client/terminals/codeExecution/helper.ts @@ -3,12 +3,12 @@ import '../../common/extensions'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; import { Range, TextEditor, Uri } from 'vscode'; import { IApplicationShell, IDocumentManager } from '../../common/application/types'; -import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants'; +import { PYTHON_LANGUAGE } from '../../common/constants'; import { traceError } from '../../common/logger'; +import * as internalScripts from '../../common/process/internal/scripts'; import { IProcessServiceFactory } from '../../common/process/types'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; @@ -36,10 +36,10 @@ export class CodeExecutionHelper implements ICodeExecutionHelper { code = code.replace(new RegExp('\\r', 'g'), ''); const interpreter = await this.interpreterService.getActiveInterpreter(resource); const processService = await this.processServiceFactory.create(resource); - const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'normalizeForInterpreter.py'), code]; + const [args, parse] = internalScripts.normalizeForInterpreter(code); const proc = await processService.exec(interpreter?.path || 'python', args, { throwOnStdErr: true }); - return proc.stdout; + return parse(proc.stdout); } catch (ex) { traceError(ex, 'Python: Failed to normalize code for execution in terminal'); return code; diff --git a/src/client/testing/common/debugLauncher.ts b/src/client/testing/common/debugLauncher.ts index 8e707629bc69..969c02e19eb7 100644 --- a/src/client/testing/common/debugLauncher.ts +++ b/src/client/testing/common/debugLauncher.ts @@ -6,6 +6,7 @@ import { IApplicationShell, IDebugService, IWorkspaceService } from '../../commo import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { traceError } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; +import * as internalScripts from '../../common/process/internal/scripts'; import { IConfigurationService, IPythonSettings } from '../../common/types'; import { noop } from '../../common/utils/misc'; import { DebuggerTypeName } from '../../debugger/constants'; @@ -156,8 +157,11 @@ export class DebugLauncher implements ITestDebugLauncher { ): Promise { const configArgs = debugConfig as LaunchRequestArguments; - configArgs.program = this.getTestLauncherScript(options.testProvider); - configArgs.args = this.fixArgs(options.args, options.testProvider); + const testArgs = this.fixArgs(options.args, options.testProvider); + const script = this.getTestLauncherScript(options.testProvider); + const args = script(testArgs); + configArgs.program = args[0]; + configArgs.args = args.slice(1); // We leave configArgs.request as "test" so it will be sent in telemetry. const launchArgs = await this.launchResolver.resolveDebugConfiguration( @@ -184,11 +188,11 @@ export class DebugLauncher implements ITestDebugLauncher { private getTestLauncherScript(testProvider: TestProvider) { switch (testProvider) { case 'unittest': { - return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'visualstudio_py_testlauncher.py'); + return internalScripts.visualstudio_py_testlauncher; } case 'pytest': case 'nosetest': { - return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testlauncher.py'); + return internalScripts.testlauncher; } default: { throw new Error(`Unknown test provider '${testProvider}'`); diff --git a/src/client/testing/common/services/discovery.ts b/src/client/testing/common/services/discovery.ts index 30ca5f177ce8..0042f5e003f0 100644 --- a/src/client/testing/common/services/discovery.ts +++ b/src/client/testing/common/services/discovery.ts @@ -4,25 +4,21 @@ 'use strict'; import { inject, injectable, named } from 'inversify'; -import * as path from 'path'; import { OutputChannel } from 'vscode'; import { traceError } from '../../../common/logger'; +import * as internalScripts from '../../../common/process/internal/scripts'; import { ExecutionFactoryCreateWithEnvironmentOptions, - ExecutionResult, IPythonExecutionFactory, SpawnOptions } from '../../../common/process/types'; import { IOutputChannel } from '../../../common/types'; -import { EXTENSION_ROOT_DIR } from '../../../constants'; import { captureTelemetry } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; import { TEST_OUTPUT_CHANNEL } from '../constants'; import { ITestDiscoveryService, TestDiscoveryOptions, Tests } from '../types'; import { DiscoveredTests, ITestDiscoveredTestParser } from './types'; -const DISCOVERY_FILE = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testing_tools', 'run_adapter.py'); - @injectable() export class TestsDiscoveryService implements ITestDiscoveryService { constructor( @@ -32,20 +28,19 @@ export class TestsDiscoveryService implements ITestDiscoveryService { ) {} @captureTelemetry(EventName.UNITTEST_DISCOVER_WITH_PYCODE, undefined, true) public async discoverTests(options: TestDiscoveryOptions): Promise { - let output: ExecutionResult | undefined; try { - output = await this.exec(options); - const discoveredTests = JSON.parse(output.stdout) as DiscoveredTests[]; + const discoveredTests = await this.exec(options); return this.parser.parse(options.workspaceFolder, discoveredTests); } catch (ex) { - if (output) { - traceError('Failed to parse discovered Test', new Error(output.stdout)); + if (ex.stdout) { + traceError('Failed to parse discovered Test', new Error(ex.stdout)); } traceError('Failed to parse discovered Test', ex); throw ex; } } - public async exec(options: TestDiscoveryOptions): Promise> { + public async exec(options: TestDiscoveryOptions): Promise { + const [args, parse] = internalScripts.testing_tools.run_adapter(options.args); const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = { allowEnvironmentFetchExceptions: false, resource: options.workspaceFolder @@ -56,8 +51,13 @@ export class TestsDiscoveryService implements ITestDiscoveryService { cwd: options.cwd, throwOnStdErr: true }; - const argv = [DISCOVERY_FILE, ...options.args]; - this.outChannel.appendLine(`python ${argv.join(' ')}`); - return execService.exec(argv, spawnOptions); + this.outChannel.appendLine(`python ${args.join(' ')}`); + const proc = await execService.exec(args, spawnOptions); + try { + return parse(proc.stdout); + } catch (ex) { + ex.stdout = proc.stdout; + throw ex; // re-throw + } } } diff --git a/src/client/testing/common/services/types.ts b/src/client/testing/common/services/types.ts index b6ae34eccb0f..e2b929379bbb 100644 --- a/src/client/testing/common/services/types.ts +++ b/src/client/testing/common/services/types.ts @@ -4,42 +4,17 @@ 'use strict'; import { Uri } from 'vscode'; +import * as internalScripts from '../../../common/process/internal/scripts'; import { Tests } from '../types'; -export type TestNode = { - id: string; - name: string; - parentid: string; -}; -export type TestParent = TestNode & { - kind: 'folder' | 'file' | 'suite' | 'function'; -}; -export type TestFSNode = TestParent & { - kind: 'folder' | 'file'; - relpath: string; -}; -export type TestFolder = TestFSNode & { - kind: 'folder'; -}; -export type TestFile = TestFSNode & { - kind: 'file'; -}; -export type TestSuite = TestParent & { - kind: 'suite'; -}; -// function-as-a-container is for parameterized ("sub") tests. -export type TestFunction = TestParent & { - kind: 'function'; -}; -export type Test = TestNode & { - source: string; -}; -export type DiscoveredTests = { - rootid: string; - root: string; - parents: TestParent[]; - tests: Test[]; -}; +// We expose these here as a convenience and to cut down on churn +// elsewhere in the code. +export type DiscoveredTests = internalScripts.testing_tools.DiscoveredTests; +export type Test = internalScripts.testing_tools.Test; +export type TestFolder = internalScripts.testing_tools.TestFolder; +export type TestFile = internalScripts.testing_tools.TestFile; +export type TestSuite = internalScripts.testing_tools.TestSuite; +export type TestFunction = internalScripts.testing_tools.TestFunction; export const ITestDiscoveredTestParser = Symbol('ITestDiscoveredTestParser'); export interface ITestDiscoveredTestParser { diff --git a/src/client/testing/navigation/symbolProvider.ts b/src/client/testing/navigation/symbolProvider.ts index 7a308990985d..806776d89245 100644 --- a/src/client/testing/navigation/symbolProvider.ts +++ b/src/client/testing/navigation/symbolProvider.ts @@ -4,7 +4,6 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; import { CancellationToken, DocumentSymbolProvider, @@ -16,8 +15,8 @@ import { Uri } from 'vscode'; import { traceError } from '../../common/logger'; +import * as internalScripts from '../../common/process/internal/scripts'; import { IPythonExecutionFactory } from '../../common/process/types'; -import { EXTENSION_ROOT_DIR } from '../../constants'; type RawSymbol = { namespace: string; name: string; range: Range }; type Symbols = { @@ -62,15 +61,14 @@ export class TestFileSymbolProvider implements DocumentSymbolProvider { if (document.isUntitled) { return; } - const scriptArgs: string[] = [document.uri.fsPath]; - if (document.isDirty) { - scriptArgs.push(document.getText()); - } - const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'symbolProvider.py'), ...scriptArgs]; + const [args, parse] = internalScripts.symbolProvider( + document.uri.fsPath, + document.isDirty ? document.getText() : undefined + ); const pythonService = await this.pythonServiceFactory.create({ resource: document.uri }); const proc = await pythonService.exec(args, { throwOnStdErr: true, token }); - return JSON.parse(proc.stdout); + return (parse(proc.stdout) as unknown) as Symbols; } catch (ex) { traceError('Python: Failed to get symbols', ex); return; diff --git a/src/client/testing/unittest/runner.ts b/src/client/testing/unittest/runner.ts index a5fe4353cfb9..77f23ee3de54 100644 --- a/src/client/testing/unittest/runner.ts +++ b/src/client/testing/unittest/runner.ts @@ -1,9 +1,8 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { traceError } from '../../common/logger'; +import * as internalScripts from '../../common/process/internal/scripts'; import { IDisposableRegistry } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { noop } from '../../common/utils/misc'; @@ -73,7 +72,6 @@ export class TestManagerRunner implements ITestManagerRunner { options.tests.summary.passed = 0; options.tests.summary.skipped = 0; let failFast = false; - const testLauncherFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'visualstudio_py_testlauncher.py'); this.server.on('error', (message: string, ...data: string[]) => traceError(`${message} ${data.join(' ')}`)); this.server.on('log', noop); this.server.on('connect', noop); @@ -144,8 +142,10 @@ export class TestManagerRunner implements ITestManagerRunner { }; return debugLauncher.launchDebugger(launchOptions); } else { + const args = internalScripts.visualstudio_py_testlauncher(testArgs); + const runOptions: Options = { - args: [testLauncherFile].concat(testArgs), + args: args, cwd: options.cwd, outChannel: options.outChannel, token: options.token, diff --git a/src/test/providers/importSortProvider.unit.test.ts b/src/test/providers/importSortProvider.unit.test.ts index 3d7208b21200..59d621d6edaf 100644 --- a/src/test/providers/importSortProvider.unit.test.ts +++ b/src/test/providers/importSortProvider.unit.test.ts @@ -277,7 +277,7 @@ suite('Import Sort Provider', () => { shell.verifyAll(); documentManager.verifyAll(); }); - test('Ensure temporary file is created for sorting when document is dirty', async () => { + test('Ensure temporary file is created for sorting when document is dirty (with custom isort path)', async () => { const uri = Uri.file('something.py'); const mockDoc = TypeMoq.Mock.ofType(); let tmpFileDisposed = false; @@ -352,13 +352,11 @@ suite('Import Sort Provider', () => { shell.verifyAll(); documentManager.verifyAll(); }); - test('Ensure temporary file is created for sorting when document is dirty (with custom isort path)', async () => { + test('Ensure temporary file is created for sorting when document is dirty', async () => { const uri = Uri.file('something.py'); const mockDoc = TypeMoq.Mock.ofType(); let tmpFileDisposed = false; const tmpFile: TemporaryFile = { filePath: 'TmpFile', dispose: () => (tmpFileDisposed = true) }; - const processService = TypeMoq.Mock.ofType(); - processService.setup((d: any) => d.then).returns(() => undefined); mockDoc.setup((d: any) => d.then).returns(() => undefined); mockDoc .setup((d) => d.lineCount) diff --git a/src/test/refactor/extension.refactor.extract.method.test.ts b/src/test/refactor/extension.refactor.extract.method.test.ts index 3e3e64d2f7ee..9a47914d827d 100644 --- a/src/test/refactor/extension.refactor.extract.method.test.ts +++ b/src/test/refactor/extension.refactor.extract.method.test.ts @@ -17,17 +17,16 @@ import { workspace } from 'vscode'; import { getTextEditsFromPatch } from '../../client/common/editor'; +import { IPythonExecutionFactory, IPythonExecutionService } from '../../client/common/process/types'; import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; import { InterpreterService } from '../../client/interpreter/interpreterService'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { extractMethod } from '../../client/providers/simpleRefactorProvider'; import { RefactorProxy } from '../../client/refactor/proxy'; -import { getExtensionSettings } from '../common'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; import { closeActiveWindows, initialize, initializeTest } from './../initialize'; import { MockOutputChannel } from './../mockClasses'; -const EXTENSION_DIR = path.join(__dirname, '..', '..', '..'); const refactorSourceFile = path.join( __dirname, '..', @@ -97,16 +96,17 @@ suite('Method Extraction', () => { instance(mock(InterpreterService)) ); } + function createPythonExecGetter(workspaceRoot: string): () => Promise { + return async () => { + const factory = ioc.serviceContainer.get(IPythonExecutionFactory); + return factory.create({ resource: Uri.file(workspaceRoot) }); + }; + } async function testingMethodExtraction(shouldError: boolean, startPos: Position, endPos: Position): Promise { - const pythonSettings = getExtensionSettings(Uri.file(refactorTargetFile)); const rangeOfTextToExtract = new Range(startPos, endPos); - const proxy = new RefactorProxy( - EXTENSION_DIR, - pythonSettings, - path.dirname(refactorTargetFile), - ioc.serviceContainer - ); + const workspaceRoot = path.dirname(refactorTargetFile); + const proxy = new RefactorProxy(workspaceRoot, createPythonExecGetter(workspaceRoot)); // tslint:disable-next-line:no-multiline-string const DIFF = `--- a/refactor.py\n+++ b/refactor.py\n@@ -237,9 +237,12 @@\n try:\n self._process_request(self._input.readline())\n except Exception as ex:\n- message = ex.message + ' \\n' + traceback.format_exc()\n- sys.stderr.write(str(len(message)) + ':' + message)\n- sys.stderr.flush()\n+ self.myNewMethod(ex)\n+\n+ def myNewMethod(self, ex):\n+ message = ex.message + ' \\n' + traceback.format_exc()\n+ sys.stderr.write(str(len(message)) + ':' + message)\n+ sys.stderr.flush()\n \n if __name__ == '__main__':\n RopeRefactoring().watch()\n`; @@ -167,7 +167,7 @@ suite('Method Extraction', () => { editor.selection = new Selection(rangeOfTextToExtract.start, rangeOfTextToExtract.end); try { - await extractMethod(EXTENSION_DIR, editor, rangeOfTextToExtract, ch, ioc.serviceContainer); + await extractMethod(editor, rangeOfTextToExtract, ch, ioc.serviceContainer); if (shouldError) { assert.fail('No error', 'Error', 'Extraction should fail with an error', ''); } diff --git a/src/test/refactor/extension.refactor.extract.var.test.ts b/src/test/refactor/extension.refactor.extract.var.test.ts index 4f082824cc5b..149b09310b02 100644 --- a/src/test/refactor/extension.refactor.extract.var.test.ts +++ b/src/test/refactor/extension.refactor.extract.var.test.ts @@ -16,16 +16,16 @@ import { workspace } from 'vscode'; import { getTextEditsFromPatch } from '../../client/common/editor'; +import { IPythonExecutionFactory, IPythonExecutionService } from '../../client/common/process/types'; import { ICondaService } from '../../client/interpreter/contracts'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { extractVariable } from '../../client/providers/simpleRefactorProvider'; import { RefactorProxy } from '../../client/refactor/proxy'; -import { getExtensionSettings, isPythonVersion } from '../common'; +import { isPythonVersion } from '../common'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; import { closeActiveWindows, initialize, initializeTest, IS_CI_SERVER } from './../initialize'; import { MockOutputChannel } from './../mockClasses'; -const EXTENSION_DIR = path.join(__dirname, '..', '..', '..'); const refactorSourceFile = path.join( __dirname, '..', @@ -84,7 +84,6 @@ suite('Variable Extraction', () => { } catch {} await closeActiveWindows(); }); - function initializeDI() { ioc = new UnitTestIocContainer(); ioc.registerCommonTypes(); @@ -94,20 +93,21 @@ suite('Variable Extraction', () => { ioc.serviceManager.addSingleton(ICondaService, CondaService); } + function createPythonExecGetter(workspaceRoot: string): () => Promise { + return async () => { + const factory = ioc.serviceContainer.get(IPythonExecutionFactory); + return factory.create({ resource: Uri.file(workspaceRoot) }); + }; + } async function testingVariableExtraction( shouldError: boolean, startPos: Position, endPos: Position ): Promise { - const pythonSettings = getExtensionSettings(Uri.file(refactorTargetFile)); const rangeOfTextToExtract = new Range(startPos, endPos); - const proxy = new RefactorProxy( - EXTENSION_DIR, - pythonSettings, - path.dirname(refactorTargetFile), - ioc.serviceContainer - ); + const workspaceRoot = path.dirname(refactorTargetFile); + const proxy = new RefactorProxy(workspaceRoot, createPythonExecGetter(workspaceRoot)); const DIFF = '--- a/refactor.py\n+++ b/refactor.py\n@@ -232,7 +232,8 @@\n sys.stdout.flush()\n \n def watch(self):\n- self._write_response("STARTED")\n+ myNewVariable = "STARTED"\n+ self._write_response(myNewVariable)\n while True:\n try:\n self._process_request(self._input.readline())\n'; @@ -172,7 +172,7 @@ suite('Variable Extraction', () => { editor.selections = [new Selection(rangeOfTextToExtract.start, rangeOfTextToExtract.end)]; editor.selection = new Selection(rangeOfTextToExtract.start, rangeOfTextToExtract.end); try { - await extractVariable(EXTENSION_DIR, editor, rangeOfTextToExtract, ch, ioc.serviceContainer); + await extractVariable(editor, rangeOfTextToExtract, ch, ioc.serviceContainer); if (shouldError) { assert.fail('No error', 'Error', 'Extraction should fail with an error', ''); } diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index fdd5a1604c68..23bb74d1e7f5 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -8,13 +8,26 @@ import { EOL } from 'os'; import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import * as typeMoq from 'typemoq'; -import { Range, TextEditorCursorStyle, TextEditorLineNumbersStyle, TextEditorOptions, window, workspace } from 'vscode'; +import { + Range, + TextEditorCursorStyle, + TextEditorLineNumbersStyle, + TextEditorOptions, + Uri, + window, + workspace +} from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../client/common/constants'; import '../../client/common/extensions'; import { BufferDecoder } from '../../client/common/process/decoder'; import { ProcessService } from '../../client/common/process/proc'; import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory'; -import { IProcessLogger, IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; +import { + IProcessLogger, + IProcessServiceFactory, + IPythonExecutionFactory, + IPythonExecutionService +} from '../../client/common/process/types'; import { IConfigurationService, IPythonSettings } from '../../client/common/types'; import { IEnvironmentActivationService } from '../../client/interpreter/activation/types'; import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; @@ -106,6 +119,12 @@ suite('Refactor Rename', () => { }); teardown(closeActiveWindows); suiteTeardown(closeActiveWindows); + function createPythonExecGetter(workspaceRoot: string): () => Promise { + return async () => { + const factory = serviceContainer.object.get(IPythonExecutionFactory); + return factory.create({ resource: Uri.file(workspaceRoot) }); + }; + } test('Rename function in source without a trailing empty line', async () => { const sourceFile = path.join( @@ -122,13 +141,9 @@ suite('Refactor Rename', () => { )}${EOL}@@ -1,8 +1,8 @@${EOL} import os${EOL} ${EOL}-def one():${EOL}+def three():${EOL} return True${EOL} ${EOL} def two():${EOL}- if one():${EOL}- print(\"A\" + one())${EOL}+ if three():${EOL}+ print(\"A\" + three())${EOL}`.splitLines( { removeEmptyEntries: false, trim: false } ); + const workspaceRoot = path.dirname(sourceFile); - const proxy = new RefactorProxy( - EXTENSION_ROOT_DIR, - pythonSettings.object, - path.dirname(sourceFile), - serviceContainer.object - ); + const proxy = new RefactorProxy(workspaceRoot, createPythonExecGetter(workspaceRoot)); const textDocument = await workspace.openTextDocument(sourceFile); await window.showTextDocument(textDocument); @@ -159,13 +174,9 @@ suite('Refactor Rename', () => { )}${EOL}@@ -1,8 +1,8 @@${EOL} import os${EOL} ${EOL}-def one():${EOL}+def three():${EOL} return True${EOL} ${EOL} def two():${EOL}- if one():${EOL}- print(\"A\" + one())${EOL}+ if three():${EOL}+ print(\"A\" + three())${EOL}`.splitLines( { removeEmptyEntries: false, trim: false } ); + const workspaceRoot = path.dirname(sourceFile); - const proxy = new RefactorProxy( - EXTENSION_ROOT_DIR, - pythonSettings.object, - path.dirname(sourceFile), - serviceContainer.object - ); + const proxy = new RefactorProxy(workspaceRoot, createPythonExecGetter(workspaceRoot)); const textDocument = await workspace.openTextDocument(sourceFile); await window.showTextDocument(textDocument); diff --git a/src/test/testing/common/services/discovery.unit.test.ts b/src/test/testing/common/services/discovery.unit.test.ts index 972cbfe6acb8..e7b54d2ec0cc 100644 --- a/src/test/testing/common/services/discovery.unit.test.ts +++ b/src/test/testing/common/services/discovery.unit.test.ts @@ -46,8 +46,7 @@ suite('Unit Tests - Common Discovery', () => { }; const discoveredTests: DiscoveredTests[] = [{ hello: 1 } as any]; const parsedResult = ({ done: true } as any) as Tests; - const json = JSON.stringify(discoveredTests); - discovery.exec = () => Promise.resolve({ stdout: json }); + discovery.exec = () => Promise.resolve(discoveredTests); when(parser.parse(options.workspaceFolder, deepEqual(discoveredTests))).thenResolve(parsedResult as any); const tests = await discovery.discoverTests(options); @@ -78,7 +77,7 @@ suite('Unit Tests - Common Discovery', () => { }; when(executionFactory.createActivatedEnvironment(deepEqual(creationOptions))).thenResolve(execService.object); - const executionResult = { stdout: discoveredTests }; + const executionResult = { stdout: JSON.stringify(discoveredTests) }; execService .setup((e) => e.exec(typemoq.It.isValue([pythonFile, ...options.args]), typemoq.It.isValue(spawnOptions))) .returns(() => Promise.resolve(executionResult)); @@ -86,7 +85,7 @@ suite('Unit Tests - Common Discovery', () => { const result = await discovery.exec(options); execService.verifyAll(); - assert.deepEqual(result, executionResult); + assert.deepEqual(result, discoveredTests); }); });