Fix no interpreter error message (#57)

* Add verification for interpreter in debugAdapter

* Fix tests

* Add error

* Fix unused import

* Fix tests

* fix import error

Author: paulacamargo25

src/extension/common/python.ts

@@ -26,7 +26,7 @@ interface IExtensionApi {
         known: Environment[];
         getActiveEnvironmentPath(resource?: Resource): EnvironmentPath;
         resolveEnvironment(
-            environment: Environment | EnvironmentPath | string,
+            environment: Environment | EnvironmentPath | string | undefined,
         ): Promise<ResolvedEnvironment | undefined>;
         readonly onDidChangeActiveEnvironmentPath: Event<ActiveEnvironmentPathChangeEvent>;
         getEnvironmentVariables(resource?: Resource): EnvironmentVariables;
@@ -80,17 +80,6 @@ export async function initializePython(disposables: Disposable[]): Promise<void>
     }
 }
 
-export async function getInterpreterDetails(resource?: Uri): Promise<IInterpreterDetails> {
-    const api = await getPythonExtensionAPI();
-    const environment = await api?.environments.resolveEnvironment(
-        api?.environments.getActiveEnvironmentPath(resource),
-    );
-    if (environment?.executable.uri) {
-        return { path: [environment?.executable.uri.fsPath], resource };
-    }
-    return { path: undefined, resource };
-}
-
 export async function getDebuggerPath(): Promise<string | undefined> {
     const api = await getPythonExtensionAPI();
     return api?.debug.getDebuggerPackagePath();
@@ -111,7 +100,7 @@ export async function getEnvironmentVariables(resource?: Resource) {
     return api?.environments.getEnvironmentVariables(resource);
 }
 
-export async function resolveEnvironment(env: Environment | EnvironmentPath | string) {
+export async function resolveEnvironment(env: Environment | EnvironmentPath | string | undefined) {
     const api = await getPythonExtensionAPI();
     return api?.environments.resolveEnvironment(env);
 }
@@ -121,6 +110,17 @@ export async function getActiveEnvironmentPath(resource?: Resource) {
     return api?.environments.getActiveEnvironmentPath(resource);
 }
 
+export async function getInterpreterDetails(resource?: Uri): Promise<IInterpreterDetails> {
+    const api = await getPythonExtensionAPI();
+    const environment = await api?.environments.resolveEnvironment(
+        api?.environments.getActiveEnvironmentPath(resource),
+    );
+    if (environment?.executable.uri) {
+        return { path: [environment?.executable.uri.fsPath], resource };
+    }
+    return { path: undefined, resource };
+}
+
 export async function hasInterpreters() {
     const api = await getPythonExtensionAPI();
     if (api) {

src/extension/common/utils/localize.ts

@@ -13,6 +13,7 @@ export namespace AttachProcess {
 }
 
 export namespace DebugConfigStrings {
+    export const debugStopped = l10n.t('Debug Stopped');
     export const selectConfiguration = {
         title: l10n.t('Select a debug configuration'),
         placeholder: l10n.t('Debug Configuration'),
@@ -142,44 +143,6 @@ export namespace DebugConfigStrings {
     }
 }
 
-export namespace Diagnostics {
-    export const warnSourceMaps = l10n.t(
-        'Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.',
-    );
-    export const disableSourceMaps = l10n.t('Disable Source Map Support');
-    export const warnBeforeEnablingSourceMaps = l10n.t(
-        'Enabling source map support in the Python Extension will adversely impact performance of the extension.',
-    );
-    export const enableSourceMapsAndReloadVSC = l10n.t('Enable and reload Window.');
-    export const lsNotSupported = l10n.t(
-        'Your operating system does not meet the minimum requirements of the Python Language Server. Reverting to the alternative autocompletion provider, Jedi.',
-    );
-    export const invalidPythonPathInDebuggerSettings = l10n.t(
-        'You need to select a Python interpreter before you start debugging.\n\nTip: click on "Select Interpreter" in the status bar.',
-    );
-    export const invalidPythonPathInDebuggerLaunch = l10n.t('The Python path in your debug configuration is invalid.');
-    export const invalidDebuggerTypeDiagnostic = l10n.t(
-        'Your launch.json file needs to be updated to change the "pythonExperimental" debug configurations to use the "python" debugger type, otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?',
-    );
-    export const consoleTypeDiagnostic = l10n.t(
-        'Your launch.json file needs to be updated to change the console type string from "none" to "internalConsole", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?',
-    );
-    export const justMyCodeDiagnostic = l10n.t(
-        'Configuration "debugStdLib" in launch.json is no longer supported. It\'s recommended to replace it with "justMyCode", which is the exact opposite of using "debugStdLib". Would you like to automatically update your launch.json file to do that?',
-    );
-    export const yesUpdateLaunch = l10n.t('Yes, update launch.json');
-    export const invalidTestSettings = l10n.t(
-        'Your settings needs to be updated to change the setting "python.unitTest." to "python.testing.", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?',
-    );
-    export const updateSettings = l10n.t('Yes, update settings');
-    export const checkIsort5UpgradeGuide = l10n.t(
-        'We found outdated configuration for sorting imports in this workspace. Check the [isort upgrade guide](https://aka.ms/AA9j5x4) to update your settings.',
-    );
-    export const pylanceDefaultMessage = l10n.t(
-        "The Python extension now includes Pylance to improve completions, code navigation, overall performance and much more! You can learn more about the update and learn how to change your language server [here](https://aka.ms/new-python-bundle).\n\nRead Pylance's license [here](https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license).",
-    );
-}
-
 export namespace Common {
     export const loadingExtension = l10n.t('Python Debugger extension loading...');
     export const doNotShowAgain = l10n.t('Do not show again');

src/extension/debugger/adapter/factory.ts

@@ -16,22 +16,15 @@ import {
 } from 'vscode';
 import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
 import { IDebugAdapterDescriptorFactory } from '../types';
-import { showErrorMessage } from '../../common/vscodeapi';
+import { executeCommand, showErrorMessage } from '../../common/vscodeapi';
 import { traceLog, traceVerbose } from '../../common/log/logging';
 import { EventName } from '../../telemetry/constants';
 import { sendTelemetryEvent } from '../../telemetry';
-import {
-    getActiveEnvironmentPath,
-    getInterpreters,
-    hasInterpreters,
-    resolveEnvironment,
-    runPythonExtensionCommand,
-} from '../../common/python';
+import { getActiveEnvironmentPath, resolveEnvironment, runPythonExtensionCommand } from '../../common/python';
 import { Commands, EXTENSION_ROOT_DIR } from '../../common/constants';
-import { Common, Interpreters } from '../../common/utils/localize';
+import { Common, DebugConfigStrings, Interpreters } from '../../common/utils/localize';
 import { IPersistentStateFactory } from '../../common/types';
 import { Environment } from '../../common/pythonTypes';
-import { ignoreErrors } from '../../common/promiseUtils';
 
 // persistent state names, exported to make use of in testing
 export enum debugStateKeys {
@@ -45,7 +38,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
     public async createDebugAdapterDescriptor(
         session: DebugSession,
         _executable: DebugAdapterExecutable | undefined,
-    ): Promise<DebugAdapterDescriptor> {
+    ): Promise<DebugAdapterDescriptor | undefined> {
         const configuration = session.configuration as LaunchRequestArguments | AttachRequestArguments;
 
         // There are four distinct scenarios here:
@@ -98,16 +91,14 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
             traceLog(`DAP Server launched with command: ${executable} ${args.join(' ')}`);
             sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: true });
             return new DebugAdapterExecutable(executable, args);
+        } else {
+            throw new Error(DebugConfigStrings.debugStopped);
         }
-
-        // Unlikely scenario.
-        throw new Error('Debug Adapter Executable not provided');
     }
 
-    // FIX THIS
     /**
      * Get the python executable used to launch the Python Debug Adapter.
-     * In the case of `attach` scenarios, just use the workspace interpreter, else first available one.
+     * In the case of `attach` scenarios, just use the workspace interpreter.
      * It is unlike user won't have a Python interpreter
      *
      * @private
@@ -128,21 +119,25 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
 
         const resourceUri = workspaceFolder ? workspaceFolder.uri : undefined;
         const interpreterPath = await getActiveEnvironmentPath(resourceUri);
+        const interpreter = await resolveEnvironment(interpreterPath);
 
-        if (interpreterPath) {
+        if (interpreter?.path) {
             traceVerbose(`Selecting active interpreter as Python Executable for DA '${interpreterPath}'`);
             return this.getExecutableCommand(await resolveEnvironment(interpreterPath));
         }
 
-        await hasInterpreters(); // Wait until we know whether we have an interpreter
-        const interpreters = await getInterpreters();
-        if (interpreters.length === 0) {
-            ignoreErrors(this.notifySelectInterpreter());
-            return [];
+        const prompts = [Interpreters.changePythonInterpreter];
+        const selection = await showErrorMessage(
+            l10n.t(
+                'You need to select a Python interpreter before you start debugging.\n\nTip: click on "Select Interpreter" in the status bar.',
+            ),
+            { modal: true },
+            ...prompts,
+        );
+        if (selection === Interpreters.changePythonInterpreter) {
+            await executeCommand(Commands.Set_Interpreter);
         }
-
-        traceVerbose(`Picking first available interpreter to launch the DA '${interpreters[0].path}'`);
-        return this.getExecutableCommand(interpreters[0]);
+        return [];
     }
 
     private async showDeprecatedPythonMessage() {
@@ -185,16 +180,4 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
         }
         return [];
     }
-
-    /**
-     * Notify user about the requirement for Python.
-     * Unlikely scenario, as ex expect users to have Python in order to use the extension.
-     * However it is possible to ignore the warnings and continue using the extension.
-     *
-     * @private
-     * @memberof DebugAdapterDescriptorFactory
-     */
-    private async notifySelectInterpreter() {
-        await showErrorMessage(l10n.t('Please install Python or select a Python Interpreter to use the debugger.'));
-    }
 }

src/test/unittest/adapter/factory.unit.test.ts

@@ -22,6 +22,7 @@ import * as vscodeApi from '../../../extension/common/vscodeapi';
 import { EXTENSION_ROOT_DIR } from '../../../extension/common/constants';
 import { Architecture } from '../../../extension/common/platform';
 import * as pythonApi from '../../../extension/common/python';
+import { DebugConfigStrings } from '../../../extension/common/utils/localize';
 
 use(chaiAsPromised);
 
@@ -125,21 +126,14 @@ suite('Debugging - Adapter Factory', () => {
         assert.deepStrictEqual(descriptor, debugExecutable);
     });
 
-    test('Return the path of the first available interpreter as the current python path, configuration.pythonPath is not defined and there is no active interpreter', async () => {
-        const session = createSession({});
-        const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);
-
-        const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
-
-        assert.deepStrictEqual(descriptor, debugExecutable);
-    });
-
-    test('Display a message if no python interpreter is set', async () => {
-        getInterpretersStub.returns([]);
+    test.only('Display a message if no python interpreter is set', async () => {
+        getActiveEnvironmentPathStub.resolves(undefined);
         const session = createSession({});
         const promise = factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
-        await expect(promise).to.eventually.be.rejectedWith('Debug Adapter Executable not provided');
+        await expect(promise).to.eventually.be.rejectedWith(DebugConfigStrings.debugStopped);
+
+        //check error message
         sinon.assert.calledOnce(showErrorMessageStub);
     });
 
@@ -191,11 +185,10 @@ suite('Debugging - Adapter Factory', () => {
     test('Return Debug Adapter executable if request is "attach", and listen is specified', async () => {
         const session = createSession({ request: 'attach', listen: { port: 5678, host: 'localhost' } });
         const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);
-
         getActiveEnvironmentPathStub.resolves(interpreter.path);
         resolveEnvironmentStub.resolves(interpreter);
-
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
+
         assert.deepStrictEqual(descriptor, debugExecutable);
     });
 
@@ -223,6 +216,9 @@ suite('Debugging - Adapter Factory', () => {
             EXTENSION_ROOT_DIR,
         ]);
 
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
+
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.deepStrictEqual(descriptor, debugExecutable);
@@ -232,6 +228,9 @@ suite('Debugging - Adapter Factory', () => {
         const session = createSession({});
         const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);
 
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
+
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.deepStrictEqual(descriptor, debugExecutable);
@@ -241,20 +240,28 @@ suite('Debugging - Adapter Factory', () => {
         const session = createSession({ logToFile: false });
         const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);
 
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
+
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.deepStrictEqual(descriptor, debugExecutable);
     });
 
     test('Send attach to local process telemetry if attaching to a local process', async () => {
         const session = createSession({ request: 'attach', processId: 1234 });
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
+
         await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.ok(Reporter.eventNames.includes(EventName.DEBUGGER_ATTACH_TO_LOCAL_PROCESS));
     });
 
     test("Don't send any telemetry if not attaching to a local process", async () => {
         const session = createSession({});
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
 
         await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
@@ -265,7 +272,8 @@ suite('Debugging - Adapter Factory', () => {
         const customAdapterPath = 'custom/debug/adapter/path';
         const session = createSession({ debugAdapterPath: customAdapterPath });
         const debugExecutable = new DebugAdapterExecutable(pythonPath, [customAdapterPath]);
-
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.deepStrictEqual(descriptor, debugExecutable);
@@ -292,7 +300,8 @@ suite('Debugging - Adapter Factory', () => {
     test('Do not use "python" to spawn the debug adapter', async () => {
         const session = createSession({ python: '/bin/custompy' });
         const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);
-
+        getActiveEnvironmentPathStub.resolves(interpreter.path);
+        resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
         const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         assert.deepStrictEqual(descriptor, debugExecutable);