Add setting to control severity of missing package diagnostic. (microsoft#21794)

karthiknadig · anthonykim1 · commit 1f49b4c4c3aa · 2023-09-12T11:03:19.000-07:00
Closes microsoft#21792
diff --git a/package.json b/package.json
@@ -1151,6 +1151,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%",
diff --git a/package.nls.json b/package.nls.json
@@ -201,6 +201,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.",
diff --git a/pythonFiles/installed_check.py b/pythonFiles/installed_check.py
@@ -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):
@@ -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]]]:
@@ -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
@@ -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
diff --git a/pythonFiles/tests/test_installed_check.py b/pythonFiles/tests/test_installed_check.py
@@ -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"
@@ -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,
@@ -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""
@@ -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]
diff --git a/src/client/pythonEnvironments/creation/common/installCheckUtils.ts b/src/client/pythonEnvironments/creation/common/installCheckUtils.ts
@@ -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;
@@ -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,
@@ -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);
diff --git a/src/test/pythonEnvironments/creation/common/installCheckUtils.unit.test.ts b/src/test/pythonEnvironments/creation/common/installCheckUtils.unit.test.ts
@@ -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);
 
@@ -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();
+        });
     });
 });
