Fix linters to make use of the new python code execution framework (#360)

Fixes #351
* Refactored linter code to make use of new execution framework
* Reverted to two output windows with hardcoded names

Author: DonJayamanne

package-lock.json

@@ -44,7 +44,6 @@ export interface IUnitTestSettings {
     pyTestArgs: string[];
     unittestEnabled: boolean;
     unittestArgs: string[];
-    outputWindow: string;
     cwd?: string;
 }
 export interface IPylintCategorySeverity {
@@ -96,7 +95,6 @@ export interface ILintingSettings {
     pylamaPath: string;
     flake8Path: string;
     pydocstylePath: string;
-    outputWindow: string;
     mypyEnabled: boolean;
     mypyArgs: string[];
     mypyPath: string;
@@ -107,7 +105,6 @@ export interface IFormattingSettings {
     autopep8Args: string[];
     yapfPath: string;
     yapfArgs: string[];
-    outputWindow: string;
 }
 export interface IAutoCompeteSettings {
     addBrackets: boolean;
@@ -244,7 +241,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
             flake8Args: [], flake8Enabled: false, flake8Path: 'flake',
             lintOnSave: false, maxNumberOfProblems: 100,
             mypyArgs: [], mypyEnabled: false, mypyPath: 'mypy',
-            outputWindow: 'python', pep8Args: [], pep8Enabled: false, pep8Path: 'pep8',
+            pep8Args: [], pep8Enabled: false, pep8Path: 'pep8',
             pylamaArgs: [], pylamaEnabled: false, pylamaPath: 'pylama',
             prospectorArgs: [], prospectorEnabled: false, prospectorPath: 'prospector',
             pydocstyleArgs: [], pydocstyleEnabled: false, pydocstylePath: 'pydocstyle',
@@ -288,7 +285,6 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
         // Support for travis.
         this.formatting = this.formatting ? this.formatting : {
             autopep8Args: [], autopep8Path: 'autopep8',
-            outputWindow: 'python',
             provider: 'autopep8',
             yapfArgs: [], yapfPath: 'yapf'
         };
@@ -339,7 +335,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
                     nosetestArgs: [], pyTestArgs: [], unittestArgs: [],
                     promptToConfigure: true, debugPort: 3000,
                     nosetestsEnabled: false, pyTestEnabled: false, unittestEnabled: false,
-                    nosetestPath: 'nosetests', pyTestPath: 'py.test', outputWindow: 'Python Test Log'
+                    nosetestPath: 'nosetests', pyTestPath: 'py.test'
                 } as IUnitTestSettings;
             }
         }
@@ -349,7 +345,6 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
             promptToConfigure: true,
             debugPort: 3000,
             nosetestArgs: [], nosetestPath: 'nosetest', nosetestsEnabled: false,
-            outputWindow: 'python',
             pyTestArgs: [], pyTestEnabled: false, pyTestPath: 'pytest',
             unittestArgs: [], unittestEnabled: false
         };

package.json

@@ -57,3 +57,5 @@ export namespace LinterErrors {
         export const InvalidSyntax = 'E999';
     }
 }
+
+export const STANDARD_OUTPUT_CHANNEL = 'STANDARD_OUTPUT_CHANNEL';

src/client/common/configSettings.ts

@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+/**
+ * @typedef {Object} SplitLinesOptions
+ * @property {boolean} [trim=true] - Whether to trim the lines.
+ * @property {boolean} [removeEmptyEntries=true] - Whether to remove empty entries.
+ */
+
+// tslint:disable-next-line:interface-name
+interface String {
+    /**
+     * Split a string using the cr and lf characters and return them as an array.
+     * By default lines are trimmed and empty lines are removed.
+     * @param {SplitLinesOptions=} splitOptions - Options used for splitting the string.
+     */
+    splitLines(splitOptions?: { trim: boolean, removeEmptyEntries: boolean }): string[];
+}
+
+/**
+ * Split a string using the cr and lf characters and return them as an array.
+ * By default lines are trimmed and empty lines are removed.
+ * @param {SplitLinesOptions=} splitOptions - Options used for splitting the string.
+ */
+String.prototype.splitLines = function (this: string, splitOptions: { trim: boolean, removeEmptyEntries: boolean } = { removeEmptyEntries: true, trim: true }): string[] {
+    let lines = this.split(/\r?\n/g);
+    if (splitOptions && splitOptions.trim) {
+        lines = lines.filter(line => line.trim());
+    }
+    if (splitOptions && splitOptions.removeEmptyEntries) {
+        lines = lines.filter(line => line.length > 0);
+    }
+    return lines;
+};

src/client/common/constants.ts

@@ -1,26 +1,16 @@
+import { inject, injectable, named } from 'inversify';
 import * as os from 'os';
-import * as vscode from 'vscode';
+import 'reflect-metadata';
+import 'reflect-metadata';
 import { ConfigurationTarget, Uri, window, workspace } from 'vscode';
+import * as vscode from 'vscode';
 import * as settings from './configSettings';
+import { STANDARD_OUTPUT_CHANNEL } from './constants';
 import { isNotInstalledError } from './helpers';
+import { IInstaller, InstallerResponse, IOutputChannel, Product } from './types';
 import { execPythonFile, getFullyQualifiedPythonInterpreterPath, IS_WINDOWS } from './utils';
 
-export enum Product {
-    pytest = 1,
-    nosetest = 2,
-    pylint = 3,
-    flake8 = 4,
-    pep8 = 5,
-    pylama = 6,
-    prospector = 7,
-    pydocstyle = 8,
-    yapf = 9,
-    autopep8 = 10,
-    mypy = 11,
-    unittest = 12,
-    ctags = 13,
-    rope = 14
-}
+export { Product } from './types';
 
 // tslint:disable-next-line:variable-name
 const ProductInstallScripts = new Map<Product, string[]>();
@@ -156,15 +146,11 @@ ProductTypes.set(Product.rope, ProductType.RefactoringLibrary);
 
 const IS_POWERSHELL = /powershell.exe$/i;
 
-export enum InstallerResponse {
-    Installed,
-    Disabled,
-    Ignore
-}
-export class Installer implements vscode.Disposable {
+@injectable()
+export class Installer implements IInstaller {
     private static terminal: vscode.Terminal | undefined | null;
     private disposables: vscode.Disposable[] = [];
-    constructor(private outputChannel?: vscode.OutputChannel) {
+    constructor( @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel?: vscode.OutputChannel) {
         this.disposables.push(vscode.window.onDidCloseTerminal(term => {
             if (term === Installer.terminal) {
                 Installer.terminal = null;

src/client/common/extensions.ts

@@ -1,42 +1,23 @@
-import * as vscode from "vscode";
-import * as settings from "./configSettings";
+import { injectable } from 'inversify';
+import 'reflect-metadata';
+import { ILogger } from './types';
 
-let outChannel: vscode.OutputChannel;
+const PREFIX = 'Python Extension: ';
 
-class Logger {
-    public static IsDebug: boolean;
-    static initializeChannel() {
-        if (settings.PythonSettings.getInstance().devOptions.indexOf("DEBUG") >= 0) {
-            Logger.IsDebug = true;
-            outChannel = vscode.window.createOutputChannel("PythonExtLog");
-        }
+@injectable()
+export class Logger implements ILogger {
+    public logError(message: string, ex?: Error) {
+        console.error(`${PREFIX}${message}`, error);
     }
-
-    static write(category: string = "log", title: string = "", message: any) {
-        Logger.initializeChannel();
-        if (title.length > 0) {
-            Logger.writeLine(category, "---------------------------");
-            Logger.writeLine(category, title);
-        }
-
-        Logger.writeLine(category, message);
+    public logWarning(message: string, ex?: Error) {
+        console.warn(`${PREFIX}${message}`, ex);
     }
-    static writeLine(category: string = "log", line: any) {
-        if (process.env['VSC_PYTHON_CI_TEST'] !== '1') {
-            console[category](line);
-        }
-        if (outChannel) {
-            outChannel.appendLine(line);
-        }
-    }
-}
-export function error(title: string = "", message: any) {
-    Logger.write.apply(Logger, ["error", title, message]);
 }
-export function warn(title: string = "", message: any) {
-    Logger.write.apply(Logger, ["warn", title, message]);
+// tslint:disable-next-line:no-any
+export function error(title: string = '', message: any) {
+    new Logger().logError(`${title}, ${message}`);
 }
-export function log(title: string = "", message: any) {
-    if (!Logger.IsDebug) return;
-    Logger.write.apply(Logger, ["log", title, message]);
+// tslint:disable-next-line:no-any
+export function warn(title: string = '', message: any) {
+    new Logger().logWarning(`${title}, ${message}`);
 }

src/client/common/installer.ts

@@ -2,12 +2,20 @@
 // Licensed under the MIT License.
 
 import 'reflect-metadata';
+import { Disposable } from 'vscode';
 import { IServiceManager } from '../ioc/types';
+import { Installer } from './installer';
+import { Logger } from './logger';
 import { PersistentStateFactory } from './persistentState';
 import { IS_WINDOWS as isWindows } from './platform/constants';
-import { IPersistentStateFactory, IsWindows } from './types';
+import { IDiposableRegistry, IInstaller, ILogger, IPersistentStateFactory, IsWindows } from './types';
 
 export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingletonInstance<boolean>(IsWindows, isWindows);
     serviceManager.addSingleton<IPersistentStateFactory>(IPersistentStateFactory, PersistentStateFactory);
+    serviceManager.addSingleton<IInstaller>(IInstaller, Installer);
+    serviceManager.addSingleton<ILogger>(ILogger, Logger);
+
+    const disposableRegistry = serviceManager.get<Disposable[]>(IDiposableRegistry);
+    disposableRegistry.push(serviceManager.get<IInstaller>(IInstaller));
 }

src/client/common/logger.ts

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { Disposable, Uri } from 'vscode';
+
 export const IOutputChannel = Symbol('IOutputChannel');
 export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
 export const IsWindows = Symbol('IS_WINDOWS');
@@ -19,3 +21,49 @@ export interface IPersistentStateFactory {
     createGlobalPersistentState<T>(key: string, defaultValue: T): IPersistentState<T>;
     createWorkspacePersistentState<T>(key: string, defaultValue: T): IPersistentState<T>;
 }
+
+export type ExecutionInfo = {
+    execPath: string;
+    moduleName?: string;
+    args: string[];
+    product?: Product;
+};
+
+export const ILogger = Symbol('ILogger');
+
+export interface ILogger {
+    logError(message: string, error?: Error);
+    logWarning(message: string, error?: Error);
+}
+
+export enum InstallerResponse {
+    Installed,
+    Disabled,
+    Ignore
+}
+
+export enum Product {
+    pytest = 1,
+    nosetest = 2,
+    pylint = 3,
+    flake8 = 4,
+    pep8 = 5,
+    pylama = 6,
+    prospector = 7,
+    pydocstyle = 8,
+    yapf = 9,
+    autopep8 = 10,
+    mypy = 11,
+    unittest = 12,
+    ctags = 13,
+    rope = 14
+}
+
+export const IInstaller = Symbol('IInstaller');
+
+export interface IInstaller extends Disposable {
+    promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse>;
+    install(product: Product, resource?: Uri): Promise<InstallerResponse>;
+    isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined>;
+    disableLinter(product: Product, resource?: Uri): Promise<void>;
+}