From 8aadd40a580f7c95ecdba8f86028338e7c359e4a Mon Sep 17 00:00:00 2001
From: Sean McManus <seanmcm@microsoft.com>
Date: Tue, 19 Mar 2019 13:44:58 -0700
Subject: [PATCH] Fix squiggles with WSL compilerPath with args. (#3313)

* Fix squiggles with WSL compilerPath with args.
* Normalize path separators.
---
 .../src/LanguageServer/configurations.ts      | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts
index dadcc86b7b..9ef947e65c 100644
--- a/Extension/src/LanguageServer/configurations.ts
+++ b/Extension/src/LanguageServer/configurations.ts
@@ -753,15 +753,9 @@ export class CppProperties {
                 paths.add(`"${this.CurrentConfiguration.compileCommands}"`);
             }
 
-            const isWindows: boolean = os.platform() === 'win32';
             if (this.CurrentConfiguration.compilerPath) {
-                let compilerPathAndArgs: util.CompilerPathAndArgs;
-                compilerPathAndArgs = util.extractCompilerPathAndArgs(this.CurrentConfiguration.compilerPath);
-                if (!(isWindows && compilerPathAndArgs.compilerPath.endsWith("cl.exe"))) {
-                    // Unlike other cases, compilerPath may not start or end with " due to trimming of whitespace.
-                    // This is checked to determine if the path is a compilerPath later on.
-                    paths.add(`${compilerPathAndArgs.compilerPath}`);
-                }
+                // Unlike other cases, compilerPath may not start or end with " due to trimming of whitespace and the possibility of compiler args.
+                paths.add(`${this.CurrentConfiguration.compilerPath}`);
             }
 
             // Get the start/end for properties that are file-only.
@@ -776,10 +770,11 @@ export class CppProperties {
                 this.prevSquiggleMetrics[this.CurrentConfiguration.name] = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0 };
             }
             let newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0 };
+            const isWindows: boolean = os.platform() === 'win32';
 
             for (let curPath of paths) {
-                const isCompilerPath: boolean = !curPath.startsWith('"'); // This check probably will need to change later.
-                let resolvedPath: string = curPath.substr((!isCompilerPath ? 1 : 0), curPath.length + (!isCompilerPath ? - 2 : 0));
+                const isCompilerPath: boolean = curPath === this.CurrentConfiguration.compilerPath;
+                let resolvedPath: string = isCompilerPath ? curPath : curPath.substr(1, curPath.length - 2); // Remove the surrounding quotes.
                 // Resolve special path cases.
                 if (resolvedPath === "${default}") {
                     // TODO: Add squiggles for when the C_Cpp.default.* paths are invalid.
@@ -799,6 +794,8 @@ export class CppProperties {
                     resolvedPath = resolvedPath.replace(/\*/g, "");
                 }
 
+                // TODO: Invalid paths created from environment variables are not detected.
+
                 // Handle WSL paths.
                 const isWSL: boolean = isWindows && resolvedPath.startsWith("/");
                 if (isWSL) {
@@ -808,11 +805,19 @@ export class CppProperties {
                         resolvedPath = resolvedPath.substr(0, 1) + ":" + resolvedPath.substr(1);
                     } else if (this.rootfs && this.rootfs.length > 0) {
                         resolvedPath = this.rootfs + resolvedPath.substr(1);
-                        resolvedPath = resolvedPath.replace(/\//g, path.sep);
                         // TODO: Handle WSL symlinks.
                     }
                 }
 
+                if (isCompilerPath) {
+                    let compilerPathAndArgs: util.CompilerPathAndArgs = util.extractCompilerPathAndArgs(resolvedPath);
+                    if (isWindows && compilerPathAndArgs.compilerPath.endsWith("cl.exe")) {
+                        continue; // Don't squiggle invalid cl.exe paths because it could be for an older preview build.
+                    }
+                    resolvedPath = compilerPathAndArgs.compilerPath;
+                    curPath = curPath.replace(/\"/g, `\\"`);
+                }
+
                 let pathExists: boolean = true;
                 let existsWithExeAdded: (path: string) => boolean = (path: string) => {
                     return isCompilerPath && isWindows && !isWSL && fs.existsSync(path + ".exe");
@@ -835,6 +840,13 @@ export class CppProperties {
                     }
                 }
 
+                // Normalize path separators.
+                if (path.sep === "/") {
+                    resolvedPath = resolvedPath.replace(/\\/g, path.sep);
+                } else {
+                    resolvedPath = resolvedPath.replace(/\//g, path.sep);
+                }
+
                 // Iterate through the text and apply squiggles.
                 for (let curOffset: number = curText.indexOf(curPath); curOffset !== -1; curOffset = curText.indexOf(curPath, curOffset + curPath.length)) {
                     let message: string;
@@ -861,7 +873,7 @@ export class CppProperties {
                     }
                     let diagnostic: vscode.Diagnostic = new vscode.Diagnostic(
                         new vscode.Range(document.positionAt(curTextStartOffset + curOffset),
-                            document.positionAt(curTextStartOffset + curOffset + curPath.length + (!isCompilerPath ? - 1 : 0))),
+                            document.positionAt(curTextStartOffset + curOffset + curPath.length + (!isCompilerPath ? -1 : 0))),
                         message, vscode.DiagnosticSeverity.Warning);
                     diagnostics.push(diagnostic);
                 }