Skip to content

Commit df39217

Browse files
author
Mikhail Arkhipov
authored
Implements linting configuration (#599)
* 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 * Round I * Round 2 * Round 3 * Round 4 * Round 5 * no message * Round 6 * Round 7 * Clean up targets and messages * Settings propagation * Tests * Test warning * Fix installer tests * Tests * Test fixes * Fix terminal service and tests async/await * Fix mock setup * Test fix * Test async/await fix * Test fix + activate tslint on awaits * Use command manager * Work around updateSettings * Multiroot fixes, partial * More workarounds * Multiroot tests * Fix installer test * Test fixes * Disable prospector * Enable dispose in all cases * Fix event firing * Min pylint options * Min checkers & pylintrc discovery * Fix Windows path in tests for Travis * Fix Mac test * Test fix
1 parent 0253995 commit df39217

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+8007
-849
lines changed

‎package-lock.json

Lines changed: 6322 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@
7676
"onCommand:python.buildWorkspaceSymbols",
7777
"onCommand:python.updateSparkLibrary",
7878
"onCommand:python.startREPL",
79-
"onCommand:python.goToPythonObject"
79+
"onCommand:python.goToPythonObject",
80+
"onCommand:python.setLinter",
81+
"onCommand:python.enableLinting"
8082
],
8183
"main": "./out/client/extension",
8284
"contributes": {
@@ -213,6 +215,16 @@
213215
"command": "python.goToPythonObject",
214216
"title": "%python.command.python.goToPythonObject.title%",
215217
"category": "Python"
218+
},
219+
{
220+
"command": "python.setLinter",
221+
"title": "%python.command.python.setLinter.title%",
222+
"category": "Python"
223+
},
224+
{
225+
"command": "python.enableLinting",
226+
"title": "%python.command.python.enableLinting.title%",
227+
"category": "Python"
216228
}
217229
],
218230
"menus": {
@@ -949,12 +961,6 @@
949961
"description": "Whether to lint Python files.",
950962
"scope": "resource"
951963
},
952-
"python.linting.enabledWithoutWorkspace": {
953-
"type": "boolean",
954-
"default": true,
955-
"description": "Whether to lint Python files when no workspace is opened.",
956-
"scope": "resource"
957-
},
958964
"python.linting.prospectorEnabled": {
959965
"type": "boolean",
960966
"default": false,
@@ -1003,6 +1009,12 @@
10031009
"description": "Controls the maximum number of problems produced by the server.",
10041010
"scope": "resource"
10051011
},
1012+
"python.linting.pylintUseMinimalCheckers": {
1013+
"type": "boolean",
1014+
"default": true,
1015+
"description": "Whether to run Pylint with minimal set of rules.",
1016+
"scope": "resource"
1017+
},
10061018
"python.linting.pylintCategorySeverity.convention": {
10071019
"type": "string",
10081020
"default": "Information",

‎package.nls.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
"python.command.jupyter.gotToPreviousCell.title": "Go to Previous Cell",
2424
"python.command.jupyter.gotToNextCell.title": "Go to Next Cell",
2525
"python.command.python.goToPythonObject.title": "Go to Python Object",
26+
"python.command.python.setLinter.title": "Select Linter",
27+
"python.command.python.enableLinting.title": "Enable Linting",
2628
"python.snippet.launch.standard.label": "Python",
2729
"python.snippet.launch.standard.description": "Debug a Python program with standard output",
2830
"python.snippet.launch.pyspark.label": "Python: PySpark",

‎package.nls.ru.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
"python.command.jupyter.gotToPreviousCell.title": "Перейти к предыдущей ячейке",
2424
"python.command.jupyter.gotToNextCell.title": "Перейти к следующей ячейке",
2525
"python.command.python.goToPythonObject.title": "Перейти к объекту Python",
26+
"python.command.python.setLinter.title": "Выбрать анализатор кода",
27+
"python.command.python.enableLinting.title": "Включить анализатор кода",
2628
"python.snippet.launch.standard.label": "Python",
2729
"python.snippet.launch.standard.description": "Отладить программу Python со стандартным выводом",
2830
"python.snippet.launch.pyspark.label": "Python: PySpark",

‎src/client/common/configSettings.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as child_process from 'child_process';
44
import { EventEmitter } from 'events';
55
import * as path from 'path';
66
import * as vscode from 'vscode';
7-
import { Uri } from 'vscode';
7+
import { ConfigurationTarget, Uri } from 'vscode';
88
import {
99
IAutoCompeteSettings,
1010
IFormattingSettings,
@@ -61,19 +61,29 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
6161
}
6262
// tslint:disable-next-line:function-name
6363
public static getInstance(resource?: Uri): PythonSettings {
64-
const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined;
65-
let workspaceFolderUri: Uri | undefined = workspaceFolder ? workspaceFolder.uri : undefined;
66-
if (!workspaceFolderUri && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) {
67-
workspaceFolderUri = vscode.workspace.workspaceFolders[0].uri;
68-
}
64+
const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource).uri;
6965
const workspaceFolderKey = workspaceFolderUri ? workspaceFolderUri.fsPath : '';
66+
7067
if (!PythonSettings.pythonSettings.has(workspaceFolderKey)) {
7168
const settings = new PythonSettings(workspaceFolderUri);
7269
PythonSettings.pythonSettings.set(workspaceFolderKey, settings);
7370
}
7471
// tslint:disable-next-line:no-non-null-assertion
7572
return PythonSettings.pythonSettings.get(workspaceFolderKey)!;
7673
}
74+
75+
public static getSettingsUriAndTarget(resource?: Uri): { uri: Uri | undefined, target: ConfigurationTarget } {
76+
const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined;
77+
let workspaceFolderUri: Uri | undefined = workspaceFolder ? workspaceFolder.uri : undefined;
78+
79+
if (!workspaceFolderUri && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) {
80+
workspaceFolderUri = vscode.workspace.workspaceFolders[0].uri;
81+
}
82+
83+
const target = workspaceFolderUri ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Global;
84+
return { uri: workspaceFolderUri, target };
85+
}
86+
7787
// tslint:disable-next-line:function-name
7888
public static dispose() {
7989
if (!isTestExecution()) {
@@ -138,7 +148,6 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
138148
// Support for travis.
139149
this.linting = this.linting ? this.linting : {
140150
enabled: false,
141-
enabledWithoutWorkspace: false,
142151
ignorePatterns: [],
143152
flake8Args: [], flake8Enabled: false, flake8Path: 'flake',
144153
lintOnSave: false, maxNumberOfProblems: 100,
@@ -167,7 +176,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
167176
mypyCategorySeverity: {
168177
error: vscode.DiagnosticSeverity.Error,
169178
note: vscode.DiagnosticSeverity.Hint
170-
}
179+
},
180+
pylintUseMinimalCheckers: false
171181
};
172182
this.linting.pylintPath = getAbsolutePath(systemVariables.resolveAny(this.linting.pylintPath), workspaceRoot);
173183
this.linting.flake8Path = getAbsolutePath(systemVariables.resolveAny(this.linting.flake8Path), workspaceRoot);

‎src/client/common/configuration/service.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33

44
import { injectable } from 'inversify';
5-
import { Uri } from 'vscode';
5+
import { ConfigurationTarget, Uri, workspace, WorkspaceConfiguration } from 'vscode';
66
import { PythonSettings } from '../configSettings';
77
import { IConfigurationService, IPythonSettings } from '../types';
88

@@ -11,4 +11,48 @@ export class ConfigurationService implements IConfigurationService {
1111
public getSettings(resource?: Uri): IPythonSettings {
1212
return PythonSettings.getInstance(resource);
1313
}
14+
public async updateSettingAsync(setting: string, value?: {}, resource?: Uri, configTarget?: ConfigurationTarget): Promise<void> {
15+
const settingsInfo = PythonSettings.getSettingsUriAndTarget(resource);
16+
17+
const pythonConfig = workspace.getConfiguration('python', settingsInfo.uri);
18+
const currentValue = pythonConfig.inspect(setting);
19+
20+
if (currentValue !== undefined &&
21+
((settingsInfo.target === ConfigurationTarget.Global && currentValue.globalValue === value) ||
22+
(settingsInfo.target === ConfigurationTarget.Workspace && currentValue.workspaceValue === value) ||
23+
(settingsInfo.target === ConfigurationTarget.WorkspaceFolder && currentValue.workspaceFolderValue === value))) {
24+
return;
25+
}
26+
27+
await pythonConfig.update(setting, value, settingsInfo.target);
28+
await this.verifySetting(pythonConfig, settingsInfo.target, setting, value);
29+
}
30+
31+
public isTestExecution(): boolean {
32+
return process.env.VSC_PYTHON_CI_TEST === '1';
33+
}
34+
35+
private async verifySetting(pythonConfig: WorkspaceConfiguration, target: ConfigurationTarget, settingName: string, value?: {}): Promise<void> {
36+
if (this.isTestExecution()) {
37+
let retries = 0;
38+
do {
39+
const setting = pythonConfig.inspect(settingName);
40+
if (!setting && value === undefined) {
41+
break; // Both are unset
42+
}
43+
if (setting && value !== undefined) {
44+
// Both specified
45+
const actual = target === ConfigurationTarget.Global
46+
? setting.globalValue
47+
: target === ConfigurationTarget.Workspace ? setting.workspaceValue : setting.workspaceFolderValue;
48+
if (actual === value) {
49+
break;
50+
}
51+
}
52+
// Wait for settings to get refreshed.
53+
await new Promise((resolve, reject) => setTimeout(resolve, 250));
54+
retries += 1;
55+
} while (retries < 20);
56+
}
57+
}
1458
}

‎src/client/common/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ export const PythonLanguage = { language: 'python' };
33

44
export namespace Commands {
55
export const Set_Interpreter = 'python.setInterpreter';
6+
export const Set_ShebangInterpreter = 'python.setShebangInterpreter';
67
export const Exec_In_Terminal = 'python.execInTerminal';
78
export const Exec_Selection_In_Terminal = 'python.execSelectionInTerminal';
89
export const Exec_Selection_In_Django_Shell = 'python.execSelectionInDjangoShell';
@@ -27,6 +28,8 @@ export namespace Commands {
2728
export const Update_SparkLibrary = 'python.updateSparkLibrary';
2829
export const Build_Workspace_Symbols = 'python.buildWorkspaceSymbols';
2930
export const Start_REPL = 'python.startREPL';
31+
export const Set_Linter = 'python.setLinter';
32+
export const Enable_Linter = 'python.enableLinting';
3033
}
3134
export namespace Octicons {
3235
export const Test_Pass = '$(check)';

‎src/client/common/errors/moduleNotInstalledError.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@
33

44
export class ModuleNotInstalledError extends Error {
55
constructor(moduleName: string) {
6-
super(`Module '${moduleName} not installed.`);
6+
super(`Module '${moduleName}' not installed.`);
77
}
88
}

‎src/client/common/installer/installer.ts

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ConfigurationTarget, QuickPickItem, Uri, window, workspace } from 'vsco
55
import * as vscode from 'vscode';
66
import { IFormatterHelper } from '../../formatters/types';
77
import { IServiceContainer } from '../../ioc/types';
8-
import { ILinterHelper } from '../../linters/types';
8+
import { ILinterManager } from '../../linters/types';
99
import { ITestsHelper } from '../../unittests/common/types';
1010
import { PythonSettings } from '../configSettings';
1111
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
@@ -34,17 +34,6 @@ ProductNames.set(Product.pytest, 'pytest');
3434
ProductNames.set(Product.yapf, 'yapf');
3535
ProductNames.set(Product.rope, 'rope');
3636

37-
export const SettingToDisableProduct = new Map<Product, string>();
38-
SettingToDisableProduct.set(Product.flake8, 'linting.flake8Enabled');
39-
SettingToDisableProduct.set(Product.mypy, 'linting.mypyEnabled');
40-
SettingToDisableProduct.set(Product.nosetest, 'unitTest.nosetestsEnabled');
41-
SettingToDisableProduct.set(Product.pep8, 'linting.pep8Enabled');
42-
SettingToDisableProduct.set(Product.pylama, 'linting.pylamaEnabled');
43-
SettingToDisableProduct.set(Product.prospector, 'linting.prospectorEnabled');
44-
SettingToDisableProduct.set(Product.pydocstyle, 'linting.pydocstyleEnabled');
45-
SettingToDisableProduct.set(Product.pylint, 'linting.pylintEnabled');
46-
SettingToDisableProduct.set(Product.pytest, 'unitTest.pyTestEnabled');
47-
4837
// tslint:disable-next-line:variable-name
4938
const ProductInstallationPrompt = new Map<Product, string>();
5039
ProductInstallationPrompt.set(Product.ctags, 'Install CTags to enable Python workspace symbols');
@@ -92,25 +81,14 @@ export class Installer implements IInstaller {
9281
const productTypeName = ProductTypeNames.get(productType)!;
9382
const productName = ProductNames.get(product)!;
9483

95-
if (!this.shouldDisplayPrompt(product)) {
96-
const message = `${productTypeName} '${productName}' not installed.`;
97-
this.outputChannel.appendLine(message);
98-
return InstallerResponse.Ignore;
99-
}
100-
10184
const installOption = ProductInstallationPrompt.has(product) ? ProductInstallationPrompt.get(product)! : `Install ${productName}`;
102-
const disableOption = `Disable ${productTypeName}`;
103-
const dontShowAgain = 'Don\'t show this prompt again';
10485
const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8';
10586
const useOtherFormatter = `Use '${alternateFormatter}' formatter`;
10687
const options: string[] = [];
10788
options.push(installOption);
10889
if (productType === ProductType.Formatter) {
10990
options.push(...[useOtherFormatter]);
11091
}
111-
if (SettingToDisableProduct.has(product)) {
112-
options.push(...[disableOption, dontShowAgain]);
113-
}
11492
const item = await window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options);
11593
if (!item) {
11694
return InstallerResponse.Ignore;
@@ -119,24 +97,10 @@ export class Installer implements IInstaller {
11997
case installOption: {
12098
return this.install(product, resource);
12199
}
122-
case disableOption: {
123-
if (ProductTypes.has(product) && ProductTypes.get(product)! === ProductType.Linter) {
124-
return this.disableLinter(product, resource).then(() => InstallerResponse.Disabled);
125-
} else {
126-
const settingToDisable = SettingToDisableProduct.get(product)!;
127-
return this.updateSetting(settingToDisable, false, resource).then(() => InstallerResponse.Disabled);
128-
}
129-
}
130100
case useOtherFormatter: {
131101
return this.updateSetting('formatting.provider', alternateFormatter, resource)
132102
.then(() => InstallerResponse.Installed);
133103
}
134-
case dontShowAgain: {
135-
const pythonConfig = workspace.getConfiguration('python');
136-
const features = pythonConfig.get('disablePromptForFeatures', [] as string[]);
137-
features.push(productName);
138-
return pythonConfig.update('disablePromptForFeatures', features, true).then(() => InstallerResponse.Ignore);
139-
}
140104
default: {
141105
throw new Error('Invalid selection');
142106
}
@@ -210,24 +174,6 @@ export class Installer implements IInstaller {
210174
.catch(() => false);
211175
}
212176
}
213-
public async disableLinter(product: Product, resource?: Uri) {
214-
if (resource && workspace.getWorkspaceFolder(resource)) {
215-
const settingToDisable = SettingToDisableProduct.get(product)!;
216-
const pythonConfig = workspace.getConfiguration('python', resource);
217-
const isMultiroot = Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 1;
218-
const configTarget = isMultiroot ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace;
219-
return pythonConfig.update(settingToDisable, false, configTarget);
220-
} else {
221-
const pythonConfig = workspace.getConfiguration('python');
222-
return pythonConfig.update('linting.enabledWithoutWorkspace', false, true);
223-
}
224-
}
225-
private shouldDisplayPrompt(product: Product) {
226-
const productName = ProductNames.get(product)!;
227-
const pythonConfig = workspace.getConfiguration('python');
228-
const disablePromptForFeatures = pythonConfig.get('disablePromptForFeatures', [] as string[]);
229-
return disablePromptForFeatures.indexOf(productName) === -1;
230-
}
231177
private installCTags() {
232178
if (this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows) {
233179
this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols');
@@ -302,9 +248,8 @@ export class Installer implements IInstaller {
302248
}
303249
case ProductType.RefactoringLibrary: return this.translateProductToModuleName(product, ModuleNamePurpose.run);
304250
case ProductType.Linter: {
305-
const linterHelper = this.serviceContainer.get<ILinterHelper>(ILinterHelper);
306-
const settingsPropNames = linterHelper.getSettingsPropertyNames(product);
307-
return settings.linting[settingsPropNames.pathName] as string;
251+
const linterManager = this.serviceContainer.get<ILinterManager>(ILinterManager);
252+
return linterManager.getLinterInfo(product).pathName(resource);
308253
}
309254
default: {
310255
throw new Error(`Unrecognized Product '${product}'`);

‎src/client/common/installer/pipInstaller.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,14 @@ import { IModuleInstaller } from './types';
1111

1212
@injectable()
1313
export class PipInstaller extends ModuleInstaller implements IModuleInstaller {
14-
private isCondaAvailable: boolean | undefined;
1514
public get displayName() {
1615
return 'Pip';
1716
}
1817
constructor( @inject(IServiceContainer) serviceContainer: IServiceContainer) {
1918
super(serviceContainer);
2019
}
2120
public isSupported(resource?: Uri): Promise<boolean> {
22-
const pythonExecutionFactory = this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
23-
return pythonExecutionFactory.create(resource)
24-
.then(proc => proc.isModuleInstalled('pip'))
25-
.catch(() => false);
21+
return this.isPipAvailable(resource);
2622
}
2723
protected async getExecutionInfo(moduleName: string, resource?: Uri): Promise<ExecutionInfo> {
2824
const proxyArgs = [];
@@ -37,7 +33,7 @@ export class PipInstaller extends ModuleInstaller implements IModuleInstaller {
3733
moduleName: 'pip'
3834
};
3935
}
40-
private isPipAvailable(resource?: Uri) {
36+
private isPipAvailable(resource?: Uri): Promise<boolean> {
4137
const pythonExecutionFactory = this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
4238
return pythonExecutionFactory.create(resource)
4339
.then(proc => proc.isModuleInstalled('pip'))

0 commit comments

Comments
 (0)