microsoft · karthiknadig · May 4, 2021 · Apr 28, 2021 · Apr 29, 2021 · Apr 29, 2021
diff --git a/.github/test_plan.md b/.github/test_plan.md
@@ -141,37 +141,15 @@ SPAM='hello ${WHO}'
     -   [ ] Create a virtual environment
     -   [ ] Install `requests` into the virtual environment
 
-#### Pylint/default linting
-
-[Prompting to install Pylint is covered under `Environments` above]
-
-For testing the disablement of the default linting rules for Pylint:
-
-```ini
-# pylintrc
-[MESSAGES CONTROL]
-enable=bad-names
-```
-
-```python3
-# example.py
-foo = 42  # Marked as a disallowed name.
-```
-
--   [ ] Installation via the prompt installs Pylint as appropriate
-    -   [ ] Uses `--user` for system-install of Python
-    -   [ ] Installs into a virtual environment environment directly
--   [ ] Pylint works
--   [ ] `"python.linting.pylintUseMinimalCheckers": false` turns off the default rules w/ no `pylintrc` file present
--   [ ] The existence of a `pylintrc` file turns off the default rules
-
-#### Other linters
+#### Linting
 
 **Note**:
 
 -   You can use the `Run Linting` command to run a newly installed linter
 -   When the extension installs a new linter, it turns off all other linters
 
+-   [ ] pylint works
+    -   [ ] `Select linter` lists the linter and installs it if necessary
 -   [ ] flake8 works
     -   [ ] `Select linter` lists the linter and installs it if necessary
 -   [ ] mypy works

diff --git a/package.json b/package.json
@@ -1635,7 +1635,7 @@
                 },
                 "python.linting.pylintEnabled": {
                     "type": "boolean",
-                    "default": true,
+                    "default": false,
                     "description": "Whether to lint Python files using pylint.",
                     "scope": "resource"
                 },
@@ -1645,12 +1645,6 @@
                     "description": "Path to Pylint, you can use a custom version of pylint by modifying this setting to include the full path.",
                     "scope": "resource"
                 },
-                "python.linting.pylintUseMinimalCheckers": {
-                    "type": "boolean",
-                    "default": true,
-                    "description": "Whether to run Pylint with minimal set of rules.",
-                    "scope": "resource"
-                },
                 "python.pythonPath": {
                     "type": "string",
                     "default": "python",

diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts
@@ -397,7 +397,6 @@ export class PythonSettings implements IPythonSettings {
                       error: DiagnosticSeverity.Error,
                       note: DiagnosticSeverity.Hint,
                   },
-                  pylintUseMinimalCheckers: false,
               };
         this.linting.pylintPath = getAbsolutePath(systemVariables.resolveAny(this.linting.pylintPath), workspaceRoot);
         this.linting.flake8Path = getAbsolutePath(systemVariables.resolveAny(this.linting.flake8Path), workspaceRoot);

diff --git a/src/client/common/types.ts b/src/client/common/types.ts
@@ -271,7 +271,6 @@ export interface ILintingSettings {
     banditEnabled: boolean;
     banditArgs: string[];
     banditPath: string;
-    readonly pylintUseMinimalCheckers: boolean;
 }
 export interface IFormattingSettings {
     readonly provider: string;

diff --git a/src/client/linters/linterAvailability.ts b/src/client/linters/linterAvailability.ts
@@ -6,11 +6,10 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Uri } from 'vscode';
-import { LanguageServerType } from '../activation/types';
 import { IApplicationShell, IWorkspaceService } from '../common/application/types';
 import '../common/extensions';
 import { IFileSystem } from '../common/platform/types';
-import { IConfigurationService, IPersistentStateFactory, Resource } from '../common/types';
+import { IPersistentStateFactory, Resource } from '../common/types';
 import { Common, Linters } from '../common/utils/localize';
 import { sendTelemetryEvent } from '../telemetry';
 import { EventName } from '../telemetry/constants';
@@ -23,7 +22,6 @@ export class AvailableLinterActivator implements IAvailableLinterActivator {
         @inject(IApplicationShell) private appShell: IApplicationShell,
         @inject(IFileSystem) private fs: IFileSystem,
         @inject(IWorkspaceService) private workspaceService: IWorkspaceService,
-        @inject(IConfigurationService) private configService: IConfigurationService,
         @inject(IPersistentStateFactory) private persistentStateFactory: IPersistentStateFactory,
     ) {}
 
@@ -38,11 +36,6 @@ export class AvailableLinterActivator implements IAvailableLinterActivator {
      * @returns true if configuration was updated in any way, false otherwise.
      */
     public async promptIfLinterAvailable(linterInfo: ILinterInfo, resource?: Uri): Promise<boolean> {
-        // Has the feature been enabled yet?
-        if (!this.isFeatureEnabled) {
-            return false;
-        }
-
         // Has the linter in question has been configured explicitly? If so, no need to continue.
         if (!this.isLinterUsingDefaultConfiguration(linterInfo, resource)) {
             return false;
@@ -128,17 +121,4 @@ export class AvailableLinterActivator implements IAvailableLinterActivator {
             pe!.globalValue === undefined && pe!.workspaceValue === undefined && pe!.workspaceFolderValue === undefined
         );
     }
-
-    /**
-     * Check if this feature is enabled yet.
-     *
-     * This is a feature of the vscode-python extension that will become enabled once the
-     * Python Language Server becomes the default, replacing Jedi as the default. Testing
-     * the global default setting for `"python.languageServer": !Jedi` enables it.
-     *
-     * @returns true if the global default for python.languageServer is not Jedi.
-     */
-    public get isFeatureEnabled(): boolean {
-        return this.configService.getSettings().languageServer !== LanguageServerType.Jedi;
-    }
 }
diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts
@@ -36,7 +36,7 @@ export class LinterCommands implements IDisposable {
         const linters = this.linterManager.getAllLinterInfos();
         const suggestions = linters.map((x) => x.id).sort();
         const linterList = ['Disable Linting', ...suggestions];
-        const activeLinters = await this.linterManager.getActiveLinters(true, this.settingsUri);
+        const activeLinters = await this.linterManager.getActiveLinters(this.settingsUri);
 
         let current: string;
         switch (activeLinters.length) {
@@ -82,7 +82,7 @@ export class LinterCommands implements IDisposable {
 
     public async enableLintingAsync(): Promise<void> {
         const options = ['Enable', 'Disable'];
-        const current = (await this.linterManager.isLintingEnabled(true, this.settingsUri)) ? options[0] : options[1];
+        const current = (await this.linterManager.isLintingEnabled(this.settingsUri)) ? options[0] : options[1];
 
         const quickPickOptions: QuickPickOptions = {
             matchOnDetail: true,

diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts
@@ -3,8 +3,6 @@
 
 import * as path from 'path';
 import { Uri } from 'vscode';
-import { LanguageServerType } from '../activation/types';
-import { IWorkspaceService } from '../common/application/types';
 import { ExecutionInfo, IConfigurationService, Product } from '../common/types';
 import { ILinterInfo, LinterId } from './types';
 
@@ -74,38 +72,3 @@ export class LinterInfo implements ILinterInfo {
         return { execPath, moduleName, args, product: this.product };
     }
 }
-
-export class PylintLinterInfo extends LinterInfo {
-    constructor(
-        configService: IConfigurationService,
-        private readonly workspaceService: IWorkspaceService,
-        configFileNames: string[] = [],
-    ) {
-        super(Product.pylint, LinterId.PyLint, configService, configFileNames);
-    }
-    public isEnabled(resource?: Uri): boolean {
-        // We want to be sure the setting is not default since default is `true` and hence
-        // missing setting yields `true`. When setting is missing and LS is non-Jedi,
-        // we want default to be `false`. So inspection here makes sure we are not getting
-        // `true` because there is no setting and LS is active.
-        const enabled = super.isEnabled(resource); // Is it enabled by settings?
-        const usingJedi = this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi;
-        if (usingJedi) {
-            // In Jedi case adhere to default behavior. Missing setting means `enabled`.
-            return enabled;
-        }
-        // If we're using LS, then by default Pylint is disabled unless user provided
-        // the value. We have to resort to direct inspection of settings here.
-        const configuration = this.workspaceService.getConfiguration('python', resource);
-        const inspection = configuration.inspect<boolean>(`linting.${this.enabledSettingName}`);
-        if (
-            !inspection ||
-            (inspection.globalValue === undefined &&
-                inspection.workspaceFolderValue === undefined &&
-                inspection.workspaceValue === undefined)
-        ) {
-            return false;
-        }
-        return enabled;
-    }
-}
diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts
@@ -5,20 +5,19 @@
 
 import { inject, injectable } from 'inversify';
 import { CancellationToken, OutputChannel, TextDocument, Uri } from 'vscode';
-import { IWorkspaceService } from '../common/application/types';
 import { traceError } from '../common/logger';
 import { IConfigurationService, Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { Bandit } from './bandit';
 import { Flake8 } from './flake8';
-import { LinterInfo, PylintLinterInfo } from './linterInfo';
+import { LinterInfo } from './linterInfo';
 import { MyPy } from './mypy';
 import { Prospector } from './prospector';
 import { Pycodestyle } from './pycodestyle';
 import { PyDocStyle } from './pydocstyle';
 import { PyLama } from './pylama';
 import { Pylint } from './pylint';
-import { IAvailableLinterActivator, ILinter, ILinterInfo, ILinterManager, ILintMessage, LinterId } from './types';
+import { ILinter, ILinterInfo, ILinterManager, ILintMessage, LinterId } from './types';
 
 class DisabledLinter implements ILinter {
     constructor(private configService: IConfigurationService) {}
@@ -33,19 +32,13 @@ class DisabledLinter implements ILinter {
 @injectable()
 export class LinterManager implements ILinterManager {
     protected linters: ILinterInfo[];
-    private configService: IConfigurationService;
-    private checkedForInstalledLinters = new Set<string>();
 
-    constructor(
-        @inject(IServiceContainer) private serviceContainer: IServiceContainer,
-        @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
-    ) {
-        this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
+    constructor(@inject(IConfigurationService) private configService: IConfigurationService) {
         // Note that we use unit tests to ensure all the linters are here.
         this.linters = [
             new LinterInfo(Product.bandit, LinterId.Bandit, this.configService),
             new LinterInfo(Product.flake8, LinterId.Flake8, this.configService),
-            new PylintLinterInfo(this.configService, this.workspaceService, ['.pylintrc', 'pylintrc']),
+            new LinterInfo(Product.pylint, LinterId.PyLint, this.configService, ['pylintrc', '.pylintrc']),
             new LinterInfo(Product.mypy, LinterId.MyPy, this.configService),
             new LinterInfo(Product.pycodestyle, LinterId.PyCodeStyle, this.configService),
             new LinterInfo(Product.prospector, LinterId.Prospector, this.configService),
@@ -66,20 +59,17 @@ export class LinterManager implements ILinterManager {
         throw new Error(`Invalid linter '${Product[product]}'`);
     }
 
-    public async isLintingEnabled(silent: boolean, resource?: Uri): Promise<boolean> {
+    public async isLintingEnabled(resource?: Uri): Promise<boolean> {
         const settings = this.configService.getSettings(resource);
-        const activeLintersPresent = await this.getActiveLinters(silent, resource);
+        const activeLintersPresent = await this.getActiveLinters(resource);
         return settings.linting.enabled && activeLintersPresent.length > 0;
     }
 
     public async enableLintingAsync(enable: boolean, resource?: Uri): Promise<void> {
         await this.configService.updateSetting('linting.enabled', enable, resource);
     }
 
-    public async getActiveLinters(silent: boolean, resource?: Uri): Promise<ILinterInfo[]> {
-        if (!silent) {
-            await this.enableUnconfiguredLinters(resource);
-        }
+    public async getActiveLinters(resource?: Uri): Promise<ILinterInfo[]> {
         return this.linters.filter((x) => x.isEnabled(resource));
     }
 
@@ -93,7 +83,7 @@ export class LinterManager implements ILinterManager {
 
         // if we have valid linter product(s), enable only those
         if (validProducts.length > 0) {
-            const active = await this.getActiveLinters(true, resource);
+            const active = await this.getActiveLinters(resource);
             for (const x of active) {
                 await x.enableAsync(false, resource);
             }
@@ -113,7 +103,7 @@ export class LinterManager implements ILinterManager {
         serviceContainer: IServiceContainer,
         resource?: Uri,
     ): Promise<ILinter> {
-        if (!(await this.isLintingEnabled(true, resource))) {
+        if (!(await this.isLintingEnabled(resource))) {
             return new DisabledLinter(this.configService);
         }
         const error = 'Linter manager: Unknown linter';
@@ -140,22 +130,4 @@ export class LinterManager implements ILinterManager {
         }
         throw new Error(error);
     }
-
-    protected async enableUnconfiguredLinters(resource?: Uri): Promise<void> {
-        const settings = this.configService.getSettings(resource);
-        if (!settings.linting.pylintEnabled || !settings.linting.enabled) {
-            return;
-        }
-        // If we've already checked during this session for the same workspace and Python path, then don't bother again.
-        const workspaceKey = `${this.workspaceService.getWorkspaceFolderIdentifier(resource)}${settings.pythonPath}`;
-        if (this.checkedForInstalledLinters.has(workspaceKey)) {
-            return;
-        }
-        this.checkedForInstalledLinters.add(workspaceKey);
-
-        // only check & ask the user if they'd like to enable pylint
-        const pylintInfo = this.linters.find((linter) => linter.id === 'pylint');
-        const activator = this.serviceContainer.get<IAvailableLinterActivator>(IAvailableLinterActivator);
-        await activator.promptIfLinterAvailable(pylintInfo!, resource);
-    }
 }
diff --git a/src/client/linters/lintingEngine.ts b/src/client/linters/lintingEngine.ts
@@ -90,7 +90,7 @@ export class LintingEngine implements ILintingEngine {
 
         this.pendingLintings.set(document.uri.fsPath, cancelToken);
 
-        const activeLinters = await this.linterManager.getActiveLinters(false, document.uri);
+        const activeLinters = await this.linterManager.getActiveLinters(document.uri);
         const promises: Promise<ILintMessage[]>[] = activeLinters.map(async (info: ILinterInfo) => {
             const stopWatch = new StopWatch();
             const linter = await this.linterManager.createLinter(
@@ -161,7 +161,7 @@ export class LintingEngine implements ILintingEngine {
     }
 
     private async shouldLintDocument(document: vscode.TextDocument): Promise<boolean> {
-        if (!(await this.linterManager.isLintingEnabled(false, document.uri))) {
+        if (!(await this.linterManager.isLintingEnabled(document.uri))) {
             this.diagnosticCollection.set(document.uri, []);
             return false;
         }