From 2eda86a6e4e8b0092742bec28b14206adb373ba8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 12 Oct 2017 13:30:22 -0700 Subject: [PATCH] #1288 installer config and tests (#1302) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified installer to pass resource for workspace resolution * null test in installer * canges to config settings to support multiroot workspace * changes to code refactoring to support workspace symbols * oops * modified to settings are resolved using document uri * unit tests for multi root support * fix unittests for multiroot * exclude files * add new line * config changes for multiroot workspace * new lines and enabled multi root linter tests * fix sys variables * added unit test to resolve ${workspaceRoot} in settings.json * fixed code review comments * fixed code review comments --- .gitignore | 2 + .vscode/launch.json | 17 ++ .vscode/settings.json | 5 +- package.json | 2 +- src/client/common/configSettings.ts | 14 +- src/client/common/editor.ts | 4 +- src/client/common/installer.ts | 172 ++++++++++------- src/client/common/logger.ts | 4 +- src/client/common/systemVariables.ts | 5 +- src/client/formatters/baseFormatter.ts | 6 +- src/client/linters/baseLinter.ts | 8 +- .../linters/errorHandlers/invalidArgs.ts | 6 +- src/client/linters/errorHandlers/main.ts | 4 +- .../linters/errorHandlers/notInstalled.ts | 5 +- src/client/linters/errorHandlers/standard.ts | 15 +- src/client/linters/prospector.ts | 6 +- src/client/linters/pydocstyle.ts | 2 +- src/client/providers/renameProvider.ts | 17 +- .../providers/simpleRefactorProvider.ts | 24 ++- src/client/refactor/proxy.ts | 2 +- .../unittests/common/baseTestManager.ts | 2 +- src/client/workspaceSymbols/main.ts | 33 +--- .../common/configSettings.multiroot.test.ts | 180 ++++++++++++++++++ src/test/common/configSettings.test.ts | 4 +- src/test/index.ts | 9 +- src/test/initialize.ts | 4 + src/test/linters/lint.multiroot.test.ts | 80 ++++++++ src/test/multiRootTest.ts | 8 + .../multiRootWkspc/disableLinters/file.py | 87 +++++++++ src/test/multiRootWkspc/multi.code-workspace | 28 +++ src/test/multiRootWkspc/parent/child/file.py | 87 +++++++++ .../workspace1/.vscode/settings.json | 5 + src/test/multiRootWkspc/workspace1/file.py | 87 +++++++++ .../workspace2/.vscode/settings.json | 3 + src/test/multiRootWkspc/workspace2/file.py | 87 +++++++++ .../workspace3/.vscode/settings.json | 3 + src/test/multiRootWkspc/workspace3/file.py | 87 +++++++++ .../extension.refactor.extract.method.test.ts | 2 +- .../extension.refactor.extract.var.test.ts | 2 +- 39 files changed, 972 insertions(+), 146 deletions(-) create mode 100644 src/test/common/configSettings.multiroot.test.ts create mode 100644 src/test/linters/lint.multiroot.test.ts create mode 100644 src/test/multiRootTest.ts create mode 100644 src/test/multiRootWkspc/disableLinters/file.py create mode 100644 src/test/multiRootWkspc/multi.code-workspace create mode 100644 src/test/multiRootWkspc/parent/child/file.py create mode 100644 src/test/multiRootWkspc/workspace1/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace1/file.py create mode 100644 src/test/multiRootWkspc/workspace2/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace2/file.py create mode 100644 src/test/multiRootWkspc/workspace3/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace3/file.py diff --git a/.gitignore b/.gitignore index 58ce4158ddbe..6c4d63cbb81a 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,9 @@ node_modules *.pyc .vscode/.ropeproject/** src/test/.vscode/** +**/.vscode/tags **/testFiles/**/.cache/** *.noseids .vscode-test __pycache__ +npm-debug.log diff --git a/.vscode/launch.json b/.vscode/launch.json index 284cb90bbaa5..6d4317be09b1 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -67,6 +67,23 @@ ], "preLaunchTask": "npm" }, + { + "name": "Launch Multiroot Tests", + "type": "extensionHost", + "request": "launch", + "runtimeExecutable": "${execPath}", + "args": [ + "${workspaceRoot}/src/test/multiRootWkspc/multi.code-workspace", + "--extensionDevelopmentPath=${workspaceRoot}", + "--extensionTestsPath=${workspaceRoot}/out/test" + ], + "stopOnEntry": false, + "sourceMaps": true, + "outFiles": [ + "${workspaceRoot}/out/**/*.js" + ], + "preLaunchTask": "npm" + }, { "name": "Python", "type": "python", diff --git a/.vscode/settings.json b/.vscode/settings.json index 2dc22387c78a..1b512d4d5c07 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,10 +1,11 @@ // Place your settings in this file to overwrite default and user settings. { "files.exclude": { - "out": false, // set this to true to hide the "out" folder with the compiled JS files + "out": true, // set this to true to hide the "out" folder with the compiled JS files "**/*.pyc": true, "**/__pycache__": true, - "node_modules": true + "node_modules": true, + ".vscode-test":true }, "search.exclude": { "out": true // set this to false to include "out" folder in search results diff --git a/package.json b/package.json index a91044ebfce1..b0901281ecb4 100644 --- a/package.json +++ b/package.json @@ -1593,7 +1593,7 @@ "vscode:prepublish": "tsc -p ./ && webpack", "compile": "webpack && tsc -watch -p ./", "postinstall": "node ./node_modules/vscode/bin/install", - "test": "node ./node_modules/vscode/bin/test" + "test": "node ./node_modules/vscode/bin/test && node ./out/test/multiRootTest.js" }, "dependencies": { "anser": "^1.1.0", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 32ef9ec060fb..97222940b071 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -146,6 +146,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { this.initializeSettings(); } + public static dispose() { + if (!IS_TEST_EXECUTION) { + throw new Error('Dispose can only be called from unit tests'); + } + PythonSettings.pythonSettings.clear(); + } public static getInstance(resource?: Uri): PythonSettings { const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined; let workspaceFolderUri: Uri = workspaceFolder ? workspaceFolder.uri : undefined; @@ -160,15 +166,15 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { return PythonSettings.pythonSettings.get(workspaceFolderKey); } private initializeSettings() { - const systemVariables: SystemVariables = new SystemVariables(); - const workspaceRoot = (IS_TEST_EXECUTION || !this.workspaceRoot) ? __dirname : this.workspaceRoot.fsPath; + const workspaceRoot = this.workspaceRoot.fsPath; + const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined); const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot); this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; - this.pythonPath = getAbsolutePath(this.pythonPath, IS_TEST_EXECUTION ? __dirname : workspaceRoot); + this.pythonPath = getAbsolutePath(this.pythonPath, workspaceRoot); this.venvPath = systemVariables.resolveAny(pythonSettings.get('venvPath'))!; this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; if (typeof this.jediPath === 'string' && this.jediPath.length > 0) { - this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), IS_TEST_EXECUTION ? __dirname : workspaceRoot); + this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot); } else { this.jediPath = ''; diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index 9d23d38ff678..7da7a2657e8b 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -75,7 +75,7 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[] return textEdits; } -export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit { +export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?:string): WorkspaceEdit { const workspaceEdit = new WorkspaceEdit(); filePatches.forEach(patch => { const indexOfAtAt = patch.indexOf('@@'); @@ -101,7 +101,7 @@ export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit } let fileName = fileNameLines[0].substring(fileNameLines[0].indexOf(' a') + 3).trim(); - fileName = path.isAbsolute(fileName) ? fileName : path.resolve(vscode.workspace.rootPath, fileName); + fileName = workspaceRoot && !path.isAbsolute(fileName) ? path.resolve(workspaceRoot, fileName) : fileName; if (!fs.existsSync(fileName)) { return; } diff --git a/src/client/common/installer.ts b/src/client/common/installer.ts index 1b3991047944..59e22331f743 100644 --- a/src/client/common/installer.ts +++ b/src/client/common/installer.ts @@ -1,9 +1,10 @@ +import { error } from './logger'; import * as vscode from 'vscode'; import * as settings from './configSettings'; import * as os from 'os'; +import { commands, ConfigurationTarget, Disposable, OutputChannel, Terminal, Uri, window, workspace } from 'vscode'; import { isNotInstalledError } from './helpers'; import { execPythonFile, getFullyQualifiedPythonInterpreterPath } from './utils'; -import { Documentation } from './constants'; export enum Product { pytest, @@ -111,6 +112,9 @@ SettingToDisableProduct.set(Product.pydocstyle, 'linting.pydocstyleEnabled'); SettingToDisableProduct.set(Product.pylint, 'linting.pylintEnabled'); SettingToDisableProduct.set(Product.pytest, 'unitTest.pyTestEnabled'); +const ProductInstallationPrompt = new Map(); +ProductInstallationPrompt.set(Product.ctags, 'Install CTags to enable Python workspace symbols'); + enum ProductType { Linter, Formatter, @@ -142,6 +146,11 @@ ProductTypes.set(Product.autopep8, ProductType.Formatter); ProductTypes.set(Product.yapf, ProductType.Formatter); ProductTypes.set(Product.rope, ProductType.RefactoringLibrary); +export enum InstallerResponse { + Installed, + Disabled, + Ignore +} export class Installer implements vscode.Disposable { private static terminal: vscode.Terminal | undefined | null; private disposables: vscode.Disposable[] = []; @@ -155,12 +164,14 @@ export class Installer implements vscode.Disposable { public dispose() { this.disposables.forEach(d => d.dispose()); } - public shouldDisplayPrompt(product: Product) { + private shouldDisplayPrompt(product: Product) { const productName = ProductNames.get(product)!; - return settings.PythonSettings.getInstance().disablePromptForFeatures.indexOf(productName) === -1; + const pythonConfig = workspace.getConfiguration('python'); + const disablePromptForFeatures = pythonConfig.get('disablePromptForFeatures', [] as string[]); + return disablePromptForFeatures.indexOf(productName) === -1; } - async promptToInstall(product: Product) { + public async promptToInstall(product: Product, resource?: Uri): Promise { const productType = ProductTypes.get(product)!; const productTypeName = ProductTypeNames.get(productType); const productName = ProductNames.get(product)!; @@ -173,10 +184,10 @@ export class Installer implements vscode.Disposable { else { console.warn(message); } - return; + return InstallerResponse.Ignore; } - const installOption = 'Install ' + productName; + const installOption = ProductInstallationPrompt.has(product) ? ProductInstallationPrompt.get(product) : 'Install ' + productName; const disableOption = 'Disable ' + productTypeName; const dontShowAgain = `Don't show this prompt again`; const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8'; @@ -189,46 +200,53 @@ export class Installer implements vscode.Disposable { if (SettingToDisableProduct.has(product)) { options.push(...[disableOption, dontShowAgain]); } - return vscode.window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options).then(item => { - switch (item) { - case installOption: { - return this.install(product); - } - case disableOption: { - if (Linters.indexOf(product) >= 0) { - return disableLinter(product); - } - else { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const settingToDisable = SettingToDisableProduct.get(product)!; - return pythonConfig.update(settingToDisable, false); - } - } - case useOtherFormatter: { - const pythonConfig = vscode.workspace.getConfiguration('python'); - return pythonConfig.update('formatting.provider', alternateFormatter); - } - case dontShowAgain: { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const features = pythonConfig.get('disablePromptForFeatures', [] as string[]); - features.push(productName); - return pythonConfig.update('disablePromptForFeatures', features, true); + const item = await window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options); + switch (item) { + case installOption: { + return this.install(product, resource); + } + case disableOption: { + if (Linters.indexOf(product) >= 0) { + return this.disableLinter(product, resource).then(() => InstallerResponse.Disabled); } - case 'Help': { - return Promise.resolve(); + else { + const settingToDisable = SettingToDisableProduct.get(product)!; + return this.updateSetting(settingToDisable, false, resource).then(() => InstallerResponse.Disabled); } } - }); + case useOtherFormatter: { + return this.updateSetting('formatting.provider', alternateFormatter, resource) + .then(() => InstallerResponse.Installed); + } + case dontShowAgain: { + const pythonConfig = workspace.getConfiguration('python'); + const features = pythonConfig.get('disablePromptForFeatures', [] as string[]); + features.push(productName); + return pythonConfig.update('disablePromptForFeatures', features, true).then(() => InstallerResponse.Ignore); + } + default: { + throw new Error('Invalid selection'); + } + } } - - install(product: Product): Promise { + public async install(product: Product, resource?: Uri): Promise { if (!this.outputChannel && !Installer.terminal) { - Installer.terminal = vscode.window.createTerminal('Python Installer'); + Installer.terminal = window.createTerminal('Python Installer'); } - if (product === Product.ctags && os.platform() === 'win32') { - vscode.commands.executeCommand('python.displayHelp', Documentation.Workspace.InstallOnWindows); - return Promise.resolve(); + if (product === Product.ctags && settings.IS_WINDOWS) { + if (this.outputChannel) { + this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols'); + this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.'); + this.outputChannel.appendLine('Option 1: Extract ctags.exe from the downloaded zip to any folder within your PATH so that Visual Studio Code can run it.'); + this.outputChannel.appendLine('Option 2: Extract to any folder and add the path to this folder to the command setting.'); + this.outputChannel.appendLine('Option 3: Extract to any folder and define that path in the python.workspaceSymbols.ctagsPath setting of your user settings file (settings.json).'); + this.outputChannel.show(); + } + else { + window.showInformationMessage('Install Universal Ctags and set it in your path or define the path in your python.workspaceSymbols.ctagsPath settings'); + } + return InstallerResponse.Ignore; } let installArgs = ProductInstallScripts.get(product)!; @@ -241,19 +259,19 @@ export class Installer implements vscode.Disposable { installArgs.splice(2, 0, '--proxy'); } } + let installationPromise: Promise; if (this.outputChannel && installArgs[0] === '-m') { // Errors are just displayed to the user this.outputChannel.show(); - return execPythonFile(settings.PythonSettings.getInstance().pythonPath, installArgs, vscode.workspace.rootPath!, true, (data) => { - this.outputChannel!.append(data); - }); + installationPromise = execPythonFile(settings.PythonSettings.getInstance(resource).pythonPath, + installArgs, getCwdForInstallScript(resource), true, (data) => { this.outputChannel!.append(data); }); } else { // When using terminal get the fully qualitified path // Cuz people may launch vs code from terminal when they have activated the appropriate virtual env // Problem is terminal doesn't use the currently activated virtual env // Must have something to do with the process being launched in the terminal - return getFullyQualifiedPythonInterpreterPath() + installationPromise = getFullyQualifiedPythonInterpreterPath() .then(pythonPath => { let installScript = installArgs.join(' '); @@ -269,42 +287,68 @@ export class Installer implements vscode.Disposable { Installer.terminal!.show(false); }); } + + return installationPromise + .then(() => this.isInstalled(product)) + .then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore); } - isInstalled(product: Product): Promise { - return isProductInstalled(product); + public isInstalled(product: Product, resource?: Uri): Promise { + return isProductInstalled(product, resource); } - uninstall(product: Product): Promise { - return uninstallproduct(product); + public uninstall(product: Product, resource?: Uri): Promise { + return uninstallproduct(product, resource); + } + public disableLinter(product: Product, resource: Uri) { + if (resource && !workspace.getWorkspaceFolder(resource)) { + const settingToDisable = SettingToDisableProduct.get(product)!; + const pythonConfig = workspace.getConfiguration('python', resource); + return pythonConfig.update(settingToDisable, false, ConfigurationTarget.Workspace); + } + else { + const pythonConfig = workspace.getConfiguration('python'); + return pythonConfig.update('linting.enabledWithoutWorkspace', false, true); + } + } + private updateSetting(setting: string, value: any, resource?: Uri) { + if (resource && !workspace.getWorkspaceFolder(resource)) { + const pythonConfig = workspace.getConfiguration('python', resource); + return pythonConfig.update(setting, value, ConfigurationTarget.Workspace); + } + else { + const pythonConfig = workspace.getConfiguration('python'); + return pythonConfig.update(setting, value, true); + } } } -export function disableLinter(product: Product, global?: boolean) { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const settingToDisable = SettingToDisableProduct.get(product)!; - if (vscode.workspace.rootPath) { - return pythonConfig.update(settingToDisable, false, global); +function getCwdForInstallScript(resource?: Uri) { + const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined; + if (workspaceFolder) { + return workspaceFolder.uri.fsPath; } - else { - return pythonConfig.update('linting.enabledWithoutWorkspace', false, true); + if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) { + return workspace.workspaceFolders[0].uri.fsPath; } + return __dirname; } -async function isProductInstalled(product: Product): Promise { +async function isProductInstalled(product: Product, resource?: Uri): Promise { if (!ProductExecutableAndArgs.has(product)) { return; } const prodExec = ProductExecutableAndArgs.get(product)!; - return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), vscode.workspace.rootPath!, false) - .then(() => { - return true; - }).catch(reason => { - return !isNotInstalledError(reason); - }); + const cwd = getCwdForInstallScript(resource); + return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), cwd, false) + .then(() => true) + .catch(reason => !isNotInstalledError(reason)); } -function uninstallproduct(product: Product): Promise { +function uninstallproduct(product: Product, resource?: Uri): Promise { + if (!ProductUninstallScripts.has(product)) { + return Promise.resolve(); + } const uninstallArgs = ProductUninstallScripts.get(product)!; - return execPythonFile('python', uninstallArgs, vscode.workspace.rootPath!, false); -} + return execPythonFile('python', uninstallArgs, getCwdForInstallScript(resource), false); +} \ No newline at end of file diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index ac372199e258..173da70d78ab 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -22,7 +22,9 @@ class Logger { Logger.writeLine(category, message); } static writeLine(category: string = "log", line: any) { - console[category](line); + if (process.env['PYTHON_DONJAYAMANNE_TEST'] !== '1') { + console[category](line); + } if (outChannel) { outChannel.appendLine(line); } diff --git a/src/client/common/systemVariables.ts b/src/client/common/systemVariables.ts index f0df0218c2d9..444d782d6eea 100644 --- a/src/client/common/systemVariables.ts +++ b/src/client/common/systemVariables.ts @@ -133,11 +133,10 @@ export abstract class AbstractSystemVariables implements ISystemVariables { export class SystemVariables extends AbstractSystemVariables { private _workspaceRoot: string; private _workspaceRootFolderName: string; - private _execPath: string; - constructor() { + constructor(workspaceRoot?: string) { super(); - this._workspaceRoot = typeof vscode.workspace.rootPath === 'string' ? vscode.workspace.rootPath : __dirname; + this._workspaceRoot = typeof workspaceRoot === 'string' ? workspaceRoot : __dirname; this._workspaceRootFolderName = Path.basename(this._workspaceRoot); Object.keys(process.env).forEach(key => { this[`env:${key}`] = this[`env.${key}`] = process.env[key]; diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 3fcce0d76661..0beaa02e9291 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -61,14 +61,14 @@ export abstract class BaseFormatter { } return getTextEditsFromPatch(document.getText(), data[1]); }).catch(error => { - this.handleError(this.Id, command, error); + this.handleError(this.Id, command, error, document.uri); return []; }); vscode.window.setStatusBarMessage(`Formatting with ${this.Id}`, promise); return promise; } - protected handleError(expectedFileName: string, fileName: string, error: Error) { + protected handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { let customError = `Formatting with ${this.Id} failed.`; if (isNotInstalledError(error)) { @@ -84,7 +84,7 @@ export abstract class BaseFormatter { } else { customError += `\nYou could either install the '${this.Id}' formatter, turn it off or use another formatter.`; - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, resource); } } diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 9e1dcd8a54f5..d4d6f89d58de 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -1,7 +1,7 @@ 'use strict'; import { IPythonSettings, PythonSettings } from '../common/configSettings'; import { execPythonFile } from './../common/utils'; -import { OutputChannel } from 'vscode'; +import { OutputChannel, Uri } from 'vscode'; import { Installer, Product } from '../common/installer'; import * as vscode from 'vscode'; import { ErrorHandler } from './errorHandlers/main'; @@ -139,12 +139,12 @@ export abstract class BaseLinter { let outputLines = data.split(/\r?\n/g); return this.parseLines(outputLines, regEx); }).catch(error => { - this.handleError(this.Id, command, error); + this.handleError(this.Id, command, error, document.uri); return []; }); } - protected handleError(expectedFileName: string, fileName: string, error: Error) { - this._errorHandler.handleError(expectedFileName, fileName, error); + protected handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { + this._errorHandler.handleError(expectedFileName, fileName, error, resource); } } diff --git a/src/client/linters/errorHandlers/invalidArgs.ts b/src/client/linters/errorHandlers/invalidArgs.ts index 514b2b82e523..b2d4b155cadb 100644 --- a/src/client/linters/errorHandlers/invalidArgs.ts +++ b/src/client/linters/errorHandlers/invalidArgs.ts @@ -1,6 +1,6 @@ 'use strict'; import { isNotInstalledError } from '../../common/helpers'; -import * as vscode from 'vscode'; +import { Uri, window } from 'vscode'; import { StandardErrorHandler } from './standard'; export class InvalidArgumentsErrorHandler extends StandardErrorHandler { @@ -13,14 +13,14 @@ export class InvalidArgumentsErrorHandler extends StandardErrorHandler { private displayInvalidArgsError() { // Ok if we have a space after the file name, this means we have some arguments defined and this isn't supported - vscode.window.showErrorMessage(`Unsupported configuration for '${this.id}'`, 'View Errors').then(item => { + window.showErrorMessage(`Unsupported configuration for '${this.id}'`, 'View Errors').then(item => { if (item === 'View Errors') { this.outputChannel.show(); } }); } - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (!isNotInstalledError(error)) { return false; } diff --git a/src/client/linters/errorHandlers/main.ts b/src/client/linters/errorHandlers/main.ts index 9503e01e4a69..cc9120627929 100644 --- a/src/client/linters/errorHandlers/main.ts +++ b/src/client/linters/errorHandlers/main.ts @@ -1,5 +1,5 @@ 'use strict'; -import { OutputChannel } from 'vscode'; +import { OutputChannel, Uri } from 'vscode'; import { Installer, Product } from '../../common/installer'; import { InvalidArgumentsErrorHandler } from './invalidArgs'; import { StandardErrorHandler } from './standard'; @@ -15,7 +15,7 @@ export class ErrorHandler { ]; } - public handleError(expectedFileName: string, fileName: string, error: Error) { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { this._errorHandlers.some(handler => handler.handleError(expectedFileName, fileName, error)); } } \ No newline at end of file diff --git a/src/client/linters/errorHandlers/notInstalled.ts b/src/client/linters/errorHandlers/notInstalled.ts index bc7c676a9390..f875997b184f 100644 --- a/src/client/linters/errorHandlers/notInstalled.ts +++ b/src/client/linters/errorHandlers/notInstalled.ts @@ -1,14 +1,15 @@ 'use strict'; +import { Uri } from 'vscode'; import { isNotInstalledError } from '../../common/helpers'; import { StandardErrorHandler } from './standard'; export class NotInstalledErrorHandler extends StandardErrorHandler { - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (!isNotInstalledError(error)) { return false; } - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, resource); const customError = `Linting with ${this.id} failed.\nYou could either install the '${this.id}' linter or turn it off in setings.json via "python.linting.${this.id}Enabled = false".`; this.outputChannel.appendLine(`\n${customError}\n${error + ''}`); return true; diff --git a/src/client/linters/errorHandlers/standard.ts b/src/client/linters/errorHandlers/standard.ts index 114d99e5c79b..e9f665a0d09c 100644 --- a/src/client/linters/errorHandlers/standard.ts +++ b/src/client/linters/errorHandlers/standard.ts @@ -1,18 +1,17 @@ 'use strict'; -import { OutputChannel } from 'vscode'; -import { Installer, Product, disableLinter } from '../../common/installer'; -import * as vscode from 'vscode'; +import { OutputChannel, Uri, window } from 'vscode'; +import { Installer, Product } from '../../common/installer'; export class StandardErrorHandler { constructor(protected id: string, protected product: Product, protected installer: Installer, protected outputChannel: OutputChannel) { } - private displayLinterError() { + private displayLinterError(resource?: Uri) { const message = `There was an error in running the linter '${this.id}'`; - vscode.window.showErrorMessage(message, 'Disable linter', 'View Errors').then(item => { + window.showErrorMessage(message, 'Disable linter', 'View Errors').then(item => { switch (item) { case 'Disable linter': { - disableLinter(this.product); + this.installer.disableLinter(this.product, resource); break; } case 'View Errors': { @@ -23,7 +22,7 @@ export class StandardErrorHandler { }); } - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (typeof error === 'string' && (error as string).indexOf("OSError: [Errno 2] No such file or directory: '/") > 0) { return false; } @@ -31,7 +30,7 @@ export class StandardErrorHandler { console.error(error); this.outputChannel.appendLine(`Linting with ${this.id} failed.\n${error + ''}`); - this.displayLinterError(); + this.displayLinterError(resource); return true; } } diff --git a/src/client/linters/prospector.ts b/src/client/linters/prospector.ts index e02a10f210be..bf1c6b1af4e9 100644 --- a/src/client/linters/prospector.ts +++ b/src/client/linters/prospector.ts @@ -36,8 +36,8 @@ export class Linter extends baseLinter.BaseLinter { let prospectorPath = this.pythonSettings.linting.prospectorPath; let outputChannel = this.outputChannel; let prospectorArgs = Array.isArray(this.pythonSettings.linting.prospectorArgs) ? this.pythonSettings.linting.prospectorArgs : []; - - if (prospectorArgs.length === 0 && ProductExecutableAndArgs.has(Product.prospector) && prospectorPath.toLocaleLowerCase() === 'prospector'){ + + if (prospectorArgs.length === 0 && ProductExecutableAndArgs.has(Product.prospector) && prospectorPath.toLocaleLowerCase() === 'prospector') { prospectorPath = ProductExecutableAndArgs.get(Product.prospector).executable; prospectorArgs = ProductExecutableAndArgs.get(Product.prospector).args; } @@ -70,7 +70,7 @@ export class Linter extends baseLinter.BaseLinter { resolve(diagnostics); }).catch(error => { - this.handleError(this.Id, prospectorPath, error); + this.handleError(this.Id, prospectorPath, error, document.uri); resolve([]); }); }); diff --git a/src/client/linters/pydocstyle.ts b/src/client/linters/pydocstyle.ts index e58fa4edd5a5..7759ee23a26f 100644 --- a/src/client/linters/pydocstyle.ts +++ b/src/client/linters/pydocstyle.ts @@ -99,7 +99,7 @@ export class Linter extends baseLinter.BaseLinter { }); resolve(diagnostics); }, error => { - this.handleError(this.Id, commandLine, error); + this.handleError(this.Id, commandLine, error, document.uri); resolve([]); }); }); diff --git a/src/client/providers/renameProvider.ts b/src/client/providers/renameProvider.ts index 9afcb4db6c47..e65b915b856b 100644 --- a/src/client/providers/renameProvider.ts +++ b/src/client/providers/renameProvider.ts @@ -7,7 +7,6 @@ import * as path from 'path'; import { PythonSettings } from '../common/configSettings'; import { Installer, Product } from '../common/installer'; -const pythonSettings = PythonSettings.getInstance(); const EXTENSION_DIR = path.join(__dirname, '..', '..', '..'); interface RenameResponse { results: [{ diff: string }]; @@ -41,14 +40,20 @@ export class PythonRenameProvider implements vscode.RenameProvider { return; } - let proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, vscode.workspace.rootPath); + let workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); + + let proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, workspaceRoot); return proxy.rename(document, newName, document.uri.fsPath, range).then(response => { - //return response.results[0].diff; - const workspaceEdit = getWorkspaceEditsFromPatch(response.results.map(fileChanges => fileChanges.diff)); - return workspaceEdit; + const fileDiffs = response.results.map(fileChanges => fileChanges.diff); + return getWorkspaceEditsFromPatch(fileDiffs, workspaceRoot); }).catch(reason => { if (reason === 'Not installed') { - this.installer.promptToInstall(Product.rope); + this.installer.promptToInstall(Product.rope, document.uri); return Promise.reject(''); } else { diff --git a/src/client/providers/simpleRefactorProvider.ts b/src/client/providers/simpleRefactorProvider.ts index 99e62ac55b5a..35f765165eb1 100644 --- a/src/client/providers/simpleRefactorProvider.ts +++ b/src/client/providers/simpleRefactorProvider.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import { RefactorProxy } from '../refactor/proxy'; import { getTextEditsFromPatch } from '../common/editor'; -import { PythonSettings, IPythonSettings } from '../common/configSettings'; +import { PythonSettings } from '../common/configSettings'; import { Installer, Product } from '../common/installer'; interface RenameResponse { @@ -34,8 +34,14 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo // Exported for unit testing export function extractVariable(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, - outputChannel: vscode.OutputChannel, workspaceRoot: string = vscode.workspace.rootPath, - pythonSettings: IPythonSettings = PythonSettings.getInstance()): Promise { + outputChannel: vscode.OutputChannel): Promise { + + let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { let newName = 'newvariable' + new Date().getMilliseconds().toString(); @@ -50,8 +56,14 @@ export function extractVariable(extensionDir: string, textEditor: vscode.TextEdi // Exported for unit testing export function extractMethod(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, - outputChannel: vscode.OutputChannel, workspaceRoot: string = vscode.workspace.rootPath, - pythonSettings: IPythonSettings = PythonSettings.getInstance()): Promise { + outputChannel: vscode.OutputChannel): Promise { + + let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { let newName = 'newmethod' + new Date().getMilliseconds().toString(); @@ -127,7 +139,7 @@ function extractName(extensionDir: string, textEditor: vscode.TextEditor, range: } }).catch(error => { if (error === 'Not installed') { - installer.promptToInstall(Product.rope); + installer.promptToInstall(Product.rope, textEditor.document.uri); return Promise.reject(''); } let errorMessage = error + ''; diff --git a/src/client/refactor/proxy.ts b/src/client/refactor/proxy.ts index 41a717036754..aada24c2323d 100644 --- a/src/client/refactor/proxy.ts +++ b/src/client/refactor/proxy.ts @@ -17,7 +17,7 @@ export class RefactorProxy extends vscode.Disposable { private _commandResolve: (value?: any | PromiseLike) => void; private _commandReject: (reason?: any) => void; private _initializeReject: (reason?: any) => void; - constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string = vscode.workspace.rootPath) { + constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string) { super(() => { }); this._extensionDir = extensionDir; } diff --git a/src/client/unittests/common/baseTestManager.ts b/src/client/unittests/common/baseTestManager.ts index e6c31d31a0be..1dc38f7259b2 100644 --- a/src/client/unittests/common/baseTestManager.ts +++ b/src/client/unittests/common/baseTestManager.ts @@ -90,7 +90,7 @@ export abstract class BaseTestManager { return tests; }).catch(reason => { if (isNotInstalledError(reason) && !quietMode) { - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, vscode.Uri.file(this.rootDirectory)); } this.tests = null; diff --git a/src/client/workspaceSymbols/main.ts b/src/client/workspaceSymbols/main.ts index 835c7cec545c..5c0206290d0e 100644 --- a/src/client/workspaceSymbols/main.ts +++ b/src/client/workspaceSymbols/main.ts @@ -1,6 +1,6 @@ import * as vscode from 'vscode'; import { Generator } from './generator'; -import { Product, Installer } from '../common/installer'; +import { Installer, InstallerResponse, Product } from '../common/installer'; import { PythonSettings } from '../common/configSettings'; import { fsExistsAsync } from '../common/utils'; import { isNotInstalledError } from '../common/helpers'; @@ -52,11 +52,6 @@ export class WorkspaceSymbols implements vscode.Disposable { dispose() { this.disposables.forEach(d => d.dispose()); } - disableDocumentLanguageProvider(): Thenable { - const pythonConfig = vscode.workspace.getConfiguration('python'); - return pythonConfig.update('python.workspaceSymbols.enabled', false); - - } buildWorkspaceSymbols(rebuild: boolean = true, token?: vscode.CancellationToken): Promise { if (!pythonSettings.workspaceSymbols.enabled || (token && token.isCancellationRequested)) { return Promise.resolve([]); @@ -81,23 +76,15 @@ export class WorkspaceSymbols implements vscode.Disposable { if (!token || token.isCancellationRequested) { return; } - return new Promise((resolve, reject) => { - vscode.window.showErrorMessage('CTags needs to be installed to get support for Python workspace symbols', - 'Install', `Don't ask again`).then(item => { - switch (item) { - case 'Install': { - this.installer.install(Product.ctags).then(() => { - return this.buildWorkspaceSymbols(rebuild, token); - }).catch(reason => reject(reason)); - break; - } - case `Don't ask again`: { - this.disableDocumentLanguageProvider().then(() => resolve(), reason => reject(reason)); - break; - } - } - }); - }); + return this.installer.promptToInstall(Product.ctags) + .then(result => { + if (!token || token.isCancellationRequested) { + return; + } + if (result === InstallerResponse.Installed) { + return this.buildWorkspaceSymbols(rebuild, token); + } + }); }); }); } diff --git a/src/test/common/configSettings.multiroot.test.ts b/src/test/common/configSettings.multiroot.test.ts new file mode 100644 index 000000000000..ca7d9cc68762 --- /dev/null +++ b/src/test/common/configSettings.multiroot.test.ts @@ -0,0 +1,180 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import { ConfigurationTarget, Uri, workspace } from 'vscode'; +import { initialize, closeActiveWindows } from '../initialize'; +import { PythonSettings } from '../../client/common/configSettings'; + +const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); + +suite('Multiroot Config Settings', () => { + suiteSetup(async () => { + await initialize(); + await resetSettings(); + }); + suiteTeardown(() => closeActiveWindows()); + teardown(async () => { + await resetSettings(); + // await closeActiveWindows(); + }); + + async function resetSettings() { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + let value = settings.inspect('pythonPath'); + if (value.workspaceValue && value.workspaceValue !== 'python') { + await settings.update('pythonPath', undefined, ConfigurationTarget.Workspace); + } + if (value.workspaceFolderValue && value.workspaceFolderValue !== 'python') { + await settings.update('pythonPath', undefined, ConfigurationTarget.WorkspaceFolder); + } + PythonSettings.dispose(); + } + async function enableDisableSetting(resource: Uri, configTarget: ConfigurationTarget, setting: string, value: boolean) { + let settings = workspace.getConfiguration('python.linting', resource); + await settings.update(setting, value, configTarget); + settings = workspace.getConfiguration('python.linting', resource); + return settings.get(setting); + } + + async function testLinterSetting(resource: Uri, configTarget: ConfigurationTarget, setting: string, value: boolean) { + const valueInSetting = await enableDisableSetting(resource, configTarget, setting, value); + PythonSettings.dispose(); + const cfgSetting = PythonSettings.getInstance(resource); + assert.equal(valueInSetting, cfgSetting.linting[setting], `Both settings ${setting} should be ${value} for ${resource.fsPath}`); + } + + test('Workspace folder should inherit Python Path from workspace root', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + + settings = workspace.getConfiguration('python', workspaceUri); + assert.equal(settings.get('pythonPath'), pythonPath, 'Python path not set in workspace root'); + + const cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.pythonPath, pythonPath, 'Python Path not inherited from workspace'); + }); + + test('Workspace folder should not inherit Python Path from workspace root', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + await settings.update('pythonPath', 'privatePythonPath', ConfigurationTarget.WorkspaceFolder); + + const cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.pythonPath, 'privatePythonPath', 'Python Path for workspace folder is incorrect'); + }); + + + test('Workspace folder should inherit Python Path from workspace root when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + const settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + + const document = await workspace.openTextDocument(fileToOpen); + const cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.pythonPath, pythonPath, 'Python Path not inherited from workspace'); + }); + + test('Workspace folder should not inherit Python Path from workspace root when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + const settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + await settings.update('pythonPath', 'privatePythonPath', ConfigurationTarget.WorkspaceFolder); + + const document = await workspace.openTextDocument(fileToOpen); + const cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.pythonPath, 'privatePythonPath', 'Python Path for workspace folder is incorrect'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + }); + + test('Enabling/Disabling Pylint in root and workspace should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + + let cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.linting.pylintEnabled, false, 'Workspace folder pylint setting is true when it should not be'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + + cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.linting.pylintEnabled, true, 'Workspace folder pylint setting is false when it should not be'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, true, 'Pylint should be enabled in workspace'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, false, 'Pylint should not be enabled in workspace'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, true, 'Pylint should be enabled in workspace'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, false, 'Pylint should not be enabled in workspace'); + }); + + test('${workspaceRoot} variable in settings should be replaced with the right value', async () => { + const workspace2Uri = Uri.file(path.join(multirootPath, 'workspace2')); + let fileToOpen = path.join(workspace2Uri.fsPath, 'file.py'); + + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(path.dirname(cfg.workspaceSymbols.ctagsPath), workspace2Uri.fsPath, 'ctags file for workspace2 is incorrect'); + PythonSettings.dispose(); + + const workspace3Uri = Uri.file(path.join(multirootPath, 'workspace2')); + fileToOpen = path.join(workspace3Uri.fsPath, 'file.py'); + + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(path.dirname(cfg.workspaceSymbols.ctagsPath), workspace3Uri.fsPath, 'ctags file for workspace3 is incorrect'); + PythonSettings.dispose(); + }); +}); diff --git a/src/test/common/configSettings.test.ts b/src/test/common/configSettings.test.ts index 8382b8f4e1c9..07cfe597033a 100644 --- a/src/test/common/configSettings.test.ts +++ b/src/test/common/configSettings.test.ts @@ -9,11 +9,13 @@ import * as assert from 'assert'; // You can import and use all API from the 'vscode' module // as well as import your extension to test it import * as vscode from 'vscode'; +import * as path from 'path'; import { initialize, IS_TRAVIS } from './../initialize'; import { PythonSettings } from '../../client/common/configSettings'; import { SystemVariables } from '../../client/common/systemVariables'; const pythonSettings = PythonSettings.getInstance(); +const workspaceRoot = path.join(__dirname, '..', '..', '..', 'src', 'test'); // Defines a Mocha test suite to group tests of similar kind together suite('Configuration Settings', () => { @@ -21,7 +23,7 @@ suite('Configuration Settings', () => { if (!IS_TRAVIS) { test('Check Values', done => { - const systemVariables: SystemVariables = new SystemVariables(); + const systemVariables: SystemVariables = new SystemVariables(workspaceRoot); const pythonConfig = vscode.workspace.getConfiguration('python'); Object.keys(pythonSettings).forEach(key => { let settingValue = pythonConfig.get(key, 'Not a config'); diff --git a/src/test/index.ts b/src/test/index.ts index af916a981db9..e5fe465f9647 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -1,4 +1,4 @@ -import { initializePython } from './initialize'; +import { initializePython, isMultitrootTest } from './initialize'; // // PLEASE DO NOT MODIFY / DELETE UNLESS YOU KNOW WHAT YOU ARE DOING // @@ -11,14 +11,17 @@ import { initializePython } from './initialize'; // to report the results back to the caller. When the tests are finished, return // a possible error to the callback or null if none. -let testRunner = require('vscode/lib/testrunner'); +const testRunner = require('vscode/lib/testrunner'); +const invert = isMultitrootTest() ? undefined : 'invert'; // You can directly control Mocha options by uncommenting the following lines // See https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically#set-options for more info testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000 + timeout: 25000, + grep : 'Multiroot', + invert }); initializePython(); diff --git a/src/test/initialize.ts b/src/test/initialize.ts index 1dcd6e247fd4..97b22674894f 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -89,3 +89,7 @@ export function initializePython() { const pythonConfig = vscode.workspace.getConfiguration('python'); pythonConfig.update('pythonPath', PYTHON_PATH); } + +export function isMultitrootTest() { + return Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 1; +} diff --git a/src/test/linters/lint.multiroot.test.ts b/src/test/linters/lint.multiroot.test.ts new file mode 100644 index 000000000000..e038ac9a1339 --- /dev/null +++ b/src/test/linters/lint.multiroot.test.ts @@ -0,0 +1,80 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import * as baseLinter from '../../client/linters/baseLinter'; +import * as pyLint from '../../client/linters/pylint'; +import * as flake8 from '../../client/linters/flake8'; +import { PythonSettings } from '../../client/common/configSettings'; +import { CancellationTokenSource, ConfigurationTarget, Uri, window, workspace } from 'vscode'; +import { initialize, closeActiveWindows } from '../initialize'; +import { MockOutputChannel } from '../mockClasses'; + +const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); + +suite('Multiroot Linting', () => { + suiteSetup(async () => { + await initialize(); + PythonSettings.dispose(); + }); + suiteTeardown(() => closeActiveWindows()); + teardown(async () => { + await closeActiveWindows(); + PythonSettings.dispose(); + }); + + async function testLinterInWorkspaceFolder(linter: baseLinter.BaseLinter, workspaceFolderRelativePath: string, mustHaveErrors: boolean) { + const fileToLint = path.join(multirootPath, workspaceFolderRelativePath, 'file.py'); + const cancelToken = new CancellationTokenSource(); + const document = await workspace.openTextDocument(fileToLint); + const editor = await window.showTextDocument(document); + const messages = await linter.lint(editor.document, cancelToken.token); + const errorMessage = mustHaveErrors ? 'No errors returned by linter' : 'Errors returned by linter'; + assert.equal(messages.length > 0, mustHaveErrors, errorMessage); + } + async function enableDisableSetting(workspaceFolder, configTarget: ConfigurationTarget, setting: string, value: boolean) { + const folderUri = Uri.file(workspaceFolder); + const settings = workspace.getConfiguration('python.linting', folderUri); + await settings.update(setting, value, configTarget); + } + + test('Enabling Pylint in root and also in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); + }); + + test('Enabling Pylint in root and disabling in Workspace, should not return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', false); + }); + + test('Disabling Pylint in root and enabling in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); + }); + + test('Enabling Flake8 in root and also in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); + }); + + test('Enabling Flake8 in root and disabling in Workspace, should not return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', false); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', false); + }); + + test('Disabling Flake8 in root and enabling in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', false); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); + }); +}); diff --git a/src/test/multiRootTest.ts b/src/test/multiRootTest.ts new file mode 100644 index 000000000000..b7ad307ef20c --- /dev/null +++ b/src/test/multiRootTest.ts @@ -0,0 +1,8 @@ +import * as path from 'path'; + +process.env.CODE_TESTS_WORKSPACE = path.join(__dirname, '..', '..', 'src', 'test', 'multiRootWkspc', 'multi.code-workspace'); + +function start() { + require('../../node_modules/vscode/bin/test'); +} +start(); diff --git a/src/test/multiRootWkspc/disableLinters/file.py b/src/test/multiRootWkspc/disableLinters/file.py new file mode 100644 index 000000000000..27509dd2fcd6 --- /dev/null +++ b/src/test/multiRootWkspc/disableLinters/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print(self) + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print (self\ + + "foo") + + def meth3(self): + """test one line disabling""" + # no error + print (self.bla) # pylint: disable=no-member + # error + print (self.blop) + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) + # pylint: enable=no-member + # error + print (self.blip) + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + if self.blop: + # pylint: enable=no-member + # error + print (self.blip) + else: + # no error + print (self.blip) + # no error + print (self.blip) + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + try: + # pylint: enable=no-member + # error + print (self.blip) + except UndefinedName: # pylint: disable=undefined-variable + # no error + print (self.blip) + # no error + print (self.blip) + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print (self.blip) + else: + # error + print (self.blip) + # error + print (self.blip) + + + def meth8(self): + """test late disabling""" + # error + print (self.blip) + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/multi.code-workspace b/src/test/multiRootWkspc/multi.code-workspace new file mode 100644 index 000000000000..67ea6996216d --- /dev/null +++ b/src/test/multiRootWkspc/multi.code-workspace @@ -0,0 +1,28 @@ +{ + "folders": [ + { + "path": "workspace1" + }, + { + "path": "workspace2" + }, + { + "path": "workspace3" + }, + { + "path": "parent\\child" + }, + { + "path": "disableLinters" + } + ], + "settings": { + "python.linting.flake8Enabled": false, + "python.linting.mypyEnabled": true, + "python.linting.pydocstyleEnabled": true, + "python.linting.pylamaEnabled": true, + "python.linting.pylintEnabled": false, + "python.linting.pep8Enabled": true, + "python.linting.prospectorEnabled": true + } +} diff --git a/src/test/multiRootWkspc/parent/child/file.py b/src/test/multiRootWkspc/parent/child/file.py new file mode 100644 index 000000000000..27509dd2fcd6 --- /dev/null +++ b/src/test/multiRootWkspc/parent/child/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print(self) + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print (self\ + + "foo") + + def meth3(self): + """test one line disabling""" + # no error + print (self.bla) # pylint: disable=no-member + # error + print (self.blop) + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) + # pylint: enable=no-member + # error + print (self.blip) + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + if self.blop: + # pylint: enable=no-member + # error + print (self.blip) + else: + # no error + print (self.blip) + # no error + print (self.blip) + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + try: + # pylint: enable=no-member + # error + print (self.blip) + except UndefinedName: # pylint: disable=undefined-variable + # no error + print (self.blip) + # no error + print (self.blip) + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print (self.blip) + else: + # error + print (self.blip) + # error + print (self.blip) + + + def meth8(self): + """test late disabling""" + # error + print (self.blip) + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace1/.vscode/settings.json b/src/test/multiRootWkspc/workspace1/.vscode/settings.json new file mode 100644 index 000000000000..a783cfe01962 --- /dev/null +++ b/src/test/multiRootWkspc/workspace1/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "python.linting.pylintEnabled": true, + "python.linting.enabled": false, + "python.linting.flake8Enabled": true +} diff --git a/src/test/multiRootWkspc/workspace1/file.py b/src/test/multiRootWkspc/workspace1/file.py new file mode 100644 index 000000000000..27509dd2fcd6 --- /dev/null +++ b/src/test/multiRootWkspc/workspace1/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print(self) + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print (self\ + + "foo") + + def meth3(self): + """test one line disabling""" + # no error + print (self.bla) # pylint: disable=no-member + # error + print (self.blop) + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) + # pylint: enable=no-member + # error + print (self.blip) + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + if self.blop: + # pylint: enable=no-member + # error + print (self.blip) + else: + # no error + print (self.blip) + # no error + print (self.blip) + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + try: + # pylint: enable=no-member + # error + print (self.blip) + except UndefinedName: # pylint: disable=undefined-variable + # no error + print (self.blip) + # no error + print (self.blip) + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print (self.blip) + else: + # error + print (self.blip) + # error + print (self.blip) + + + def meth8(self): + """test late disabling""" + # error + print (self.blip) + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace2/.vscode/settings.json b/src/test/multiRootWkspc/workspace2/.vscode/settings.json new file mode 100644 index 000000000000..1398df90f45b --- /dev/null +++ b/src/test/multiRootWkspc/workspace2/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "python.workspaceSymbols.ctagsPath": "${workspaceRoot}/workspace2.tags.file" +} diff --git a/src/test/multiRootWkspc/workspace2/file.py b/src/test/multiRootWkspc/workspace2/file.py new file mode 100644 index 000000000000..27509dd2fcd6 --- /dev/null +++ b/src/test/multiRootWkspc/workspace2/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print(self) + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print (self\ + + "foo") + + def meth3(self): + """test one line disabling""" + # no error + print (self.bla) # pylint: disable=no-member + # error + print (self.blop) + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) + # pylint: enable=no-member + # error + print (self.blip) + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + if self.blop: + # pylint: enable=no-member + # error + print (self.blip) + else: + # no error + print (self.blip) + # no error + print (self.blip) + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + try: + # pylint: enable=no-member + # error + print (self.blip) + except UndefinedName: # pylint: disable=undefined-variable + # no error + print (self.blip) + # no error + print (self.blip) + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print (self.blip) + else: + # error + print (self.blip) + # error + print (self.blip) + + + def meth8(self): + """test late disabling""" + # error + print (self.blip) + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace3/.vscode/settings.json b/src/test/multiRootWkspc/workspace3/.vscode/settings.json new file mode 100644 index 000000000000..1398df90f45b --- /dev/null +++ b/src/test/multiRootWkspc/workspace3/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "python.workspaceSymbols.ctagsPath": "${workspaceRoot}/workspace2.tags.file" +} diff --git a/src/test/multiRootWkspc/workspace3/file.py b/src/test/multiRootWkspc/workspace3/file.py new file mode 100644 index 000000000000..27509dd2fcd6 --- /dev/null +++ b/src/test/multiRootWkspc/workspace3/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print(self) + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print (self\ + + "foo") + + def meth3(self): + """test one line disabling""" + # no error + print (self.bla) # pylint: disable=no-member + # error + print (self.blop) + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) + # pylint: enable=no-member + # error + print (self.blip) + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + if self.blop: + # pylint: enable=no-member + # error + print (self.blip) + else: + # no error + print (self.blip) + # no error + print (self.blip) + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print (self.bla) + try: + # pylint: enable=no-member + # error + print (self.blip) + except UndefinedName: # pylint: disable=undefined-variable + # no error + print (self.blip) + # no error + print (self.blip) + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print (self.blip) + else: + # error + print (self.blip) + # error + print (self.blip) + + + def meth8(self): + """test late disabling""" + # error + print (self.blip) + # pylint: disable=no-member + # no error + print (self.bla) + print (self.blop) diff --git a/src/test/refactor/extension.refactor.extract.method.test.ts b/src/test/refactor/extension.refactor.extract.method.test.ts index f3cd520f608c..021bb90532ff 100644 --- a/src/test/refactor/extension.refactor.extract.method.test.ts +++ b/src/test/refactor/extension.refactor.extract.method.test.ts @@ -119,7 +119,7 @@ suite('Method Extraction', () => { textEditor = editor; return; }).then(() => { - return extractMethod(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch, path.dirname(refactorTargetFile), pythonSettings).then(() => { + return extractMethod(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch).then(() => { if (shouldError) { ignoreErrorHandling = true; 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 7428a6b1d43d..1e2a896a4e56 100644 --- a/src/test/refactor/extension.refactor.extract.var.test.ts +++ b/src/test/refactor/extension.refactor.extract.var.test.ts @@ -117,7 +117,7 @@ suite('Variable Extraction', () => { textEditor = editor; return; }).then(() => { - return extractVariable(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch, path.dirname(refactorTargetFile), pythonSettings).then(() => { + return extractVariable(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch).then(() => { if (shouldError) { ignoreErrorHandling = true; assert.fail('No error', 'Error', 'Extraction should fail with an error', '');