diff --git a/src/client/common/installer.ts b/src/client/common/installer.ts index e8e93e76c7ef..97884405434d 100644 --- a/src/client/common/installer.ts +++ b/src/client/common/installer.ts @@ -1,9 +1,8 @@ import * as os from 'os'; import * as vscode from 'vscode'; -import { commands, ConfigurationTarget, Disposable, OutputChannel, Terminal, Uri, window, workspace } from 'vscode'; +import { ConfigurationTarget, Uri, window, workspace } from 'vscode'; import * as settings from './configSettings'; import { isNotInstalledError } from './helpers'; -import { error } from './logger'; import { execPythonFile, getFullyQualifiedPythonInterpreterPath, IS_WINDOWS } from './utils'; export enum Product { @@ -203,12 +202,13 @@ export class Installer implements vscode.Disposable { return InstallerResponse.Ignore; } - const installOption = ProductInstallationPrompt.has(product) ? ProductInstallationPrompt.get(product) : `Install ${productName}`; + // tslint:disable-next-line:no-non-null-assertion + 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'; const useOtherFormatter = `Use '${alternateFormatter}' formatter`; - const options = []; + const options: string[] = []; options.push(installOption); if (productType === ProductType.Formatter) { options.push(...[useOtherFormatter]); @@ -217,6 +217,9 @@ export class Installer implements vscode.Disposable { options.push(...[disableOption, dontShowAgain]); } const item = await window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options); + if (!item) { + return InstallerResponse.Ignore; + } switch (item) { case installOption: { return this.install(product, resource); @@ -313,26 +316,28 @@ export class Installer implements vscode.Disposable { } return installationPromise - .then(() => this.isInstalled(product)) + .then(async () => this.isInstalled(product)) .then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore); } // tslint:disable-next-line:member-ordering - public isInstalled(product: Product, resource?: Uri): Promise { + public async isInstalled(product: Product, resource?: Uri): Promise { return isProductInstalled(product, resource); } // tslint:disable-next-line:member-ordering no-any - public uninstall(product: Product, resource?: Uri): Promise { + public async uninstall(product: Product, resource?: Uri): Promise { return uninstallproduct(product, resource); } // tslint:disable-next-line:member-ordering - public disableLinter(product: Product, resource: Uri) { - if (resource && !workspace.getWorkspaceFolder(resource)) { + public async disableLinter(product: Product, resource?: Uri) { + if (resource && workspace.getWorkspaceFolder(resource)) { // tslint:disable-next-line:no-non-null-assertion const settingToDisable = SettingToDisableProduct.get(product)!; const pythonConfig = workspace.getConfiguration('python', resource); - return pythonConfig.update(settingToDisable, false, ConfigurationTarget.Workspace); + const isMultiroot = Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 1; + const configTarget = isMultiroot ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace; + return pythonConfig.update(settingToDisable, false, configTarget); } else { const pythonConfig = workspace.getConfiguration('python'); return pythonConfig.update('linting.enabledWithoutWorkspace', false, true); @@ -382,7 +387,7 @@ async function isProductInstalled(product: Product, resource?: Uri): Promise { +async function uninstallproduct(product: Product, resource?: Uri): Promise { if (!ProductUninstallScripts.has(product)) { return Promise.resolve(); } diff --git a/src/test/common.ts b/src/test/common.ts index fe8bc2b35547..851ed6cae5a9 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -14,9 +14,10 @@ export type PythonSettingKeys = 'workspaceSymbols.enabled' | 'pythonPath' | 'linting.prospectorEnabled' | 'linting.pydocstyleEnabled' | 'linting.mypyEnabled' | 'unitTest.nosetestArgs' | 'unitTest.pyTestArgs' | 'unitTest.unittestArgs' | 'formatting.formatOnSave' | 'formatting.provider' | 'sortImports.args' | - 'unitTest.nosetestsEnabled' | 'unitTest.pyTestEnabled' | 'unitTest.unittestEnabled'; + 'unitTest.nosetestsEnabled' | 'unitTest.pyTestEnabled' | 'unitTest.unittestEnabled' | + 'linting.enabledWithoutWorkspace'; -export async function updateSetting(setting: PythonSettingKeys, value: {}, resource: Uri, configTarget: ConfigurationTarget) { +export async function updateSetting(setting: PythonSettingKeys, value: {}, resource: Uri | undefined, configTarget: ConfigurationTarget) { const settings = workspace.getConfiguration('python', resource); const currentValue = settings.inspect(setting); if (currentValue !== undefined && ((configTarget === ConfigurationTarget.Global && currentValue.globalValue === value) || @@ -50,8 +51,7 @@ export function retryAsync(wrapped: Function, retryCount: number = 2) { const reasons: any[] = []; const makeCall = () => { - // tslint:disable-next-line:no-unsafe-any no-any - // tslint:disable-next-line:no-invalid-this + // tslint:disable-next-line:no-unsafe-any no-any no-invalid-this wrapped.call(this, ...args) // tslint:disable-next-line:no-unsafe-any no-any .then(resolve, (reason: any) => { @@ -86,7 +86,9 @@ async function setPythonPathInWorkspace(resource: string | Uri | undefined, conf } async function restoreGlobalPythonPathSetting(): Promise { const pythonConfig = workspace.getConfiguration('python'); - const currentGlobalPythonPathSetting = pythonConfig.inspect('pythonPath').globalValue; + // tslint:disable-next-line:no-non-null-assertion + const currentGlobalPythonPathSetting = pythonConfig.inspect('pythonPath')!.globalValue; + // tslint:disable-next-line:no-use-before-declare if (globalPythonPathSetting !== currentGlobalPythonPathSetting) { await pythonConfig.update('pythonPath', undefined, true); } @@ -106,7 +108,8 @@ export async function deleteFile(file: string) { } } -const globalPythonPathSetting = workspace.getConfiguration('python').inspect('pythonPath').globalValue; +// tslint:disable-next-line:no-non-null-assertion +const globalPythonPathSetting = workspace.getConfiguration('python').inspect('pythonPath')!.globalValue; export const clearPythonPathInWorkspaceFolder = async (resource: string | Uri) => retryAsync(setPythonPathInWorkspace)(resource, ConfigurationTarget.WorkspaceFolder); export const setPythonPathInWorkspaceRoot = async (pythonPath: string) => retryAsync(setPythonPathInWorkspace)(undefined, ConfigurationTarget.Workspace, pythonPath); export const resetGlobalPythonPathSetting = async () => retryAsync(restoreGlobalPythonPathSetting)(); diff --git a/src/test/common/installer.multiroot.test.ts b/src/test/common/installer.multiroot.test.ts new file mode 100644 index 000000000000..16723f0c435f --- /dev/null +++ b/src/test/common/installer.multiroot.test.ts @@ -0,0 +1,58 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import { ConfigurationTarget, Uri, workspace } from 'vscode'; +import { Installer, Product } from '../../client/common/installer'; +import { rootWorkspaceUri } from '../common'; +import { updateSetting } from '../common'; +import { closeActiveWindows, initializeTest, IS_MULTI_ROOT_TEST } from './../initialize'; +import { MockOutputChannel } from './../mockClasses'; + +// tslint:disable-next-line:no-suspicious-comment +// TODO: Need to mock the command runner, to check what commands are being sent. +// Instead of altering the environment. + +suite('Installer', () => { + let outputChannel: MockOutputChannel; + let installer: Installer; + const workspaceUri = Uri.file(path.join(__dirname, '..', '..', '..', 'src', 'test')); + suiteSetup(async function () { + if (!IS_MULTI_ROOT_TEST) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + outputChannel = new MockOutputChannel('Installer'); + installer = new Installer(outputChannel); + await initializeTest(); + }); + setup(async () => { + await initializeTest(); + await resetSettings(); + }); + suiteTeardown(async () => { + await closeActiveWindows(); + await resetSettings(); + }); + teardown(closeActiveWindows); + + async function resetSettings() { + await updateSetting('linting.enabledWithoutWorkspace', true, undefined, ConfigurationTarget.Global); + await updateSetting('linting.pylintEnabled', true, rootWorkspaceUri, ConfigurationTarget.Workspace); + if (IS_MULTI_ROOT_TEST) { + await updateSetting('linting.pylintEnabled', true, rootWorkspaceUri, ConfigurationTarget.WorkspaceFolder); + } + } + + test('Disable linting of files contained in a multi-root workspace', async function () { + if (!IS_MULTI_ROOT_TEST) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + await installer.disableLinter(Product.pylint, workspaceUri); + const pythonWConfig = workspace.getConfiguration('python', workspaceUri); + const value = pythonWConfig.inspect('linting.pylintEnabled'); + // tslint:disable-next-line:no-non-null-assertion + assert.equal(value!.workspaceValue, true, 'Workspace setting has been disabled'); + // tslint:disable-next-line:no-non-null-assertion + assert.equal(value!.workspaceFolderValue, false, 'Workspace folder setting not disabled'); + }); +}); diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 786ae7bb278e..3abf205d2bc9 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -1,11 +1,14 @@ import * as assert from 'assert'; import * as path from 'path'; -import { Uri } from 'vscode'; +import { ConfigurationTarget, Uri, workspace } from 'vscode'; import { EnumEx } from '../../client/common/enumUtils'; import { Installer, Product } from '../../client/common/installer'; +import { rootWorkspaceUri } from '../common'; +import { updateSetting } from '../common'; import { closeActiveWindows, initializeTest, IS_MULTI_ROOT_TEST, IS_TRAVIS } from './../initialize'; import { MockOutputChannel } from './../mockClasses'; +// tslint:disable-next-line:no-suspicious-comment // TODO: Need to mock the command runner, to check what commands are being sent. // Instead of altering the environment. @@ -19,16 +22,27 @@ suite('Installer', () => { installer = new Installer(outputChannel); await initializeTest(); }); - setup(initializeTest); - suiteTeardown(closeActiveWindows); + setup(async () => { + await initializeTest(); + await resetSettings(); + }); + suiteTeardown(async () => { + await closeActiveWindows(); + await resetSettings(); + }); teardown(closeActiveWindows); + async function resetSettings() { + await updateSetting('linting.enabledWithoutWorkspace', true, undefined, ConfigurationTarget.Global); + await updateSetting('linting.pylintEnabled', true, rootWorkspaceUri, ConfigurationTarget.Workspace); + } + async function testUninstallingProduct(product: Product) { let isInstalled = await installer.isInstalled(product, resource); if (isInstalled) { await installer.uninstall(product, resource); isInstalled = await installer.isInstalled(product, resource); - // Someimtes installation doesn't work on Travis + // Sometimes installation doesn't work on Travis if (!IS_TRAVIS) { assert.equal(isInstalled, false, 'Product uninstall failed'); } @@ -50,7 +64,7 @@ suite('Installer', () => { await installer.install(product, resource); } const checkIsInstalledAgain = await installer.isInstalled(product, resource); - // Someimtes installation doesn't work on Travis + // Sometimes installation doesn't work on Travis if (!IS_TRAVIS) { assert.notEqual(checkIsInstalledAgain, false, 'Product installation failed'); } @@ -63,4 +77,20 @@ suite('Installer', () => { await testInstallingProduct(prod.value); }); }); + + test('Disable linting of files not contained in a workspace', async () => { + await installer.disableLinter(Product.pylint, undefined); + const pythonConfig = workspace.getConfiguration('python'); + assert.equal(pythonConfig.get('linting.enabledWithoutWorkspace'), false, 'Incorrect setting'); + }); + + test('Disable linting of files contained in a workspace', async function () { + if (IS_MULTI_ROOT_TEST) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + await installer.disableLinter(Product.pylint, workspaceUri); + const pythonConfig = workspace.getConfiguration('python', workspaceUri); + assert.equal(pythonConfig.get('linting.pylintEnabled'), false, 'Incorrect setting'); + }); }); diff --git a/tslint.json b/tslint.json index 9acc3a190ee9..b6e81a889f85 100644 --- a/tslint.json +++ b/tslint.json @@ -43,6 +43,7 @@ "PromiseLike" ], "completed-docs": false, + "no-backbone-get-set-outside-model": false, "no-unsafe-any": false } }