Address PR feedback

Author: bobbrow

Extension/c_cpp_properties.schema.json

@@ -18,7 +18,10 @@
                     "compilerPath": {
                         "markdownDescription": "Full path of the compiler being used, e.g. `/usr/bin/gcc`, to enable more accurate IntelliSense.",
                         "descriptionHint": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered.",
-                        "type": "string"
+                        "type": [
+                            "string",
+                            "null"
+                        ]
                     },
                     "compilerArgs": {
                         "markdownDescription": "Compiler arguments to modify the includes or defines used, e.g. `-nostdinc++`, `-m32`, etc. Arguments that take additional space-delimited arguments should be entered as separate arguments in the array, e.g. for `--sysroot <arg>` use `\"--sysroot\", \"<arg>\"`.",

Extension/src/LanguageServer/configurations.ts

@@ -63,7 +63,7 @@ export interface ConfigurationJson {
 export interface Configuration {
     name: string;
     compilerPathInCppPropertiesJson?: string | null;
-    compilerPath?: string;
+    compilerPath?: string; // Can be set to null based on the schema, but it will be fixed in parsePropertiesFile.
     compilerPathIsExplicit?: boolean;
     compilerArgs?: string[];
     compilerArgsLegacy?: string[];
@@ -1443,10 +1443,17 @@ export class CppProperties {
                 }
             }
 
-            // Configuration.compileCommands is allowed to be defined as a string in the schema, but we send an array to the language server.
-            // For having a predictable behavior, we convert it here to an array of strings.
+            // Special sanitization of the newly parsed configuration file happens here:
             for (let i: number = 0; i < newJson.configurations.length; i++) {
+                // Configuration.compileCommands is allowed to be defined as a string in the schema, but we send an array to the language server.
+                // For having a predictable behavior, we convert it here to an array of strings.
                 newJson.configurations[i].compileCommands = this.forceCompileCommandsAsArray(<any>newJson.configurations[i].compileCommands);
+
+                // `compilerPath` is allowed to be set to null in the schema so that empty string is not the default value (which has another meaning).
+                // If we detect this, we treat it as undefined.
+                if (newJson.configurations[i].compilerPath === null) {
+                    delete newJson.configurations[i].compilerPath;
+                }
             }
 
             this.configurationJson = newJson;
@@ -1647,7 +1654,7 @@ export class CppProperties {
 
             const compilerPathErrors: string[] = [];
             if (compilerPathMayNeedQuotes && !pathExists) {
-                compilerPathErrors.push(localize("path.with.spaces", 'Compiler path with spaces and arguments is missing double quotes " around the path.'));
+                compilerPathErrors.push(localize("path.with.spaces", 'Compiler path with spaces could not be found. If this was intended to include compiler arguments, surround the compiler path with double quotes (").'));
                 telemetry.CompilerPathMissingQuotes = 1;
             }
 

Extension/test/scenarios/SimpleCppProject/tests/compilerPath.test.ts

@@ -263,7 +263,7 @@ describe('validateCompilerPath', () => {
         equal(result.compilerName, 'icc', 'compilerName should be found');
         deepEqual(result.allCompilerArgs, ['-O2'], 'args should match');
         ok(result.error?.includes('Cannot find'), 'Should have an error for unknown compiler');
-        ok(result.error?.includes('missing double quotes'), 'Should have an error for missing double quotes');
+        ok(result.error?.includes('surround the compiler path with double quotes'), 'Should have an error for missing double quotes');
         equal(result.telemetry?.PathNonExistent, 1, 'Should have telemetry for relative paths');
         equal(result.telemetry?.PathNotAFile, undefined, 'Should not have telemetry for invalid paths');
         equal(result.telemetry?.CompilerPathMissingQuotes, 1, 'Should have telemetry for missing quotes');