Pylint no longer enabled by default.

Fix for microsoft#974

- Pylint is easily enabled for workspaces that have it available

news/1 Enhancements/974.md

@@ -0,0 +1 @@
+Pylint is no longer enabled by default. Users that have not configured pylint but who have installed it in their workspace will be asked if they'd like to enable it.

package.json

@@ -1245,7 +1245,7 @@
                 },
                 "python.linting.pylintEnabled": {
                     "type": "boolean",
-                    "default": true,
+                    "default": false,
                     "description": "Whether to lint Python files using pylint.",
                     "scope": "resource"
                 },
@@ -1678,4 +1678,4 @@
         "publisherDisplayName": "Microsoft",
         "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8"
     }
-}
+}

src/client/linters/baseLinter.ts

@@ -86,10 +86,6 @@ export abstract class BaseLinter implements ILinter {
         return this._info;
     }
 
-    public isLinterExecutableSpecified(resource: vscode.Uri) {
-        const executablePath = this.info.pathName(resource);
-        return path.basename(executablePath).length > 0 && path.basename(executablePath) !== executablePath;
-    }
     public async lint(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise<ILintMessage[]> {
         this._pythonSettings = this.configService.getSettings(document.uri);
         return this.runLinter(document, cancellation);

src/client/linters/linterCommands.ts

@@ -28,7 +28,7 @@ export class LinterCommands implements vscode.Disposable {
     public async setLinterAsync(): Promise<void> {
         const linters = this.linterManager.getAllLinterInfos();
         const suggestions = linters.map(x => x.id).sort();
-        const activeLinters = this.linterManager.getActiveLinters(this.settingsUri);
+        const activeLinters = await this.linterManager.getActiveLinters(false, this.settingsUri);
 
         let current: string;
         switch (activeLinters.length) {
@@ -64,7 +64,7 @@ export class LinterCommands implements vscode.Disposable {
 
     public async enableLintingAsync(): Promise<void> {
         const options = ['on', 'off'];
-        const current = this.linterManager.isLintingEnabled(this.settingsUri) ? options[0] : options[1];
+        const current = await this.linterManager.isLintingEnabled(false, this.settingsUri) ? options[0] : options[1];
 
         const quickPickOptions: vscode.QuickPickOptions = {
             matchOnDetail: true,

src/client/linters/linterManager.ts

@@ -1,9 +1,20 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+'use strict';
+
 import { inject, injectable } from 'inversify';
-import { CancellationToken, OutputChannel, TextDocument, Uri } from 'vscode';
-import { IConfigurationService, ILogger, Product } from '../common/types';
+import {
+    CancellationToken, MessageItem, OutputChannel,
+    TextDocument, Uri
+} from 'vscode';
+import { IApplicationShell, IWorkspaceService } from '../common/application/types';
+import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
+import { LinterInstaller } from '../common/installer/productInstaller';
+import {
+    IConfigurationService, ILogger,
+    IOutputChannel, Product
+} from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { Bandit } from './bandit';
 import { Flake8 } from './flake8';
@@ -31,8 +42,9 @@ export class LinterManager implements ILinterManager {
     private lintingEnabledSettingName = 'enabled';
     private linters: ILinterInfo[];
     private configService: IConfigurationService;
+    private checkedForInstalledLinters: boolean = false;
 
-    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
+    constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
         this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
         this.linters = [
             new LinterInfo(Product.bandit, 'bandit', this.configService),
@@ -58,26 +70,116 @@ export class LinterManager implements ILinterManager {
         throw new Error('Invalid linter');
     }
 
-    public isLintingEnabled(resource?: Uri): boolean {
+    public async isLintingEnabled(checkAvailable: boolean, resource?: Uri): Promise<boolean> {
         const settings = this.configService.getSettings(resource);
-        return (settings.linting[this.lintingEnabledSettingName] as boolean) && this.getActiveLinters(resource).length > 0;
+        const activeLintersPresent = await this.getActiveLinters(checkAvailable, resource);
+        return (settings.linting[this.lintingEnabledSettingName] as boolean) && activeLintersPresent.length > 0;
     }
 
     public async enableLintingAsync(enable: boolean, resource?: Uri): Promise<void> {
         await this.configService.updateSetting(`linting.${this.lintingEnabledSettingName}`, enable, resource);
+    }
+
+    /// Check if it is possible to enable an otherwise-unconfigured linter in the current
+    /// workspace, and if so ask the user if they want that linter configured explicitly.
+    /// Return true if a configuration change was made.
+    public async notifyUserOfInstalledLinters(linterInfo: ILinterInfo, resource?: Uri): Promise<boolean> {
+        // if we've already checked during this session, don't bother again
+        if (!this.checkedForInstalledLinters) {
+            this.checkedForInstalledLinters = true;
+        } else {
+            return false;
+        }
+
+        // If linting is disabled, we are finished.
+        if (!await this.isLintingEnabled(false, resource)) {
+            return false;
+        }
+
+        // Has the linter in question has been configured explicitly? If so, no need to continue.
+        if (!this.isLinterUnset(linterInfo, resource)) {
+            return false;
+        }
 
-        // If nothing is enabled, fix it up to PyLint (default).
-        if (enable && this.getActiveLinters(resource).length === 0) {
-            await this.setActiveLintersAsync([Product.pylint], resource);
+        // Is the linter available in the current workspace?
+        if (await this.isLinterAvailable(linterInfo.product, resource)) {
+            // ...ask the user if they would like to enable it.
+            return this.notifyUserAndConfigureLinter(linterInfo);
+        } else {
+            return false;
         }
+
     }
 
-    public getActiveLinters(resource?: Uri): ILinterInfo[] {
-        return this.linters.filter(x => x.isEnabled(resource));
+    /// Raise a dialog asking the user if they would like to explicitly configure a linter or not.
+    /// Return true if a config change was made.
+    public async notifyUserAndConfigureLinter(linterInfo: ILinterInfo): Promise<boolean> {
+        const appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
+
+        class ConfigureLinterMessage implements MessageItem {
+            public enabled: boolean;
+            public title: string;
+            constructor() {
+                this.enabled = true;
+                this.title = '';
+            }
+        }
+
+        const optButtons: ConfigureLinterMessage[] = [
+            {
+                title: `Enable ${linterInfo.id}`,
+                enabled: true
+            },
+            {
+                title: `Disable ${linterInfo.id}`,
+                enabled: false
+            }
+        ];
+        const pick = await appShell.showInformationMessage(`Linter ${linterInfo.id} is available but not enabled.`, ...optButtons);
+        if (pick) {
+            await linterInfo.enableAsync(pick.enabled);
+            return true;
+        }
+
+        return false;
+    }
+
+    /// Check if the linter itself is available in the workspace's Python environment or not.
+    /// Return true if the linter is present.
+    public async isLinterAvailable(linterProduct: Product, resource?: Uri): Promise<boolean | undefined> {
+        const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
+        const linterInstaller = new LinterInstaller(this.serviceContainer, outputChannel);
+
+        return linterInstaller.isInstalled(linterProduct, resource);
+    }
+
+    /// Check if the given linter has been configured by the user in this workspace or not.
+    /// Return true if no explicit setting for the linter has been made.
+    public isLinterUnset(linterInfo: ILinterInfo, resource?: Uri) {
+        const workspaceConfig = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
+        const ws = workspaceConfig.getConfiguration('python.linting', resource);
+        const pe = ws!.inspect(linterInfo.enabledSettingName);
+        return (pe!.globalValue === undefined && pe!.workspaceValue === undefined && pe!.workspaceFolderValue === undefined);
+    }
+
+    public async getActiveLinters(checkAvailable: boolean, resource?: Uri): Promise<ILinterInfo[]> {
+        let activeLinters = this.linters.filter(x => x.isEnabled(resource));
+
+        if (checkAvailable) {
+            // only ask the user if they'd like to enable pylint when it is available... others may follow.
+            const pylintInfo = this.linters.find((linter: ILinterInfo) => linter.id === 'pylint');
+            if (pylintInfo) {
+                const change = await this.notifyUserOfInstalledLinters(pylintInfo, resource);
+                if (change) {
+                    activeLinters = this.linters.filter(x => x.isEnabled(resource));
+                }
+            }
+        }
+        return activeLinters;
     }
 
     public async setActiveLintersAsync(products: Product[], resource?: Uri): Promise<void> {
-        const active = this.getActiveLinters(resource);
+        const active = await this.getActiveLinters(false, resource);
         for (const x of active) {
             await x.enableAsync(false, resource);
         }
@@ -90,8 +192,8 @@ export class LinterManager implements ILinterManager {
         }
     }
 
-    public createLinter(product: Product, outputChannel: OutputChannel, serviceContainer: IServiceContainer, resource?: Uri): ILinter {
-        if (!this.isLintingEnabled(resource)) {
+    public async createLinter(product: Product, outputChannel: OutputChannel, serviceContainer: IServiceContainer, resource?: Uri): Promise<ILinter> {
+        if (!await this.isLintingEnabled(false, resource)) {
             return new DisabledLinter(this.configService);
         }
         const error = 'Linter manager: Unknown linter';

src/client/linters/lintingEngine.ts

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+'use strict';
+
 import { inject, injectable } from 'inversify';
 import { Minimatch } from 'minimatch';
 import * as path from 'path';
@@ -93,10 +95,16 @@ export class LintingEngine implements ILintingEngine {
 
         this.pendingLintings.set(document.uri.fsPath, cancelToken);
 
-        const promises: Promise<ILintMessage[]>[] = this.linterManager.getActiveLinters(document.uri)
-            .map(info => {
+        const activeLinters = await this.linterManager.getActiveLinters(true, document.uri);
+        const promises: Promise<ILintMessage[]>[] = activeLinters
+            .map(async (info: ILinterInfo) => {
                 const stopWatch = new StopWatch();
-                const linter = this.linterManager.createLinter(info.product, this.outputChannel, this.serviceContainer, document.uri);
+                const linter = await this.linterManager.createLinter(
+                    info.product,
+                    this.outputChannel,
+                    this.serviceContainer,
+                    document.uri
+                );
                 const promise = linter.lint(document, cancelToken.token);
                 this.sendLinterRunTelemetry(info, document.uri, promise, stopWatch, trigger);
                 return promise;
@@ -175,7 +183,7 @@ export class LintingEngine implements ILintingEngine {
     }
 
     private async shouldLintDocument(document: vscode.TextDocument): Promise<boolean> {
-        if (!this.linterManager.isLintingEnabled(document.uri)) {
+        if (!await this.linterManager.isLintingEnabled(true, document.uri)) {
             this.diagnosticCollection.set(document.uri, []);
             return false;
         }

src/client/linters/types.ts

@@ -19,7 +19,7 @@ export interface ILinterInfo {
     readonly argsSettingName: string;
     readonly enabledSettingName: string;
     readonly configFileNames: string[];
-    enableAsync(flag: boolean, resource?: vscode.Uri): Promise<void>;
+    enableAsync(enabled: boolean, resource?: vscode.Uri): Promise<void>;
     isEnabled(resource?: vscode.Uri): boolean;
     pathName(resource?: vscode.Uri): string;
     linterArgs(resource?: vscode.Uri): string[];
@@ -35,11 +35,11 @@ export const ILinterManager = Symbol('ILinterManager');
 export interface ILinterManager {
     getAllLinterInfos(): ILinterInfo[];
     getLinterInfo(product: Product): ILinterInfo;
-    getActiveLinters(resource?: vscode.Uri): ILinterInfo[];
-    isLintingEnabled(resource?: vscode.Uri): boolean;
+    getActiveLinters(checkAvailable: boolean, resource?: vscode.Uri): Promise<ILinterInfo[]>;
+    isLintingEnabled(checkAvailable: boolean, resource?: vscode.Uri): Promise<boolean>;
     enableLintingAsync(enable: boolean, resource?: vscode.Uri): Promise<void>;
     setActiveLintersAsync(products: Product[], resource?: vscode.Uri): Promise<void>;
-    createLinter(product: Product, outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer, resource?: vscode.Uri): ILinter;
+    createLinter(product: Product, outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer, resource?: vscode.Uri): Promise<ILinter>;
 }
 
 export interface ILintMessage {

src/client/providers/linterProvider.ts

@@ -1,9 +1,13 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+'use strict';
+
 import * as path from 'path';
-import * as vscode from 'vscode';
-import { ConfigurationTarget, Uri, workspace } from 'vscode';
+import {
+    ConfigurationTarget, Disposable, ExtensionContext,
+    TextDocument, Uri, workspace
+} from 'vscode';
 import { IDocumentManager } from '../common/application/types';
 import { ConfigSettingMonitor } from '../common/configSettingMonitor';
 import { isTestExecution } from '../common/constants';
@@ -13,9 +17,9 @@ import { IInterpreterService } from '../interpreter/contracts';
 import { IServiceContainer } from '../ioc/types';
 import { ILinterManager, ILintingEngine } from '../linters/types';
 
-export class LinterProvider implements vscode.Disposable {
-    private context: vscode.ExtensionContext;
-    private disposables: vscode.Disposable[];
+export class LinterProvider implements Disposable {
+    private context: ExtensionContext;
+    private disposables: Disposable[];
     private configMonitor: ConfigSettingMonitor;
     private interpreterService: IInterpreterService;
     private documents: IDocumentManager;
@@ -24,7 +28,7 @@ export class LinterProvider implements vscode.Disposable {
     private engine: ILintingEngine;
     private fs: IFileSystem;
 
-    public constructor(context: vscode.ExtensionContext, serviceContainer: IServiceContainer) {
+    public constructor(context: ExtensionContext, serviceContainer: IServiceContainer) {
         this.context = context;
         this.disposables = [];
 
@@ -39,7 +43,7 @@ export class LinterProvider implements vscode.Disposable {
 
         this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions);
         this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions);
-        this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions);
+        this.documents.onDidSaveTextDocument(async (e) => this.onDocumentSaved(e), this.context.subscriptions);
 
         this.configMonitor = new ConfigSettingMonitor('linting');
         this.configMonitor.on('change', this.lintSettingsChangedHandler.bind(this));
@@ -56,7 +60,7 @@ export class LinterProvider implements vscode.Disposable {
         this.configMonitor.dispose();
     }
 
-    private isDocumentOpen(uri: vscode.Uri): boolean {
+    private isDocumentOpen(uri: Uri): boolean {
         return this.documents.textDocuments.some(document => this.fs.arePathsSame(document.uri.fsPath, uri.fsPath));
     }
 
@@ -74,26 +78,26 @@ export class LinterProvider implements vscode.Disposable {
         });
     }
 
-    private onDocumentOpened(document: vscode.TextDocument): void {
+    private onDocumentOpened(document: TextDocument): void {
         this.engine.lintDocument(document, 'auto').ignoreErrors();
     }
 
-    private onDocumentSaved(document: vscode.TextDocument): void {
+    private async onDocumentSaved(document: TextDocument): Promise<void> {
         const settings = this.configuration.getSettings(document.uri);
         if (document.languageId === 'python' && settings.linting.enabled && settings.linting.lintOnSave) {
             this.engine.lintDocument(document, 'save').ignoreErrors();
             return;
         }
 
-        const linters = this.linterManager.getActiveLinters(document.uri);
+        const linters = await this.linterManager.getActiveLinters(true, document.uri);
         const fileName = path.basename(document.uri.fsPath).toLowerCase();
         const watchers = linters.filter((info) => info.configFileNames.indexOf(fileName) >= 0);
         if (watchers.length > 0) {
             setTimeout(() => this.engine.lintOpenPythonFiles(), 1000);
         }
     }
 
-    private onDocumentClosed(document: vscode.TextDocument) {
+    private onDocumentClosed(document: TextDocument) {
         if (!document || !document.fileName || !document.uri) {
             return;
         }