Skip to content

Commit 60fec3a

Browse files
authored
Ensures file paths are properly encoded when passing them as arguments to linters. (#1155)
* 🐛 format file names when passing as command line args * 🐛 fix header check * ✅ tests * 📝 news entry * Fixes #199
1 parent 5416af2 commit 60fec3a

File tree

10 files changed

+188
-26
lines changed

10 files changed

+188
-26
lines changed

gulpfile.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const fs = require('fs');
2424
const remapIstanbul = require('remap-istanbul');
2525
const istanbul = require('istanbul');
2626
const glob = require('glob');
27+
const os = require('os');
2728
const _ = require('lodash');
2829

2930
/**
@@ -68,7 +69,8 @@ const copyrightHeader = [
6869
'// Licensed under the MIT License.',
6970
'',
7071
'\'use strict\';'
71-
].join('\n');
72+
];
73+
const copyrightHeaders = [copyrightHeader.join('\n'), copyrightHeader.join('\r\n')];
7274

7375
gulp.task('hygiene', () => run({ mode: 'all', skipFormatCheck: true, skipIndentationCheck: true }));
7476

@@ -178,10 +180,13 @@ const hygiene = (options) => {
178180
const addedFiles = options.skipCopyrightCheck ? [] : getAddedFilesSync();
179181
console.log(colors.blue('Hygiene started.'));
180182
const copyrights = es.through(function (file) {
181-
if (addedFiles.indexOf(file.path) !== -1 && file.contents.toString('utf8').indexOf(copyrightHeader) !== 0) {
182-
// Use tslint format.
183-
console.error(`ERROR: (copyright) ${file.relative}[1,1]: Missing or bad copyright statement`);
184-
errorCount++;
183+
if (addedFiles.indexOf(file.path) !== -1) {
184+
const contents = file.contents.toString('utf8');
185+
if (!copyrightHeaders.some(header => contents.indexOf(header) === 0)) {
186+
// Use tslint format.
187+
console.error(`ERROR: (copyright) ${file.relative}[1,1]: Missing or bad copyright statement`);
188+
errorCount++;
189+
}
185190
}
186191

187192
this.emit('data', file);

news/2 Fixes/199.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensures file paths are properly encoded when passing them as arguments to linters.

src/client/linters/flake8.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { OutputChannel } from 'vscode';
2-
import { CancellationToken, TextDocument } from 'vscode';
1+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
2+
import '../common/extensions';
33
import { Product } from '../common/types';
44
import { IServiceContainer } from '../ioc/types';
55
import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class Flake8 extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation);
16+
const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
1717
messages.forEach(msg => {
1818
msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.flake8CategorySeverity);
1919
});

src/client/linters/mypy.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { OutputChannel } from 'vscode';
2-
import { CancellationToken, TextDocument } from 'vscode';
1+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
2+
import '../common/extensions';
33
import { Product } from '../common/types';
44
import { IServiceContainer } from '../ioc/types';
55
import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class MyPy extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX);
16+
const messages = await this.run([document.uri.fsPath.fileToCommandArgument()], document, cancellation, REGEX);
1717
messages.forEach(msg => {
1818
msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity);
1919
msg.code = msg.type;

src/client/linters/pep8.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { OutputChannel } from 'vscode';
2-
import { CancellationToken, TextDocument } from 'vscode';
1+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
2+
import '../common/extensions';
33
import { Product } from '../common/types';
44
import { IServiceContainer } from '../ioc/types';
55
import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class Pep8 extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation);
16+
const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
1717
messages.forEach(msg => {
1818
msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.pep8CategorySeverity);
1919
});

src/client/linters/prospector.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { OutputChannel } from 'vscode';
2-
import { CancellationToken, TextDocument } from 'vscode';
1+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
2+
import '../common/extensions';
33
import { Product } from '../common/types';
44
import { IServiceContainer } from '../ioc/types';
55
import { BaseLinter } from './baseLinter';
@@ -28,7 +28,7 @@ export class Prospector extends BaseLinter {
2828
}
2929

3030
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
31-
return await this.run(['--absolute-paths', '--output-format=json', document.uri.fsPath], document, cancellation);
31+
return this.run(['--absolute-paths', '--output-format=json', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
3232
}
3333
protected async parseMessages(output: string, document: TextDocument, token: CancellationToken, regEx: string) {
3434
let parsedData: IProspectorResponse;

src/client/linters/pydocstyle.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as path from 'path';
2-
import { OutputChannel } from 'vscode';
3-
import { CancellationToken, TextDocument } from 'vscode';
2+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
3+
import '../common/extensions';
44
import { Product } from '../common/types';
55
import { IServiceContainer } from '../ioc/types';
66
import { IS_WINDOWS } from './../common/utils';
@@ -13,7 +13,7 @@ export class PyDocStyle extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run([document.uri.fsPath], document, cancellation);
16+
const messages = await this.run([document.uri.fsPath.fileToCommandArgument()], document, cancellation);
1717
// All messages in pep8 are treated as warnings for now.
1818
messages.forEach(msg => {
1919
msg.severity = LintMessageSeverity.Warning;
@@ -61,6 +61,7 @@ export class PyDocStyle extends BaseLinter {
6161
const trmmedSourceLine = sourceLine.trim();
6262
const sourceStart = sourceLine.indexOf(trmmedSourceLine);
6363

64+
// tslint:disable-next-line:no-object-literal-type-assertion
6465
return {
6566
code: code,
6667
message: message,

src/client/linters/pylama.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { OutputChannel } from 'vscode';
2-
import { CancellationToken, TextDocument } from 'vscode';
1+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
2+
import '../common/extensions';
33
import { Product } from '../common/types';
44
import { IServiceContainer } from '../ioc/types';
55
import { BaseLinter } from './baseLinter';
@@ -14,7 +14,7 @@ export class PyLama extends BaseLinter {
1414
}
1515

1616
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
17-
const messages = await this.run(['--format=parsable', document.uri.fsPath], document, cancellation, REGEX);
17+
const messages = await this.run(['--format=parsable', document.uri.fsPath.fileToCommandArgument()], document, cancellation, REGEX);
1818
// All messages in pylama are treated as warnings for now.
1919
messages.forEach(msg => {
2020
msg.severity = LintMessageSeverity.Warning;

src/client/linters/pylint.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
import * as os from 'os';
66
import * as path from 'path';
7-
import { OutputChannel } from 'vscode';
8-
import { CancellationToken, TextDocument } from 'vscode';
7+
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
8+
import '../common/extensions';
99
import { IFileSystem, IPlatformService } from '../common/platform/types';
1010
import { Product } from '../common/types';
1111
import { IServiceContainer } from '../ioc/types';
@@ -70,7 +70,7 @@ export class Pylint extends BaseLinter {
7070
'--msg-template=\'{line},{column},{category},{msg_id}:{msg}\'',
7171
'--reports=n',
7272
'--output-format=text',
73-
uri.fsPath
73+
uri.fsPath.fileToCommandArgument()
7474
];
7575
const messages = await this.run(minArgs.concat(args), document, cancellation);
7676
messages.forEach(msg => {

src/test/linters/lint.args.test.ts

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
// tslint:disable:no-any
7+
8+
import { expect } from 'chai';
9+
import { Container } from 'inversify';
10+
import * as path from 'path';
11+
import * as TypeMoq from 'typemoq';
12+
import { CancellationTokenSource, OutputChannel, TextDocument, Uri } from 'vscode';
13+
import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
14+
import '../../client/common/extensions';
15+
import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
16+
import { IConfigurationService, IInstaller, ILintingSettings, ILogger, IOutputChannel, IPythonSettings } from '../../client/common/types';
17+
import { IInterpreterService } from '../../client/interpreter/contracts';
18+
import { ServiceContainer } from '../../client/ioc/container';
19+
import { ServiceManager } from '../../client/ioc/serviceManager';
20+
import { BaseLinter } from '../../client/linters/baseLinter';
21+
import { Flake8 } from '../../client/linters/flake8';
22+
import { LinterManager } from '../../client/linters/linterManager';
23+
import { MyPy } from '../../client/linters/mypy';
24+
import { Pep8 } from '../../client/linters/pep8';
25+
import { Prospector } from '../../client/linters/prospector';
26+
import { PyDocStyle } from '../../client/linters/pydocstyle';
27+
import { PyLama } from '../../client/linters/pylama';
28+
import { Pylint } from '../../client/linters/pylint';
29+
import { ILinterManager, ILintingEngine } from '../../client/linters/types';
30+
import { initialize } from '../initialize';
31+
32+
// tslint:disable-next-line:max-func-body-length
33+
suite('Linting - Arguments', () => {
34+
let interpreterService: TypeMoq.IMock<IInterpreterService>;
35+
let engine: TypeMoq.IMock<ILintingEngine>;
36+
let configService: TypeMoq.IMock<IConfigurationService>;
37+
let docManager: TypeMoq.IMock<IDocumentManager>;
38+
let settings: TypeMoq.IMock<IPythonSettings>;
39+
let lm: ILinterManager;
40+
let serviceContainer: ServiceContainer;
41+
let document: TypeMoq.IMock<TextDocument>;
42+
let outputChannel: TypeMoq.IMock<OutputChannel>;
43+
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
44+
const cancellationToken = new CancellationTokenSource().token;
45+
46+
suiteSetup(initialize);
47+
setup(async () => {
48+
const cont = new Container();
49+
const serviceManager = new ServiceManager(cont);
50+
51+
serviceContainer = new ServiceContainer(cont);
52+
outputChannel = TypeMoq.Mock.ofType<OutputChannel>();
53+
54+
const fs = TypeMoq.Mock.ofType<IFileSystem>();
55+
fs.setup(x => x.fileExistsAsync(TypeMoq.It.isAny())).returns(() => new Promise<boolean>((resolve, reject) => resolve(true)));
56+
fs.setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => true);
57+
serviceManager.addSingletonInstance<IFileSystem>(IFileSystem, fs.object);
58+
59+
serviceManager.addSingletonInstance(IOutputChannel, outputChannel.object);
60+
61+
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
62+
serviceManager.addSingletonInstance<IInterpreterService>(IInterpreterService, interpreterService.object);
63+
64+
engine = TypeMoq.Mock.ofType<ILintingEngine>();
65+
serviceManager.addSingletonInstance<ILintingEngine>(ILintingEngine, engine.object);
66+
67+
docManager = TypeMoq.Mock.ofType<IDocumentManager>();
68+
serviceManager.addSingletonInstance<IDocumentManager>(IDocumentManager, docManager.object);
69+
70+
const lintSettings = TypeMoq.Mock.ofType<ILintingSettings>();
71+
lintSettings.setup(x => x.enabled).returns(() => true);
72+
lintSettings.setup(x => x.lintOnSave).returns(() => true);
73+
74+
settings = TypeMoq.Mock.ofType<IPythonSettings>();
75+
settings.setup(x => x.linting).returns(() => lintSettings.object);
76+
77+
configService = TypeMoq.Mock.ofType<IConfigurationService>();
78+
configService.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
79+
serviceManager.addSingletonInstance<IConfigurationService>(IConfigurationService, configService.object);
80+
81+
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
82+
serviceManager.addSingletonInstance<IWorkspaceService>(IWorkspaceService, workspaceService.object);
83+
84+
const logger = TypeMoq.Mock.ofType<ILogger>();
85+
serviceManager.addSingletonInstance<ILogger>(ILogger, logger.object);
86+
87+
const installer = TypeMoq.Mock.ofType<IInstaller>();
88+
serviceManager.addSingletonInstance<IInstaller>(IInstaller, installer.object);
89+
90+
const platformService = TypeMoq.Mock.ofType<IPlatformService>();
91+
serviceManager.addSingletonInstance<IPlatformService>(IPlatformService, platformService.object);
92+
93+
lm = new LinterManager(serviceContainer);
94+
serviceManager.addSingletonInstance<ILinterManager>(ILinterManager, lm);
95+
document = TypeMoq.Mock.ofType<TextDocument>();
96+
});
97+
98+
async function testLinter(linter: BaseLinter, fileUri: Uri, expectedArgs: string[]) {
99+
document.setup(d => d.uri).returns(() => fileUri);
100+
101+
let invoked = false;
102+
(linter as any).run = (args, doc, token) => {
103+
expect(args).to.deep.equal(expectedArgs);
104+
invoked = true;
105+
return Promise.resolve([]);
106+
};
107+
await linter.lint(document.object, cancellationToken);
108+
expect(invoked).to.be.equal(true, 'method not invoked');
109+
}
110+
[Uri.file(path.join('users', 'development path to', 'one.py')), Uri.file(path.join('users', 'development', 'one.py'))].forEach(fileUri => {
111+
test(`Flake8 (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
112+
const linter = new Flake8(outputChannel.object, serviceContainer);
113+
const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath.fileToCommandArgument()];
114+
await testLinter(linter, fileUri, expectedArgs);
115+
});
116+
test(`Pep8 (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
117+
const linter = new Pep8(outputChannel.object, serviceContainer);
118+
const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath.fileToCommandArgument()];
119+
await testLinter(linter, fileUri, expectedArgs);
120+
});
121+
test(`Prospector (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
122+
const linter = new Prospector(outputChannel.object, serviceContainer);
123+
const expectedArgs = ['--absolute-paths', '--output-format=json', fileUri.fsPath.fileToCommandArgument()];
124+
await testLinter(linter, fileUri, expectedArgs);
125+
});
126+
test(`Pylama (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
127+
const linter = new PyLama(outputChannel.object, serviceContainer);
128+
const expectedArgs = ['--format=parsable', fileUri.fsPath.fileToCommandArgument()];
129+
await testLinter(linter, fileUri, expectedArgs);
130+
});
131+
test(`MyPy (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
132+
const linter = new MyPy(outputChannel.object, serviceContainer);
133+
const expectedArgs = [fileUri.fsPath.fileToCommandArgument()];
134+
await testLinter(linter, fileUri, expectedArgs);
135+
});
136+
test(`Pydocstyle (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
137+
const linter = new PyDocStyle(outputChannel.object, serviceContainer);
138+
const expectedArgs = [fileUri.fsPath.fileToCommandArgument()];
139+
await testLinter(linter, fileUri, expectedArgs);
140+
});
141+
test(`Pylint (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
142+
const linter = new Pylint(outputChannel.object, serviceContainer);
143+
document.setup(d => d.uri).returns(() => fileUri);
144+
145+
let invoked = false;
146+
(linter as any).run = (args, doc, token) => {
147+
expect(args[args.length - 1]).to.equal(fileUri.fsPath.fileToCommandArgument());
148+
invoked = true;
149+
return Promise.resolve([]);
150+
};
151+
await linter.lint(document.object, cancellationToken);
152+
expect(invoked).to.be.equal(true, 'method not invoked');
153+
});
154+
});
155+
});

0 commit comments

Comments
 (0)