Fix microsoft#14674: Enable overriding "pythonPath" in the launcher

Fix microsoft#12462: Update launch.json schema to add "python" and remove "pythonPath" Split the "pythonPath" debug property into "python", "debugAdapterPython", and "debugLauncherPython". Properly register the python.interpreterPath command to expand the ${command:interpreterPath} debug configuration variable. Do most debug config validation on fully expanded property values via resolveDebugConfigurationWithSubstitutedVariables(). Add fixups for legacy launch.json with "pythonPath" and/or "${command:python.interpreterPath}".
int19h · Nov 9, 2020 · ae5120a · ae5120a
1 parent 98f5b49
commit ae5120a
Show file tree

Hide file tree

Showing 14 changed files with 504 additions and 235 deletions.
diff --git a/package.json b/package.json
@@ -1555,7 +1555,8 @@
                     "python"
                 ],
                 "variables": {
-                    "pickProcess": "python.pickLocalProcess"
+                    "pickProcess": "python.pickLocalProcess",
+                    "interpreterPath": "python.interpreterPath"
                 },
                 "configurationSnippets": [],
                 "configurationAttributes": {
@@ -1571,10 +1572,10 @@
                                 "description": "Absolute path to the program.",
                                 "default": "${file}"
                             },
-                            "pythonPath": {
+                            "python": {
                                 "type": "string",
-                                "description": "Path (fully qualified) to python executable. Defaults to the value in settings",
-                                "default": "${command:python.interpreterPath}"
+                                "description": "Absolute path to the Python interpreter executable; overrides workspace configuration if set.",
+                                "default": "${command:interpreterPath}"
                             },
                             "pythonArgs": {
                                 "type": "array",
@@ -3685,4 +3686,4 @@
         "publisherDisplayName": "Microsoft",
         "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8"
     }
-}
+}
diff --git a/src/client/application/diagnostics/checks/invalidLaunchJsonDebugger.ts b/src/client/application/diagnostics/checks/invalidLaunchJsonDebugger.ts
@@ -114,8 +114,10 @@ export class InvalidLaunchJsonDebuggerService extends BaseDiagnosticsService {
             diagnostics.push(new InvalidLaunchJsonDebuggerDiagnostic(DiagnosticCodes.ConsoleTypeDiagnostic, resource));
         }
         if (
+            fileContents.indexOf('"pythonPath":') > 0 ||
             fileContents.indexOf('{config:python.pythonPath}') > 0 ||
-            fileContents.indexOf('{config:python.interpreterPath}') > 0
+            fileContents.indexOf('{config:python.interpreterPath}') > 0 ||
+            fileContents.indexOf('{command:python.interpreterPath}') > 0
         ) {
             diagnostics.push(
                 new InvalidLaunchJsonDebuggerDiagnostic(DiagnosticCodes.ConfigPythonPathDiagnostic, resource, false)
@@ -169,15 +171,21 @@ export class InvalidLaunchJsonDebuggerService extends BaseDiagnosticsService {
                 break;
             }
             case DiagnosticCodes.ConfigPythonPathDiagnostic: {
+                fileContents = this.findAndReplace(fileContents, '"pythonPath":', '"python":');
                 fileContents = this.findAndReplace(
                     fileContents,
                     '{config:python.pythonPath}',
-                    '{command:python.interpreterPath}'
+                    '{command:interpreterPath}'
                 );
                 fileContents = this.findAndReplace(
                     fileContents,
                     '{config:python.interpreterPath}',
-                    '{command:python.interpreterPath}'
+                    '{command:interpreterPath}'
+                );
+                fileContents = this.findAndReplace(
+                    fileContents,
+                    '{command:python.interpreterPath}',
+                    '{command:interpreterPath}'
                 );
                 break;
             }

diff --git a/src/client/debugger/extension/adapter/factory.ts b/src/client/debugger/extension/adapter/factory.ts
@@ -26,7 +26,8 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
     constructor(
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
         @inject(IApplicationShell) private readonly appShell: IApplicationShell
-    ) {}
+    ) { }
+
     public async createDebugAdapterDescriptor(
         session: DebugSession,
         _executable: DebugAdapterExecutable | undefined
@@ -54,7 +55,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
             }
         }
 
-        const pythonPath = await this.getPythonPath(configuration, session.workspaceFolder);
+        const pythonPath = await this.getDebugAdapterPython(configuration, session.workspaceFolder);
         if (pythonPath.length !== 0) {
             if (configuration.request === 'attach' && configuration.processId !== undefined) {
                 sendTelemetryEvent(EventName.DEBUGGER_ATTACH_TO_LOCAL_PROCESS);
@@ -96,13 +97,16 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
      * @returns {Promise<string>} Path to the python interpreter for this workspace.
      * @memberof DebugAdapterDescriptorFactory
      */
-    private async getPythonPath(
+    private async getDebugAdapterPython(
         configuration: LaunchRequestArguments | AttachRequestArguments,
         workspaceFolder?: WorkspaceFolder
     ): Promise<string> {
-        if (configuration.pythonPath) {
+        if (configuration.debugAdapterPython !== undefined) {
+            return configuration.debugAdapterPython;
+        } else if (configuration.pythonPath) {
             return configuration.pythonPath;
         }
+
         const resourceUri = workspaceFolder ? workspaceFolder.uri : undefined;
         const interpreter = await this.interpreterService.getActiveInterpreter(resourceUri);
         if (interpreter) {

diff --git a/src/client/debugger/extension/configuration/debugConfigurationService.ts b/src/client/debugger/extension/configuration/debugConfigurationService.ts
@@ -28,7 +28,8 @@ export class PythonDebugConfigurationService implements IDebugConfigurationServi
         @inject(IDebugConfigurationProviderFactory)
         private readonly providerFactory: IDebugConfigurationProviderFactory,
         @inject(IMultiStepInputFactory) private readonly multiStepFactory: IMultiStepInputFactory
-    ) {}
+    ) { }
+
     public async provideDebugConfigurations(
         folder: WorkspaceFolder | undefined,
         token?: CancellationToken
@@ -46,6 +47,7 @@ export class PythonDebugConfigurationService implements IDebugConfigurationServi
             return [state.config as DebugConfiguration];
         }
     }
+
     public async resolveDebugConfiguration(
         folder: WorkspaceFolder | undefined,
         debugConfiguration: DebugConfiguration,
@@ -76,6 +78,18 @@ export class PythonDebugConfigurationService implements IDebugConfigurationServi
             );
         }
     }
+
+    public async resolveDebugConfigurationWithSubstitutedVariables(
+        folder: WorkspaceFolder | undefined,
+        debugConfiguration: DebugConfiguration,
+        token?: CancellationToken
+    ): Promise<DebugConfiguration | undefined> {
+        function resolve<T extends DebugConfiguration>(resolver: IDebugConfigurationResolver<T>) {
+            return resolver.resolveDebugConfigurationWithSubstitutedVariables(folder, debugConfiguration as T, token);
+        }
+        return debugConfiguration.request === 'attach' ? resolve(this.attachResolver) : resolve(this.launchResolver);
+    }
+
     protected async pickDebugConfiguration(
         input: IMultiStepInput<DebugConfigurationState>,
         state: DebugConfigurationState

diff --git a/src/client/debugger/extension/configuration/resolvers/attach.ts b/src/client/debugger/extension/configuration/resolvers/attach.ts
@@ -21,7 +21,8 @@ export class AttachConfigurationResolver extends BaseConfigurationResolver<Attac
     ) {
         super(workspaceService, documentManager, platformService, configurationService);
     }
-    public async resolveDebugConfiguration(
+
+    public async resolveDebugConfigurationWithSubstitutedVariables(
         folder: WorkspaceFolder | undefined,
         debugConfiguration: AttachRequestArguments,
         _token?: CancellationToken
@@ -38,6 +39,7 @@ export class AttachConfigurationResolver extends BaseConfigurationResolver<Attac
         }
         return debugConfiguration;
     }
+
     // tslint:disable-next-line:cyclomatic-complexity
     protected async provideAttachDefaults(
         workspaceFolder: Uri | undefined,

diff --git a/src/client/debugger/extension/configuration/resolvers/base.ts b/src/client/debugger/extension/configuration/resolvers/base.ts
@@ -24,17 +24,34 @@ import { IDebugConfigurationResolver } from '../types';
 export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
     implements IDebugConfigurationResolver<T> {
     protected pythonPathSource: PythonPathSource = PythonPathSource.launchJson;
+
     constructor(
         protected readonly workspaceService: IWorkspaceService,
         protected readonly documentManager: IDocumentManager,
         protected readonly platformService: IPlatformService,
         protected readonly configurationService: IConfigurationService
-    ) {}
-    public abstract resolveDebugConfiguration(
+    ) { }
+
+    // This is a legacy hook used solely for backwards-compatible manual substitution
+    // of ${command:python.interpreterPath} in "pythonPath". Newly added debug variables
+    // should be properly registered in package.json, so that they're automatically
+    // substituted by VSCode itself. All other validation in derived classes should be
+    // performed in resolveDebugConfigurationWithSubstitutedVariables() instead, where
+    // all variables are already substituted.
+    public async resolveDebugConfiguration(
+        _folder: WorkspaceFolder | undefined,
+        debugConfiguration: DebugConfiguration,
+        _token?: CancellationToken
+    ): Promise<T | undefined> {
+        return debugConfiguration as T;
+    }
+
+    public abstract resolveDebugConfigurationWithSubstitutedVariables(
         folder: WorkspaceFolder | undefined,
         debugConfiguration: DebugConfiguration,
         token?: CancellationToken
     ): Promise<T | undefined>;
+
     protected getWorkspaceFolder(folder: WorkspaceFolder | undefined): Uri | undefined {
         if (folder) {
             return folder.uri;
@@ -56,19 +73,22 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
             }
         }
     }
+
     protected getProgram(): string | undefined {
         const editor = this.documentManager.activeTextEditor;
         if (editor && editor.document.languageId === PYTHON_LANGUAGE) {
             return editor.document.fileName;
         }
     }
+
     protected resolveAndUpdatePaths(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments
     ): void {
         this.resolveAndUpdateEnvFilePath(workspaceFolder, debugConfiguration);
         this.resolveAndUpdatePythonPath(workspaceFolder, debugConfiguration);
     }
+
     protected resolveAndUpdateEnvFilePath(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments
@@ -84,6 +104,7 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
             debugConfiguration.envFile = systemVariables.resolveAny(debugConfiguration.envFile);
         }
     }
+
     protected resolveAndUpdatePythonPath(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments
@@ -99,16 +120,19 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
             this.pythonPathSource = PythonPathSource.launchJson;
         }
     }
+
     protected debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions) {
         if (debugOptions.indexOf(debugOption) >= 0) {
             return;
         }
         debugOptions.push(debugOption);
     }
+
     protected isLocalHost(hostName?: string) {
         const LocalHosts = ['localhost', '127.0.0.1', '::1'];
         return hostName && LocalHosts.indexOf(hostName.toLowerCase()) >= 0 ? true : false;
     }
+
     protected fixUpPathMappings(
         pathMappings: PathMapping[],
         defaultLocalRoot?: string,
@@ -153,9 +177,11 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
 
         return pathMappings;
     }
+
     protected isDebuggingFlask(debugConfiguration: Partial<LaunchRequestArguments & AttachRequestArguments>) {
         return debugConfiguration.module && debugConfiguration.module.toUpperCase() === 'FLASK' ? true : false;
     }
+
     protected sendTelemetry(
         trigger: 'launch' | 'attach' | 'test',
         debugConfiguration: Partial<LaunchRequestArguments & AttachRequestArguments>

diff --git a/src/client/debugger/extension/configuration/resolvers/launch.ts b/src/client/debugger/extension/configuration/resolvers/launch.ts
@@ -29,47 +29,69 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
     ) {
         super(workspaceService, documentManager, platformService, configurationService);
     }
+
     public async resolveDebugConfiguration(
         folder: WorkspaceFolder | undefined,
         debugConfiguration: LaunchRequestArguments,
         _token?: CancellationToken
     ): Promise<LaunchRequestArguments | undefined> {
-        const workspaceFolder = this.getWorkspaceFolder(folder);
-
-        const config = debugConfiguration as LaunchRequestArguments;
-        const numberOfSettings = Object.keys(config);
-
-        if ((config.noDebug === true && numberOfSettings.length === 1) || numberOfSettings.length === 0) {
+        if (
+            debugConfiguration.name === undefined &&
+            debugConfiguration.type === undefined &&
+            debugConfiguration.request === undefined &&
+            debugConfiguration.program === undefined &&
+            debugConfiguration.env === undefined
+        ) {
             const defaultProgram = this.getProgram();
-
-            config.name = 'Launch';
-            config.type = DebuggerTypeName;
-            config.request = 'launch';
-            config.program = defaultProgram ? defaultProgram : '';
-            config.env = {};
+            debugConfiguration.name = 'Launch';
+            debugConfiguration.type = DebuggerTypeName;
+            debugConfiguration.request = 'launch';
+            debugConfiguration.program = defaultProgram ?? '';
+            debugConfiguration.env = {};
         }
 
-        await this.provideLaunchDefaults(workspaceFolder, config);
+        const workspaceFolder = this.getWorkspaceFolder(folder);
+        this.resolveAndUpdatePaths(workspaceFolder, debugConfiguration);
+        return debugConfiguration;
+    }
 
-        const isValid = await this.validateLaunchConfiguration(folder, config);
+    public async resolveDebugConfigurationWithSubstitutedVariables(
+        folder: WorkspaceFolder | undefined,
+        debugConfiguration: LaunchRequestArguments,
+        _token?: CancellationToken
+    ): Promise<LaunchRequestArguments | undefined> {
+        const workspaceFolder = this.getWorkspaceFolder(folder);
+        await this.provideLaunchDefaults(workspaceFolder, debugConfiguration);
+
+        const isValid = await this.validateLaunchConfiguration(folder, debugConfiguration);
         if (!isValid) {
             return;
         }
 
-        const dbgConfig = debugConfiguration;
-        if (Array.isArray(dbgConfig.debugOptions)) {
-            dbgConfig.debugOptions = dbgConfig.debugOptions!.filter(
-                (item, pos) => dbgConfig.debugOptions!.indexOf(item) === pos
+        if (Array.isArray(debugConfiguration.debugOptions)) {
+            debugConfiguration.debugOptions = debugConfiguration.debugOptions!.filter(
+                (item, pos) => debugConfiguration.debugOptions!.indexOf(item) === pos
             );
         }
         return debugConfiguration;
     }
+
     // tslint:disable-next-line:cyclomatic-complexity
     protected async provideLaunchDefaults(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments
     ): Promise<void> {
-        this.resolveAndUpdatePaths(workspaceFolder, debugConfiguration);
+        if (debugConfiguration.python === undefined) {
+            debugConfiguration.python = debugConfiguration.pythonPath;
+        }
+        if (debugConfiguration.debugAdapterPython === undefined) {
+            debugConfiguration.debugAdapterPython = debugConfiguration.pythonPath;
+        }
+        if (debugConfiguration.debugLauncherPython === undefined) {
+            debugConfiguration.debugLauncherPython = debugConfiguration.pythonPath;
+        }
+        delete debugConfiguration.pythonPath;
+
         if (typeof debugConfiguration.cwd !== 'string' && workspaceFolder) {
             debugConfiguration.cwd = workspaceFolder.fsPath;
         }
@@ -160,10 +182,19 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
         debugConfiguration: LaunchRequestArguments
     ): Promise<boolean> {
         const diagnosticService = this.invalidPythonPathInDebuggerService;
-        return diagnosticService.validatePythonPath(
-            debugConfiguration.pythonPath,
-            this.pythonPathSource,
-            folder ? folder.uri : undefined
+        const resource = folder ? folder.uri : undefined;
+        return (
+            diagnosticService.validatePythonPath(debugConfiguration.python, this.pythonPathSource, resource) &&
+            diagnosticService.validatePythonPath(
+                debugConfiguration.debugAdapterPython,
+                this.pythonPathSource,
+                resource
+            ) &&
+            diagnosticService.validatePythonPath(
+                debugConfiguration.debugLauncherPython,
+                this.pythonPathSource,
+                resource
+            )
         );
     }
 }
diff --git a/src/client/debugger/extension/configuration/types.ts b/src/client/debugger/extension/configuration/types.ts
@@ -13,6 +13,12 @@ export interface IDebugConfigurationResolver<T extends DebugConfiguration> {
         debugConfiguration: T,
         token?: CancellationToken
     ): Promise<T | undefined>;
+
+    resolveDebugConfigurationWithSubstitutedVariables(
+        folder: WorkspaceFolder | undefined,
+        debugConfiguration: T,
+        token?: CancellationToken
+    ): Promise<T | undefined>;
 }
 
 export const IDebugConfigurationProviderFactory = Symbol('IDebugConfigurationProviderFactory');