diff --git a/news/1 Enhancements/974.md b/news/1 Enhancements/974.md new file mode 100644 index 000000000000..cb43b6ceedfb --- /dev/null +++ b/news/1 Enhancements/974.md @@ -0,0 +1 @@ +Pylint is no longer enabled by default. Users that have not configured pylint but who have installed it in their workspace will be asked if they'd like to enable it. diff --git a/package.json b/package.json index 1b8d5f262ba9..abb50ce13654 100644 --- a/package.json +++ b/package.json @@ -1690,4 +1690,4 @@ "publisherDisplayName": "Microsoft", "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8" } -} +} \ No newline at end of file diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 2f31cf970b77..05289316c1f2 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -86,10 +86,6 @@ export abstract class BaseLinter implements ILinter { return this._info; } - public isLinterExecutableSpecified(resource: vscode.Uri) { - const executablePath = this.info.pathName(resource); - return path.basename(executablePath).length > 0 && path.basename(executablePath) !== executablePath; - } public async lint(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise { this._pythonSettings = this.configService.getSettings(document.uri); return this.runLinter(document, cancellation); diff --git a/src/client/linters/linterAvailability.ts b/src/client/linters/linterAvailability.ts new file mode 100644 index 000000000000..ec89c5b98453 --- /dev/null +++ b/src/client/linters/linterAvailability.ts @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { Uri } from 'vscode'; +import { IApplicationShell, IWorkspaceService } from '../common/application/types'; +import { traceError } from '../common/logger'; +import { IConfigurationService, IInstaller, Product } from '../common/types'; +import { IAvailableLinterActivator, ILinterInfo } from './types'; + +@injectable() +export class AvailableLinterActivator implements IAvailableLinterActivator { + constructor( + @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IInstaller) private installer: IInstaller, + @inject(IWorkspaceService) private workspaceConfig: IWorkspaceService, + @inject(IConfigurationService) private configService: IConfigurationService + ) { } + + /** + * Check if it is possible to enable an otherwise-unconfigured linter in + * the current workspace, and if so ask the user if they want that linter + * configured explicitly. + * + * @param linterInfo The linter to check installation status. + * @param resource Context for the operation (required when in multi-root workspaces). + * + * @returns true if configuration was updated in any way, false otherwise. + */ + public async promptIfLinterAvailable(linterInfo: ILinterInfo, resource?: Uri): Promise { + // Has the feature been enabled yet? + if (!this.isFeatureEnabled) { + return false; + } + + // Has the linter in question has been configured explicitly? If so, no need to continue. + if (!this.isLinterUsingDefaultConfiguration(linterInfo, resource)) { + return false; + } + + // Is the linter available in the current workspace? + if (await this.isLinterAvailable(linterInfo.product, resource)) { + + // great, it is - ask the user if they'd like to enable it. + return this.promptToConfigureAvailableLinter(linterInfo); + } + return false; + } + + /** + * Raise a dialog asking the user if they would like to explicitly configure a + * linter or not in their current workspace. + * + * @param linterInfo The linter to ask the user to enable or not. + * + * @returns true if the user requested a configuration change, false otherwise. + */ + public async promptToConfigureAvailableLinter(linterInfo: ILinterInfo): Promise { + type ConfigureLinterMessage = { + enabled: boolean; + title: string; + }; + + const optButtons: ConfigureLinterMessage[] = [ + { + title: `Enable ${linterInfo.id}`, + enabled: true + }, + { + title: `Disable ${linterInfo.id}`, + enabled: false + } + ]; + const pick = await this.appShell.showInformationMessage(`Linter ${linterInfo.id} is available but not enabled.`, ...optButtons); + if (pick) { + await linterInfo.enableAsync(pick.enabled); + return true; + } + + return false; + } + + /** + * Check if the linter itself is available in the workspace's Python environment or + * not. + * + * @param linterProduct Linter to check in the current workspace environment. + * @param resource Context information for workspace. + */ + public async isLinterAvailable(linterProduct: Product, resource?: Uri): Promise { + return this.installer.isInstalled(linterProduct, resource) + .catch((reason) => { + // report and continue, assume the linter is unavailable. + traceError(`[WARNING]: Failed to discover if linter ${linterProduct} is installed.`, reason); + return false; + }); + } + + /** + * Check if the given linter has been configured by the user in this workspace or not. + * + * @param linterInfo Linter to check for configuration status. + * @param resource Context information. + * + * @returns true if the linter has not been configured at the user, workspace, or workspace-folder scope. false otherwise. + */ + public isLinterUsingDefaultConfiguration(linterInfo: ILinterInfo, resource?: Uri): boolean { + const ws = this.workspaceConfig.getConfiguration('python.linting', resource); + const pe = ws!.inspect(linterInfo.enabledSettingName); + return (pe!.globalValue === undefined && pe!.workspaceValue === undefined && pe!.workspaceFolderValue === undefined); + } + + /** + * Check if this feature is enabled yet. + * + * This is a feature of the vscode-python extension that will become enabled once the + * Python Language Server becomes the default, replacing Jedi as the default. Testing + * the global default setting for `"python.jediEnabled": false` enables it. + * + * @returns true if the global default for python.jediEnabled is false. + */ + public get isFeatureEnabled(): boolean { + return !this.configService.getSettings().jediEnabled; + } +} diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts index 9ab7a673467a..04db197fc9f6 100644 --- a/src/client/linters/linterCommands.ts +++ b/src/client/linters/linterCommands.ts @@ -28,7 +28,7 @@ export class LinterCommands implements vscode.Disposable { public async setLinterAsync(): Promise { const linters = this.linterManager.getAllLinterInfos(); const suggestions = linters.map(x => x.id).sort(); - const activeLinters = this.linterManager.getActiveLinters(this.settingsUri); + const activeLinters = await this.linterManager.getActiveLinters(true, this.settingsUri); let current: string; switch (activeLinters.length) { @@ -64,7 +64,7 @@ export class LinterCommands implements vscode.Disposable { public async enableLintingAsync(): Promise { const options = ['on', 'off']; - const current = this.linterManager.isLintingEnabled(this.settingsUri) ? options[0] : options[1]; + const current = await this.linterManager.isLintingEnabled(true, this.settingsUri) ? options[0] : options[1]; const quickPickOptions: vscode.QuickPickOptions = { matchOnDetail: true, diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts index c07d0b48e037..1778736f5e79 100644 --- a/src/client/linters/linterManager.ts +++ b/src/client/linters/linterManager.ts @@ -1,9 +1,15 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +'use strict'; + import { inject, injectable } from 'inversify'; -import { CancellationToken, OutputChannel, TextDocument, Uri } from 'vscode'; -import { IConfigurationService, ILogger, Product } from '../common/types'; +import { + CancellationToken, OutputChannel, TextDocument, Uri +} from 'vscode'; +import { + IConfigurationService, ILogger, Product +} from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { Bandit } from './bandit'; import { Flake8 } from './flake8'; @@ -14,7 +20,9 @@ import { Prospector } from './prospector'; import { PyDocStyle } from './pydocstyle'; import { PyLama } from './pylama'; import { Pylint } from './pylint'; -import { ILinter, ILinterInfo, ILinterManager, ILintMessage } from './types'; +import { + IAvailableLinterActivator, ILinter, ILinterInfo, ILinterManager, ILintMessage +} from './types'; class DisabledLinter implements ILinter { constructor(private configService: IConfigurationService) { } @@ -31,8 +39,9 @@ export class LinterManager implements ILinterManager { private lintingEnabledSettingName = 'enabled'; private linters: ILinterInfo[]; private configService: IConfigurationService; + private checkedForInstalledLinters: boolean = false; - constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.configService = serviceContainer.get(IConfigurationService); this.linters = [ new LinterInfo(Product.bandit, 'bandit', this.configService), @@ -58,40 +67,49 @@ export class LinterManager implements ILinterManager { throw new Error('Invalid linter'); } - public isLintingEnabled(resource?: Uri): boolean { + public async isLintingEnabled(silent: boolean, resource?: Uri): Promise { const settings = this.configService.getSettings(resource); - return (settings.linting[this.lintingEnabledSettingName] as boolean) && this.getActiveLinters(resource).length > 0; + const activeLintersPresent = await this.getActiveLinters(silent, resource); + return (settings.linting[this.lintingEnabledSettingName] as boolean) && activeLintersPresent.length > 0; } public async enableLintingAsync(enable: boolean, resource?: Uri): Promise { await this.configService.updateSetting(`linting.${this.lintingEnabledSettingName}`, enable, resource); - - // If nothing is enabled, fix it up to PyLint (default). - if (enable && this.getActiveLinters(resource).length === 0) { - await this.setActiveLintersAsync([Product.pylint], resource); - } } - public getActiveLinters(resource?: Uri): ILinterInfo[] { + public async getActiveLinters(silent: boolean, resource?: Uri): Promise { + if (!silent) { + await this.enableUnconfiguredLinters(resource); + } return this.linters.filter(x => x.isEnabled(resource)); } public async setActiveLintersAsync(products: Product[], resource?: Uri): Promise { - const active = this.getActiveLinters(resource); - for (const x of active) { - await x.enableAsync(false, resource); - } - if (products.length > 0) { - const toActivate = this.linters.filter(x => products.findIndex(p => x.product === p) >= 0); - for (const x of toActivate) { - await x.enableAsync(true, resource); + // ensure we only allow valid linters to be set, otherwise leave things alone. + // filter out any invalid products: + const validProducts = products.filter(product => { + const foundIndex = this.linters.findIndex(validLinter => validLinter.product === product); + return foundIndex !== -1; + }); + + // if we have valid linter product(s), enable only those + if (validProducts.length > 0) { + const active = await this.getActiveLinters(true, resource); + for (const x of active) { + await x.enableAsync(false, resource); + } + if (products.length > 0) { + const toActivate = this.linters.filter(x => products.findIndex(p => x.product === p) >= 0); + for (const x of toActivate) { + await x.enableAsync(true, resource); + } + await this.enableLintingAsync(true, resource); } - await this.enableLintingAsync(true, resource); } } - public createLinter(product: Product, outputChannel: OutputChannel, serviceContainer: IServiceContainer, resource?: Uri): ILinter { - if (!this.isLintingEnabled(resource)) { + public async createLinter(product: Product, outputChannel: OutputChannel, serviceContainer: IServiceContainer, resource?: Uri): Promise { + if (!await this.isLintingEnabled(true, resource)) { return new DisabledLinter(this.configService); } const error = 'Linter manager: Unknown linter'; @@ -118,4 +136,24 @@ export class LinterManager implements ILinterManager { } throw new Error(error); } + + protected async enableUnconfiguredLinters(resource?: Uri): Promise { + // if we've already checked during this session, don't bother again + if (this.checkedForInstalledLinters) { + return false; + } + this.checkedForInstalledLinters = true; + + // only check & ask the user if they'd like to enable pylint + const pylintInfo = this.linters.find( + (linter: ILinterInfo) => linter.id === 'pylint' + ); + + // If linting is disabled, don't bother checking further. + if (pylintInfo && await this.isLintingEnabled(true, resource)) { + const activator = this.serviceContainer.get(IAvailableLinterActivator); + return activator.promptIfLinterAvailable(pylintInfo, resource); + } + return false; + } } diff --git a/src/client/linters/lintingEngine.ts b/src/client/linters/lintingEngine.ts index 7f318f7c3b7c..f27618c4f2a5 100644 --- a/src/client/linters/lintingEngine.ts +++ b/src/client/linters/lintingEngine.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +'use strict'; + import { inject, injectable } from 'inversify'; import { Minimatch } from 'minimatch'; import * as path from 'path'; @@ -93,10 +95,16 @@ export class LintingEngine implements ILintingEngine { this.pendingLintings.set(document.uri.fsPath, cancelToken); - const promises: Promise[] = this.linterManager.getActiveLinters(document.uri) - .map(info => { + const activeLinters = await this.linterManager.getActiveLinters(false, document.uri); + const promises: Promise[] = activeLinters + .map(async (info: ILinterInfo) => { const stopWatch = new StopWatch(); - const linter = this.linterManager.createLinter(info.product, this.outputChannel, this.serviceContainer, document.uri); + const linter = await this.linterManager.createLinter( + info.product, + this.outputChannel, + this.serviceContainer, + document.uri + ); const promise = linter.lint(document, cancelToken.token); this.sendLinterRunTelemetry(info, document.uri, promise, stopWatch, trigger); return promise; @@ -175,7 +183,7 @@ export class LintingEngine implements ILintingEngine { } private async shouldLintDocument(document: vscode.TextDocument): Promise { - if (!this.linterManager.isLintingEnabled(document.uri)) { + if (!await this.linterManager.isLintingEnabled(false, document.uri)) { this.diagnosticCollection.set(document.uri, []); return false; } diff --git a/src/client/linters/serviceRegistry.ts b/src/client/linters/serviceRegistry.ts index 89f7abb8e11f..0067340fe5ec 100644 --- a/src/client/linters/serviceRegistry.ts +++ b/src/client/linters/serviceRegistry.ts @@ -2,11 +2,15 @@ // Licensed under the MIT License. import { IServiceManager } from '../ioc/types'; +import { AvailableLinterActivator } from './linterAvailability'; import { LinterManager } from './linterManager'; import { LintingEngine } from './lintingEngine'; -import { ILinterManager, ILintingEngine } from './types'; +import { + IAvailableLinterActivator, ILinterManager, ILintingEngine +} from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ILintingEngine, LintingEngine); serviceManager.addSingleton(ILinterManager, LinterManager); + serviceManager.add(IAvailableLinterActivator, AvailableLinterActivator); } diff --git a/src/client/linters/types.ts b/src/client/linters/types.ts index 32db0d65282b..0c3e6f501d09 100644 --- a/src/client/linters/types.ts +++ b/src/client/linters/types.ts @@ -19,7 +19,7 @@ export interface ILinterInfo { readonly argsSettingName: string; readonly enabledSettingName: string; readonly configFileNames: string[]; - enableAsync(flag: boolean, resource?: vscode.Uri): Promise; + enableAsync(enabled: boolean, resource?: vscode.Uri): Promise; isEnabled(resource?: vscode.Uri): boolean; pathName(resource?: vscode.Uri): string; linterArgs(resource?: vscode.Uri): string[]; @@ -31,15 +31,20 @@ export interface ILinter { lint(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise; } +export const IAvailableLinterActivator = Symbol('IAvailableLinterActivator'); +export interface IAvailableLinterActivator { + promptIfLinterAvailable(linter: ILinterInfo, resource?: vscode.Uri): Promise; +} + export const ILinterManager = Symbol('ILinterManager'); export interface ILinterManager { getAllLinterInfos(): ILinterInfo[]; getLinterInfo(product: Product): ILinterInfo; - getActiveLinters(resource?: vscode.Uri): ILinterInfo[]; - isLintingEnabled(resource?: vscode.Uri): boolean; + getActiveLinters(silent: boolean, resource?: vscode.Uri): Promise; + isLintingEnabled(silent: boolean, resource?: vscode.Uri): Promise; enableLintingAsync(enable: boolean, resource?: vscode.Uri): Promise; setActiveLintersAsync(products: Product[], resource?: vscode.Uri): Promise; - createLinter(product: Product, outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer, resource?: vscode.Uri): ILinter; + createLinter(product: Product, outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer, resource?: vscode.Uri): Promise; } export interface ILintMessage { diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index fb66aab3971b..70a7c49b3916 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -1,21 +1,26 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +'use strict'; + import * as path from 'path'; -import * as vscode from 'vscode'; -import { ConfigurationTarget, Uri, workspace } from 'vscode'; +import { + ConfigurationTarget, Disposable, ExtensionContext, + TextDocument, Uri, workspace +} from 'vscode'; import { IDocumentManager } from '../common/application/types'; import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; +import '../common/extensions'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; -export class LinterProvider implements vscode.Disposable { - private context: vscode.ExtensionContext; - private disposables: vscode.Disposable[]; +export class LinterProvider implements Disposable { + private context: ExtensionContext; + private disposables: Disposable[]; private configMonitor: ConfigSettingMonitor; private interpreterService: IInterpreterService; private documents: IDocumentManager; @@ -24,7 +29,7 @@ export class LinterProvider implements vscode.Disposable { private engine: ILintingEngine; private fs: IFileSystem; - public constructor(context: vscode.ExtensionContext, serviceContainer: IServiceContainer) { + public constructor(context: ExtensionContext, serviceContainer: IServiceContainer) { this.context = context; this.disposables = []; @@ -39,7 +44,7 @@ export class LinterProvider implements vscode.Disposable { this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); - this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); + this.documents.onDidSaveTextDocument(e => this.onDocumentSaved(e), this.context.subscriptions); this.configMonitor = new ConfigSettingMonitor('linting'); this.configMonitor.on('change', this.lintSettingsChangedHandler.bind(this)); @@ -56,7 +61,7 @@ export class LinterProvider implements vscode.Disposable { this.configMonitor.dispose(); } - private isDocumentOpen(uri: vscode.Uri): boolean { + private isDocumentOpen(uri: Uri): boolean { return this.documents.textDocuments.some(document => this.fs.arePathsSame(document.uri.fsPath, uri.fsPath)); } @@ -74,26 +79,28 @@ export class LinterProvider implements vscode.Disposable { }); } - private onDocumentOpened(document: vscode.TextDocument): void { + private onDocumentOpened(document: TextDocument): void { this.engine.lintDocument(document, 'auto').ignoreErrors(); } - private onDocumentSaved(document: vscode.TextDocument): void { + private onDocumentSaved(document: TextDocument): void { const settings = this.configuration.getSettings(document.uri); if (document.languageId === 'python' && settings.linting.enabled && settings.linting.lintOnSave) { this.engine.lintDocument(document, 'save').ignoreErrors(); return; } - const linters = this.linterManager.getActiveLinters(document.uri); - const fileName = path.basename(document.uri.fsPath).toLowerCase(); - const watchers = linters.filter((info) => info.configFileNames.indexOf(fileName) >= 0); - if (watchers.length > 0) { - setTimeout(() => this.engine.lintOpenPythonFiles(), 1000); - } + this.linterManager.getActiveLinters(false, document.uri) + .then((linters) => { + const fileName = path.basename(document.uri.fsPath).toLowerCase(); + const watchers = linters.filter((info) => info.configFileNames.indexOf(fileName) >= 0); + if (watchers.length > 0) { + setTimeout(() => this.engine.lintOpenPythonFiles(), 1000); + } + }).ignoreErrors(); } - private onDocumentClosed(document: vscode.TextDocument) { + private onDocumentClosed(document: TextDocument) { if (!document || !document.fileName || !document.uri) { return; } diff --git a/src/test/common.ts b/src/test/common.ts index 7258f1b7a0ee..e3ea8cf0aa0f 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -124,6 +124,7 @@ export async function deleteDirectory(dir: string) { await fs.remove(dir); } } + export async function deleteFile(file: string) { const exists = await fs.pathExists(file); if (exists) { diff --git a/src/test/linters/lint.commands.test.ts b/src/test/linters/lint.commands.test.ts index 5cac8d6f997f..c9e202f1c2ac 100644 --- a/src/test/linters/lint.commands.test.ts +++ b/src/test/linters/lint.commands.test.ts @@ -30,7 +30,7 @@ suite('Linting - Linter Selector', () => { initializeServices(); }); suiteTeardown(closeActiveWindows); - teardown(async () => await closeActiveWindows()); + teardown(async () => closeActiveWindows()); function initializeServices() { const cont = new Container(); @@ -105,7 +105,7 @@ suite('Linting - Linter Selector', () => { assert.equal(options!.matchOnDescription, true, 'Quick pick options are incorrect'); assert.equal(options!.matchOnDetail, true, 'Quick pick options are incorrect'); assert.equal(options!.placeHolder, `current: ${current}`, 'Quick pick current option is incorrect'); - assert.equal(lm.isLintingEnabled(undefined), enable, 'Linting selector did not change linting on/off flag'); + assert.equal(await lm.isLintingEnabled(true, undefined), enable, 'Linting selector did not change linting on/off flag'); } async function selectLinterAsync(products: Product[]): Promise { @@ -129,7 +129,7 @@ suite('Linting - Linter Selector', () => { await lm.setActiveLintersAsync(products); let current: string; - let activeLinters = lm.getActiveLinters(); + let activeLinters = await lm.getActiveLinters(true); switch (activeLinters.length) { case 0: current = 'none'; @@ -154,7 +154,7 @@ suite('Linting - Linter Selector', () => { assert.equal(options!.matchOnDetail, true, 'Quick pick options are incorrect'); assert.equal(options!.placeHolder, `current: ${current}`, 'Quick pick current option is incorrect'); - activeLinters = lm.getActiveLinters(); + activeLinters = await lm.getActiveLinters(true); assert.equal(activeLinters.length, 1, 'Linting selector did not change active linter'); assert.equal(activeLinters[0].product, Product.pylint, 'Linting selector did not change to pylint'); diff --git a/src/test/linters/lint.manager.test.ts b/src/test/linters/lint.manager.test.ts index d8d62ad25f2c..e6a0a3a63361 100644 --- a/src/test/linters/lint.manager.test.ts +++ b/src/test/linters/lint.manager.test.ts @@ -81,15 +81,15 @@ suite('Linting - Manager', () => { test('Enable/disable linting', async () => { await lm.enableLintingAsync(false); - assert.equal(lm.isLintingEnabled(), false, 'Linting not disabled'); + assert.equal(await lm.isLintingEnabled(true), false, 'Linting not disabled'); await lm.enableLintingAsync(true); - assert.equal(lm.isLintingEnabled(), true, 'Linting not enabled'); + assert.equal(await lm.isLintingEnabled(true), true, 'Linting not enabled'); }); test('Set single linter', async () => { for (const linter of lm.getAllLinterInfos()) { await lm.setActiveLintersAsync([linter.product]); - const selected = lm.getActiveLinters(); + const selected = await lm.getActiveLinters(true); assert.notEqual(selected.length, 0, 'Current linter is undefined'); assert.equal(linter!.id, selected![0].id, `Selected linter ${selected} does not match requested ${linter.id}`); } @@ -97,18 +97,18 @@ suite('Linting - Manager', () => { test('Set multiple linters', async () => { await lm.setActiveLintersAsync([Product.flake8, Product.pydocstyle]); - const selected = lm.getActiveLinters(); + const selected = await lm.getActiveLinters(true); assert.equal(selected.length, 2, 'Selected linters lengths does not match'); assert.equal(Product.flake8, selected[0].product, `Selected linter ${selected[0].id} does not match requested 'flake8'`); assert.equal(Product.pydocstyle, selected[1].product, `Selected linter ${selected[1].id} does not match requested 'pydocstyle'`); }); test('Try setting unsupported linter', async () => { - const before = lm.getActiveLinters(); + const before = await lm.getActiveLinters(true); assert.notEqual(before, undefined, 'Current/before linter is undefined'); await lm.setActiveLintersAsync([Product.nosetest]); - const after = lm.getActiveLinters(); + const after = await lm.getActiveLinters(true); assert.notEqual(after, undefined, 'Current/after linter is undefined'); assert.equal(after![0].id, before![0].id, 'Should not be able to set unsupported linter'); diff --git a/src/test/linters/lint.manager.unit.test.ts b/src/test/linters/lint.manager.unit.test.ts new file mode 100644 index 000000000000..66208a404628 --- /dev/null +++ b/src/test/linters/lint.manager.unit.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import * as TypeMoq from 'typemoq'; +import { Uri } from 'vscode'; +import { IConfigurationService, IPythonSettings } from '../../client/common/types'; +import { IServiceContainer } from '../../client/ioc/types'; +import { LinterManager } from '../../client/linters/linterManager'; + +// setup class instance +class TestLinterManager extends LinterManager { + public enableUnconfiguredLintersCallCount: number = 0; + + protected async enableUnconfiguredLinters(resource?: Uri): Promise { + this.enableUnconfiguredLintersCallCount += 1; + return false; + } +} + +function getServiceContainerMockForLinterManagerTests(): TypeMoq.IMock { + // setup test mocks + const serviceContainerMock = TypeMoq.Mock.ofType(); + const configMock = TypeMoq.Mock.ofType(); + const pythonSettingsMock = TypeMoq.Mock.ofType(); + configMock.setup(cm => cm.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettingsMock.object); + serviceContainerMock.setup(c => c.get(IConfigurationService)).returns(() => configMock.object); + + return serviceContainerMock; +} + +// tslint:disable-next-line:max-func-body-length +suite('Lint Manager Unit Tests', () => { + + test('Linter manager isLintingEnabled checks availability when silent = false.', async () => { + // set expectations + const expectedCallCount = 1; + const silentFlag = false; + + // get setup + const serviceContainerMock = getServiceContainerMockForLinterManagerTests(); + + // make the call + const lm = new TestLinterManager(serviceContainerMock.object); + await lm.isLintingEnabled(silentFlag); + + // test expectations + expect(lm.enableUnconfiguredLintersCallCount).to.equal(expectedCallCount); + }); + + test('Linter manager isLintingEnabled does not check availability when silent = true.', async () => { + // set expectations + const expectedCallCount = 0; + const silentFlag = true; + + // get setup + const serviceContainerMock = getServiceContainerMockForLinterManagerTests(); + + // make the call + const lm: TestLinterManager = new TestLinterManager(serviceContainerMock.object); + await lm.isLintingEnabled(silentFlag); + + // test expectations + expect(lm.enableUnconfiguredLintersCallCount).to.equal(expectedCallCount); + }); + + test('Linter manager getActiveLinters checks availability when silent = false.', async () => { + // set expectations + const expectedCallCount = 1; + const silentFlag = false; + + // get setup + const serviceContainerMock = getServiceContainerMockForLinterManagerTests(); + + // make the call + const lm: TestLinterManager = new TestLinterManager(serviceContainerMock.object); + await lm.getActiveLinters(silentFlag); + + // test expectations + expect(lm.enableUnconfiguredLintersCallCount).to.equal(expectedCallCount); + }); + + test('Linter manager getActiveLinters checks availability when silent = true.', async () => { + // set expectations + const expectedCallCount = 0; + const silentFlag = true; + + // get setup + const serviceContainerMock = getServiceContainerMockForLinterManagerTests(); + + // make the call + const lm: TestLinterManager = new TestLinterManager(serviceContainerMock.object); + await lm.getActiveLinters(silentFlag); + + // test expectations + expect(lm.enableUnconfiguredLintersCallCount).to.equal(expectedCallCount); + }); + +}); diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 07de8b37ad45..9466788eae56 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -4,15 +4,23 @@ import { Container } from 'inversify'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; -import { IDocumentManager } from '../../client/common/application/types'; +import { + IApplicationShell, IDocumentManager, IWorkspaceService +} from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; -import { IConfigurationService, ILintingSettings, IPythonSettings, Product } from '../../client/common/types'; +import { + IConfigurationService, IInstaller, ILintingSettings, + IPythonSettings, Product +} from '../../client/common/types'; import { createDeferred } from '../../client/common/utils/async'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { ServiceContainer } from '../../client/ioc/container'; import { ServiceManager } from '../../client/ioc/serviceManager'; +import { AvailableLinterActivator } from '../../client/linters/linterAvailability'; import { LinterManager } from '../../client/linters/linterManager'; -import { ILinterManager, ILintingEngine } from '../../client/linters/types'; +import { + IAvailableLinterActivator, ILinterManager, ILintingEngine +} from '../../client/linters/types'; import { LinterProvider } from '../../client/providers/linterProvider'; import { initialize } from '../initialize'; @@ -29,6 +37,9 @@ suite('Linting - Provider', () => { let emitter: vscode.EventEmitter; let document: TypeMoq.IMock; let fs: TypeMoq.IMock; + let appShell: TypeMoq.IMock; + let linterInstaller: TypeMoq.IMock; + let workspaceService: TypeMoq.IMock; suiteSetup(initialize); setup(async () => { @@ -63,6 +74,14 @@ suite('Linting - Provider', () => { configService.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); serviceManager.addSingletonInstance(IConfigurationService, configService.object); + appShell = TypeMoq.Mock.ofType(); + linterInstaller = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); + serviceManager.addSingletonInstance(IApplicationShell, appShell.object); + serviceManager.addSingletonInstance(IInstaller, linterInstaller.object); + serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); + serviceManager.add(IAvailableLinterActivator, AvailableLinterActivator); + lm = new LinterManager(serviceContainer); serviceManager.addSingletonInstance(ILinterManager, lm); emitter = new vscode.EventEmitter(); @@ -80,7 +99,7 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'auto'), TypeMoq.Times.once()); }); - test('Lint on save file', () => { + test('Lint on save file', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('test.py')); document.setup(x => x.languageId).returns(() => 'python'); diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index 4c7f57c242a7..4b0310dbfdac 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -163,7 +163,7 @@ suite('Linting - General Tests', () => { await linterManager.setActiveLintersAsync([product]); await linterManager.enableLintingAsync(enabled); - const linter = linterManager.createLinter(product, output, ioc.serviceContainer); + const linter = await linterManager.createLinter(product, output, ioc.serviceContainer); const messages = await linter.lint(document, cancelToken.token); if (enabled) { @@ -211,7 +211,7 @@ suite('Linting - General Tests', () => { const document = await workspace.openTextDocument(pythonFile); await linterManager.setActiveLintersAsync([product], document.uri); - const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer); + const linter = await linterManager.createLinter(product, outputChannel, ioc.serviceContainer); const messages = await linter.lint(document, cancelToken.token); if (messagesToBeReceived.length === 0) { @@ -260,11 +260,11 @@ suite('Linting - General Tests', () => { }); // tslint:disable-next-line:no-function-expression test('Multiple linters', async function () { -// Unreliable test being skipped until we can sort it out. See gh-2609. -// - Fails about 1/3 of runs on Windows -// - Symptom: lintingEngine::lintOpenPythonFiles returns values *after* command await resolves in lint.tests -// - lintOpenPythonFiles returns 3 sets of values, not what I expect (1). -// - Haven't yet found a way to await on this properly. + // Unreliable test being skipped until we can sort it out. See gh-2609. + // - Fails about 1/3 of runs on Windows + // - Symptom: lintingEngine::lintOpenPythonFiles returns values *after* command await resolves in lint.tests + // - lintOpenPythonFiles returns 3 sets of values, not what I expect (1). + // - Haven't yet found a way to await on this properly. const skipped = true; if (skipped) { // tslint:disable-next-line:no-invalid-this @@ -295,7 +295,7 @@ suite('Linting - General Tests', () => { const document = await workspace.openTextDocument(pythonFile); await linterManager.setActiveLintersAsync([product], document.uri); - const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer); + const linter = await linterManager.createLinter(product, outputChannel, ioc.serviceContainer); const messages = await linter.lint(document, cancelToken.token); assert.equal(messages.length, messageCountToBeReceived, 'Expected number of lint errors does not match lint error count'); diff --git a/src/test/linters/lintengine.test.ts b/src/test/linters/lintengine.test.ts index 92bb25d17ba4..f4350c4b197e 100644 --- a/src/test/linters/lintengine.test.ts +++ b/src/test/linters/lintengine.test.ts @@ -47,7 +47,7 @@ suite('Linting - LintingEngine', () => { serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isValue(STANDARD_OUTPUT_CHANNEL))).returns(() => outputChannel.object); lintManager = TypeMoq.Mock.ofType(); - lintManager.setup(x => x.isLintingEnabled(TypeMoq.It.isAny())).returns(() => true); + lintManager.setup(x => x.isLintingEnabled(TypeMoq.It.isAny())).returns(async () => true); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ILinterManager), TypeMoq.It.isAny())).returns(() => lintManager.object); lintingEngine = new LintingEngine(serviceContainer.object); @@ -59,7 +59,7 @@ suite('Linting - LintingEngine', () => { try { lintingEngine.lintDocument(doc, 'auto').ignoreErrors(); } catch { - lintManager.verify(l => l.isLintingEnabled(TypeMoq.It.isValue(doc.uri)), TypeMoq.Times.once()); + lintManager.verify(l => l.isLintingEnabled(TypeMoq.It.isAny(), TypeMoq.It.isValue(doc.uri)), TypeMoq.Times.once()); } }); test('Ensure document.uri is passed into createLinter', () => { diff --git a/src/test/linters/linter.availability.unit.test.ts b/src/test/linters/linter.availability.unit.test.ts new file mode 100644 index 000000000000..d66691aa50b3 --- /dev/null +++ b/src/test/linters/linter.availability.unit.test.ts @@ -0,0 +1,447 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import * as TypeMoq from 'typemoq'; +import { Uri, WorkspaceConfiguration } from 'vscode'; +import { + IApplicationShell, IWorkspaceService +} from '../../client/common/application/types'; +import { + IConfigurationService, IInstaller, IPythonSettings, Product +} from '../../client/common/types'; +import { AvailableLinterActivator } from '../../client/linters/linterAvailability'; +import { LinterInfo } from '../../client/linters/linterInfo'; +import { IAvailableLinterActivator } from '../../client/linters/types'; + +// tslint:disable-next-line:max-func-body-length +suite('Linter Availability Provider tests', () => { + + test('Availability feature is disabled when global default for jediEnabled=true.', async () => { + // set expectations + const jediEnabledValue = true; + const expectedResult = false; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock] = getDependenciesForAvailabilityTests(); + setupConfigurationServiceForJediSettingsTest(jediEnabledValue, configServiceMock); + + // call + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + // check expectaions + expect(availabilityProvider.isFeatureEnabled).is.equal(expectedResult, 'Avaialability feature should be disabled when python.jediEnabled is true'); + workspaceServiceMock.verifyAll(); + }); + + test('Availability feature is enabled when global default for jediEnabled=false.', async () => { + // set expectations + const jediEnabledValue = false; + const expectedResult = true; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock] = getDependenciesForAvailabilityTests(); + setupConfigurationServiceForJediSettingsTest(jediEnabledValue, configServiceMock); + + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + expect(availabilityProvider.isFeatureEnabled).is.equal(expectedResult, 'Avaialability feature should be enabled when python.jediEnabled defaults to false'); + workspaceServiceMock.verifyAll(); + }); + + test('Prompt will be performed when linter is not configured at all for the workspace, workspace-folder, or the user', async () => { + // setup expectations + const pylintUserValue = undefined; + const pylintWorkspaceValue = undefined; + const pylintWorkspaceFolderValue = undefined; + const expectedResult = true; + + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupWorkspaceMockForLinterConfiguredTests(pylintUserValue, pylintWorkspaceValue, pylintWorkspaceFolderValue, workspaceServiceMock); + + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + const result = availabilityProvider.isLinterUsingDefaultConfiguration(linterInfo); + + expect(result).to.equal(expectedResult, 'Linter is unconfigured but prompt did not get raised'); + workspaceServiceMock.verifyAll(); + }); + + test('No prompt performed when linter is configured as enabled for the workspace', async () => { + // setup expectations + const pylintUserValue = undefined; + const pylintWorkspaceValue = true; + const pylintWorkspaceFolderValue = undefined; + const expectedResult = false; + + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupWorkspaceMockForLinterConfiguredTests(pylintUserValue, pylintWorkspaceValue, pylintWorkspaceFolderValue, workspaceServiceMock); + + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + const result = availabilityProvider.isLinterUsingDefaultConfiguration(linterInfo); + expect(result).to.equal(expectedResult, 'Available linter prompt should not be shown when linter is configured for workspace.'); + workspaceServiceMock.verifyAll(); + }); + + test('No prompt performed when linter is configured as enabled for the entire user', async () => { + // setup expectations + const pylintUserValue = true; + const pylintWorkspaceValue = undefined; + const pylintWorkspaceFolderValue = undefined; + const expectedResult = false; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupWorkspaceMockForLinterConfiguredTests(pylintUserValue, pylintWorkspaceValue, pylintWorkspaceFolderValue, workspaceServiceMock); + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + const result = availabilityProvider.isLinterUsingDefaultConfiguration(linterInfo); + expect(result).to.equal(expectedResult, 'Available linter prompt should not be shown when linter is configured for user.'); + workspaceServiceMock.verifyAll(); + }); + + test('No prompt performed when linter is configured as enabled for the workspace-folder', async () => { + // setup expectations + const pylintUserValue = undefined; + const pylintWorkspaceValue = undefined; + const pylintWorkspaceFolderValue = true; + const expectedResult = false; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupWorkspaceMockForLinterConfiguredTests(pylintUserValue, pylintWorkspaceValue, pylintWorkspaceFolderValue, workspaceServiceMock); + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + + const result = availabilityProvider.isLinterUsingDefaultConfiguration(linterInfo); + expect(result).to.equal(expectedResult, 'Available linter prompt should not be shown when linter is configured for workspace-folder.'); + workspaceServiceMock.verifyAll(); + }); + + async function testForLinterPromptResponse(promptReply: { title: string; enabled: boolean } | undefined): Promise { + // arrange + const [appShellMock, installerMock, workspaceServiceMock] = getDependenciesForAvailabilityTests(); + const configServiceMock = TypeMoq.Mock.ofType(); + + const linterInfo = new class extends LinterInfo { + public testIsEnabled: boolean = promptReply ? promptReply.enabled : false; + + public async enableAsync(enabled: boolean, resource?: Uri): Promise { + this.testIsEnabled = enabled; + return Promise.resolve(); + } + + }(Product.pylint, 'pylint', configServiceMock.object, ['.pylintrc', 'pylintrc']); + + appShellMock.setup(ap => ap.showInformationMessage( + TypeMoq.It.isValue(`Linter ${linterInfo.id} is available but not enabled.`), + TypeMoq.It.isAny(), + TypeMoq.It.isAny()) + ) + .returns(() => { + // tslint:disable-next-line:no-any + return promptReply as any; + }) + .verifiable(TypeMoq.Times.once()); + + // perform test + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + const result = await availabilityProvider.promptToConfigureAvailableLinter(linterInfo); + if (promptReply) { + expect(linterInfo.testIsEnabled).to.equal(promptReply.enabled, 'LinterInfo test class was not updated as a result of the test.'); + } + + return result; + } + + test('Linter is enabled after being prompted and "Enable " is selected', async () => { + // set expectations + const expectedResult = true; + const promptReply = { title: 'Enable pylint', enabled: true }; + + // run scenario + const result = await testForLinterPromptResponse(promptReply); + + // test results + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + }); + + test('Linter is disabled after being prompted and "Disable " is selected', async () => { + // set expectation + const promptReply = { title: 'Disable pylint', enabled: false }; + const expectedResult = true; + + // run scenario + const result = await testForLinterPromptResponse(promptReply); + + // test results + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + }); + + test('Linter is left unconfigured after being prompted and the prompt is disabled without any selection made', async () => { + // set expectation + const promptReply = undefined; + const expectedResult = false; + + // run scenario + const result = await testForLinterPromptResponse(promptReply); + + // test results + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + }); + + // Options to test the implementation of the IAvailableLinterActivator. + // All options default to values that would otherwise allow the prompt to appear. + class AvailablityTestOverallOptions { + public jediEnabledValue: boolean = false; + public pylintUserEnabled?: boolean; + public pylintWorkspaceEnabled?: boolean; + public pylintWorkspaceFolderEnabled?: boolean; + public linterIsInstalled: boolean = true; + public promptReply?: { title: string; enabled: boolean }; + } + + async function performTestOfOverallImplementation(options: AvailablityTestOverallOptions): Promise { + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + appShellMock.setup(ap => ap.showInformationMessage( + TypeMoq.It.isValue(`Linter ${linterInfo.id} is available but not enabled.`), + TypeMoq.It.isAny(), + TypeMoq.It.isAny()) + ) + // tslint:disable-next-line:no-any + .returns(() => options.promptReply as any) + .verifiable(TypeMoq.Times.once()); + + installerMock.setup(im => im.isInstalled(linterInfo.product, TypeMoq.It.isAny())) + .returns(async () => options.linterIsInstalled) + .verifiable(TypeMoq.Times.once()); + + setupConfigurationServiceForJediSettingsTest(options.jediEnabledValue, configServiceMock); + setupWorkspaceMockForLinterConfiguredTests( + options.pylintUserEnabled, + options.pylintWorkspaceEnabled, + options.pylintWorkspaceFolderEnabled, + workspaceServiceMock + ); + + // perform test + const availabilityProvider: IAvailableLinterActivator = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + return availabilityProvider.promptIfLinterAvailable(linterInfo); + } + + test('Overall implementation does not change configuration when feature disabled', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.jediEnabledValue = true; + const expectedResult = false; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + // perform test + expect(expectedResult).to.equal(result, 'promptIfLinterAvailable should not change any configuration when python.jediEnabled is true.'); + }); + + test('Overall implementation does not change configuration when linter is configured (enabled)', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.pylintWorkspaceEnabled = true; + const expectedResult = false; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + // perform test + expect(expectedResult).to.equal(result, 'Configuration should not change if the linter is configured in any way.'); + }); + + test('Overall implementation does not change configuration when linter is configured (disabled)', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.pylintWorkspaceEnabled = false; + const expectedResult = false; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + expect(expectedResult).to.equal(result, 'Configuration should not change if the linter is disabled in any way.'); + }); + + test('Overall implementation does not change configuration when linter is unavailable in current workspace environment', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.pylintWorkspaceEnabled = true; + const expectedResult = false; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + expect(expectedResult).to.equal(result, 'Configuration should not change if the linter is unavailable in the current workspace environment.'); + }); + + test('Overall implementation does not change configuration when user is prompted and prompt is dismissed', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.promptReply = undefined; // just being explicit for test readability - this is the default + const expectedResult = false; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + expect(expectedResult).to.equal(result, 'Configuration should not change if the user is prompted and they dismiss the prompt.'); + }); + + test('Overall implementation changes configuration when user is prompted and "Disable " is selected', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.promptReply = { title: 'Disable pylint', enabled: false }; + const expectedResult = true; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + expect(expectedResult).to.equal(result, 'Configuration should change if the user is prompted and they choose to update the linter config.'); + }); + + test('Overall implementation changes configuration when user is prompted and "Enable " is selected', async () => { + // set expectations + const testOpts = new AvailablityTestOverallOptions(); + testOpts.promptReply = { title: 'Enable pylint', enabled: true }; + const expectedResult = true; + + // arrange + const result = await performTestOfOverallImplementation(testOpts); + + expect(expectedResult).to.equal(result, 'Configuration should change if the user is prompted and they choose to update the linter config.'); + }); + + test('Discovery of linter is available in the environment returns true when it succeeds and is present', async () => { + // set expectations + const linterIsInstalled = true; + const expectedResult = true; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupInstallerForAvailabilityTest(linterInfo, linterIsInstalled, installerMock); + + // perform test + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + const result = await availabilityProvider.isLinterAvailable(linterInfo.product); + + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + installerMock.verifyAll(); + }); + + test('Discovery of linter is available in the environment returns false when it succeeds and is not present', async () => { + // set expectations + const linterIsInstalled = false; + const expectedResult = false; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + setupInstallerForAvailabilityTest(linterInfo, linterIsInstalled, installerMock); + + // perform test + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + const result = await availabilityProvider.isLinterAvailable(linterInfo.product); + + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + installerMock.verifyAll(); + }); + + test('Discovery of linter is available in the environment returns false when it fails', async () => { + // set expectations + const expectedResult = false; + + // arrange + const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests(); + installerMock.setup(im => im.isInstalled(linterInfo.product, TypeMoq.It.isAny())) + .returns(() => Promise.reject('error testfail')) + .verifiable(TypeMoq.Times.once()); + + // perform test + const availabilityProvider = new AvailableLinterActivator(appShellMock.object, installerMock.object, workspaceServiceMock.object, configServiceMock.object); + const result = await availabilityProvider.isLinterAvailable(linterInfo.product); + + expect(result).to.equal(expectedResult, 'Expected promptToConfigureAvailableLinter to return true because the configuration was updated.'); + installerMock.verifyAll(); + }); +}); + +function setupWorkspaceMockForLinterConfiguredTests( + enabledForUser: boolean | undefined, + enabeldForWorkspace: boolean | undefined, + enabledForWorkspaceFolder: boolean | undefined, + workspaceServiceMock?: TypeMoq.IMock): TypeMoq.IMock { + + if (!workspaceServiceMock) { + workspaceServiceMock = TypeMoq.Mock.ofType(); + } + const workspaceConfiguration = TypeMoq.Mock.ofType(); + workspaceConfiguration.setup(wc => wc.inspect(TypeMoq.It.isValue('pylintEnabled'))) + .returns(() => { + return { + key: '', + globalValue: enabledForUser, + defaultValue: false, + workspaceFolderValue: enabeldForWorkspace, + workspaceValue: enabledForWorkspaceFolder + }; + }) + .verifiable(TypeMoq.Times.once()); + + workspaceServiceMock.setup(ws => ws.getConfiguration(TypeMoq.It.isValue('python.linting'), TypeMoq.It.isAny())) + .returns(() => workspaceConfiguration.object) + .verifiable(TypeMoq.Times.once()); + + return workspaceServiceMock; +} + +function setupConfigurationServiceForJediSettingsTest( + jediEnabledValue: boolean, + configServiceMock: TypeMoq.IMock +): [ + TypeMoq.IMock, + TypeMoq.IMock + ] { + + if (!configServiceMock) { + configServiceMock = TypeMoq.Mock.ofType(); + } + const pythonSettings = TypeMoq.Mock.ofType(); + pythonSettings.setup(ps => ps.jediEnabled).returns(() => jediEnabledValue); + + configServiceMock.setup(cs => cs.getSettings()).returns(() => pythonSettings.object); + return [configServiceMock, pythonSettings]; +} + +function setupInstallerForAvailabilityTest(linterInfo: LinterInfo, linterIsInstalled: boolean, installerMock): TypeMoq.IMock { + if (!installerMock) { + installerMock = TypeMoq.Mock.ofType(); + } + installerMock.setup(im => im.isInstalled(linterInfo.product, TypeMoq.It.isAny())) + .returns(async () => linterIsInstalled) + .verifiable(TypeMoq.Times.once()); + + return installerMock; +} + +function getDependenciesForAvailabilityTests(): [ + TypeMoq.IMock, + TypeMoq.IMock, + TypeMoq.IMock, + TypeMoq.IMock, + LinterInfo +] { + const configServiceMock = TypeMoq.Mock.ofType(); + return [ + TypeMoq.Mock.ofType(), + TypeMoq.Mock.ofType(), + TypeMoq.Mock.ofType(), + TypeMoq.Mock.ofType(), + new LinterInfo(Product.pylint, 'pylint', configServiceMock.object, ['.pylintrc', 'pylintrc']) + ]; +}