Skip to content

Commit

Permalink
Fix pylintrc search (#805)
Browse files Browse the repository at this point in the history
* Fix pylint search

* Handle quote escapes in strings

* Escapes in strings

* CR feedback

* Discover pylintrc better + tests
  • Loading branch information
Mikhail Arkhipov authored Feb 19, 2018
1 parent 9aea5f1 commit 3fe37a2
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 25 deletions.
9 changes: 8 additions & 1 deletion src/client/language/tokenizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 10 additions & 8 deletions src/client/linters/baseLinter.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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>(ILinterManager).getLinterInfo(product);
this.errorHandler = new ErrorHandler(this.info.product, outputChannel, serviceContainer);
this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
this.workspace = serviceContainer.get<IWorkspaceService>(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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -107,7 +109,7 @@ export abstract class BaseLinter implements ILinter {
const cwd = this.getWorkspaceRootPath(document);
const pythonToolsExecutionService = this.serviceContainer.get<IPythonToolExecutionService>(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) {
Expand All @@ -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'));
}
Expand Down
34 changes: 27 additions & 7 deletions src/client/linters/pylint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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'
Expand All @@ -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<boolean> {
public static async hasConfigurationFile(fs: IFileSystem, folder: string, platformService: IPlatformService): Promise<boolean> {
// 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
Expand All @@ -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;
}
Expand All @@ -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))) {
Expand All @@ -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<boolean> {
// 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;
}
}
15 changes: 15 additions & 0 deletions src/test/language/tokenizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ');
Expand Down
Loading

0 comments on commit 3fe37a2

Please sign in to comment.