Add input validation on configuration UI (#3563)

* show errors on config UI

Author: michelleangela

Extension/src/LanguageServer/configurations.ts

@@ -64,6 +64,12 @@ export interface Configuration {
     browse?: Browse;
 }
 
+export interface ConfigurationErrors {
+    compilerPath?: string;
+    includePath?: string;
+    intelliSenseMode?: string;
+}
+
 export interface Browse {
     path?: string[];
     limitSymbolsToIncludedHeaders?: boolean | string;
@@ -227,7 +233,9 @@ export class CppProperties {
                     // Clear out any modifications we may have made internally by parsing the json file
                     if (this.parsePropertiesFile(false)) {
                         // Update the UI with new selected configuration
-                        this.settingsPanel.updateConfigUI(this.configurationJson.configurations[this.currentConfigurationIndex.Value]);
+                        this.settingsPanel.updateConfigUI(
+                            this.configurationJson.configurations[this.currentConfigurationIndex.Value],
+                            this.getErrorsForConfigUI());
                     } else {
                         // Parse failed, open json file
                         vscode.workspace.openTextDocument(this.propertiesFile);
@@ -392,8 +400,13 @@ export class CppProperties {
     private isCompilerIntelliSenseModeCompatible(): boolean {
         // Check if intelliSenseMode and compilerPath are compatible
         // cl.exe and msvc mode should be used together
-        // Ignore if compiler path is not set
-        if (this.CurrentConfiguration.compilerPath === undefined) {
+        // Ignore if compiler path is not set or intelliSenseMode is not set
+        if (this.CurrentConfiguration.compilerPath === undefined ||
+            this.CurrentConfiguration.compilerPath === "" ||
+            this.CurrentConfiguration.compilerPath === "${default}" ||
+            this.CurrentConfiguration.intelliSenseMode === undefined || 
+            this.CurrentConfiguration.intelliSenseMode === "" || 
+            this.CurrentConfiguration.intelliSenseMode === "${default}")  {
             return true;
         }
         let compilerPathAndArgs: util.CompilerPathAndArgs = util.extractCompilerPathAndArgs(this.CurrentConfiguration.compilerPath);
@@ -628,7 +641,9 @@ export class CppProperties {
                         this.settingsPanel.ConfigValuesChanged(() => this.saveConfigurationUI());
                         this.disposables.push(this.settingsPanel);
                     }
-                    this.settingsPanel.createOrShow(this.configurationJson.configurations[this.currentConfigurationIndex.Value]);
+                    this.settingsPanel.createOrShow(
+                        this.configurationJson.configurations[this.currentConfigurationIndex.Value],
+                        this.getErrorsForConfigUI());
                 } else {
                     // Parse failed, open json file
                     vscode.workspace.openTextDocument(this.propertiesFile).then((document: vscode.TextDocument) => {
@@ -646,7 +661,9 @@ export class CppProperties {
                     if (this.parsePropertiesFile(false)) {
                         // The settings UI became visible or active.
                         // Ensure settingsPanel has copy of latest current configuration
-                        this.settingsPanel.updateConfigUI(this.configurationJson.configurations[this.currentConfigurationIndex.Value]);
+                        this.settingsPanel.updateConfigUI(
+                            this.configurationJson.configurations[this.currentConfigurationIndex.Value],
+                            this.getErrorsForConfigUI());
                     } else {
                         // Parse failed, open json file
                         vscode.workspace.openTextDocument(this.propertiesFile);
@@ -660,6 +677,7 @@ export class CppProperties {
         this.parsePropertiesFile(false); // Clear out any modifications we may have made internally.
         let config: Configuration = this.settingsPanel.getLastValuesFromConfigUI();
         this.configurationJson.configurations[this.currentConfigurationIndex.Value] = config;
+        this.settingsPanel.updateErrors(this.getErrorsForConfigUI());
         this.writeToJson();
     }
 
@@ -822,6 +840,150 @@ export class CppProperties {
         return success;
     }
 
+    private resolvePath(path: string, isWindows: boolean): string {
+        if (!path || path === "${default}") {
+            return "";
+        }
+
+        let result: string = "";
+
+        // first resolve variables
+        result = util.resolveVariables(path, this.ExtendedEnvironment);
+        if (result.includes("${workspaceFolder}")) {
+            result = result.replace("${workspaceFolder}", this.rootUri.fsPath);
+        }
+        if (result.includes("${workspaceRoot}")) {
+            result = result.replace("${workspaceRoot}", this.rootUri.fsPath);
+        }
+        if (result.includes("${vcpkgRoot}") && util.getVcpkgRoot()) {
+            result = result.replace("${vcpkgRoot}", util.getVcpkgRoot());
+        }
+        if (result.includes("*")) {
+            result = result.replace(/\*/g, "");
+        }
+
+        // resolve WSL paths
+        if (isWindows && result.startsWith("/")) {
+            const mntStr: string = "/mnt/";
+            if (result.length > "/mnt/c/".length && result.substr(0, mntStr.length) === mntStr) {
+                result = result.substr(mntStr.length);
+                result = result.substr(0, 1) + ":" + result.substr(1);
+            } else if (this.rootfs && this.rootfs.length > 0) {
+                result = this.rootfs + result.substr(1);
+                // TODO: Handle WSL symlinks.
+            }
+        }
+
+        return result;
+    }
+
+    private getErrorsForConfigUI(): ConfigurationErrors {
+        let errors: ConfigurationErrors = {};
+        const isWindows: boolean = os.platform() === 'win32';
+
+        // Validate compilerPath
+        let resolvedCompilerPath: string = this.resolvePath(this.CurrentConfiguration.compilerPath, isWindows);
+        let compilerPathAndArgs: util.CompilerPathAndArgs = util.extractCompilerPathAndArgs(resolvedCompilerPath);
+        if (resolvedCompilerPath &&
+            // Don't error cl.exe paths because it could be for an older preview build.
+            !(isWindows && compilerPathAndArgs.compilerPath.endsWith("cl.exe"))) {
+            resolvedCompilerPath = resolvedCompilerPath.trim();
+
+            // Error when the compiler's path has spaces without quotes but args are used.
+            // Except, exclude cl.exe paths because it could be for an older preview build.
+            let compilerPathNeedsQuotes: boolean =
+                compilerPathAndArgs.additionalArgs &&
+                !resolvedCompilerPath.startsWith('"') &&
+                compilerPathAndArgs.compilerPath.includes(" ");
+            
+            let compilerPathErrors: string[] = [];
+            if (compilerPathNeedsQuotes) {
+                compilerPathErrors.push(`Compiler path with spaces and arguments is missing double quotes " around the path.`);
+            }
+
+            // Get compiler path without arguments before checking if it exists
+            resolvedCompilerPath = compilerPathAndArgs.compilerPath;
+
+            let pathExists: boolean = true;
+            let existsWithExeAdded: (path: string) => boolean = (path: string) => {
+                return isWindows && !path.startsWith("/") && fs.existsSync(path + ".exe");
+            };
+            if (!fs.existsSync(resolvedCompilerPath)) {
+                if (existsWithExeAdded(resolvedCompilerPath)) {
+                    resolvedCompilerPath += ".exe";
+                } else {
+                    // Check again for a relative path.
+                    const relativePath: string = this.rootUri.fsPath + path.sep + resolvedCompilerPath;
+                    if (!fs.existsSync(relativePath)) {
+                        if (existsWithExeAdded(resolvedCompilerPath)) {
+                            resolvedCompilerPath += ".exe";
+                        } else {
+                            pathExists = false;
+                        }
+                    } else {
+                        resolvedCompilerPath = relativePath;
+                    }
+                }
+            }
+            
+            if (!pathExists) {
+                let message: string = `Cannot find: ${resolvedCompilerPath}`;
+                compilerPathErrors.push(message);
+            } else if (!util.checkFileExistsSync(resolvedCompilerPath)) {
+                let message: string = `Path is not a file: ${resolvedCompilerPath}`;
+                compilerPathErrors.push(message);
+            }
+
+            if (compilerPathErrors.length > 0) {
+                errors.compilerPath = compilerPathErrors.join('\n');
+            }
+        }
+
+        // Validate includePath
+        let includePathErrors: string[] = [];
+        for (let includePath of this.CurrentConfiguration.includePath) {
+            let pathExists: boolean = true;
+            let resolvedIncludePath: string = this.resolvePath(includePath, isWindows);
+            if (!resolvedIncludePath) {
+                continue;
+            }
+
+            // Check if resolved path exists
+            if (!fs.existsSync(resolvedIncludePath)) {
+                // Check for relative path if resolved path does not exists
+                const relativePath: string = this.rootUri.fsPath + path.sep + resolvedIncludePath;
+                if (!fs.existsSync(relativePath)) {
+                    pathExists = false;
+                } else {
+                    resolvedIncludePath = relativePath;
+                }
+            }
+
+            if (!pathExists) {
+                let message: string = `Cannot find: ${resolvedIncludePath}`;
+                includePathErrors.push(message);
+                continue;
+            }
+
+            // Check if path is a directory
+            if (!util.checkDirectoryExistsSync(resolvedIncludePath)) {
+                let message: string = `Path is not a directory: ${resolvedIncludePath}`;
+                includePathErrors.push(message);
+            }
+        }
+
+        if (includePathErrors.length > 0) {
+            errors.includePath = includePathErrors.join('\n');
+        }
+
+        // Validate intelliSenseMode
+        if (isWindows && !this.isCompilerIntelliSenseModeCompatible()) {
+            errors.intelliSenseMode = `IntelliSense mode ${this.CurrentConfiguration.intelliSenseMode} is incompatible with compiler path.`;
+        }
+
+        return errors;
+    }
+
     private handleSquiggles(): void {
         if (!this.propertiesFile) {
             return;
@@ -926,35 +1088,14 @@ export class CppProperties {
                     // TODO: Add squiggles for when the C_Cpp.default.* paths are invalid.
                     continue;
                 }
-                let resolvedPath: string = util.resolveVariables(curPath, this.ExtendedEnvironment);
-                if (resolvedPath.includes("${workspaceFolder}")) {
-                    resolvedPath = resolvedPath.replace("${workspaceFolder}", this.rootUri.fsPath);
-                }
-                if (resolvedPath.includes("${workspaceRoot}")) {
-                    resolvedPath = resolvedPath.replace("${workspaceRoot}", this.rootUri.fsPath);
-                }
-                if (resolvedPath.includes("${vcpkgRoot}")) {
-                    resolvedPath = resolvedPath.replace("${vcpkgRoot}", util.getVcpkgRoot());
-                }
-                if (resolvedPath.includes("*")) {
-                    resolvedPath = resolvedPath.replace(/\*/g, "");
+
+                let resolvedPath: string = this.resolvePath(curPath, isWindows);
+                if (!resolvedPath) {
+                    continue;
                 }
 
                 // TODO: Invalid paths created from environment variables are not detected.
 
-                // Handle WSL paths.
-                const isWSL: boolean = isWindows && resolvedPath.startsWith("/");
-                if (isWSL) {
-                    const mntStr: string = "/mnt/";
-                    if (resolvedPath.length > "/mnt/c/".length && resolvedPath.substr(0, mntStr.length) === mntStr) {
-                        resolvedPath = resolvedPath.substr(mntStr.length);
-                        resolvedPath = resolvedPath.substr(0, 1) + ":" + resolvedPath.substr(1);
-                    } else if (this.rootfs && this.rootfs.length > 0) {
-                        resolvedPath = this.rootfs + resolvedPath.substr(1);
-                        // TODO: Handle WSL symlinks.
-                    }
-                }
-
                 let compilerPathNeedsQuotes: boolean = false;
                 if (isCompilerPath) {
                     resolvedPath = resolvedPath.trim();
@@ -967,6 +1108,7 @@ export class CppProperties {
                     resolvedPath = compilerPathAndArgs.compilerPath;
                 }
 
+                const isWSL: boolean = isWindows && resolvedPath.startsWith("/");
                 let pathExists: boolean = true;
                 let existsWithExeAdded: (path: string) => boolean = (path: string) => {
                     return isCompilerPath && isWindows && !isWSL && fs.existsSync(path + ".exe");

Extension/src/LanguageServer/settingsPanel.ts

@@ -40,7 +40,7 @@ export class SettingsPanel {
         );
     }
 
-    public createOrShow(activeConfiguration: config.Configuration): void {
+    public createOrShow(activeConfiguration: config.Configuration, errors: config.ConfigurationErrors): void {
         const column: vscode.ViewColumn = vscode.window.activeTextEditor
                 ? vscode.window.activeTextEditor.viewColumn
                 : undefined;
@@ -80,7 +80,7 @@ export class SettingsPanel {
 
         this.panel.webview.html = this.getHtml();
 
-        this.updateWebview(activeConfiguration);
+        this.updateWebview(activeConfiguration, errors);
     }
 
     public get SettingsPanelActivated(): vscode.Event<void> { 
@@ -95,18 +95,17 @@ export class SettingsPanel {
         return this.configValues;
     }
 
-    public updateConfigUI(configuration: config.Configuration): void {
+    public updateConfigUI(configuration: config.Configuration, errors: config.ConfigurationErrors): void {
         if (this.panel) {
-            this.updateWebview(configuration);
+            this.updateWebview(configuration, errors);
         }
     }
 
-    //TODO: validate input paths
-    // public validatePaths(invalid: boolean) {
-    //     if (this.panel) {
-    //         this.panel.webview.postMessage({ command: 'validateCompilerPath', invalid: invalid });
-    //      }
-    // }
+    public updateErrors(errors: config.ConfigurationErrors): void {
+        if (this.panel) {
+            this.panel.webview.postMessage({ command: 'updateErrors', errors: errors});
+        }
+    }
 
     public dispose(): void {
         // Clean up resources
@@ -128,12 +127,13 @@ export class SettingsPanel {
         }
     }
 
-    private updateWebview(configuration: config.Configuration): void {
+    private updateWebview(configuration: config.Configuration, errors: config.ConfigurationErrors): void {
         this.configValues = Object.assign({}, configuration); // Copy configuration values
         this.isIntelliSenseModeDefined = (this.configValues.intelliSenseMode !== undefined);
         if (this.panel) {
-            // Send a message to the webview to update the values
-           this.panel.webview.postMessage({ command: 'update', config: this.configValues });
+            // Send a message to the webview to update the values and errors
+           this.panel.webview.postMessage({ command: 'updateConfig', config: this.configValues});
+           this.panel.webview.postMessage({ command: 'updateErrors', errors: errors});
         }
     }