Merge branch 'topic/vscode-env-init' into 'master'

Handle null environment variable in vscode settings

Closes #1283

See merge request eng/ide/ada_language_server!1486

Author: eliericha

integration/vscode/ada/src/debugConfigProvider.ts

@@ -100,7 +100,7 @@ function getOrFindGdb(): string | undefined {
         const env = getEvaluatedTerminalEnv();
         let pathVal: string;
         if (env && 'PATH' in env) {
-            pathVal = env.PATH;
+            pathVal = env.PATH ?? '';
         } else if ('PATH' in process.env) {
             pathVal = process.env.PATH ?? '';
         } else {

integration/vscode/ada/src/extension.ts

@@ -131,8 +131,8 @@ async function activateExtension(context: vscode.ExtensionContext) {
     if (customEnv && Object.keys(customEnv).length > 0) {
         logger.info(`Custom environment variables from ${TERMINAL_ENV_SETTING_NAME}`);
         for (const varName in customEnv) {
-            const varValue: string = customEnv[varName];
-            logger.info(`${varName}=${varValue}`);
+            const varValue = customEnv[varName];
+            logger.info(`${varName}=${varValue ?? '<null>'}`);
         }
     } else {
         logger.debug('No custom environment variables set in %s', TERMINAL_ENV_SETTING_NAME);

integration/vscode/ada/src/helpers.ts

@@ -121,12 +121,12 @@ export const TERMINAL_ENV_SETTING_NAME =
  * @returns the value of the applicable `terminal.integrated.env.*` setting,
  * without evaluation of macros such as `${env:...}`.
  */
-export function getTerminalEnv() {
+export function getTerminalEnv(): { [name: string]: string | null } {
     const custom_env = vscode.workspace
         .getConfiguration()
-        .get<{ [name: string]: string }>(TERMINAL_ENV_SETTING_NAME);
+        .get<{ [name: string]: string | null }>(TERMINAL_ENV_SETTING_NAME);
 
-    return custom_env;
+    return custom_env ?? {};
 }
 
 /**
@@ -139,11 +139,18 @@ export function getEvaluatedTerminalEnv() {
 
     if (custom_env) {
         for (const var_name in custom_env) {
-            // Substitute VS Code variable references that might be present
-            // in the JSON settings configuration (e.g: "PATH": "${workspaceFolder}/obj")
-            custom_env[var_name] = custom_env[var_name].replace(/(\$\{.*\})/, (substring) =>
-                substituteVariables(substring, false)
-            );
+            /**
+             * The User can specify `"VAR": null` in his settings, so we only
+             * apply substitution to non-null values.
+             */
+            if (custom_env[var_name] != null) {
+                // Substitute VS Code variable references that might be present
+                // in the JSON settings configuration (e.g: "PATH": "${workspaceFolder}/obj")
+                custom_env[var_name] =
+                    custom_env[var_name]?.replace(/(\$\{.*\})/, (substring) =>
+                        substituteVariables(substring, false)
+                    ) ?? null;
+            }
         }
     }
 
@@ -157,15 +164,30 @@ export function getEvaluatedTerminalEnv() {
  * The targetEnv can be `process.env` to apply the changes to the environment of
  * the running process.
  */
-export function setTerminalEnvironment(targetEnv: NodeJS.ProcessEnv) {
-    // Retrieve the user's custom environment variables if specified in their
-    // settings/workspace
-    const custom_env = getEvaluatedTerminalEnv();
+export function setTerminalEnvironment(
+    targetEnv: NodeJS.ProcessEnv,
+    custom_env?: { [name: string]: string | null }
+) {
+    if (custom_env == undefined) {
+        // Retrieve the user's custom environment variables if specified in their
+        // settings/workspace
+        custom_env = getEvaluatedTerminalEnv();
+    }
 
     if (custom_env) {
         for (const var_name in custom_env) {
-            const var_value: string = custom_env[var_name];
-            targetEnv[var_name] = var_value;
+            const var_value = custom_env[var_name];
+            if (var_value == null) {
+                /**
+                 * If the value is null, delete it from the target env if it
+                 * exists.
+                 */
+                if (var_name in targetEnv) {
+                    delete targetEnv[var_name];
+                }
+            } else {
+                targetEnv[var_name] = var_value;
+            }
         }
     }
 }

integration/vscode/ada/test/suite/general/env.test.ts

@@ -0,0 +1,43 @@
+import assert from 'assert';
+import { suite, test } from 'mocha';
+import { setTerminalEnvironment } from '../../../src/helpers';
+
+suite('Environment init', () => {
+    test('Env init', () => {
+        /**
+         * We want to test the cases [existing variable, non-existing variable]
+         * x [null value, empty string, non-empty string] exhaustively. So we
+         * should end up with 6 test cases.
+         */
+
+        const targetEnv = {
+            VAR1: '',
+            VAR2: '',
+            VAR3: '',
+            VAR7: 'Should be preserved',
+        };
+
+        const userEnv: { [name: string]: string | null } = {
+            // Existing variables
+            VAR1: null,
+            VAR2: '',
+            VAR3: 'Some new value',
+            // Non-existing variables
+            VAR4: null,
+            VAR5: '',
+            VAR6: 'Some other value',
+        };
+
+        setTerminalEnvironment(targetEnv, userEnv);
+
+        const expected = {
+            VAR2: '',
+            VAR3: 'Some new value',
+            VAR5: '',
+            VAR6: 'Some other value',
+            VAR7: 'Should be preserved',
+        };
+
+        assert.deepEqual(targetEnv, expected);
+    });
+});