refactor resolvingWorkspaceVars and implement deprecation telemetry tracker (#841)

eleanorjboyd · Copilot · web-flow · commit 704dae6f290c · 2025-10-27T13:21:51.000-07:00
* Initial plan

* Add comprehensive tests for resolveAndUpdatePythonPath prioritization logic

Co-authored-by: eleanorjboyd &lt;26030610+eleanorjboyd@users.noreply.github.com&gt;

* add tests for resolveAndUpdatePythonPath

* refactor resolvingWorkspaceVars and implement deprecation telemetry tracker

* refactor resolveAndUpdatePythonPath

* fix item

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
diff --git a/src/extension/debugger/configuration/providers/pyramidLaunch.ts b/src/extension/debugger/configuration/providers/pyramidLaunch.ts
@@ -13,7 +13,7 @@ import { EventName } from '../../../telemetry/constants';
 import { DebuggerTypeName } from '../../../constants';
 import { LaunchRequestArguments } from '../../../types';
 import { DebugConfigurationState, DebugConfigurationType } from '../../types';
-import { resolveVariables } from '../utils/common';
+import { resolveWorkspaceVariables } from '../utils/common';
 
 const workspaceFolderToken = '${workspaceFolder}';
 
@@ -73,7 +73,7 @@ export async function validateIniPath(
     if (!selected || selected.trim().length === 0) {
         return error;
     }
-    const resolvedPath = resolveVariables(selected, undefined, folder);
+    const resolvedPath = resolveWorkspaceVariables(selected, undefined, folder);
     if (resolvedPath) {
         if (selected !== defaultValue && !fs.pathExists(resolvedPath)) {
             return error;
diff --git a/src/extension/debugger/configuration/resolvers/base.ts b/src/extension/debugger/configuration/resolvers/base.ts
@@ -12,7 +12,7 @@ import { getWorkspaceFolders, getWorkspaceFolder as getVSCodeWorkspaceFolder } f
 import { AttachRequestArguments, DebugOptions, LaunchRequestArguments, PathMapping } from '../../../types';
 import { PythonPathSource } from '../../types';
 import { IDebugConfigurationResolver } from '../types';
-import { resolveVariables } from '../utils/common';
+import { resolveWorkspaceVariables } from '../utils/common';
 import { getProgram } from './helper';
 import { getSettingsPythonPath, getInterpreterDetails } from '../../../common/python';
 import { getOSType, OSType } from '../../../common/platform';
@@ -80,6 +80,16 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
         return undefined;
     }
 
+    /**
+     * Resolves and updates file paths and Python interpreter paths in the debug configuration.
+     *
+     * This method performs two main operations:
+     * 1. Resolves workspace variables in the envFile path (if specified)
+     * 2. Resolves and updates Python interpreter paths, handling legacy pythonPath deprecation
+     *
+     * @param workspaceFolder The workspace folder URI for variable resolution
+     * @param debugConfiguration The launch configuration to update
+     */
     protected async resolveAndUpdatePaths(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments,
@@ -88,76 +98,131 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
         await this.resolveAndUpdatePythonPath(workspaceFolder, debugConfiguration);
     }
 
+    /**
+     * Resolves workspace variables in the envFile path.
+     *
+     * Expands variables like ${workspaceFolder} in the envFile configuration using the
+     * workspace folder path or current working directory as the base for resolution.
+     *
+     * @param workspaceFolder The workspace folder URI for variable resolution
+     * @param debugConfiguration The launch configuration containing the envFile path
+     */
     protected static resolveAndUpdateEnvFilePath(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments,
     ): void {
-        if (!debugConfiguration) {
+        // Early exit if no configuration or no envFile to resolve
+        if (!debugConfiguration?.envFile) {
             return;
         }
-        if (debugConfiguration.envFile && (workspaceFolder || debugConfiguration.cwd)) {
-            debugConfiguration.envFile = resolveVariables(
-                debugConfiguration.envFile,
-                (workspaceFolder ? workspaceFolder.fsPath : undefined) || debugConfiguration.cwd,
-                undefined,
-            );
+
+        const basePath = workspaceFolder?.fsPath || debugConfiguration.cwd;
+
+        if (basePath) {
+            // update envFile with resolved variables
+            debugConfiguration.envFile = resolveWorkspaceVariables(debugConfiguration.envFile, basePath, undefined);
         }
     }
 
+    /**
+     * Resolves Python interpreter paths and handles the legacy pythonPath deprecation.
+     *
+     * @param workspaceFolder The workspace folder URI for variable resolution and interpreter detection
+     * @param debugConfiguration The launch configuration to update with resolved Python paths
+     */
     protected async resolveAndUpdatePythonPath(
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments,
     ): Promise<void> {
         if (!debugConfiguration) {
             return;
         }
+
+        // get the interpreter details in the context of the workspace folder
+        const interpreterDetail = await getInterpreterDetails(workspaceFolder);
+        const interpreterPath = interpreterDetail?.path ?? (await getSettingsPythonPath(workspaceFolder));
+        const resolvedInterpreterPath = interpreterPath ? interpreterPath[0] : interpreterPath;
+
+        traceLog(
+            `resolveAndUpdatePythonPath - Initial state: ` +
+                `pythonPath='${debugConfiguration.pythonPath}', ` +
+                `python='${debugConfiguration.python}', ` +
+                `debugAdapterPython='${debugConfiguration.debugAdapterPython}', ` +
+                `debugLauncherPython='${debugConfiguration.debugLauncherPython}', ` +
+                `workspaceFolder='${workspaceFolder?.fsPath}'` +
+                `resolvedInterpreterPath='${resolvedInterpreterPath}'`,
+        );
+
+        // STEP 1: Resolve legacy pythonPath property (DEPRECATED)
+        // pythonPath will equal user set value, or getInterpreterDetails if undefined or set to command
         if (debugConfiguration.pythonPath === '${command:python.interpreterPath}' || !debugConfiguration.pythonPath) {
-            const interpreterDetail = await getInterpreterDetails(workspaceFolder);
-            const interpreterPath = interpreterDetail
-                ? interpreterDetail.path
-                : await getSettingsPythonPath(workspaceFolder);
-            debugConfiguration.pythonPath = interpreterPath ? interpreterPath[0] : interpreterPath;
+            this.pythonPathSource = PythonPathSource.settingsJson;
+            debugConfiguration.pythonPath = resolvedInterpreterPath;
         } else {
-            debugConfiguration.pythonPath = resolveVariables(
-                debugConfiguration.pythonPath ? debugConfiguration.pythonPath : undefined,
+            // User provided explicit pythonPath in launch.json
+            debugConfiguration.pythonPath = resolveWorkspaceVariables(
+                debugConfiguration.pythonPath,
                 workspaceFolder?.fsPath,
                 undefined,
             );
         }
 
+        // STEP 2: Resolve current python property (CURRENT STANDARD)
         if (debugConfiguration.python === '${command:python.interpreterPath}') {
+            // if python is set to the command, resolve it
             this.pythonPathSource = PythonPathSource.settingsJson;
-            const interpreterDetail = await getInterpreterDetails(workspaceFolder);
-            const interpreterPath = interpreterDetail.path
-                ? interpreterDetail.path
-                : await getSettingsPythonPath(workspaceFolder);
-            debugConfiguration.python = interpreterPath ? interpreterPath[0] : interpreterPath;
-        } else if (debugConfiguration.python === undefined) {
+            debugConfiguration.python = resolvedInterpreterPath;
+        } else if (!debugConfiguration.python) {
+            // fallback to pythonPath if python undefined
             this.pythonPathSource = PythonPathSource.settingsJson;
             debugConfiguration.python = debugConfiguration.pythonPath;
         } else {
+            // User provided explicit python path in launch.json
             this.pythonPathSource = PythonPathSource.launchJson;
-            debugConfiguration.python = resolveVariables(
-                debugConfiguration.python ?? debugConfiguration.pythonPath,
+            debugConfiguration.python = resolveWorkspaceVariables(
+                debugConfiguration.python,
                 workspaceFolder?.fsPath,
                 undefined,
             );
         }
 
-        if (
+        // STEP 3: Set debug adapter and launcher Python paths (backwards compatible)
+        this.setDebugComponentPythonPaths(debugConfiguration);
+
+        // STEP 4: Clean up - remove the deprecated pythonPath property
+        delete debugConfiguration.pythonPath;
+    }
+
+    /**
+     * Sets debugAdapterPython and debugLauncherPython with backwards compatibility.
+     * Prefers pythonPath over python for these internal properties.
+     *
+     * @param debugConfiguration The debug configuration to update
+     */
+    private setDebugComponentPythonPaths(debugConfiguration: LaunchRequestArguments): void {
+        const shouldSetDebugAdapter =
             debugConfiguration.debugAdapterPython === '${command:python.interpreterPath}' ||
-            debugConfiguration.debugAdapterPython === undefined
-        ) {
-            debugConfiguration.debugAdapterPython = debugConfiguration.pythonPath ?? debugConfiguration.python;
-        }
-        if (
+            debugConfiguration.debugAdapterPython === undefined;
+
+        const shouldSetDebugLauncher =
             debugConfiguration.debugLauncherPython === '${command:python.interpreterPath}' ||
-            debugConfiguration.debugLauncherPython === undefined
-        ) {
-            debugConfiguration.debugLauncherPython = debugConfiguration.pythonPath ?? debugConfiguration.python;
+            debugConfiguration.debugLauncherPython === undefined;
+
+        // Default fallback path (prefer pythonPath for backwards compatibility)
+        const fallbackPath = debugConfiguration.pythonPath ?? debugConfiguration.python;
+
+        if (debugConfiguration.pythonPath !== debugConfiguration.python) {
+            sendTelemetryEvent(EventName.DEPRECATED_CODE_PATH_USAGE, undefined, {
+                codePath: 'different_python_paths_in_debug_config',
+            });
         }
 
-        delete debugConfiguration.pythonPath;
+        if (shouldSetDebugAdapter) {
+            debugConfiguration.debugAdapterPython = fallbackPath;
+        }
+        if (shouldSetDebugLauncher) {
+            debugConfiguration.debugLauncherPython = fallbackPath;
+        }
     }
 
     protected static debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions): void {
@@ -194,7 +259,7 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
         } else {
             // Expand ${workspaceFolder} variable first if necessary.
             pathMappings = pathMappings.map(({ localRoot: mappedLocalRoot, remoteRoot }) => {
-                const resolvedLocalRoot = resolveVariables(mappedLocalRoot, defaultLocalRoot, undefined);
+                const resolvedLocalRoot = resolveWorkspaceVariables(mappedLocalRoot, defaultLocalRoot, undefined);
                 return {
                     localRoot: resolvedLocalRoot || '',
                     // TODO: Apply to remoteRoot too?
diff --git a/src/extension/debugger/configuration/resolvers/launch.ts b/src/extension/debugger/configuration/resolvers/launch.ts
@@ -8,7 +8,7 @@ import { getOSType, OSType } from '../../../common/platform';
 import { getEnvFile } from '../../../common/settings';
 import { DebuggerTypeName } from '../../../constants';
 import { DebugOptions, DebugPurpose, LaunchRequestArguments } from '../../../types';
-import { resolveVariables } from '../utils/common';
+import { resolveWorkspaceVariables } from '../utils/common';
 import { BaseConfigurationResolver } from './base';
 import { getDebugEnvironmentVariables, getProgram } from './helper';
 import { getConfiguration } from '../../../common/vscodeapi';
@@ -83,7 +83,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
             debugConfiguration.cwd = workspaceFolder.fsPath;
         }
         if (typeof debugConfiguration.envFile !== 'string' && workspaceFolder) {
-            debugConfiguration.envFile = resolveVariables(
+            debugConfiguration.envFile = resolveWorkspaceVariables(
                 getEnvFile('python', workspaceFolder),
                 workspaceFolder.fsPath,
                 undefined,
diff --git a/src/extension/debugger/configuration/utils/common.ts b/src/extension/debugger/configuration/utils/common.ts
@@ -8,36 +8,63 @@
 
 import { Uri, WorkspaceFolder } from 'vscode';
 import { getWorkspaceFolder } from '../../../common/vscodeapi';
+import { sendTelemetryEvent } from '../../../telemetry';
+import { EventName } from '../../../telemetry/constants';
 
 /**
  * @returns whether the provided parameter is a JavaScript String or not.
  */
 function isString(str: any): str is string {
-    if (typeof str === 'string' || str instanceof String) {
-        return true;
-    }
-
-    return false;
+    return typeof str === 'string' || str instanceof String;
 }
 
-export function resolveVariables(
+/**
+ * Resolves VS Code variable placeholders in a string value.
+ *
+ * Specifically handles:
+ * - `${workspaceFolder}` - replaced with the workspace folder path
+ * - `${env.VAR}` or `${env:VAR}` - replaced with empty string
+ * - Unknown variables - left as-is in the original `${variable}` format
+ *
+ * @param value The string containing variable placeholders to resolve
+ * @param rootFolder Fallback folder path to use if no workspace folder is available
+ * @param folder The workspace folder context for variable resolution
+ * @returns The string with variables resolved, or undefined if input was undefined
+ */
+export function resolveWorkspaceVariables(
     value: string | undefined,
     rootFolder: string | Uri | undefined,
     folder: WorkspaceFolder | undefined,
 ): string | undefined {
-    if (value) {
-        const workspaceFolder = folder ? getWorkspaceFolder(folder.uri) : undefined;
-        const variablesObject: { [key: string]: any } = {};
-        variablesObject.workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : rootFolder;
-
-        const regexp = /\$\{(.*?)\}/g;
-        return value.replace(regexp, (match: string, name: string) => {
-            const newValue = variablesObject[name];
-            if (isString(newValue)) {
-                return newValue;
-            }
-            return match && (match.indexOf('env.') > 0 || match.indexOf('env:') > 0) ? '' : match;
-        });
+    if (!value) {
+        return value;
     }
-    return value;
+
+    // opt for folder with fallback to rootFolder
+    const workspaceFolder = folder ? getWorkspaceFolder(folder.uri) : undefined;
+    const workspaceFolderPath = workspaceFolder ? workspaceFolder.uri.fsPath : rootFolder;
+
+    // Replace all ${variable} patterns
+    return value.replace(/\$\{([^}]+)\}/g, (match: string, variableName: string) => {
+        // Handle workspaceFolder variable
+        if (variableName === 'workspaceFolder' && isString(workspaceFolderPath)) {
+            // Track usage of this potentially deprecated code path
+            sendTelemetryEvent(EventName.DEPRECATED_CODE_PATH_USAGE, undefined, {
+                codePath: 'workspaceFolder_substitution',
+            });
+            return workspaceFolderPath;
+        }
+
+        // Replace environment variables with empty string
+        if (variableName.startsWith('env.') || variableName.startsWith('env:')) {
+            // Track usage of this potentially deprecated code path
+            sendTelemetryEvent(EventName.DEPRECATED_CODE_PATH_USAGE, undefined, {
+                codePath: 'env_variable_substitution',
+            });
+            return '';
+        }
+
+        // Unknown variables are left unchanged
+        return match;
+    });
 }
diff --git a/src/extension/telemetry/constants.ts b/src/extension/telemetry/constants.ts
@@ -24,4 +24,5 @@ export enum EventName {
     USE_REPORT_ISSUE_COMMAND = 'USE_REPORT_ISSUE_COMMAND',
     DEBUGGER_PYTHON_37_DEPRECATED = 'DEBUGGER_PYTHON_37_DEPRECATED',
     DEBUGGER_SHOW_PYTHON_INLINE_VALUES = 'DEBUGGER_SHOW_PYTHON_INLINE_VALUES',
+    DEPRECATED_CODE_PATH_USAGE = 'DEPRECATED_CODE_PATH_USAGE',
 }
diff --git a/src/extension/telemetry/index.ts b/src/extension/telemetry/index.ts
@@ -691,4 +691,21 @@ export interface IEventNamePropertyMapping {
        "DEBUGGER_SHOW_PYTHON_INLINE_VALUES" : { "owner": "eleanorjboyd" }
      */
     [EventName.DEBUGGER_SHOW_PYTHON_INLINE_VALUES]: never | undefined;
+    /**
+     * Telemetry event sent when potentially deprecated code paths are executed.
+     */
+    /* __GDPR__
+       "deprecated_code_path_usage" : {
+          "codepath" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }
+       }
+     */
+    [EventName.DEPRECATED_CODE_PATH_USAGE]: {
+        /**
+         * Identifier for the specific deprecated code path that was executed.
+         * Examples: 'workspaceFolder_substitution', 'env_variable_substitution'
+         *
+         * @type {string}
+         */
+        codePath: string;
+    };
 }
diff --git a/src/test/unittest/configuration/providers/pyramidLaunch.unit.test.ts b/src/test/unittest/configuration/providers/pyramidLaunch.unit.test.ts
@@ -12,10 +12,10 @@ import { Uri } from 'vscode';
 import { DebugConfigStrings } from '../../../../extension/common/utils/localize';
 import { MultiStepInput } from '../../../../extension/common/multiStepInput';
 import { DebuggerTypeName } from '../../../../extension/constants';
-import { resolveVariables } from '../../../../extension/debugger/configuration/utils/common';
 import * as pyramidLaunch from '../../../../extension/debugger/configuration/providers/pyramidLaunch';
 import { DebugConfigurationState } from '../../../../extension/debugger/types';
 import * as vscodeapi from '../../../../extension/common/vscodeapi';
+import { resolveWorkspaceVariables } from '../../../../extension/debugger/configuration/utils/common';
 
 suite('Debugging - Configuration Provider Pyramid', () => {
     let input: MultiStepInput<DebugConfigurationState>;
@@ -52,7 +52,7 @@ suite('Debugging - Configuration Provider Pyramid', () => {
     test('Resolve variables (with resource)', async () => {
         const folder = { uri: Uri.parse(path.join('one', 'two')), name: '1', index: 0 };
         workspaceStub.returns(folder);
-        const resolvedPath = resolveVariables('${workspaceFolder}/one.py', undefined, folder);
+        const resolvedPath = resolveWorkspaceVariables('${workspaceFolder}/one.py', undefined, folder);
 
         expect(resolvedPath).to.be.equal(`${folder.uri.fsPath}/one.py`);
     });

Original file line number	Diff line number	Diff line change
`@@ -24,4 +24,5 @@ export enum EventName {`
`24`	`24`	`USE_REPORT_ISSUE_COMMAND = 'USE_REPORT_ISSUE_COMMAND',`
`25`	`25`	`DEBUGGER_PYTHON_37_DEPRECATED = 'DEBUGGER_PYTHON_37_DEPRECATED',`
`26`	`26`	`DEBUGGER_SHOW_PYTHON_INLINE_VALUES = 'DEBUGGER_SHOW_PYTHON_INLINE_VALUES',`
	`27`	`+ DEPRECATED_CODE_PATH_USAGE = 'DEPRECATED_CODE_PATH_USAGE',`
`27`	`28`	`}`