Warning processing fix and tests for env files

This commit builds on the work of @SebastianPfliegel to finish up support for
env files in launch.json.

Two changes:
1. Fixes a build break in the way warnings were hooked up. I also decided to
add a button to open up the env file that has the warning.
2. Moves the parser into a seperate class and adds a unit test.

src/configurationProvider.ts

@@ -7,13 +7,15 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import * as serverUtils from './omnisharp/utils';
 import * as vscode from 'vscode';
+import { ParsedEnvironmentFile } from './coreclr-debug/ParsedEnvironmentFile';
 
 import { AssetGenerator, addTasksJsonIfNecessary, createAttachConfiguration, createLaunchConfiguration, createWebLaunchConfiguration } from './assets';
 
 import { OmniSharpServer } from './omnisharp/server';
 import { containsDotNetCoreProjects } from './omnisharp/protocol';
 import { isSubfolderOf } from './common';
 import { parse } from 'jsonc-parser';
+import { MessageItem } from './vscodeAdapter';
 
 export class CSharpConfigurationProvider implements vscode.DebugConfigurationProvider {
     private server: OmniSharpServer;
@@ -104,58 +106,24 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro
      * Parse envFile and add to config.env
      */
     private parseEnvFile(envFile: string, config: vscode.DebugConfiguration): vscode.DebugConfiguration {
-        if(envFile) {
+        if (envFile) {
             try {
-                let content: string = fs.readFileSync(envFile, "utf8");
-                let parseErrors: string[] = [];
-
-                // Remove UTF-8 BOM if present
-                if(content.charAt(0) === '\uFEFF') {
-                    content = content.substr(1);
-                }
-
-                content.split("\n").forEach(line => {
-                    const r: RegExpMatchArray = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/);
-
-                    if (r !== null) {
-                        const key: string = r[1];
-                        let value: string = r[2] || "";
-                        if ((value.length > 0) && (value.charAt(0) === '"') && (value.charAt(value.length - 1) === '"')) {
-                            value = value.replace(/\\n/gm, "\n");
-                        }
-
-                        value = value.replace(/(^['"]|['"]$)/g, "");
-
-                        if(!config.env) {
-                            config.env = {};
-                        }
-                        config.env[key] = value;
-                    }
-                    else {
-                        // Blank lines and lines starting with # are no parse errors
-                        const comments: RegExp = new RegExp(/^\s*(#|$)/);
-                        if (!comments.test(line)) {
-                            parseErrors.push(line);
-                        }
-                    }
-                });
-
+                const parsedFile: ParsedEnvironmentFile = ParsedEnvironmentFile.CreateFromFile(envFile, config["env"]);
+
                 // show error message if single lines cannot get parsed
-                if(parseErrors.length !== 0) {
-                    let parseError: string = "Ignoring non-parseable lines in envFile " + envFile + ": ";
-                    parseErrors.forEach(function (value, idx, array) {
-                        parseError += "\"" + value + "\"" + ((idx !== array.length - 1) ? ", " : ".");
-                    });
-                    showErrorMessage(vscode, parseError);
+                if (parsedFile.Warning) {
+                    CSharpConfigurationProvider.showFileWarningAsync(parsedFile.Warning, envFile);
                 }
+
+                config.env = parsedFile.Env;
             }
             catch (e) {
                 throw new Error("Can't parse envFile " + envFile);
             }
         }
 
         // remove envFile from config after parsing
-        if(config.envFile) {
+        if (config.envFile) {
             delete config.envFile;
         }
 
@@ -166,13 +134,24 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro
 	 * Try to add all missing attributes to the debug configuration being launched.
 	 */
     resolveDebugConfiguration(folder: vscode.WorkspaceFolder | undefined, config: vscode.DebugConfiguration, token?: vscode.CancellationToken): vscode.ProviderResult<vscode.DebugConfiguration> {
-        // vsdbg does the error checking
 
         // read from envFile and set config.env
-        if(config.envFile) {
+        if (config.envFile) {
             config = this.parseEnvFile(config.envFile.replace(/\${workspaceFolder}/g, folder.uri.path), config);
         }
 
+        // vsdbg will error check the debug configuration fields      
         return config;
     }   
+
+    private static async showFileWarningAsync(message: string, fileName: string) {
+        const openItem: MessageItem = { title: 'Open envFile' };
+        let result: MessageItem = await vscode.window.showWarningMessage(message, openItem);
+        if (result && result.title === openItem.title) {
+            let doc: vscode.TextDocument = await vscode.workspace.openTextDocument(fileName);
+            if (doc) {
+                vscode.window.showTextDocument(doc);
+            }
+        }
+    }
 }

src/coreclr-debug/ParsedEnvironmentFile.ts

@@ -0,0 +1,72 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as fs from 'fs-extra';
+
+export class ParsedEnvironmentFile
+{
+    public Env: { [key: string]: any };
+    public Warning: string | null;
+
+    private constructor(env: { [key: string]: any }, warning: string | null)
+    {
+        this.Env = env;
+        this.Warning = warning;
+    }
+
+    public static CreateFromFile(envFile: string, initialEnv: { [key: string]: any } | undefined): ParsedEnvironmentFile {
+        let content: string = fs.readFileSync(envFile, "utf8");
+        return this.CreateFromContent(content, envFile, initialEnv);
+    }
+
+    public static CreateFromContent(content: string, envFile: string, initialEnv: { [key: string]: any } | undefined): ParsedEnvironmentFile {
+
+        // Remove UTF-8 BOM if present
+        if(content.charAt(0) === '\uFEFF') {
+            content = content.substr(1);
+        }
+
+        let parseErrors: string[] = [];
+        let env: { [key: string]: any } = initialEnv;
+        if (!env) {
+            env = {};
+        }
+
+        content.split("\n").forEach(line => {
+            // Split the line between key and value
+            const r: RegExpMatchArray = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/);
+
+            if (r !== null) {
+                const key: string = r[1];
+                let value: string = r[2] || "";
+                if ((value.length > 0) && (value.charAt(0) === '"') && (value.charAt(value.length - 1) === '"')) {
+                    value = value.replace(/\\n/gm, "\n");
+                }
+
+                value = value.replace(/(^['"]|['"]$)/g, "");
+
+                env[key] = value;
+            }
+            else {
+                // Blank lines and lines starting with # are no parse errors
+                const comments: RegExp = new RegExp(/^\s*(#|$)/);
+                if (!comments.test(line)) {
+                    parseErrors.push(line);
+                }
+            }
+        });
+
+        // show error message if single lines cannot get parsed
+        let warning: string = null;
+        if(parseErrors.length !== 0) {
+            warning = "Ignoring non-parseable lines in envFile " + envFile + ": ";
+            parseErrors.forEach(function (value, idx, array) {
+                warning += "\"" + value + "\"" + ((idx !== array.length - 1) ? ", " : ".");
+            });
+        }
+
+        return new ParsedEnvironmentFile(env, warning);
+    }
+}

test/unitTests/ParsedEnvironmentFile.test.ts

@@ -0,0 +1,99 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { ParsedEnvironmentFile } from '../../src/coreclr-debug/ParsedEnvironmentFile';
+import { should, expect } from 'chai';
+
+suite("ParsedEnvironmentFile", () => {
+    suiteSetup(() => should());
+
+    test("Add single variable", () => {
+        const content = `MyName=VALUE`;
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName"].should.equal("VALUE");
+    });
+
+    test("Handle quoted values", () => {
+        const content = `MyName="VALUE"`;
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName"].should.equal("VALUE");
+    });
+
+    test("Handle BOM", () => {
+        const content = "\uFEFFMyName=VALUE";
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName"].should.equal("VALUE");
+    });
+
+    test("Add multiple variables", () => {
+        const content = `
+MyName1=Value1
+MyName2=Value2
+
+`;
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName1"].should.equal("Value1");
+        result.Env["MyName2"].should.equal("Value2");
+    });
+
+    test("Update variable", () => {
+        const content = `
+MyName1=Value1
+MyName2=Value2
+
+`;
+        const initialEnv : { [key: string]: any } = {
+            "MyName1": "Value7",
+            "ThisShouldNotChange": "StillHere"
+        };
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", initialEnv);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName1"].should.equal("Value1");
+        result.Env["MyName2"].should.equal("Value2");
+        result.Env["ThisShouldNotChange"].should.equal("StillHere");
+    });
+
+    test("Handle comments", () => {
+        const content = `# This is an environment file    
+MyName1=Value1
+# This is a comment in the middle of the file
+MyName2=Value2
+`;
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        expect(result.Warning).to.be.null;
+        result.Env["MyName1"].should.equal("Value1");
+        result.Env["MyName2"].should.equal("Value2");
+    });
+
+    test("Handle invalid lines", () => {
+        const content = `
+This_Line_Is_Wrong
+MyName1=Value1
+MyName2=Value2
+
+`;
+        const fakeConfig : { [key: string]: any } = {};
+        const result = ParsedEnvironmentFile.CreateFromContent(content, "TestEnvFileName", fakeConfig["env"]);
+
+        result.Warning.should.startWith("Ignoring non-parseable lines in envFile TestEnvFileName");
+        result.Env["MyName1"].should.equal("Value1");
+        result.Env["MyName2"].should.equal("Value2");
+    });
+});