Ensures file paths are properly encoded when passing them as arguments to linters.

gulpfile.js

@@ -24,6 +24,7 @@ const fs = require('fs');
 const remapIstanbul = require('remap-istanbul');
 const istanbul = require('istanbul');
 const glob = require('glob');
+const os = require('os');
 const _ = require('lodash');
 
 /**
@@ -68,7 +69,8 @@ const copyrightHeader = [
     '// Licensed under the MIT License.',
     '',
     '\'use strict\';'
-].join('\n');
+];
+const copyrightHeaders = [copyrightHeader.join('\n'), copyrightHeader.join('\r\n')];
 
 gulp.task('hygiene', () => run({ mode: 'all', skipFormatCheck: true, skipIndentationCheck: true }));
 
@@ -178,10 +180,13 @@ const hygiene = (options) => {
     const addedFiles = options.skipCopyrightCheck ? [] : getAddedFilesSync();
     console.log(colors.blue('Hygiene started.'));
     const copyrights = es.through(function (file) {
-        if (addedFiles.indexOf(file.path) !== -1 && file.contents.toString('utf8').indexOf(copyrightHeader) !== 0) {
-            // Use tslint format.
-            console.error(`ERROR: (copyright) ${file.relative}[1,1]: Missing or bad copyright statement`);
-            errorCount++;
+        if (addedFiles.indexOf(file.path) !== -1) {
+            const contents = file.contents.toString('utf8');
+            if (!copyrightHeaders.some(header => contents.indexOf(header) === 0)) {
+                // Use tslint format.
+                console.error(`ERROR: (copyright) ${file.relative}[1,1]: Missing or bad copyright statement`);
+                errorCount++;
+            }
         }
 
         this.emit('data', file);

news/2 Fixes/199.md

@@ -0,0 +1 @@
+Ensures file paths are properly encoded when passing them as arguments to linters.

src/client/linters/flake8.ts

@@ -1,5 +1,5 @@
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class Flake8 extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation);
+        const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
         messages.forEach(msg => {
             msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.flake8CategorySeverity);
         });

src/client/linters/mypy.ts

@@ -1,5 +1,5 @@
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class MyPy extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX);
+        const messages = await this.run([document.uri.fsPath.fileToCommandArgument()], document, cancellation, REGEX);
         messages.forEach(msg => {
             msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity);
             msg.code = msg.type;

src/client/linters/pep8.ts

@@ -1,5 +1,5 @@
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { BaseLinter } from './baseLinter';
@@ -13,7 +13,7 @@ export class Pep8 extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation);
+        const messages = await this.run(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
         messages.forEach(msg => {
             msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.pep8CategorySeverity);
         });

src/client/linters/prospector.ts

@@ -1,5 +1,5 @@
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { BaseLinter } from './baseLinter';
@@ -28,7 +28,7 @@ export class Prospector extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        return await this.run(['--absolute-paths', '--output-format=json', document.uri.fsPath], document, cancellation);
+        return this.run(['--absolute-paths', '--output-format=json', document.uri.fsPath.fileToCommandArgument()], document, cancellation);
     }
     protected async parseMessages(output: string, document: TextDocument, token: CancellationToken, regEx: string) {
         let parsedData: IProspectorResponse;

src/client/linters/pydocstyle.ts

@@ -1,6 +1,6 @@
 import * as path from 'path';
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { IS_WINDOWS } from './../common/utils';
@@ -13,7 +13,7 @@ export class PyDocStyle extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run([document.uri.fsPath], document, cancellation);
+        const messages = await this.run([document.uri.fsPath.fileToCommandArgument()], document, cancellation);
         // All messages in pep8 are treated as warnings for now.
         messages.forEach(msg => {
             msg.severity = LintMessageSeverity.Warning;
@@ -61,6 +61,7 @@ export class PyDocStyle extends BaseLinter {
                     const trmmedSourceLine = sourceLine.trim();
                     const sourceStart = sourceLine.indexOf(trmmedSourceLine);
 
+                    // tslint:disable-next-line:no-object-literal-type-assertion
                     return {
                         code: code,
                         message: message,

src/client/linters/pylama.ts

@@ -1,5 +1,5 @@
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
+import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { BaseLinter } from './baseLinter';
@@ -14,7 +14,7 @@ export class PyLama extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run(['--format=parsable', document.uri.fsPath], document, cancellation, REGEX);
+        const messages = await this.run(['--format=parsable', document.uri.fsPath.fileToCommandArgument()], document, cancellation, REGEX);
         // All messages in pylama are treated as warnings for now.
         messages.forEach(msg => {
             msg.severity = LintMessageSeverity.Warning;

src/client/linters/pylint.ts

@@ -4,8 +4,8 @@
 
 import * as os from 'os';
 import * as path from 'path';
-import { OutputChannel } from 'vscode';
-import { CancellationToken, TextDocument } from 'vscode';
+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';
@@ -70,7 +70,7 @@ export class Pylint extends BaseLinter {
             '--msg-template=\'{line},{column},{category},{msg_id}:{msg}\'',
             '--reports=n',
             '--output-format=text',
-            uri.fsPath
+            uri.fsPath.fileToCommandArgument()
         ];
         const messages = await this.run(minArgs.concat(args), document, cancellation);
         messages.forEach(msg => {

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

@@ -0,0 +1,155 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+// tslint:disable:no-any
+
+import { expect } from 'chai';
+import { Container } from 'inversify';
+import * as path from 'path';
+import * as TypeMoq from 'typemoq';
+import { CancellationTokenSource, OutputChannel, TextDocument, Uri } from 'vscode';
+import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
+import '../../client/common/extensions';
+import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
+import { IConfigurationService, IInstaller, ILintingSettings, ILogger, IOutputChannel, IPythonSettings } from '../../client/common/types';
+import { IInterpreterService } from '../../client/interpreter/contracts';
+import { ServiceContainer } from '../../client/ioc/container';
+import { ServiceManager } from '../../client/ioc/serviceManager';
+import { BaseLinter } from '../../client/linters/baseLinter';
+import { Flake8 } from '../../client/linters/flake8';
+import { LinterManager } from '../../client/linters/linterManager';
+import { MyPy } from '../../client/linters/mypy';
+import { Pep8 } from '../../client/linters/pep8';
+import { Prospector } from '../../client/linters/prospector';
+import { PyDocStyle } from '../../client/linters/pydocstyle';
+import { PyLama } from '../../client/linters/pylama';
+import { Pylint } from '../../client/linters/pylint';
+import { ILinterManager, ILintingEngine } from '../../client/linters/types';
+import { initialize } from '../initialize';
+
+// tslint:disable-next-line:max-func-body-length
+suite('Linting - Arguments', () => {
+    let interpreterService: TypeMoq.IMock<IInterpreterService>;
+    let engine: TypeMoq.IMock<ILintingEngine>;
+    let configService: TypeMoq.IMock<IConfigurationService>;
+    let docManager: TypeMoq.IMock<IDocumentManager>;
+    let settings: TypeMoq.IMock<IPythonSettings>;
+    let lm: ILinterManager;
+    let serviceContainer: ServiceContainer;
+    let document: TypeMoq.IMock<TextDocument>;
+    let outputChannel: TypeMoq.IMock<OutputChannel>;
+    let workspaceService: TypeMoq.IMock<IWorkspaceService>;
+    const cancellationToken = new CancellationTokenSource().token;
+
+    suiteSetup(initialize);
+    setup(async () => {
+        const cont = new Container();
+        const serviceManager = new ServiceManager(cont);
+
+        serviceContainer = new ServiceContainer(cont);
+        outputChannel = TypeMoq.Mock.ofType<OutputChannel>();
+
+        const fs = TypeMoq.Mock.ofType<IFileSystem>();
+        fs.setup(x => x.fileExistsAsync(TypeMoq.It.isAny())).returns(() => new Promise<boolean>((resolve, reject) => resolve(true)));
+        fs.setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => true);
+        serviceManager.addSingletonInstance<IFileSystem>(IFileSystem, fs.object);
+
+        serviceManager.addSingletonInstance(IOutputChannel, outputChannel.object);
+
+        interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
+        serviceManager.addSingletonInstance<IInterpreterService>(IInterpreterService, interpreterService.object);
+
+        engine = TypeMoq.Mock.ofType<ILintingEngine>();
+        serviceManager.addSingletonInstance<ILintingEngine>(ILintingEngine, engine.object);
+
+        docManager = TypeMoq.Mock.ofType<IDocumentManager>();
+        serviceManager.addSingletonInstance<IDocumentManager>(IDocumentManager, docManager.object);
+
+        const lintSettings = TypeMoq.Mock.ofType<ILintingSettings>();
+        lintSettings.setup(x => x.enabled).returns(() => true);
+        lintSettings.setup(x => x.lintOnSave).returns(() => true);
+
+        settings = TypeMoq.Mock.ofType<IPythonSettings>();
+        settings.setup(x => x.linting).returns(() => lintSettings.object);
+
+        configService = TypeMoq.Mock.ofType<IConfigurationService>();
+        configService.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
+        serviceManager.addSingletonInstance<IConfigurationService>(IConfigurationService, configService.object);
+
+        workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
+        serviceManager.addSingletonInstance<IWorkspaceService>(IWorkspaceService, workspaceService.object);
+
+        const logger = TypeMoq.Mock.ofType<ILogger>();
+        serviceManager.addSingletonInstance<ILogger>(ILogger, logger.object);
+
+        const installer = TypeMoq.Mock.ofType<IInstaller>();
+        serviceManager.addSingletonInstance<IInstaller>(IInstaller, installer.object);
+
+        const platformService = TypeMoq.Mock.ofType<IPlatformService>();
+        serviceManager.addSingletonInstance<IPlatformService>(IPlatformService, platformService.object);
+
+        lm = new LinterManager(serviceContainer);
+        serviceManager.addSingletonInstance<ILinterManager>(ILinterManager, lm);
+        document = TypeMoq.Mock.ofType<TextDocument>();
+    });
+
+    async function testLinter(linter: BaseLinter, fileUri: Uri, expectedArgs: string[]) {
+        document.setup(d => d.uri).returns(() => fileUri);
+
+        let invoked = false;
+        (linter as any).run = (args, doc, token) => {
+            expect(args).to.deep.equal(expectedArgs);
+            invoked = true;
+            return Promise.resolve([]);
+        };
+        await linter.lint(document.object, cancellationToken);
+        expect(invoked).to.be.equal(true, 'method not invoked');
+    }
+    [Uri.file(path.join('users', 'development path to', 'one.py')), Uri.file(path.join('users', 'development', 'one.py'))].forEach(fileUri => {
+        test(`Flake8 (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new Flake8(outputChannel.object, serviceContainer);
+            const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`Pep8 (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new Pep8(outputChannel.object, serviceContainer);
+            const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`Prospector (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new Prospector(outputChannel.object, serviceContainer);
+            const expectedArgs = ['--absolute-paths', '--output-format=json', fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`Pylama (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new PyLama(outputChannel.object, serviceContainer);
+            const expectedArgs = ['--format=parsable', fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`MyPy (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new MyPy(outputChannel.object, serviceContainer);
+            const expectedArgs = [fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`Pydocstyle (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new PyDocStyle(outputChannel.object, serviceContainer);
+            const expectedArgs = [fileUri.fsPath.fileToCommandArgument()];
+            await testLinter(linter, fileUri, expectedArgs);
+        });
+        test(`Pylint (${fileUri.fsPath.indexOf(' ') > 0 ? 'with spaces' : 'without spaces'})`, async () => {
+            const linter = new Pylint(outputChannel.object, serviceContainer);
+            document.setup(d => d.uri).returns(() => fileUri);
+
+            let invoked = false;
+            (linter as any).run = (args, doc, token) => {
+                expect(args[args.length - 1]).to.equal(fileUri.fsPath.fileToCommandArgument());
+                invoked = true;
+                return Promise.resolve([]);
+            };
+            await linter.lint(document.object, cancellationToken);
+            expect(invoked).to.be.equal(true, 'method not invoked');
+        });
+    });
+});