diff --git a/.github/test_plan.md b/.github/test_plan.md index b542476dc24d..6d0767571ef6 100644 --- a/.github/test_plan.md +++ b/.github/test_plan.md @@ -141,37 +141,15 @@ SPAM='hello ${WHO}' - [ ] Create a virtual environment - [ ] Install `requests` into the virtual environment -#### Pylint/default linting - -[Prompting to install Pylint is covered under `Environments` above] - -For testing the disablement of the default linting rules for Pylint: - -```ini -# pylintrc -[MESSAGES CONTROL] -enable=bad-names -``` - -```python3 -# example.py -foo = 42 # Marked as a disallowed name. -``` - -- [ ] Installation via the prompt installs Pylint as appropriate - - [ ] Uses `--user` for system-install of Python - - [ ] Installs into a virtual environment environment directly -- [ ] Pylint works -- [ ] `"python.linting.pylintUseMinimalCheckers": false` turns off the default rules w/ no `pylintrc` file present -- [ ] The existence of a `pylintrc` file turns off the default rules - -#### Other linters +#### Linting **Note**: - You can use the `Run Linting` command to run a newly installed linter - When the extension installs a new linter, it turns off all other linters +- [ ] pylint works + - [ ] `Select linter` lists the linter and installs it if necessary - [ ] flake8 works - [ ] `Select linter` lists the linter and installs it if necessary - [ ] mypy works diff --git a/package.json b/package.json index eda7b4315972..f5417cf643b4 100644 --- a/package.json +++ b/package.json @@ -1635,7 +1635,7 @@ }, "python.linting.pylintEnabled": { "type": "boolean", - "default": true, + "default": false, "description": "Whether to lint Python files using pylint.", "scope": "resource" }, @@ -1645,12 +1645,6 @@ "description": "Path to Pylint, you can use a custom version of pylint by modifying this setting to include the full path.", "scope": "resource" }, - "python.linting.pylintUseMinimalCheckers": { - "type": "boolean", - "default": true, - "description": "Whether to run Pylint with minimal set of rules.", - "scope": "resource" - }, "python.pythonPath": { "type": "string", "default": "python", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 5c8eea156b90..ecff43c6bbb9 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -397,7 +397,6 @@ export class PythonSettings implements IPythonSettings { error: DiagnosticSeverity.Error, note: DiagnosticSeverity.Hint, }, - pylintUseMinimalCheckers: false, }; this.linting.pylintPath = getAbsolutePath(systemVariables.resolveAny(this.linting.pylintPath), workspaceRoot); this.linting.flake8Path = getAbsolutePath(systemVariables.resolveAny(this.linting.flake8Path), workspaceRoot); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 801c29f580dc..f66f7ee45d9d 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -271,7 +271,6 @@ export interface ILintingSettings { banditEnabled: boolean; banditArgs: string[]; banditPath: string; - readonly pylintUseMinimalCheckers: boolean; } export interface IFormattingSettings { readonly provider: string; diff --git a/src/client/linters/linterAvailability.ts b/src/client/linters/linterAvailability.ts index 2f60096ea00e..73e9a01d2c9f 100644 --- a/src/client/linters/linterAvailability.ts +++ b/src/client/linters/linterAvailability.ts @@ -6,11 +6,10 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; -import { LanguageServerType } from '../activation/types'; import { IApplicationShell, IWorkspaceService } from '../common/application/types'; import '../common/extensions'; import { IFileSystem } from '../common/platform/types'; -import { IConfigurationService, IPersistentStateFactory, Resource } from '../common/types'; +import { IPersistentStateFactory, Resource } from '../common/types'; import { Common, Linters } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; @@ -23,7 +22,6 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { @inject(IApplicationShell) private appShell: IApplicationShell, @inject(IFileSystem) private fs: IFileSystem, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, - @inject(IConfigurationService) private configService: IConfigurationService, @inject(IPersistentStateFactory) private persistentStateFactory: IPersistentStateFactory, ) {} @@ -38,11 +36,6 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { * @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; @@ -128,17 +121,4 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { 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.languageServer": !Jedi` enables it. - * - * @returns true if the global default for python.languageServer is not Jedi. - */ - public get isFeatureEnabled(): boolean { - return this.configService.getSettings().languageServer !== LanguageServerType.Jedi; - } } diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts index c423a0f33a22..ad3decdcef63 100644 --- a/src/client/linters/linterCommands.ts +++ b/src/client/linters/linterCommands.ts @@ -36,7 +36,7 @@ export class LinterCommands implements IDisposable { const linters = this.linterManager.getAllLinterInfos(); const suggestions = linters.map((x) => x.id).sort(); const linterList = ['Disable Linting', ...suggestions]; - const activeLinters = await this.linterManager.getActiveLinters(true, this.settingsUri); + const activeLinters = await this.linterManager.getActiveLinters(this.settingsUri); let current: string; switch (activeLinters.length) { @@ -82,7 +82,7 @@ export class LinterCommands implements IDisposable { public async enableLintingAsync(): Promise { const options = ['Enable', 'Disable']; - const current = (await this.linterManager.isLintingEnabled(true, this.settingsUri)) ? options[0] : options[1]; + const current = (await this.linterManager.isLintingEnabled(this.settingsUri)) ? options[0] : options[1]; const quickPickOptions: QuickPickOptions = { matchOnDetail: true, diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index 912eea1cdf04..f946cde97f7c 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -3,8 +3,6 @@ import * as path from 'path'; import { Uri } from 'vscode'; -import { LanguageServerType } from '../activation/types'; -import { IWorkspaceService } from '../common/application/types'; import { ExecutionInfo, IConfigurationService, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; @@ -74,38 +72,3 @@ export class LinterInfo implements ILinterInfo { return { execPath, moduleName, args, product: this.product }; } } - -export class PylintLinterInfo extends LinterInfo { - constructor( - configService: IConfigurationService, - private readonly workspaceService: IWorkspaceService, - configFileNames: string[] = [], - ) { - super(Product.pylint, LinterId.PyLint, configService, configFileNames); - } - public isEnabled(resource?: Uri): boolean { - // We want to be sure the setting is not default since default is `true` and hence - // missing setting yields `true`. When setting is missing and LS is non-Jedi, - // we want default to be `false`. So inspection here makes sure we are not getting - // `true` because there is no setting and LS is active. - const enabled = super.isEnabled(resource); // Is it enabled by settings? - const usingJedi = this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi; - if (usingJedi) { - // In Jedi case adhere to default behavior. Missing setting means `enabled`. - return enabled; - } - // If we're using LS, then by default Pylint is disabled unless user provided - // the value. We have to resort to direct inspection of settings here. - const configuration = this.workspaceService.getConfiguration('python', resource); - const inspection = configuration.inspect(`linting.${this.enabledSettingName}`); - if ( - !inspection || - (inspection.globalValue === undefined && - inspection.workspaceFolderValue === undefined && - inspection.workspaceValue === undefined) - ) { - return false; - } - return enabled; - } -} diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts index 9bfba890094d..e8ec38c5ca08 100644 --- a/src/client/linters/linterManager.ts +++ b/src/client/linters/linterManager.ts @@ -5,20 +5,19 @@ import { inject, injectable } from 'inversify'; import { CancellationToken, OutputChannel, TextDocument, Uri } from 'vscode'; -import { IWorkspaceService } from '../common/application/types'; import { traceError } from '../common/logger'; import { IConfigurationService, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { Bandit } from './bandit'; import { Flake8 } from './flake8'; -import { LinterInfo, PylintLinterInfo } from './linterInfo'; +import { LinterInfo } from './linterInfo'; import { MyPy } from './mypy'; import { Prospector } from './prospector'; import { Pycodestyle } from './pycodestyle'; import { PyDocStyle } from './pydocstyle'; import { PyLama } from './pylama'; import { Pylint } from './pylint'; -import { IAvailableLinterActivator, ILinter, ILinterInfo, ILinterManager, ILintMessage, LinterId } from './types'; +import { ILinter, ILinterInfo, ILinterManager, ILintMessage, LinterId } from './types'; class DisabledLinter implements ILinter { constructor(private configService: IConfigurationService) {} @@ -33,19 +32,13 @@ class DisabledLinter implements ILinter { @injectable() export class LinterManager implements ILinterManager { protected linters: ILinterInfo[]; - private configService: IConfigurationService; - private checkedForInstalledLinters = new Set(); - constructor( - @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - ) { - this.configService = serviceContainer.get(IConfigurationService); + constructor(@inject(IConfigurationService) private configService: IConfigurationService) { // Note that we use unit tests to ensure all the linters are here. this.linters = [ new LinterInfo(Product.bandit, LinterId.Bandit, this.configService), new LinterInfo(Product.flake8, LinterId.Flake8, this.configService), - new PylintLinterInfo(this.configService, this.workspaceService, ['.pylintrc', 'pylintrc']), + new LinterInfo(Product.pylint, LinterId.PyLint, this.configService, ['pylintrc', '.pylintrc']), new LinterInfo(Product.mypy, LinterId.MyPy, this.configService), new LinterInfo(Product.pycodestyle, LinterId.PyCodeStyle, this.configService), new LinterInfo(Product.prospector, LinterId.Prospector, this.configService), @@ -66,9 +59,9 @@ export class LinterManager implements ILinterManager { throw new Error(`Invalid linter '${Product[product]}'`); } - public async isLintingEnabled(silent: boolean, resource?: Uri): Promise { + public async isLintingEnabled(resource?: Uri): Promise { const settings = this.configService.getSettings(resource); - const activeLintersPresent = await this.getActiveLinters(silent, resource); + const activeLintersPresent = await this.getActiveLinters(resource); return settings.linting.enabled && activeLintersPresent.length > 0; } @@ -76,10 +69,7 @@ export class LinterManager implements ILinterManager { await this.configService.updateSetting('linting.enabled', enable, resource); } - public async getActiveLinters(silent: boolean, resource?: Uri): Promise { - if (!silent) { - await this.enableUnconfiguredLinters(resource); - } + public async getActiveLinters(resource?: Uri): Promise { return this.linters.filter((x) => x.isEnabled(resource)); } @@ -93,7 +83,7 @@ export class LinterManager implements ILinterManager { // if we have valid linter product(s), enable only those if (validProducts.length > 0) { - const active = await this.getActiveLinters(true, resource); + const active = await this.getActiveLinters(resource); for (const x of active) { await x.enableAsync(false, resource); } @@ -113,7 +103,7 @@ export class LinterManager implements ILinterManager { serviceContainer: IServiceContainer, resource?: Uri, ): Promise { - if (!(await this.isLintingEnabled(true, resource))) { + if (!(await this.isLintingEnabled(resource))) { return new DisabledLinter(this.configService); } const error = 'Linter manager: Unknown linter'; @@ -140,22 +130,4 @@ export class LinterManager implements ILinterManager { } throw new Error(error); } - - protected async enableUnconfiguredLinters(resource?: Uri): Promise { - const settings = this.configService.getSettings(resource); - if (!settings.linting.pylintEnabled || !settings.linting.enabled) { - return; - } - // If we've already checked during this session for the same workspace and Python path, then don't bother again. - const workspaceKey = `${this.workspaceService.getWorkspaceFolderIdentifier(resource)}${settings.pythonPath}`; - if (this.checkedForInstalledLinters.has(workspaceKey)) { - return; - } - this.checkedForInstalledLinters.add(workspaceKey); - - // only check & ask the user if they'd like to enable pylint - const pylintInfo = this.linters.find((linter) => linter.id === 'pylint'); - const activator = this.serviceContainer.get(IAvailableLinterActivator); - await activator.promptIfLinterAvailable(pylintInfo!, resource); - } } diff --git a/src/client/linters/lintingEngine.ts b/src/client/linters/lintingEngine.ts index 00c7dab0ea08..c932a9e27882 100644 --- a/src/client/linters/lintingEngine.ts +++ b/src/client/linters/lintingEngine.ts @@ -90,7 +90,7 @@ export class LintingEngine implements ILintingEngine { this.pendingLintings.set(document.uri.fsPath, cancelToken); - const activeLinters = await this.linterManager.getActiveLinters(false, document.uri); + const activeLinters = await this.linterManager.getActiveLinters(document.uri); const promises: Promise[] = activeLinters.map(async (info: ILinterInfo) => { const stopWatch = new StopWatch(); const linter = await this.linterManager.createLinter( @@ -161,7 +161,7 @@ export class LintingEngine implements ILintingEngine { } private async shouldLintDocument(document: vscode.TextDocument): Promise { - if (!(await this.linterManager.isLintingEnabled(false, document.uri))) { + if (!(await this.linterManager.isLintingEnabled(document.uri))) { this.diagnosticCollection.set(document.uri, []); return false; } diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index 495787568509..10a213c2d631 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -1,171 +1,34 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as os from 'os'; -import * as path from 'path'; import { CancellationToken, OutputChannel, TextDocument } from 'vscode'; import '../common/extensions'; -import { IFileSystem, IPlatformService } from '../common/platform/types'; import { Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { BaseLinter } from './baseLinter'; import { ILintMessage } from './types'; -const pylintrc = 'pylintrc'; -const dotPylintrc = '.pylintrc'; - const REGEX = '(?\\d+),(?-?\\d+),(?\\w+),(?[\\w-]+):(?.*)\\r?(\\n|$)'; export class Pylint extends BaseLinter { - private fileSystem: IFileSystem; - private platformService: IPlatformService; - constructor(outputChannel: OutputChannel, serviceContainer: IServiceContainer) { super(Product.pylint, outputChannel, serviceContainer); - this.fileSystem = serviceContainer.get(IFileSystem); - this.platformService = serviceContainer.get(IPlatformService); } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - let minArgs: string[] = []; - // Only use minimal checkers if - // a) there are no custom arguments and - // b) there is no pylintrc file next to the file or at the workspace root const uri = document.uri; - const workspaceRoot = this.getWorkspaceRootPath(document); const settings = this.configService.getSettings(uri); - if ( - settings.linting.pylintUseMinimalCheckers && - this.info.linterArgs(uri).length === 0 && - // Check pylintrc next to the file or above up to and including the workspace root - !(await Pylint.hasConfigurationFileInWorkspace(this.fileSystem, path.dirname(uri.fsPath), workspaceRoot)) && - // Check for pylintrc at the cwd and above - !(await Pylint.hasConfigurationFile( - this.fileSystem, - this.getWorkingDirectoryPath(document), - this.platformService, - )) - ) { - // Disable all checkers up front and then selectively add back in: - // - All F checkers - // - Select W checkers - // - All E checkers _manually_ - // (see https://github.com/Microsoft/vscode-python/issues/722 for - // why; see - // https://gist.github.com/brettcannon/eff7f38a60af48d39814cbb2f33b3d1d - // for a script to regenerate the list of E checkers) - minArgs = [ - '--disable=all', - '--enable=F' + - ',unreachable,duplicate-key,unnecessary-semicolon' + - ',global-variable-not-assigned,unused-variable' + - ',unused-wildcard-import,binary-op-exception' + - ',bad-format-string,anomalous-backslash-in-string' + - ',bad-open-mode' + - ',E0001,E0011,E0012,E0100,E0101,E0102,E0103,E0104,E0105,E0107' + - ',E0108,E0110,E0111,E0112,E0113,E0114,E0115,E0116,E0117,E0118' + - ',E0202,E0203,E0211,E0213,E0236,E0237,E0238,E0239,E0240,E0241' + - ',E0301,E0302,E0303,E0401,E0402,E0601,E0602,E0603,E0604,E0611' + - ',E0632,E0633,E0701,E0702,E0703,E0704,E0710,E0711,E0712,E1003' + - ',E1101,E1102,E1111,E1120,E1121,E1123,E1124,E1125,E1126,E1127' + - ',E1128,E1129,E1130,E1131,E1132,E1133,E1134,E1135,E1136,E1137' + - ',E1138,E1139,E1200,E1201,E1205,E1206,E1300,E1301,E1302,E1303' + - ',E1304,E1305,E1306,E1310,E1700,E1701', - ]; - } const args = [ "--msg-template='{line},{column},{category},{symbol}:{msg}'", '--reports=n', '--output-format=text', uri.fsPath, ]; - const messages = await this.run(minArgs.concat(args), document, cancellation, REGEX); + const messages = await this.run(args, document, cancellation, REGEX); messages.forEach((msg) => { msg.severity = this.parseMessagesSeverity(msg.type, settings.linting.pylintCategorySeverity); }); return messages; } - - public static async hasConfigurationFile( - fs: IFileSystem, - folder: string, - platformService: IPlatformService, - ): Promise { - // https://pylint.readthedocs.io/en/latest/user_guide/run.html - // https://github.com/PyCQA/pylint/blob/975e08148c0faa79958b459303c47be1a2e1500a/pylint/config.py - // 1. pylintrc in the current working directory - // 2. .pylintrc in the current working directory - // 3. If the current working directory is in a Python module, Pylint searches - // up the hierarchy of Python modules until it finds a pylintrc file. - // This allows you to specify coding standards on a module by module basis. - // A directory is judged to be a Python module if it contains an __init__.py file. - // 4. The file named by environment variable PYLINTRC - // 5. if you have a home directory which isn’t /root: - // a) .pylintrc in your home directory - // b) .config/pylintrc in your home directory - // 6. /etc/pylintrc - if (process.env.PYLINTRC) { - return true; - } - - if ( - (await fs.fileExists(path.join(folder, pylintrc))) || - (await fs.fileExists(path.join(folder, dotPylintrc))) - ) { - return true; - } - - let current = folder; - let above = path.dirname(folder); - do { - if (!(await fs.fileExists(path.join(current, '__init__.py')))) { - break; - } - if ( - (await fs.fileExists(path.join(current, pylintrc))) || - (await fs.fileExists(path.join(current, dotPylintrc))) - ) { - return true; - } - current = above; - above = path.dirname(above); - } while (!fs.arePathsSame(current, above)); - - const home = os.homedir(); - if (await fs.fileExists(path.join(home, dotPylintrc))) { - return true; - } - if (await fs.fileExists(path.join(home, '.config', pylintrc))) { - return true; - } - - if (!platformService.isWindows) { - if (await fs.fileExists(path.join('/etc', pylintrc))) { - return true; - } - } - return false; - } - - public static async hasConfigurationFileInWorkspace( - fs: IFileSystem, - folder: string, - root: string, - ): Promise { - // Search up from file location to the workspace root - let current = folder; - let above = path.dirname(current); - do { - if ( - (await fs.fileExists(path.join(current, pylintrc))) || - (await fs.fileExists(path.join(current, dotPylintrc))) - ) { - return true; - } - current = above; - above = path.dirname(above); - } while (!fs.arePathsSame(current, root) && !fs.arePathsSame(current, above)); - return false; - } } diff --git a/src/client/linters/types.ts b/src/client/linters/types.ts index fb6d1d44e1ca..f8c48a50bef8 100644 --- a/src/client/linters/types.ts +++ b/src/client/linters/types.ts @@ -50,8 +50,8 @@ export const ILinterManager = Symbol('ILinterManager'); export interface ILinterManager { getAllLinterInfos(): ILinterInfo[]; getLinterInfo(product: Product): ILinterInfo; - getActiveLinters(silent: boolean, resource?: vscode.Uri): Promise; - isLintingEnabled(silent: boolean, resource?: vscode.Uri): Promise; + getActiveLinters(resource?: vscode.Uri): Promise; + isLintingEnabled(resource?: vscode.Uri): Promise; enableLintingAsync(enable: boolean, resource?: vscode.Uri): Promise; setActiveLintersAsync(products: Product[], resource?: vscode.Uri): Promise; createLinter( diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index 8bfde47e192b..8be6be069154 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -90,7 +90,7 @@ export class LinterProvider implements IExtensionActivationService, Disposable { } this.linterManager - .getActiveLinters(false, document.uri) + .getActiveLinters(document.uri) .then((linters) => { const fileName = path.basename(document.uri.fsPath).toLowerCase(); const watchers = linters.filter((info) => info.configFileNames.indexOf(fileName) >= 0); diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index 3d68e9e0284c..f8086cff4c3b 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -4,15 +4,7 @@ "python.workspaceSymbols.enabled": false, "python.testing.nosetestArgs": [], "python.testing.pytestArgs": [], - "python.testing.unittestArgs": [ - "-s=./tests", - "-p=test_*.py", - "-v", - "-s", - ".", - "-p", - "*test*.py" - ], + "python.testing.unittestArgs": ["-s=./tests", "-p=test_*.py", "-v", "-s", ".", "-p", "*test*.py"], "python.sortImports.args": [], "python.linting.lintOnSave": false, "python.linting.enabled": true, @@ -23,9 +15,9 @@ "python.linting.mypyEnabled": false, "python.linting.banditEnabled": false, "python.formatting.provider": "yapf", - "python.linting.pylintUseMinimalCheckers": false, // Do not set this to "Microsoft", else it will result in LS being downloaded on CI // and that slows down tests significantly. We have other tests on CI for testing // downloading of LS with this setting enabled. - "python.languageServer": "Jedi" + "python.languageServer": "Jedi", + "python.pythonPath": "C:\\GIT\\s p\\vscode-python\\.venv\\Scripts\\python.exe" } diff --git a/src/test/linters/common.ts b/src/test/linters/common.ts index 87a489cbbb31..793f2127611e 100644 --- a/src/test/linters/common.ts +++ b/src/test/linters/common.ts @@ -102,7 +102,6 @@ export class LintingSettings { public banditEnabled: boolean; public banditArgs: string[]; public banditPath: string; - public pylintUseMinimalCheckers: boolean; constructor() { // mostly from configSettings.ts @@ -164,7 +163,6 @@ export class LintingSettings { refactor: DiagnosticSeverity.Hint, warning: DiagnosticSeverity.Warning, }; - this.pylintUseMinimalCheckers = false; } } @@ -264,7 +262,7 @@ export class BaseTestFixture { // linting - this.linterManager = new LinterManager(this.serviceContainer.object, this.workspaceService.object!); + this.linterManager = new LinterManager(this.configService.object); this.serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILinterManager), TypeMoq.It.isAny())) .returns(() => this.linterManager); diff --git a/src/test/linters/lint.args.test.ts b/src/test/linters/lint.args.test.ts index 0e21a31a12e3..e398818db031 100644 --- a/src/test/linters/lint.args.test.ts +++ b/src/test/linters/lint.args.test.ts @@ -132,7 +132,7 @@ suite('Linting - Arguments', () => { const platformService = TypeMoq.Mock.ofType(); serviceManager.addSingletonInstance(IPlatformService, platformService.object); - lm = new LinterManager(serviceContainer, workspaceService.object); + lm = new LinterManager(configService.object); serviceManager.addSingletonInstance(ILinterManager, lm); document = TypeMoq.Mock.ofType(); }); diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index 37bf4330d31f..a34bfc3f5163 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -818,10 +818,6 @@ suite('Linting Functional Tests', () => { } const fixture = new TestFixture(); - if (product === Product.pydocstyle) { - fixture.lintingSettings.pylintUseMinimalCheckers = false; - } - const { filename, messagesToBeReceived, origRCFile } = await getInfoForConfig(product); let rcfile = ''; async function cleanUp() { diff --git a/src/test/linters/lint.manager.unit.test.ts b/src/test/linters/lint.manager.unit.test.ts deleted file mode 100644 index 6229c983b59f..000000000000 --- a/src/test/linters/lint.manager.unit.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -// 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 { IWorkspaceService } from '../../client/common/application/types'; -import { IConfigurationService, IPythonSettings } from '../../client/common/types'; -import { IServiceContainer } from '../../client/ioc/types'; -import { LinterManager } from '../../client/linters/linterManager'; - -const workspaceService = TypeMoq.Mock.ofType(); - -// setup class instance -class TestLinterManager extends LinterManager { - public enableUnconfiguredLintersCallCount: number = 0; - - protected async enableUnconfiguredLinters(_resource?: Uri): Promise { - this.enableUnconfiguredLintersCallCount += 1; - } -} - -function getServiceContainerMockForLinterManagerTests(): TypeMoq.IMock { - // setup test mocks - const serviceContainerMock = TypeMoq.Mock.ofType(); - - const pythonSettingsMock = TypeMoq.Mock.ofType(); - const configMock = TypeMoq.Mock.ofType(); - configMock.setup((cm) => cm.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettingsMock.object); - serviceContainerMock.setup((c) => c.get(IConfigurationService)).returns(() => configMock.object); - - const pythonConfig = { - inspect: () => {}, - }; - workspaceService - .setup((x) => x.getConfiguration('python', TypeMoq.It.isAny())) - - .returns(() => pythonConfig as any); - serviceContainerMock.setup((c) => c.get(IWorkspaceService)).returns(() => workspaceService.object); - - return serviceContainerMock; -} - -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, workspaceService.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, workspaceService.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, workspaceService.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, workspaceService.object); - await lm.getActiveLinters(silentFlag); - - // test expectations - expect(lm.enableUnconfiguredLintersCallCount).to.equal(expectedCallCount); - }); -}); diff --git a/src/test/linters/lint.multilinter.test.ts b/src/test/linters/lint.multilinter.test.ts index 881cccb4b413..b71478505530 100644 --- a/src/test/linters/lint.multilinter.test.ts +++ b/src/test/linters/lint.multilinter.test.ts @@ -17,7 +17,7 @@ import { deleteFile, IExtensionTestApi, PythonSettingKeys, rootWorkspaceUri } fr import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize'; const workspaceUri = Uri.file(path.join(__dirname, '..', '..', '..', 'src', 'test')); -const pythoFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'linting'); +const pythonFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'linting'); // Mocked out python tool execution (all we need is mocked linter return values). class MockPythonToolExecService extends PythonToolExecutionService { @@ -78,7 +78,6 @@ suite('Linting - Multiple Linters Enabled Test', () => { await configService.updateSetting('linting.enabled', true, rootWorkspaceUri, target); await configService.updateSetting('linting.lintOnSave', false, rootWorkspaceUri, target); - await configService.updateSetting('linting.pylintUseMinimalCheckers', false, workspaceUri); linterManager.getAllLinterInfos().forEach(async (x) => { await configService.updateSetting(makeSettingKey(x.product), false, rootWorkspaceUri, target); @@ -91,7 +90,7 @@ suite('Linting - Multiple Linters Enabled Test', () => { test('Multiple linters', async () => { await closeActiveWindows(); - const document = await workspace.openTextDocument(path.join(pythoFilesPath, 'print.py')); + const document = await workspace.openTextDocument(path.join(pythonFilesPath, 'print.py')); await window.showTextDocument(document); await configService.updateSetting( 'languageServer', @@ -100,7 +99,6 @@ suite('Linting - Multiple Linters Enabled Test', () => { ConfigurationTarget.Workspace, ); await configService.updateSetting('linting.enabled', true, workspaceUri); - await configService.updateSetting('linting.pylintUseMinimalCheckers', false, workspaceUri); await configService.updateSetting('linting.pylintEnabled', true, workspaceUri); await configService.updateSetting('linting.flake8Enabled', true, workspaceUri); diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 7600f59d6c9e..5d4fd019e8bc 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -118,7 +118,7 @@ suite('Linting - Provider', () => { ICommandManager, TypeMoq.Mock.ofType().object, ); - lm = new LinterManager(serviceContainer, workspaceService.object); + lm = new LinterManager(configService.object); serviceManager.addSingletonInstance(ILinterManager, lm); emitter = new vscode.EventEmitter(); document = TypeMoq.Mock.ofType(); diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index a4d655bdda98..c9e1826030ea 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -3,9 +3,7 @@ 'use strict'; import * as assert from 'assert'; -import * as path from 'path'; -import { ConfigurationTarget, Uri } from 'vscode'; -import { WorkspaceService } from '../../client/common/application/workspace'; +import { ConfigurationTarget } from 'vscode'; import { Product } from '../../client/common/installer/productInstaller'; import { FormatterProductPathService, @@ -22,9 +20,6 @@ import { rootWorkspaceUri } from '../common'; import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; -const workspaceDir = path.join(__dirname, '..', '..', '..', 'src', 'test'); -const workspaceUri = Uri.file(workspaceDir); - suite('Linting Settings', () => { let ioc: UnitTestIocContainer; let linterManager: ILinterManager; @@ -56,8 +51,8 @@ suite('Linting Settings', () => { ioc.registerLinterTypes(); ioc.registerVariableTypes(); ioc.registerPlatformTypes(); - linterManager = new LinterManager(ioc.serviceContainer, new WorkspaceService()); configService = ioc.serviceContainer.get(IConfigurationService); + linterManager = new LinterManager(configService); ioc.serviceManager.addSingletonInstance(IProductService, new ProductService()); ioc.serviceManager.addSingleton( IProductPathService, @@ -82,7 +77,6 @@ suite('Linting Settings', () => { await configService.updateSetting('linting.enabled', lintingEnabled, rootWorkspaceUri, target); await configService.updateSetting('linting.lintOnSave', false, rootWorkspaceUri, target); - await configService.updateSetting('linting.pylintUseMinimalCheckers', false, workspaceUri); linterManager.getAllLinterInfos().forEach(async (x) => { const settingKey = `linting.${x.enabledSettingName}`; diff --git a/src/test/linters/lint.unit.test.ts b/src/test/linters/lint.unit.test.ts index db16a33c54eb..2caeb7d358d0 100644 --- a/src/test/linters/lint.unit.test.ts +++ b/src/test/linters/lint.unit.test.ts @@ -711,7 +711,6 @@ suite('Linting Scenarios', () => { useMinimal ? '' : 'out' } minimal checkers`, async () => { const fixture = new TestFixture(); - fixture.lintingSettings.pylintUseMinimalCheckers = useMinimal; await testEnablingDisablingOfLinter(fixture, Product.pylint, enabled); }); } diff --git a/src/test/linters/lintengine.test.ts b/src/test/linters/lintengine.test.ts index 79f3d4badc4e..961d298bd444 100644 --- a/src/test/linters/lintengine.test.ts +++ b/src/test/linters/lintengine.test.ts @@ -73,10 +73,7 @@ suite('Linting - LintingEngine', () => { try { lintingEngine.lintDocument(doc, 'auto').ignoreErrors(); } catch { - lintManager.verify( - (l) => l.isLintingEnabled(TypeMoq.It.isAny(), TypeMoq.It.isValue(doc.uri)), - TypeMoq.Times.once(), - ); + lintManager.verify((l) => l.isLintingEnabled(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 index 86287c4e9f56..5325da6110d5 100644 --- a/src/test/linters/linter.availability.unit.test.ts +++ b/src/test/linters/linter.availability.unit.test.ts @@ -12,7 +12,6 @@ import { LanguageServerType } from '../../client/activation/types'; import { ApplicationShell } from '../../client/common/application/applicationShell'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { WorkspaceService } from '../../client/common/application/workspace'; -import { ConfigurationService } from '../../client/common/configuration/service'; import { PersistentStateFactory } from '../../client/common/persistentState'; import { FileSystem } from '../../client/common/platform/fileSystem'; import { IFileSystem } from '../../client/common/platform/types'; @@ -29,68 +28,6 @@ import { LinterInfo } from '../../client/linters/linterInfo'; import { IAvailableLinterActivator, ILinterInfo, LinterId } from '../../client/linters/types'; suite('Linter Availability Provider tests', () => { - test('Availability feature is disabled when global default for languageServer === Jedi.', async () => { - // set expectations - const languageServerValue = LanguageServerType.Jedi; - const expectedResult = false; - - // arrange - const [ - appShellMock, - fsMock, - workspaceServiceMock, - configServiceMock, - factoryMock, - ] = getDependenciesForAvailabilityTests(); - setupConfigurationServiceForJediSettingsTest(languageServerValue, configServiceMock); - - // call - const availabilityProvider = new AvailableLinterActivator( - appShellMock.object, - fsMock.object, - workspaceServiceMock.object, - configServiceMock.object, - factoryMock.object, - ); - - // check expectaions - expect(availabilityProvider.isFeatureEnabled).is.equal( - expectedResult, - 'Avaialability feature should be disabled when python.languageServer is Jedi', - ); - workspaceServiceMock.verifyAll(); - }); - - test('Availability feature is enabled when global default for languageServer is Microsoft.', async () => { - // set expectations - const languageServerValue = LanguageServerType.Microsoft; - const expectedResult = true; - - // arrange - const [ - appShellMock, - fsMock, - workspaceServiceMock, - configServiceMock, - factoryMock, - ] = getDependenciesForAvailabilityTests(); - setupConfigurationServiceForJediSettingsTest(languageServerValue, configServiceMock); - - const availabilityProvider = new AvailableLinterActivator( - appShellMock.object, - fsMock.object, - workspaceServiceMock.object, - configServiceMock.object, - factoryMock.object, - ); - - expect(availabilityProvider.isFeatureEnabled).is.equal( - expectedResult, - 'Avaialability feature should be enabled when python.languageServer defaults to non-Jedi', - ); - 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; @@ -102,7 +39,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -117,7 +53,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); @@ -138,7 +73,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -153,7 +87,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); @@ -177,7 +110,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -191,7 +123,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); @@ -215,7 +146,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -229,7 +159,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); @@ -246,7 +175,7 @@ suite('Linter Availability Provider tests', () => { promptEnabled = true, ): Promise { // arrange - const [appShellMock, fsMock, workspaceServiceMock, , factoryMock] = getDependenciesForAvailabilityTests(); + const [appShellMock, fsMock, workspaceServiceMock, factoryMock] = getDependenciesForAvailabilityTests(); const configServiceMock = TypeMoq.Mock.ofType(); const linterInfo = new (class extends LinterInfo { @@ -302,7 +231,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); const result = await availabilityProvider.promptToConfigureAvailableLinter(linterInfo); @@ -399,7 +327,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -435,6 +362,7 @@ suite('Linter Availability Provider tests', () => { .returns(async () => options.linterIsInstalled) .verifiable(TypeMoq.Times.atLeastOnce()); + const configServiceMock = TypeMoq.Mock.ofType(); setupConfigurationServiceForJediSettingsTest(options.languageServerValue, configServiceMock); setupWorkspaceMockForLinterConfiguredTests( options.pylintUserEnabled, @@ -453,7 +381,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); return availabilityProvider.promptIfLinterAvailable(linterInfo); @@ -591,7 +518,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -602,7 +528,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); const result = await availabilityProvider.isLinterAvailable(linterInfo, undefined); @@ -625,7 +550,6 @@ suite('Linter Availability Provider tests', () => { appShellMock, fsMock, workspaceServiceMock, - configServiceMock, factoryMock, linterInfo, ] = getDependenciesForAvailabilityTests(); @@ -636,7 +560,6 @@ suite('Linter Availability Provider tests', () => { appShellMock.object, fsMock.object, workspaceServiceMock.object, - configServiceMock.object, factoryMock.object, ); const result = await availabilityProvider.isLinterAvailable(linterInfo, undefined); @@ -667,7 +590,6 @@ suite('Linter Availability Provider tests', () => { instance(mock(ApplicationShell)), instance(fs), instance(workspaceService), - instance(mock(ConfigurationService)), instance(mock(PersistentStateFactory)), ); }); @@ -846,7 +768,6 @@ function getDependenciesForAvailabilityTests(): [ TypeMoq.IMock, TypeMoq.IMock, TypeMoq.IMock, - TypeMoq.IMock, TypeMoq.IMock, LinterInfo, ] { @@ -855,7 +776,6 @@ function getDependenciesForAvailabilityTests(): [ TypeMoq.Mock.ofType(), TypeMoq.Mock.ofType(), TypeMoq.Mock.ofType(), - TypeMoq.Mock.ofType(), TypeMoq.Mock.ofType(), new LinterInfo(Product.pylint, LinterId.PyLint, configServiceMock.object, ['.pylintrc', 'pylintrc']), ]; diff --git a/src/test/linters/linterCommands.unit.test.ts b/src/test/linters/linterCommands.unit.test.ts index cd345db31ba1..794bbbfa3587 100644 --- a/src/test/linters/linterCommands.unit.test.ts +++ b/src/test/linters/linterCommands.unit.test.ts @@ -57,7 +57,7 @@ suite('Linting - Linter Commands', () => { currentState: boolean, selectedState: 'Enable' | 'Disable' | undefined, ) { - when(manager.isLintingEnabled(true, anything())).thenResolve(currentState); + when(manager.isLintingEnabled(anything())).thenResolve(currentState); const expectedQuickPickOptions = { matchOnDetail: true, matchOnDescription: true, @@ -101,7 +101,7 @@ suite('Linting - Linter Commands', () => { test('Set Linter should display a quickpick', async () => { when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(true, anything())).thenResolve([]); + when(manager.getActiveLinters(anything())).thenResolve([]); when(shell.showQuickPick(anything(), anything())).thenResolve(); const expectedQuickPickOptions = { matchOnDetail: true, @@ -120,7 +120,7 @@ suite('Linting - Linter Commands', () => { const linterId = 'Hello World'; const activeLinters: ILinterInfo[] = [{ id: linterId } as any]; when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(true, anything())).thenResolve(activeLinters); + when(manager.getActiveLinters(anything())).thenResolve(activeLinters); when(shell.showQuickPick(anything(), anything())).thenResolve(); const expectedQuickPickOptions = { matchOnDetail: true, @@ -138,7 +138,7 @@ suite('Linting - Linter Commands', () => { test('Set Linter should display a quickpick and with message about multiple linters being enabled', async () => { const activeLinters: ILinterInfo[] = [{ id: 'linterId' } as any, { id: 'linterId2' } as any]; when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(true, anything())).thenResolve(activeLinters); + when(manager.getActiveLinters(anything())).thenResolve(activeLinters); when(shell.showQuickPick(anything(), anything())).thenResolve(); const expectedQuickPickOptions = { matchOnDetail: true, @@ -157,7 +157,7 @@ suite('Linting - Linter Commands', () => { const linters: ILinterInfo[] = [{ id: '1' }, { id: '2' }, { id: '3', product: 'Three' }] as any; const activeLinters: ILinterInfo[] = [{ id: '1' }, { id: '3' }] as any; when(manager.getAllLinterInfos()).thenReturn(linters); - when(manager.getActiveLinters(true, anything())).thenResolve(activeLinters); + when(manager.getActiveLinters(anything())).thenResolve(activeLinters); when(shell.showQuickPick(anything(), anything())).thenResolve('3' as any); when(shell.showWarningMessage(anything(), 'Yes', 'No')).thenResolve('Yes' as any); const expectedQuickPickOptions = { diff --git a/src/test/linters/linterManager.unit.test.ts b/src/test/linters/linterManager.unit.test.ts index d2d9babb8187..9fe79c87887e 100644 --- a/src/test/linters/linterManager.unit.test.ts +++ b/src/test/linters/linterManager.unit.test.ts @@ -39,9 +39,6 @@ suite('Linting - Linter Manager', () => { class LinterManagerTest extends LinterManager { // Override base class property to make it public. public linters!: ILinterInfo[]; - public async enableUnconfiguredLinters(resource?: Uri) { - await super.enableUnconfiguredLinters(resource); - } } setup(() => { const svcContainer = mock(ServiceContainer); @@ -57,7 +54,7 @@ suite('Linting - Linter Manager', () => { when(svcContainer.get(ILintingEngine)).thenReturn(instance(lintingEngine)); when(svcContainer.get(IConfigurationService)).thenReturn(instance(configService)); when(svcContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); - linterManager = new LinterManagerTest(instance(svcContainer), instance(workspaceService)); + linterManager = new LinterManagerTest(instance(configService)); }); test('Get all linters will return a list of all linters', () => { @@ -109,27 +106,11 @@ suite('Linting - Linter Manager', () => { linterManager.linters = [instanceOfLinterInfo]; when(linterInfo.isEnabled(resource)).thenReturn(true); - const linters = await linterManager.getActiveLinters(true, resource); + const linters = await linterManager.getActiveLinters(resource); verify(linterInfo.isEnabled(resource)).once(); expect(linters[0]).to.deep.equal(instanceOfLinterInfo); }); - test(`getActiveLinters will check if linter is enabled and not in silent mode ${testResourceSuffix}`, async () => { - const linterInfo = mock(LinterInfo); - const instanceOfLinterInfo = instance(linterInfo); - linterManager.linters = [instanceOfLinterInfo]; - when(linterInfo.isEnabled(resource)).thenReturn(true); - let enableUnconfiguredLintersInvoked = false; - linterManager.enableUnconfiguredLinters = async () => { - enableUnconfiguredLintersInvoked = true; - }; - - const linters = await linterManager.getActiveLinters(false, resource); - - verify(linterInfo.isEnabled(resource)).once(); - expect(linters[0]).to.deep.equal(instanceOfLinterInfo); - expect(enableUnconfiguredLintersInvoked).to.equal(true, 'not invoked'); - }); test(`setActiveLintersAsync with invalid products does nothing ${testResourceSuffix}`, async () => { let getActiveLintersInvoked = false; diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts deleted file mode 100644 index 31c3b4d18499..000000000000 --- a/src/test/linters/linterinfo.unit.test.ts +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { expect } from 'chai'; -import * as sinon from 'sinon'; -import { anything, instance, mock, when } from 'ts-mockito'; -import { LanguageServerType } from '../../client/activation/types'; -import { WorkspaceService } from '../../client/common/application/workspace'; -import { ConfigurationService } from '../../client/common/configuration/service'; -import { PylintLinterInfo } from '../../client/linters/linterInfo'; - -suite('Linter Info - Pylint', () => { - const workspace = mock(WorkspaceService); - const config = mock(ConfigurationService); - - test('Test disabled when Pylint is explicitly disabled', async () => { - const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: false }, - languageServer: LanguageServerType.Jedi, - } as any); - - expect(linterInfo.isEnabled()).to.be.false; - }); - test('Test disabled when Jedi is enabled and Pylint is explicitly disabled', async () => { - const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: false }, - languageServer: LanguageServerType.Jedi, - } as any); - - expect(linterInfo.isEnabled()).to.be.false; - }); - test('Test enabled when Jedi is enabled and Pylint is explicitly enabled', async () => { - const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Jedi, - } as any); - - expect(linterInfo.isEnabled()).to.be.true; - }); - test('Test disabled when using language server and Pylint is not configured', async () => { - const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Microsoft, - } as any); - - const pythonConfig = { - inspect: () => {}, - }; - when(workspace.getConfiguration('python', anything())).thenReturn(pythonConfig as any); - - expect(linterInfo.isEnabled()).to.be.false; - }); - test('Should inspect the value of linting.pylintEnabled when using language server', async () => { - const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - const inspectStub = sinon.stub(); - const pythonConfig = { - inspect: inspectStub, - }; - - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Microsoft, - } as any); - when(workspace.getConfiguration('python', anything())).thenReturn(pythonConfig as any); - - expect(linterInfo.isEnabled()).to.be.false; - expect(inspectStub.calledOnceWith('linting.pylintEnabled')).to.be.true; - }); - const testsForisEnabled = [ - { - testName: 'When workspaceFolder setting is provided', - inspection: { workspaceFolderValue: true }, - }, - { - testName: 'When workspace setting is provided', - inspection: { workspaceValue: true }, - }, - { - testName: 'When global setting is provided', - inspection: { globalValue: true }, - }, - ]; - - suite('Test is enabled when using Language Server and Pylint is configured', () => { - testsForisEnabled.forEach((testParams) => { - test(testParams.testName, async () => { - const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); - - const pythonConfig = { - inspect: () => testParams.inspection, - }; - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Microsoft, - } as any); - when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); - - expect(linterInfo.isEnabled()).to.be.true; - }); - }); - }); -}); diff --git a/src/test/linters/pylint.test.ts b/src/test/linters/pylint.test.ts index d5ddc79ee864..0973df35d854 100644 --- a/src/test/linters/pylint.test.ts +++ b/src/test/linters/pylint.test.ts @@ -19,7 +19,7 @@ import { LanguageServerType } from '../../client/activation/types'; import { IWorkspaceService } from '../../client/common/application/types'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IPythonToolExecutionService } from '../../client/common/process/types'; -import { ExecutionInfo, IConfigurationService, IInstaller, IPythonSettings } from '../../client/common/types'; +import { IConfigurationService, IInstaller, IPythonSettings } from '../../client/common/types'; import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService, @@ -33,10 +33,6 @@ import { MockLintingSettings } from '../mockClasses'; import { MockAutoSelectionService } from '../mocks/autoSelector'; suite('Linting - Pylint', () => { - const basePath = '/user/a/b/c/d'; - const pylintrc = 'pylintrc'; - const dotPylintrc = '.pylintrc'; - let fileSystem: TypeMoq.IMock; let platformService: TypeMoq.IMock; let workspace: TypeMoq.IMock; @@ -88,144 +84,12 @@ suite('Linting - Pylint', () => { workspace.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); serviceManager.addSingletonInstance(IConfigurationService, config.object); - const linterManager = new LinterManager(serviceContainer, workspace.object); + const linterManager = new LinterManager(config.object); serviceManager.addSingletonInstance(ILinterManager, linterManager); const installer = TypeMoq.Mock.ofType(); serviceManager.addSingletonInstance(IInstaller, installer.object); }); - test('pylintrc in the file folder', async () => { - fileSystem.setup((x) => x.fileExists(path.join(basePath, pylintrc))).returns(() => Promise.resolve(true)); - let result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${pylintrc}' not detected in the file folder.`); - - fileSystem.setup((x) => x.fileExists(path.join(basePath, dotPylintrc))).returns(() => Promise.resolve(true)); - result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the file folder.`); - }); - test('pylintrc up the module tree', async () => { - const module1 = path.join('/user/a/b/c/d', '__init__.py'); - const module2 = path.join('/user/a/b/c', '__init__.py'); - const module3 = path.join('/user/a/b', '__init__.py'); - const rc = path.join('/user/a/b/c', pylintrc); - - fileSystem.setup((x) => x.fileExists(module1)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(module2)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(module3)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(rc)).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${pylintrc}' not detected in the module tree.`); - }); - test('.pylintrc up the module tree', async () => { - // Don't use path.join since it will use / on Travis and Mac - const module1 = path.join('/user/a/b/c/d', '__init__.py'); - const module2 = path.join('/user/a/b/c', '__init__.py'); - const module3 = path.join('/user/a/b', '__init__.py'); - const rc = path.join('/user/a/b/c', pylintrc); - - fileSystem.setup((x) => x.fileExists(module1)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(module2)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(module3)).returns(() => Promise.resolve(true)); - fileSystem.setup((x) => x.fileExists(rc)).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the module tree.`); - }); - test('.pylintrc up the ~ folder', async () => { - const home = os.homedir(); - const rc = path.join(home, dotPylintrc); - fileSystem.setup((x) => x.fileExists(rc)).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the ~ folder.`); - }); - test('pylintrc up the ~/.config folder', async () => { - const home = os.homedir(); - const rc = path.join(home, '.config', pylintrc); - fileSystem.setup((x) => x.fileExists(rc)).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${pylintrc}' not detected in the ~/.config folder.`); - }); - test('pylintrc in the /etc folder', async () => { - const rc = path.join('/etc', pylintrc); - fileSystem.setup((x) => x.fileExists(rc)).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); - expect(result).to.be.equal(true, `'${pylintrc}' not detected in the /etc folder.`); - }); - test('pylintrc between file and workspace root', async () => { - const root = '/user/a'; - const midFolder = '/user/a/b'; - fileSystem.setup((x) => x.fileExists(path.join(midFolder, pylintrc))).returns(() => Promise.resolve(true)); - - const result = await Pylint.hasConfigurationFileInWorkspace(fileSystem.object, basePath, root); - expect(result).to.be.equal(true, `'${pylintrc}' not detected in the workspace tree.`); - }); - - test('minArgs - pylintrc between the file and the workspace root', async () => { - fileSystem.setup((x) => x.fileExists(path.join('/user/a/b', pylintrc))).returns(() => Promise.resolve(true)); - - await testPylintArguments('/user/a/b/c', '/user/a', false); - }); - - test('minArgs - no pylintrc between the file and the workspace root', async () => { - await testPylintArguments('/user/a/b/c', '/user/a', true); - }); - - test('minArgs - pylintrc next to the file', async () => { - const fileFolder = '/user/a/b/c'; - fileSystem.setup((x) => x.fileExists(path.join(fileFolder, pylintrc))).returns(() => Promise.resolve(true)); - - await testPylintArguments(fileFolder, '/user/a', false); - }); - - test('minArgs - pylintrc at the workspace root', async () => { - const root = '/user/a'; - fileSystem.setup((x) => x.fileExists(path.join(root, pylintrc))).returns(() => Promise.resolve(true)); - - await testPylintArguments('/user/a/b/c', root, false); - }); - - async function testPylintArguments(fileFolder: string, wsRoot: string, expectedMinArgs: boolean): Promise { - const outputChannel = TypeMoq.Mock.ofType(); - const pylinter = new Pylint(outputChannel.object, serviceContainer); - - const document = TypeMoq.Mock.ofType(); - document.setup((x) => x.uri).returns(() => Uri.file(path.join(fileFolder, 'test.py'))); - - const wsf = TypeMoq.Mock.ofType(); - wsf.setup((x) => x.uri).returns(() => Uri.file(wsRoot)); - - workspace.setup((x) => x.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => wsf.object); - - let execInfo: ExecutionInfo | undefined; - execService - .setup((x) => x.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .callback((e: ExecutionInfo) => { - execInfo = e; - }) - .returns(() => Promise.resolve({ stdout: '', stderr: '' })); - - const lintSettings = new MockLintingSettings(); - lintSettings.pylintUseMinimalCheckers = true; - - lintSettings.pylintPath = 'pyLint'; - - lintSettings.pylintEnabled = true; - - const settings = TypeMoq.Mock.ofType(); - settings.setup((x) => x.linting).returns(() => lintSettings); - settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Jedi); - config.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); - - await pylinter.lint(document.object, new CancellationTokenSource().token); - expect( - execInfo!.args.findIndex((x) => x.indexOf('--disable=all') >= 0), - 'Minimal args passed to pylint while pylintrc exists.', - ).to.be.eq(expectedMinArgs ? 0 : -1); - } test('Negative column numbers should be treated 0', async () => { const fileFolder = '/user/a/b/c'; const outputChannel = TypeMoq.Mock.ofType(); @@ -250,7 +114,6 @@ suite('Linting - Pylint', () => { .returns(() => Promise.resolve({ stdout: linterOutput, stderr: '' })); const lintSettings = new MockLintingSettings(); - lintSettings.pylintUseMinimalCheckers = false; lintSettings.maxNumberOfProblems = 1000; lintSettings.pylintPath = 'pyLint'; lintSettings.pylintEnabled = true; diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index a642d7400f00..46321f9787e2 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -1,296 +1,18 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { assert, expect } from 'chai'; -import * as os from 'os'; -import * as path from 'path'; +import { assert } from 'chai'; import * as sinon from 'sinon'; import { mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; -import { PlatformService } from '../../client/common/platform/platformService'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IConfigurationService, IOutputChannel } from '../../client/common/types'; import { IServiceContainer } from '../../client/ioc/types'; import { Pylint } from '../../client/linters/pylint'; import { ILinterInfo, ILinterManager, ILintMessage, LintMessageSeverity } from '../../client/linters/types'; -suite('Pylint - Function hasConfigurationFile()', () => { - const folder = path.join('user', 'a', 'b', 'c', 'd'); - const oldValueOfPYLINTRC = process.env.PYLINTRC; - const pylintrcFiles = ['pylintrc', '.pylintrc']; - const pylintrc = 'pylintrc'; - const dotPylintrc = '.pylintrc'; - let fileSystem: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let serviceContainer: TypeMoq.IMock; - - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); - fileSystem = TypeMoq.Mock.ofType(); - fileSystem - .setup((x) => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) - .returns((a, b) => a === b); - - platformService = TypeMoq.Mock.ofType(); - }); - - teardown(() => { - if (oldValueOfPYLINTRC === undefined) { - delete process.env.PYLINTRC; - } else { - process.env.PYLINTRC = oldValueOfPYLINTRC; - } - }); - - pylintrcFiles.forEach((pylintrcFile) => { - test(`If ${pylintrcFile} exists in the current working directory, return true`, async () => { - fileSystem - .setup((x) => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(pylintrc === pylintrcFile)); - fileSystem - .setup((x) => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - }); - - test(`If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a ${pylintrcFile} file. And if ${pylintrcFile} exists, return true`, async () => { - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), '__init__.py'))) - .returns(() => Promise.resolve(true)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.atLeastOnce()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.atLeastOnce()); - - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), '__init__.py'))) - .returns(() => Promise.resolve(true)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) - .returns(() => Promise.resolve(pylintrc === pylintrcFile)); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) - .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); - - test(`If ${pylintrcFile} exists in the home directory, return true`, async () => { - const home = os.homedir(); - fileSystem.setup((x) => x.fileExists(path.join(folder, pylintrc))).returns(() => Promise.resolve(false)); - fileSystem.setup((x) => x.fileExists(path.join(folder, dotPylintrc))).returns(() => Promise.resolve(false)); - fileSystem - .setup((x) => x.fileExists(path.join(folder, '__init__.py'))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(home, '.config', pylintrc))) - .returns(() => Promise.resolve(pylintrc === pylintrcFile)); - fileSystem - .setup((x) => x.fileExists(path.join(home, dotPylintrc))) - .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); - }); - - test('If /etc/pylintrc exists in non-Windows platform, return true', async function () { - if (new PlatformService().isWindows) { - return this.skip(); - } - const home = os.homedir(); - fileSystem - .setup((x) => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(folder, '__init__.py'))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(home, '.config', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(home, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - platformService.setup((x) => x.isWindows).returns(() => false); - fileSystem.setup((x) => x.fileExists(path.join('/etc', pylintrc))).returns(() => Promise.resolve(true)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); - - test('If none of the pylintrc configuration files exist anywhere, return false', async () => { - const home = os.homedir(); - fileSystem - .setup((x) => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(folder, '__init__.py'))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(home, '.config', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(home, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - platformService - .setup((x) => x.isWindows) - .returns(() => false) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join('/etc', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(false, 'Should return false'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); - - test('If process.env.PYLINTRC contains the path to pylintrc, return true', async () => { - process.env.PYLINTRC = path.join('path', 'to', 'pylintrc'); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - }); -}); - -suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { - const pylintrc = 'pylintrc'; - const dotPylintrc = '.pylintrc'; - let fileSystem: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let serviceContainer: TypeMoq.IMock; - - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); - fileSystem = TypeMoq.Mock.ofType(); - fileSystem - .setup((x) => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) - .returns((a, b) => a === b); - - platformService = TypeMoq.Mock.ofType(); - }); - - test('If none of the pylintrc files exist up to the workspace root, return false', async () => { - const folder = path.join('user', 'a', 'b', 'c'); - const root = path.join('user', 'a'); - - const rootPathItems = ['user', 'a']; - const folderPathItems = ['b', 'c']; // full folder path will be prefixed by root path - let rootPath = ''; - rootPathItems.forEach((item) => { - rootPath = path.join(rootPath, item); - fileSystem - .setup((x) => x.fileExists(path.join(rootPath, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup((x) => x.fileExists(path.join(rootPath, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - }); - let relativeFolderPath = ''; - folderPathItems.forEach((item) => { - relativeFolderPath = path.join(relativeFolderPath, item); - const absoluteFolderPath = path.join(rootPath, relativeFolderPath); - fileSystem - .setup((x) => x.fileExists(path.join(absoluteFolderPath, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(absoluteFolderPath, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - }); - - const hasConfig = await Pylint.hasConfigurationFileInWorkspace(fileSystem.object, folder, root); - expect(hasConfig).to.equal(false, 'Should return false'); - fileSystem.verifyAll(); - }); - - [pylintrc, dotPylintrc].forEach((pylintrcFile) => { - test(`If ${pylintrcFile} exists while traversing up to the workspace root, return true`, async () => { - const folder = path.join('user', 'a', 'b', 'c'); - const root = path.join('user', 'a'); - - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b'), pylintrc))) - .returns(() => Promise.resolve(pylintrc === pylintrcFile)); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a', 'b'), dotPylintrc))) - .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user', 'a'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup((x) => x.fileExists(path.join(path.join('user'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - - const hasConfig = await Pylint.hasConfigurationFileInWorkspace(fileSystem.object, folder, root); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - }); - }); -}); - suite('Pylint - Function runLinter()', () => { let fileSystem: TypeMoq.IMock; let serviceContainer: TypeMoq.IMock; @@ -302,24 +24,6 @@ suite('Pylint - Function runLinter()', () => { let platformService: TypeMoq.IMock; let run: sinon.SinonStub; let parseMessagesSeverity: sinon.SinonStub; - const minArgs = [ - '--disable=all', - '--enable=F' + - ',unreachable,duplicate-key,unnecessary-semicolon' + - ',global-variable-not-assigned,unused-variable' + - ',unused-wildcard-import,binary-op-exception' + - ',bad-format-string,anomalous-backslash-in-string' + - ',bad-open-mode' + - ',E0001,E0011,E0012,E0100,E0101,E0102,E0103,E0104,E0105,E0107' + - ',E0108,E0110,E0111,E0112,E0113,E0114,E0115,E0116,E0117,E0118' + - ',E0202,E0203,E0211,E0213,E0236,E0237,E0238,E0239,E0240,E0241' + - ',E0301,E0302,E0303,E0401,E0402,E0601,E0602,E0603,E0604,E0611' + - ',E0632,E0633,E0701,E0702,E0703,E0704,E0710,E0711,E0712,E1003' + - ',E1101,E1102,E1111,E1120,E1121,E1123,E1124,E1125,E1126,E1127' + - ',E1128,E1129,E1130,E1131,E1132,E1133,E1134,E1135,E1136,E1137' + - ',E1138,E1139,E1200,E1201,E1205,E1206,E1300,E1301,E1302,E1303' + - ',E1304,E1305,E1306,E1310,E1700,E1701', - ]; const doc = { uri: vscode.Uri.file('path/to/doc'), }; @@ -329,9 +33,6 @@ suite('Pylint - Function runLinter()', () => { '--output-format=text', doc.uri.fsPath, ]; - const original_hasConfigurationFileInWorkspace = Pylint.hasConfigurationFileInWorkspace; - const original_hasConfigurationFile = Pylint.hasConfigurationFile; - class PylintTest extends Pylint { public async run( _args: string[], @@ -386,105 +87,17 @@ suite('Pylint - Function runLinter()', () => { }); teardown(() => { - Pylint.hasConfigurationFileInWorkspace = original_hasConfigurationFileInWorkspace; - Pylint.hasConfigurationFile = original_hasConfigurationFile; sinon.restore(); }); - test('Use minimal checkers if a) setting to use minimal checkers is true, b) there are no custom arguments and c) there is no pylintrc file next to the file or at the workspace root and above', async () => { - const settings = { - linting: { - pylintUseMinimalCheckers: true, - }, - }; - configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); - _info.setup((info) => info.linterArgs(doc.uri)).returns(() => []); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(false); - Pylint.hasConfigurationFile = () => Promise.resolve(false); - run = sinon.stub(PylintTest.prototype, 'run'); - run.callsFake(() => Promise.resolve([])); - parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); - parseMessagesSeverity.callsFake(() => 'Severity'); - const pylint = new PylintTest(output.object, serviceContainer.object); - await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); - assert.deepEqual(run.args[0][0], minArgs.concat(args)); - assert.ok(parseMessagesSeverity.notCalled); - assert.ok(run.calledOnce); - }); - - test('Do not use minimal checkers if setting to use minimal checkers is false', async () => { - const settings = { - linting: { - pylintUseMinimalCheckers: false, - }, - }; - configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); - _info.setup((info) => info.linterArgs(doc.uri)).returns(() => []); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(false); - Pylint.hasConfigurationFile = () => Promise.resolve(false); - run = sinon.stub(PylintTest.prototype, 'run'); - run.callsFake(() => Promise.resolve([])); - parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); - parseMessagesSeverity.callsFake(() => 'Severity'); - const pylint = new PylintTest(output.object, serviceContainer.object); - await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); - assert.deepEqual(run.args[0][0], args); - assert.ok(parseMessagesSeverity.notCalled); - assert.ok(run.calledOnce); - }); - - test('Do not use minimal checkers if there are custom arguments', async () => { - const settings = { - linting: { - pylintUseMinimalCheckers: true, - }, - }; - configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); - _info.setup((info) => info.linterArgs(doc.uri)).returns(() => ['customArg1', 'customArg2']); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(false); - Pylint.hasConfigurationFile = () => Promise.resolve(false); - run = sinon.stub(PylintTest.prototype, 'run'); - run.callsFake(() => Promise.resolve([])); - parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); - parseMessagesSeverity.callsFake(() => 'Severity'); - const pylint = new PylintTest(output.object, serviceContainer.object); - await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); - assert.deepEqual(run.args[0][0], args); - assert.ok(parseMessagesSeverity.notCalled); - assert.ok(run.calledOnce); - }); - - test('Do not use minimal checkers if there is a pylintrc file in the current working directory or when traversing the workspace up to its root (hasConfigurationFileInWorkspace() returns true)', async () => { - const settings = { - linting: { - pylintUseMinimalCheckers: true, - }, - }; - configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); - _info.setup((info) => info.linterArgs(doc.uri)).returns(() => []); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(true); // This implies method hasConfigurationFileInWorkspace() returns true - Pylint.hasConfigurationFile = () => Promise.resolve(false); - run = sinon.stub(PylintTest.prototype, 'run'); - run.callsFake(() => Promise.resolve([])); - parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); - parseMessagesSeverity.callsFake(() => 'Severity'); - const pylint = new PylintTest(output.object, serviceContainer.object); - await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); - assert.deepEqual(run.args[0][0], args); - assert.ok(parseMessagesSeverity.notCalled); - assert.ok(run.calledOnce); - }); - - test('Do not use minimal checkers if a pylintrc file exists in the process, in the current working directory or up in the hierarchy tree (hasConfigurationFile() returns true)', async () => { + test('Test pylint with default settings.', async () => { const settings = { linting: { - pylintUseMinimalCheckers: true, + pylintEnabled: true, }, }; configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); _info.setup((info) => info.linterArgs(doc.uri)).returns(() => []); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(false); - Pylint.hasConfigurationFile = () => Promise.resolve(true); // This implies method hasConfigurationFile() returns true run = sinon.stub(PylintTest.prototype, 'run'); run.callsFake(() => Promise.resolve([])); parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); @@ -510,13 +123,11 @@ suite('Pylint - Function runLinter()', () => { ]; const settings = { linting: { - pylintUseMinimalCheckers: true, + pylintEnabled: true, }, }; configService.setup((c) => c.getSettings(doc.uri)).returns(() => settings as any); _info.setup((info) => info.linterArgs(doc.uri)).returns(() => []); - Pylint.hasConfigurationFileInWorkspace = () => Promise.resolve(false); - Pylint.hasConfigurationFile = () => Promise.resolve(false); run = sinon.stub(PylintTest.prototype, 'run'); run.callsFake(() => Promise.resolve(message as any)); parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); diff --git a/src/test/mockClasses.ts b/src/test/mockClasses.ts index d524d774aac2..16d4c32b077d 100644 --- a/src/test/mockClasses.ts +++ b/src/test/mockClasses.ts @@ -88,5 +88,4 @@ export class MockLintingSettings implements ILintingSettings { public banditEnabled!: boolean; public banditArgs!: string[]; public banditPath!: string; - public pylintUseMinimalCheckers!: boolean; }