Fix backslash escape, multiple linters output, .pyenv/versions folder search (#920)

* Basic tokenizer

* Fixed property names

* Tests, round I

* Tests, round II

* tokenizer test

* Remove temorary change

* Fix merge issue

* Merge conflict

* Merge conflict

* Completion test

* Fix last line

* Fix javascript math

* Make test await for results

* Add license headers

* Rename definitions to types

* License headers

* Fix typo in completion details (typo)

* Fix hover test

* Russian translations

* Update to better translation

* Fix typo

*  #70 How to get all parameter info when filling in a function param list

* Fix #70 How to get all parameter info when filling in a function param list

* Clean up

* Clean imports

* CR feedback

* Trim whitespace for test stability

* More tests

* Better handle no-parameters documentation

* Better handle ellipsis and Python3

* #385 Auto-Indentation doesn't work after comment

* #141 Auto indentation broken when return keyword involved

* Undo changes

* #627 Docstrings for builtin methods are not parsed correctly

* reStructuredText converter

* Fix: period is not an operator

* Minor fixes

* Restructure

* Tests

* Tests

* Code heuristics

* Baselines

* HTML handling

* Lists

* State machine

* Baselines

* Squash

* no message

* Whitespace difference

* Update Jedi to 0.11.1

* Enable Travis

* Test fixes

* Undo change

* Jedi 0.11 with parser

* Undo changes

* Undo changes

* Test fixes

* More tests

* Tests

* Fix pylint search

* Handle quote escapes in strings

* Escapes in strings

* CR feedback

* Discover pylintrc better + tests

* Fix .pyenv/versions search

* Fix multiple linters output

* Better handle markdown underscore

* Test

* Fix 916: PyLint checks wrong files

* Test stability

* Try increase timeout

* Make sure linting is enabled in tests

* Try another way of waiting

* Simplify

* Fix clear diags on close tests

* Try writing settings directly

* Increase timeout

* Measure test time

* Measure time

* Simplify

* Set timeout

Author: Mikhail Arkhipov

src/client/common/markdown/restTextConverter.ts

@@ -36,7 +36,8 @@ export class RestTextConverter {
     return text
       .replace(/\#/g, '\\#')
       .replace(/\*/g, '\\*')
-      .replace(/\_/g, '\\_');
+      .replace(/\ _/g, ' \\_')
+      .replace(/^_/, '\\_');
   }
 
   private transformLines(docstring: string): string {

src/client/interpreter/locators/services/globalVirtualEnvService.ts

@@ -42,9 +42,11 @@ export class GlobalVirtualEnvironmentsSearchPathProvider implements IVirtualEnvi
             folders.push(pyenvRoot);
             folders.push(path.join(pyenvRoot, 'versions'));
         } else {
+            // Check if .pyenv/versions is in the list
             const pyenvVersions = path.join('.pyenv', 'versions');
             if (venvFolders.indexOf('.pyenv') >= 0 && venvFolders.indexOf(pyenvVersions) < 0) {
-                folders.push(pyenvVersions);
+                // if .pyenv is in the list, but .pyenv/versions is not, add it.
+                folders.push(path.join(homedir, pyenvVersions));
             }
         }
         return folders;

src/client/linters/linterCommands.ts

@@ -79,9 +79,9 @@ export class LinterCommands implements vscode.Disposable {
         }
     }
 
-    public runLinting(): void {
+    public runLinting(): Promise<vscode.DiagnosticCollection> {
         const engine = this.serviceContainer.get<ILintingEngine>(ILintingEngine);
-        engine.lintOpenPythonFiles();
+        return engine.lintOpenPythonFiles();
     }
 
     private get settingsUri(): vscode.Uri | undefined {

src/client/linters/lintingEngine.ts

@@ -6,7 +6,8 @@ import { Minimatch } from 'minimatch';
 import * as path from 'path';
 import * as vscode from 'vscode';
 import { IDocumentManager, IWorkspaceService } from '../common/application/types';
-import { LinterErrors, PythonLanguage, STANDARD_OUTPUT_CHANNEL } from '../common/constants';
+import { LinterErrors, STANDARD_OUTPUT_CHANNEL } from '../common/constants';
+import { IFileSystem } from '../common/platform/types';
 import { IConfigurationService, IOutputChannel } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { JupyterProvider } from '../jupyter/provider';
@@ -40,6 +41,7 @@ export class LintingEngine implements ILintingEngine {
   private diagnosticCollection: vscode.DiagnosticCollection;
   private pendingLintings = new Map<string, vscode.CancellationTokenSource>();
   private outputChannel: vscode.OutputChannel;
+  private fileSystem: IFileSystem;
 
   constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
     this.documentHasJupyterCodeCells = (a, b) => Promise.resolve(false);
@@ -48,34 +50,32 @@ export class LintingEngine implements ILintingEngine {
     this.configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
     this.outputChannel = serviceContainer.get<vscode.OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
     this.linterManager = serviceContainer.get<ILinterManager>(ILinterManager);
+    this.fileSystem = serviceContainer.get<IFileSystem>(IFileSystem);
     this.diagnosticCollection = vscode.languages.createDiagnosticCollection('python');
   }
 
-  public lintOpenPythonFiles(): void {
-    this.documents.textDocuments.forEach(async document => {
-      if (document.languageId === PythonLanguage.language) {
-        await this.lintDocument(document, 'auto');
-      }
-    });
+  public get diagnostics(): vscode.DiagnosticCollection {
+    return this.diagnosticCollection;
   }
 
-  public async lintDocument(document: vscode.TextDocument, trigger: LinterTrigger): Promise<void> {
-    // Check if we need to lint this document
-    const workspaceFolder = this.workspace.getWorkspaceFolder(document.uri);
-    const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined;
-    const relativeFileName = typeof workspaceRootPath === 'string' ? path.relative(workspaceRootPath, document.fileName) : document.fileName;
-    const settings = this.configurationService.getSettings(document.uri);
-    if (document.languageId !== PythonLanguage.language) {
-      return;
+  public clearDiagnostics(document: vscode.TextDocument): void {
+    if (this.diagnosticCollection.has(document.uri)) {
+      this.diagnosticCollection.delete(document.uri);
     }
-    if (!this.linterManager.isLintingEnabled(document.uri)) {
-      this.diagnosticCollection.set(document.uri, []);
-    }
-    const ignoreMinmatches = settings.linting.ignorePatterns.map(pattern => {
-      return new Minimatch(pattern);
-    });
+  }
 
-    if (ignoreMinmatches.some(matcher => matcher.match(document.fileName) || matcher.match(relativeFileName))) {
+  public async lintOpenPythonFiles(): Promise<vscode.DiagnosticCollection> {
+    this.diagnosticCollection.clear();
+    const promises = this.documents.textDocuments.map(async document => await this.lintDocument(document, 'auto'));
+    await Promise.all(promises);
+    return this.diagnosticCollection;
+  }
+
+  public async lintDocument(document: vscode.TextDocument, trigger: LinterTrigger): Promise<void> {
+    this.diagnosticCollection.set(document.uri, []);
+
+    // Check if we need to lint this document
+    if (!await this.shouldLintDocument(document)) {
       return;
     }
 
@@ -107,14 +107,14 @@ export class LintingEngine implements ILintingEngine {
     // linters will resolve asynchronously - keep a track of all
     // diagnostics reported as them come in.
     let diagnostics: vscode.Diagnostic[] = [];
+    const settings = this.configurationService.getSettings(document.uri);
 
     for (const p of promises) {
       const msgs = await p;
       if (cancelToken.token.isCancellationRequested) {
         break;
       }
 
-      diagnostics = [];
       if (this.isDocumentOpen(document.uri)) {
         // Build the message and suffix the message with the name of the linter used.
         for (const m of msgs) {
@@ -123,17 +123,16 @@ export class LintingEngine implements ILintingEngine {
             (m.code === LinterErrors.pylint.InvalidSyntax ||
               m.code === LinterErrors.prospector.InvalidSyntax ||
               m.code === LinterErrors.flake8.InvalidSyntax)) {
-            return;
+            continue;
           }
           diagnostics.push(this.createDiagnostics(m, document));
         }
-
         // Limit the number of messages to the max value.
         diagnostics = diagnostics.filter((value, index) => index <= settings.linting.maxNumberOfProblems);
       }
-      // Set all diagnostics found in this pass, as this method always clears existing diagnostics.
-      this.diagnosticCollection.set(document.uri, diagnostics);
     }
+    // Set all diagnostics found in this pass, as this method always clears existing diagnostics.
+    this.diagnosticCollection.set(document.uri, diagnostics);
   }
 
   // tslint:disable-next-line:no-any
@@ -175,4 +174,29 @@ export class LintingEngine implements ILintingEngine {
     diagnostic.source = message.provider;
     return diagnostic;
   }
+
+  private async shouldLintDocument(document: vscode.TextDocument): Promise<boolean> {
+    if (!this.linterManager.isLintingEnabled(document.uri)) {
+      this.diagnosticCollection.set(document.uri, []);
+      return false;
+    }
+
+    if (document.languageId !== PYTHON.language) {
+      return false;
+    }
+
+    const workspaceFolder = this.workspace.getWorkspaceFolder(document.uri);
+    const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined;
+    const relativeFileName = typeof workspaceRootPath === 'string' ? path.relative(workspaceRootPath, document.fileName) : document.fileName;
+
+    const settings = this.configurationService.getSettings(document.uri);
+    const ignoreMinmatches = settings.linting.ignorePatterns.map(pattern => new Minimatch(pattern));
+    if (ignoreMinmatches.some(matcher => matcher.match(document.fileName) || matcher.match(relativeFileName))) {
+      return false;
+    }
+    if (document.uri.scheme !== 'file' || !document.uri.fsPath) {
+      return false;
+    }
+    return await this.fileSystem.fileExistsAsync(document.uri.fsPath);
+  }
 }

src/client/linters/types.ts

@@ -61,8 +61,10 @@ export enum LintMessageSeverity {
 
 export const ILintingEngine = Symbol('ILintingEngine');
 export interface ILintingEngine {
-    lintOpenPythonFiles(): void;
+    readonly diagnostics: vscode.DiagnosticCollection;
+    lintOpenPythonFiles(): Promise<vscode.DiagnosticCollection>;
     lintDocument(document: vscode.TextDocument, trigger: LinterTrigger): Promise<void>;
     // tslint:disable-next-line:no-any
     linkJupiterExtension(jupiter: vscode.Extension<any> | undefined): Promise<void>;
+    clearDiagnostics(document: vscode.TextDocument): void;
 }

src/client/providers/linterProvider.ts

@@ -6,16 +6,14 @@ import * as vscode from 'vscode';
 import { ConfigurationTarget, Uri, workspace } from 'vscode';
 import { IDocumentManager } from '../common/application/types';
 import { ConfigSettingMonitor } from '../common/configSettingMonitor';
+import { isTestExecution } from '../common/constants';
 import { IFileSystem } from '../common/platform/types';
 import { IConfigurationService } from '../common/types';
 import { IInterpreterService } from '../interpreter/contracts';
 import { IServiceContainer } from '../ioc/types';
 import { ILinterManager, ILintingEngine } from '../linters/types';
 
-const uriSchemesToIgnore = ['git', 'showModifications', 'svn'];
-
 export class LinterProvider implements vscode.Disposable {
-    private diagnosticCollection: vscode.DiagnosticCollection;
     private context: vscode.ExtensionContext;
     private disposables: vscode.Disposable[];
     private configMonitor: ConfigSettingMonitor;
@@ -37,7 +35,6 @@ export class LinterProvider implements vscode.Disposable {
         this.documents = serviceContainer.get<IDocumentManager>(IDocumentManager);
         this.configuration = serviceContainer.get<IConfigurationService>(IConfigurationService);
 
-        this.diagnosticCollection = vscode.languages.createDiagnosticCollection('python');
         this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles()));
 
         this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions);
@@ -46,10 +43,12 @@ export class LinterProvider implements vscode.Disposable {
 
         this.configMonitor = new ConfigSettingMonitor('linting');
         this.configMonitor.on('change', this.lintSettingsChangedHandler.bind(this));
-    }
 
-    public get diagnostics(): vscode.DiagnosticCollection {
-        return this.diagnosticCollection;
+        // On workspace reopen we don't get `onDocumentOpened` since it is first opened
+        // and then the extension is activated. So schedule linting pass now.
+        if (!isTestExecution()) {
+            setTimeout(() => this.engine.lintOpenPythonFiles().ignoreErrors(), 1200);
+        }
     }
 
     public dispose() {
@@ -63,35 +62,23 @@ export class LinterProvider implements vscode.Disposable {
 
     private lintSettingsChangedHandler(configTarget: ConfigurationTarget, wkspaceOrFolder: Uri) {
         if (configTarget === ConfigurationTarget.Workspace) {
-            this.engine.lintOpenPythonFiles();
+            this.engine.lintOpenPythonFiles().ignoreErrors();
             return;
         }
         // Look for python files that belong to the specified workspace folder.
         workspace.textDocuments.forEach(async document => {
             const wkspaceFolder = workspace.getWorkspaceFolder(document.uri);
             if (wkspaceFolder && wkspaceFolder.uri.fsPath === wkspaceOrFolder.fsPath) {
-                await this.engine.lintDocument(document, 'auto');
+                this.engine.lintDocument(document, 'auto').ignoreErrors();
             }
         });
     }
 
-    private async onDocumentOpened(document: vscode.TextDocument): Promise<void> {
-        const settings = this.configuration.getSettings(document.uri);
-        if (document.languageId !== 'python' || !settings.linting.enabled) {
-            return;
-        }
-        // Exclude files opened by vscode when showing a diff view.
-        if (uriSchemesToIgnore.indexOf(document.uri.scheme) >= 0) {
-            return;
-        }
-        if (!document.uri.path ||
-            (path.basename(document.uri.path) === document.uri.path && !await this.fs.fileExistsAsync(document.uri.path))) {
-            return;
-        }
+    private onDocumentOpened(document: vscode.TextDocument): void {
         this.engine.lintDocument(document, 'auto').ignoreErrors();
     }
 
-    private onDocumentSaved(document: vscode.TextDocument) {
+    private onDocumentSaved(document: vscode.TextDocument): void {
         const settings = this.configuration.getSettings(document.uri);
         if (document.languageId === 'python' && settings.linting.enabled && settings.linting.lintOnSave) {
             this.engine.lintDocument(document, 'save').ignoreErrors();
@@ -111,8 +98,8 @@ export class LinterProvider implements vscode.Disposable {
             return;
         }
         // Check if this document is still open as a duplicate editor.
-        if (!this.isDocumentOpen(document.uri) && this.diagnosticCollection.has(document.uri)) {
-            this.diagnosticCollection.set(document.uri, []);
+        if (!this.isDocumentOpen(document.uri)) {
+            this.engine.clearDiagnostics(document);
         }
     }
 }

src/test/interpreters/venv.test.ts

@@ -49,7 +49,7 @@ suite('Virtual environments', () => {
 
         let paths = pathProvider.getSearchPaths();
         let expected = folders.map(item => path.join(homedir, item));
-        expected.push(path.join('.pyenv', 'versions'));
+        expected.push(path.join(homedir, '.pyenv', 'versions'));
 
         expect(paths).to.deep.equal(expected, 'Global search folder list is incorrect.');
 

src/test/linters/lint.provider.test.ts

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { expect } from 'chai';
 import { Container } from 'inversify';
 import * as TypeMoq from 'typemoq';
 import * as vscode from 'vscode';
@@ -150,14 +149,11 @@ suite('Linting - Provider', () => {
         document.setup(x => x.isClosed).returns(() => closed);
 
         docManager.setup(x => x.textDocuments).returns(() => closed ? [] : [document.object]);
-
+        // tslint:disable-next-line:prefer-const no-unused-variable
         const provider = new LinterProvider(context.object, serviceContainer);
-        const diags: vscode.Diagnostic[] = [];
-        diags.push(new vscode.Diagnostic(new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1)), 'error'));
-        provider.diagnostics.set(uri, diags);
 
         emitter.fire(document.object);
-        const d = provider.diagnostics.get(uri);
-        expect(d).to.be.lengthOf(closed ? 0 : 1, 'Diagnostic collection not of expected length after file close.');
+        const timesExpected = closed ? TypeMoq.Times.once() : TypeMoq.Times.never();
+        engine.verify(x => x.clearDiagnostics(TypeMoq.It.isAny()), timesExpected);
     }
 });

src/test/linters/lint.test.ts

@@ -3,6 +3,7 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import { Uri } from 'vscode';
 import * as vscode from 'vscode';
+import { ICommandManager } from '../../client/common/application/types';
 import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants';
 import { Product } from '../../client/common/installer/productInstaller';
 import { IConfigurationService, IOutputChannel } from '../../client/common/types';
@@ -250,4 +251,26 @@ suite('Linting', () => {
         await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
         await testEnablingDisablingOfLinter(Product.pylint, true, file);
     });
+    // tslint:disable-next-line:no-function-expression
+    test('Multiple linters', async function () {
+        // tslint:disable-next-line:no-invalid-this
+        this.timeout(40000);
+
+        await closeActiveWindows();
+        const document = await vscode.workspace.openTextDocument(path.join(pythoFilesPath, 'print.py'));
+        await vscode.window.showTextDocument(document);
+        await configService.updateSettingAsync('linting.enabled', true, workspaceUri);
+        await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
+        await configService.updateSettingAsync('linting.pylintEnabled', true, workspaceUri);
+        await configService.updateSettingAsync('linting.flake8Enabled', true, workspaceUri);
+
+        const commands = ioc.serviceContainer.get<ICommandManager>(ICommandManager);
+        const collection = await commands.executeCommand('python.runLinting') as vscode.DiagnosticCollection;
+        assert.notEqual(collection, undefined, 'python.runLinting did not return valid diagnostics collection.');
+
+        const messages = collection!.get(document.uri);
+        assert.notEqual(messages!.length, 0, 'No diagnostic messages.');
+        assert.notEqual(messages!.filter(x => x.source === 'pylint').length, 0, 'No pylint messages.');
+        assert.notEqual(messages!.filter(x => x.source === 'flake8').length, 0, 'No flake8 messages.');
+    });
 });