Skip to content

Commit

Permalink
Add setting to control severity of missing package diagnostic. (#21794)
Browse files Browse the repository at this point in the history
Closes #21792
  • Loading branch information
karthiknadig committed Aug 10, 2023
1 parent fbbf987 commit 835eab5
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 9 deletions.
15 changes: 15 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,21 @@
"scope": "machine",
"type": "string"
},
"python.missingPackage.severity":{
"default": "Hint",
"description": "%python.missingPackage.severity.description%",
"enum": [
"Error",
"Hint",
"Information",
"Warning"
],
"scope": "resource",
"type": "string",
"tags": [
"experimental"
]
},
"python.pipenvPath": {
"default": "pipenv",
"description": "%python.pipenvPath.description%",
Expand Down
3 changes: 2 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@
"python.linting.pylintPath.deprecationMessage": "This setting will soon be deprecated. Please use the Pylint extension. Learn more here: https://aka.ms/AAlgvkb.",
"python.logging.level.description": "The logging level the extension logs at, defaults to 'error'",
"python.logging.level.deprecation": "This setting is deprecated. Please use command `Developer: Set Log Level...` to set logging level.",
"python.missingPackage.severity.description": "Set severity of missing packages in requirements.txt or pyproject.toml",
"python.pipenvPath.description": "Path to the pipenv executable to use for activation.",
"python.poetryPath.description": "Path to the poetry executable.",
"python.sortImports.args.description": "Arguments passed in. Each argument is a separate item in the array.",
Expand Down Expand Up @@ -265,4 +266,4 @@
"walkthrough.step.python.createNewNotebook.altText": "Creating a new Jupyter notebook",
"walkthrough.step.python.openInteractiveWindow.altText": "Opening Python interactive window",
"walkthrough.step.python.dataScienceLearnMore.altText": "Image representing our documentation page and mailing list resources."
}
}
13 changes: 9 additions & 4 deletions pythonFiles/installed_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
from importlib_metadata import metadata
from packaging.requirements import Requirement

DEFAULT_SEVERITY = 3
DEFAULT_SEVERITY = "3" # 'Hint'
try:
SEVERITY = int(os.getenv("VSCODE_MISSING_PGK_SEVERITY", DEFAULT_SEVERITY))
except ValueError:
SEVERITY = int(DEFAULT_SEVERITY)


def parse_args(argv: Optional[Sequence[str]] = None):
Expand All @@ -37,7 +41,8 @@ def parse_requirements(line: str) -> Optional[Requirement]:
elif req.marker.evaluate():
return req
except Exception:
return None
pass
return None


def process_requirements(req_file: pathlib.Path) -> List[Dict[str, Union[str, int]]]:
Expand All @@ -60,7 +65,7 @@ def process_requirements(req_file: pathlib.Path) -> List[Dict[str, Union[str, in
"endCharacter": len(req.name),
"package": req.name,
"code": "not-installed",
"severity": DEFAULT_SEVERITY,
"severity": SEVERITY,
}
)
return diagnostics
Expand Down Expand Up @@ -100,7 +105,7 @@ def process_pyproject(req_file: pathlib.Path) -> List[Dict[str, Union[str, int]]
"endCharacter": end,
"package": req.name,
"code": "not-installed",
"severity": DEFAULT_SEVERITY,
"severity": SEVERITY,
}
)
return diagnostics
Expand Down
53 changes: 51 additions & 2 deletions pythonFiles/tests/test_installed_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import sys

import pytest
from typing import Dict, List, Union
from typing import Dict, List, Optional, Union

SCRIPT_PATH = pathlib.Path(__file__).parent.parent / "installed_check.py"
TEST_DATA = pathlib.Path(__file__).parent / "test_data"
Expand All @@ -29,7 +29,12 @@ def generate_file(base_file: pathlib.Path):
os.unlink(str(fullpath))


def run_on_file(file_path: pathlib.Path) -> List[Dict[str, Union[str, int]]]:
def run_on_file(
file_path: pathlib.Path, severity: Optional[str] = None
) -> List[Dict[str, Union[str, int]]]:
env = os.environ.copy()
if severity:
env["VSCODE_MISSING_PGK_SEVERITY"] = severity
result = subprocess.run(
[
sys.executable,
Expand All @@ -39,6 +44,7 @@ def run_on_file(file_path: pathlib.Path) -> List[Dict[str, Union[str, int]]]:
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=True,
env=env,
)
assert result.returncode == 0
assert result.stderr == b""
Expand Down Expand Up @@ -88,3 +94,46 @@ def test_installed_check(test_name: str):
with generate_file(base_file) as file_path:
result = run_on_file(file_path)
assert result == EXPECTED_DATA[test_name]


EXPECTED_DATA2 = {
"missing-deps": [
{
"line": 6,
"character": 0,
"endLine": 6,
"endCharacter": 10,
"package": "flake8-csv",
"code": "not-installed",
"severity": 0,
},
{
"line": 10,
"character": 0,
"endLine": 10,
"endCharacter": 11,
"package": "levenshtein",
"code": "not-installed",
"severity": 0,
},
],
"pyproject-missing-deps": [
{
"line": 8,
"character": 34,
"endLine": 8,
"endCharacter": 44,
"package": "flake8-csv",
"code": "not-installed",
"severity": 0,
}
],
}


@pytest.mark.parametrize("test_name", EXPECTED_DATA2.keys())
def test_with_severity(test_name: str):
base_file = TEST_DATA / f"{test_name}.data"
with generate_file(base_file) as file_path:
result = run_on_file(file_path, severity="0")
assert result == EXPECTED_DATA2[test_name]
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { installedCheckScript } from '../../../common/process/internal/scripts';
import { plainExec } from '../../../common/process/rawProcessApis';
import { IInterpreterPathService } from '../../../common/types';
import { traceInfo, traceVerbose, traceError } from '../../../logging';
import { getConfiguration } from '../../../common/vscodeApis/workspaceApis';

interface PackageDiagnostic {
package: string;
Expand Down Expand Up @@ -39,6 +40,21 @@ function parseDiagnostics(data: string): Diagnostic[] {
return diagnostics;
}

function getMissingPackageSeverity(doc: TextDocument): number {
const config = getConfiguration('python', doc.uri);
const severity: string = config.get<string>('missingPackage.severity', 'Hint');
if (severity === 'Error') {
return DiagnosticSeverity.Error;
}
if (severity === 'Warning') {
return DiagnosticSeverity.Warning;
}
if (severity === 'Information') {
return DiagnosticSeverity.Information;
}
return DiagnosticSeverity.Hint;
}

export async function getInstalledPackagesDiagnostics(
interpreterPathService: IInterpreterPathService,
doc: TextDocument,
Expand All @@ -47,7 +63,11 @@ export async function getInstalledPackagesDiagnostics(
const scriptPath = installedCheckScript();
try {
traceInfo('Running installed packages checker: ', interpreter, scriptPath, doc.uri.fsPath);
const result = await plainExec(interpreter, [scriptPath, doc.uri.fsPath]);
const result = await plainExec(interpreter, [scriptPath, doc.uri.fsPath], {
env: {
VSCODE_MISSING_PGK_SEVERITY: `${getMissingPackageSeverity(doc)}`,
},
});
traceVerbose('Installed packages check result:\n', result.stdout);
if (result.stderr) {
traceError('Installed packages check error:\n', result.stderr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import * as chaiAsPromised from 'chai-as-promised';
import * as sinon from 'sinon';
import * as typemoq from 'typemoq';
import { assert, use as chaiUse } from 'chai';
import { Diagnostic, TextDocument, Range, Uri } from 'vscode';
import { Diagnostic, TextDocument, Range, Uri, WorkspaceConfiguration, ConfigurationScope } from 'vscode';
import * as rawProcessApis from '../../../../client/common/process/rawProcessApis';
import { getInstalledPackagesDiagnostics } from '../../../../client/pythonEnvironments/creation/common/installCheckUtils';
import { IInterpreterPathService } from '../../../../client/common/types';
import * as workspaceApis from '../../../../client/common/vscodeApis/workspaceApis';
import { SpawnOptions } from '../../../../client/common/process/types';

chaiUse(chaiAsPromised);

Expand Down Expand Up @@ -37,29 +39,76 @@ const MISSING_PACKAGES: Diagnostic[] = [
suite('Install check diagnostics tests', () => {
let plainExecStub: sinon.SinonStub;
let interpreterPathService: typemoq.IMock<IInterpreterPathService>;
let getConfigurationStub: sinon.SinonStub;
let configMock: typemoq.IMock<WorkspaceConfiguration>;

setup(() => {
configMock = typemoq.Mock.ofType<WorkspaceConfiguration>();
plainExecStub = sinon.stub(rawProcessApis, 'plainExec');
interpreterPathService = typemoq.Mock.ofType<IInterpreterPathService>();
getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
getConfigurationStub.callsFake((section?: string, _scope?: ConfigurationScope | null) => {
if (section === 'python') {
return configMock.object;
}
return undefined;
});
});

teardown(() => {
sinon.restore();
});

test('Test parse diagnostics', async () => {
configMock
.setup((c) => c.get<string>('missingPackage.severity', 'Hint'))
.returns(() => 'Error')
.verifiable(typemoq.Times.atLeastOnce());
plainExecStub.resolves({ stdout: MISSING_PACKAGES_STR, stderr: '' });
const someFile = getSomeRequirementFile();
const result = await getInstalledPackagesDiagnostics(interpreterPathService.object, someFile.object);

assert.deepStrictEqual(result, MISSING_PACKAGES);
configMock.verifyAll();
});

test('Test parse empty diagnostics', async () => {
configMock
.setup((c) => c.get<string>('missingPackage.severity', 'Hint'))
.returns(() => 'Error')
.verifiable(typemoq.Times.atLeastOnce());
plainExecStub.resolves({ stdout: '', stderr: '' });
const someFile = getSomeRequirementFile();
const result = await getInstalledPackagesDiagnostics(interpreterPathService.object, someFile.object);

assert.deepStrictEqual(result, []);
configMock.verifyAll();
});

[
['Error', '0'],
['Warning', '1'],
['Information', '2'],
['Hint', '3'],
].forEach((severityType: string[]) => {
const setting = severityType[0];
const expected = severityType[1];
test(`Test missing package severity: ${setting}`, async () => {
configMock
.setup((c) => c.get<string>('missingPackage.severity', 'Hint'))
.returns(() => setting)
.verifiable(typemoq.Times.atLeastOnce());
let severity: string | undefined;
plainExecStub.callsFake((_cmd: string, _args: string[], options: SpawnOptions) => {
severity = options.env?.VSCODE_MISSING_PGK_SEVERITY;
return { stdout: '', stderr: '' };
});
const someFile = getSomeRequirementFile();
const result = await getInstalledPackagesDiagnostics(interpreterPathService.object, someFile.object);

assert.deepStrictEqual(result, []);
assert.deepStrictEqual(severity, expected);
configMock.verifyAll();
});
});
});

0 comments on commit 835eab5

Please sign in to comment.