diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index bf20fb3f44ba..1a4d800fed0c 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -359,7 +359,14 @@ export class Tokenizer implements ITokenizer { } private skipToSingleEndQuote(quote: number): void { - while (!this.cs.isEndOfStream() && this.cs.currentChar !== quote) { + while (!this.cs.isEndOfStream()) { + if (this.cs.currentChar === Char.Backslash && this.cs.nextChar === quote) { + this.cs.advance(2); + continue; + } + if (this.cs.currentChar === quote) { + break; + } this.cs.moveNext(); } this.cs.moveNext(); diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 2c90e5c42731..742008ea7346 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -1,10 +1,10 @@ import * as path from 'path'; import * as vscode from 'vscode'; -import { CancellationToken, OutputChannel, TextDocument, Uri } from 'vscode'; +import { IWorkspaceService } from '../common/application/types'; import '../common/extensions'; import { IPythonToolExecutionService } from '../common/process/types'; -import { ExecutionInfo, ILogger, Product } from '../common/types'; import { IConfigurationService, IPythonSettings } from '../common/types'; +import { ExecutionInfo, ILogger, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { ErrorHandler } from './errorHandlers/errorHandler'; import { ILinter, ILinterInfo, ILinterManager, ILintMessage, LintMessageSeverity } from './types'; @@ -38,25 +38,27 @@ export abstract class BaseLinter implements ILinter { private errorHandler: ErrorHandler; private _pythonSettings: IPythonSettings; private _info: ILinterInfo; + private workspace: IWorkspaceService; protected get pythonSettings(): IPythonSettings { return this._pythonSettings; } constructor(product: Product, - protected readonly outputChannel: OutputChannel, + protected readonly outputChannel: vscode.OutputChannel, protected readonly serviceContainer: IServiceContainer, protected readonly columnOffset = 0) { this._info = serviceContainer.get(ILinterManager).getLinterInfo(product); this.errorHandler = new ErrorHandler(this.info.product, outputChannel, serviceContainer); this.configService = serviceContainer.get(IConfigurationService); + this.workspace = serviceContainer.get(IWorkspaceService); } public get info(): ILinterInfo { return this._info; } - public isLinterExecutableSpecified(resource: Uri) { + public isLinterExecutableSpecified(resource: vscode.Uri) { const executablePath = this.info.pathName(resource); return path.basename(executablePath).length > 0 && path.basename(executablePath) !== executablePath; } @@ -66,7 +68,7 @@ export abstract class BaseLinter implements ILinter { } protected getWorkspaceRootPath(document: vscode.TextDocument): string { - const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + const workspaceFolder = this.workspace.getWorkspaceFolder(document.uri); const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined; return typeof workspaceRootPath === 'string' ? workspaceRootPath : __dirname; } @@ -107,7 +109,7 @@ export abstract class BaseLinter implements ILinter { const cwd = this.getWorkspaceRootPath(document); const pythonToolsExecutionService = this.serviceContainer.get(IPythonToolExecutionService); try { - const result = await pythonToolsExecutionService.exec(executionInfo, {cwd, token: cancellation, mergeStdOutErr: true}, document.uri); + const result = await pythonToolsExecutionService.exec(executionInfo, { cwd, token: cancellation, mergeStdOutErr: true }, document.uri); this.displayLinterResultHeader(result.stdout); return await this.parseMessages(result.stdout, document, cancellation, regEx); } catch (error) { @@ -116,12 +118,12 @@ export abstract class BaseLinter implements ILinter { } } - protected async parseMessages(output: string, document: TextDocument, token: CancellationToken, regEx: string) { + protected async parseMessages(output: string, document: vscode.TextDocument, token: vscode.CancellationToken, regEx: string) { const outputLines = output.splitLines({ removeEmptyEntries: false, trim: false }); return this.parseLines(outputLines, regEx); } - protected handleError(error: Error, resource: Uri, execInfo: ExecutionInfo) { + protected handleError(error: Error, resource: vscode.Uri, execInfo: ExecutionInfo) { this.errorHandler.handleError(error, resource, execInfo) .catch(this.logger.logError.bind(this, 'Error in errorHandler.handleError')); } diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index c650714e45dd..53caae8a8669 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -11,6 +11,9 @@ import { IServiceContainer } from '../ioc/types'; import { BaseLinter } from './baseLinter'; import { ILintMessage } from './types'; +const pylintrc = 'pylintrc'; +const dotPylintrc = '.pylintrc'; + export class Pylint extends BaseLinter { private fileSystem: IFileSystem; private platformService: IPlatformService; @@ -25,12 +28,16 @@ export class Pylint extends BaseLinter { let minArgs: string[] = []; // Only use minimal checkers if // a) there are no custom arguments and - // b) there is no pylintrc file + // 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 - && !await Pylint.hasConfigurationFile(this.fileSystem, uri.fsPath, this.platformService)) { + // Check pylintrc next to the file or above up to and including the workspace root + && !await Pylint.hasConfigrationFileInWorkspace(this.fileSystem, path.dirname(uri.fsPath), workspaceRoot) + // Check for pylintrc at the root and above + && !await Pylint.hasConfigurationFile(this.fileSystem, this.getWorkspaceRootPath(document), this.platformService)) { minArgs = [ '--disable=all', '--enable=F,E,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' @@ -51,7 +58,7 @@ export class Pylint extends BaseLinter { } // tslint:disable-next-line:member-ordering - public static async hasConfigurationFile(fs: IFileSystem, filePath: string, platformService: IPlatformService): Promise { + 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 @@ -69,9 +76,7 @@ export class Pylint extends BaseLinter { return true; } - let dir = path.dirname(filePath); - const pylintrc = 'pylintrc'; - const dotPylintrc = '.pylintrc'; + let dir = folder; if (await fs.fileExistsAsync(path.join(dir, pylintrc)) || await fs.fileExistsAsync(path.join(dir, dotPylintrc))) { return true; } @@ -87,7 +92,7 @@ export class Pylint extends BaseLinter { } current = above; above = path.dirname(above); - } while (current !== above); + } while (!fs.arePathsSame(current, above)); dir = path.resolve('~'); if (await fs.fileExistsAsync(path.join(dir, dotPylintrc))) { @@ -104,4 +109,19 @@ export class Pylint extends BaseLinter { } return false; } + + // tslint:disable-next-line:member-ordering + public static async hasConfigrationFileInWorkspace(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.fileExistsAsync(path.join(current, pylintrc)) || await fs.fileExistsAsync(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/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index c397ffec95e5..e11df6a147b0 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -64,6 +64,21 @@ suite('Language.Tokenizer', () => { assert.equal(tokens.getItemAt(i).type, TokenType.String); } }); + test('Strings: single quote escape', async () => { + const t = new Tokenizer(); + // tslint:disable-next-line:quotemark + const tokens = t.tokenize("'\\'quoted\\''"); + assert.equal(tokens.count, 1); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 12); + }); + test('Strings: double quote escape', async () => { + const t = new Tokenizer(); + const tokens = t.tokenize('"\\"quoted\\""'); + assert.equal(tokens.count, 1); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 12); + }); test('Comments', async () => { const t = new Tokenizer(); const tokens = t.tokenize(' #co"""mment1\n\t\n#comm\'ent2 '); diff --git a/src/test/linters/pylint.test.ts b/src/test/linters/pylint.test.ts index cc527e320478..75cd78f148f3 100644 --- a/src/test/linters/pylint.test.ts +++ b/src/test/linters/pylint.test.ts @@ -2,32 +2,72 @@ // Licensed under the MIT License. import { expect } from 'chai'; +import { Container } from 'inversify'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; +import { CancellationTokenSource, OutputChannel, TextDocument, Uri, WorkspaceFolder } from 'vscode'; +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, ILogger, IPythonSettings } from '../../client/common/types'; +import { ServiceContainer } from '../../client/ioc/container'; +import { ServiceManager } from '../../client/ioc/serviceManager'; +import { LinterManager } from '../../client/linters/linterManager'; import { Pylint } from '../../client/linters/pylint'; +import { ILinterManager } from '../../client/linters/types'; +import { MockLintingSettings } from '../mockClasses'; +// tslint:disable-next-line:max-func-body-length suite('Linting - Pylintrc search', () => { const basePath = '/user/a/b/c/d'; - const file = path.join(basePath, 'file.py'); const pylintrc = 'pylintrc'; const dotPylintrc = '.pylintrc'; let fileSystem: TypeMoq.IMock; let platformService: TypeMoq.IMock; + let workspace: TypeMoq.IMock; + let execService: TypeMoq.IMock; + let config: TypeMoq.IMock; + let serviceContainer: ServiceContainer; setup(() => { fileSystem = TypeMoq.Mock.ofType(); + fileSystem + .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .returns((a, b) => a === b); + platformService = TypeMoq.Mock.ofType(); + platformService.setup(x => x.isWindows).returns(() => false); + + workspace = TypeMoq.Mock.ofType(); + execService = TypeMoq.Mock.ofType(); + + const cont = new Container(); + const serviceManager = new ServiceManager(cont); + serviceContainer = new ServiceContainer(cont); + + serviceManager.addSingletonInstance(IFileSystem, fileSystem.object); + serviceManager.addSingletonInstance(IWorkspaceService, workspace.object); + serviceManager.addSingletonInstance(IPythonToolExecutionService, execService.object); + serviceManager.addSingletonInstance(IPlatformService, platformService.object); + + config = TypeMoq.Mock.ofType(); + serviceManager.addSingletonInstance(IConfigurationService, config.object); + const linterManager = new LinterManager(serviceContainer); + serviceManager.addSingletonInstance(ILinterManager, linterManager); + const logger = TypeMoq.Mock.ofType(); + serviceManager.addSingletonInstance(ILogger, logger.object); + const installer = TypeMoq.Mock.ofType(); + serviceManager.addSingletonInstance(IInstaller, installer.object); }); test('pylintrc in the file folder', async () => { fileSystem.setup(x => x.fileExistsAsync(path.join(basePath, pylintrc))).returns(() => Promise.resolve(true)); - let result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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.fileExistsAsync(path.join(basePath, dotPylintrc))).returns(() => Promise.resolve(true)); - result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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 () => { @@ -41,7 +81,7 @@ suite('Linting - Pylintrc search', () => { fileSystem.setup(x => x.fileExistsAsync(module3)).returns(() => Promise.resolve(true)); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); - const result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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 () => { @@ -56,7 +96,7 @@ suite('Linting - Pylintrc search', () => { fileSystem.setup(x => x.fileExistsAsync(module3)).returns(() => Promise.resolve(true)); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); - const result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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 () => { @@ -64,7 +104,7 @@ suite('Linting - Pylintrc search', () => { const rc = path.join(home, dotPylintrc); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); - const result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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 () => { @@ -72,15 +112,90 @@ suite('Linting - Pylintrc search', () => { const rc = path.join(home, '.config', pylintrc); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); - const result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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 () => { - platformService.setup(x => x.isWindows).returns(() => false); const rc = path.join('/etc', pylintrc); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); - const result = await Pylint.hasConfigurationFile(fileSystem.object, file, platformService.object); + 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.fileExistsAsync(path.join(midFolder, pylintrc))) + .returns(() => Promise.resolve(true)); + + const result = await Pylint.hasConfigrationFileInWorkspace(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.fileExistsAsync(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.fileExistsAsync(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.fileExistsAsync(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, b, c) => { + execInfo = e; + }) + .returns(() => Promise.resolve({ stdout: '', stderr: '' })); + + const lintSettings = new MockLintingSettings(); + lintSettings.pylintUseMinimalCheckers = true; + // tslint:disable-next-line:no-string-literal + lintSettings['pylintPath'] = 'pyLint'; + // tslint:disable-next-line:no-string-literal + lintSettings['pylintEnabled'] = true; + + const settings = TypeMoq.Mock.ofType(); + settings.setup(x => x.linting).returns(() => lintSettings); + 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); + } }); diff --git a/src/test/mockClasses.ts b/src/test/mockClasses.ts index 3cbae4ddfc55..7849e2a2137a 100644 --- a/src/test/mockClasses.ts +++ b/src/test/mockClasses.ts @@ -1,4 +1,8 @@ import * as vscode from 'vscode'; +import { + Flake8CategorySeverity, ILintingSettings, IMypyCategorySeverity, + IPep8CategorySeverity, IPylintCategorySeverity +} from '../client/common/types'; export class MockOutputChannel implements vscode.OutputChannel { public name: string; @@ -44,3 +48,36 @@ export class MockStatusBarItem implements vscode.StatusBarItem { public dispose(): void { } } + +export class MockLintingSettings implements ILintingSettings { + public enabled: boolean; + public ignorePatterns: string[]; + public prospectorEnabled: boolean; + public prospectorArgs: string[]; + public pylintEnabled: boolean; + public pylintArgs: string[]; + public pep8Enabled: boolean; + public pep8Args: string[]; + public pylamaEnabled: boolean; + public pylamaArgs: string[]; + public flake8Enabled: boolean; + public flake8Args: string[]; + public pydocstyleEnabled: boolean; + public pydocstyleArgs: string[]; + public lintOnSave: boolean; + public maxNumberOfProblems: number; + public pylintCategorySeverity: IPylintCategorySeverity; + public pep8CategorySeverity: IPep8CategorySeverity; + public flake8CategorySeverity: Flake8CategorySeverity; + public mypyCategorySeverity: IMypyCategorySeverity; + public prospectorPath: string; + public pylintPath: string; + public pep8Path: string; + public pylamaPath: string; + public flake8Path: string; + public pydocstylePath: string; + public mypyEnabled: boolean; + public mypyArgs: string[]; + public mypyPath: string; + public pylintUseMinimalCheckers: boolean; +}