Merge pull request #12235 from microsoft/seanmcm/1_20_2_cherryPicks2

sean-mcmanus · web-flow · commit efdc8e90f6d3 · 2024-04-19T17:17:01.000-07:00
Cherry-picks for 1.20.2
diff --git a/Extension/CHANGELOG.md b/Extension/CHANGELOG.md
@@ -1,7 +1,8 @@
 # C/C++ for Visual Studio Code Changelog
 
-## Version 1.20.2: April 17, 2024
+## Version 1.20.2: April 22, 2024
 ### Bug Fixes
+* Fix non-existent relative path variables not showing a warning in `c_cpp_properties.json` (and other related issues). [#12089](https://github.com/microsoft/vscode-cpptools/issues/12089)
 * Fix duplicate URIs in calls to provideConfigurations. [#12177](https://github.com/microsoft/vscode-cpptools/issues/12177)
 * Fix a crash and deadlock with a high `C_Cpp.loggingLevel`. [#12194](https://github.com/microsoft/vscode-cpptools/issues/12194)
 * Fix handling of `-iquote` for code analysis and `#include` completions. [#12198](https://github.com/microsoft/vscode-cpptools/issues/12198)
diff --git a/Extension/src/LanguageServer/client.ts b/Extension/src/LanguageServer/client.ts
@@ -2001,8 +2001,8 @@ export class DefaultClient implements Client {
         }
         try {
             DefaultClient.isStarted.reset();
-            const status = await this.provideCustomConfigurationAsync(docUri, requestFile, replaceExisting, provider);
-            telemetry.logLanguageServerEvent('provideCustomConfiguration', { providerId, status });
+            const resultCode = await this.provideCustomConfigurationAsync(docUri, requestFile, replaceExisting, provider);
+            telemetry.logLanguageServerEvent('provideCustomConfiguration', { providerId, resultCode });
         } finally {
             onFinished();
             DefaultClient.isStarted.resolve();
diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts
@@ -1838,9 +1838,9 @@ export class CppProperties {
             curText = curText.substring(0, nextNameStart2);
         }
         if (this.prevSquiggleMetrics.get(currentConfiguration.name) === undefined) {
-            this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0 });
+            this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0 });
         }
-        const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0 };
+        const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0 };
         const isWindows: boolean = os.platform() === 'win32';
 
         // TODO: Add other squiggles.
@@ -1867,7 +1867,7 @@ export class CppProperties {
         }
 
         // Check for path-related squiggles.
-        let paths: string[] = [];
+        const paths: string[] = [];
         let compilerPath: string | undefined;
         for (const pathArray of [ currentConfiguration.browse ? currentConfiguration.browse.path : undefined,
             currentConfiguration.includePath, currentConfiguration.macFrameworkPath ]) {
@@ -1895,10 +1895,7 @@ export class CppProperties {
             compilerPath = currentConfiguration.compilerPath;
         }
 
-        // Resolve and split any environment variables
-        paths = this.resolveAndSplit(paths, undefined, this.ExtendedEnvironment);
-        compilerPath = util.resolveVariables(compilerPath, this.ExtendedEnvironment).trim();
-        compilerPath = this.resolvePath(compilerPath);
+        compilerPath = this.resolvePath(compilerPath).trim();
 
         // Get the start/end for properties that are file-only.
         const forcedIncludeStart: number = curText.search(/\s*\"forcedInclude\"\s*:\s*\[/);
@@ -1961,8 +1958,7 @@ export class CppProperties {
         let dotConfigMessage: string | undefined;
 
         dotConfigPath = currentConfiguration.dotConfig;
-        dotConfigPath = util.resolveVariables(dotConfigPath, this.ExtendedEnvironment).trim();
-        dotConfigPath = this.resolvePath(dotConfigPath);
+        dotConfigPath = this.resolvePath(dotConfigPath).trim();
         // does not try resolve if the dotConfig property is empty
         dotConfigPath = dotConfigPath !== '' ? dotConfigPath : undefined;
 
@@ -2001,25 +1997,6 @@ export class CppProperties {
                 continue;
             }
 
-            let resolvedPath: string = this.resolvePath(curPath);
-            if (!resolvedPath) {
-                continue;
-            }
-            let pathExists: boolean = true;
-            if (this.rootUri) {
-                const checkPathExists: any = util.checkPathExistsSync(resolvedPath, this.rootUri.fsPath + path.sep, isWindows, false);
-                pathExists = checkPathExists.pathExists;
-                resolvedPath = checkPathExists.path;
-            }
-            // 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.
-
             // Escape the path string for literal use in a regular expression
             // Need to escape any quotes to match the original text
             let escapedPath: string = curPath.replace(/"/g, '\\"');
@@ -2030,6 +2007,42 @@ export class CppProperties {
             const pattern: RegExp = new RegExp(`"[^"]*?(?<="|;)${escapedPath}(?="|;).*?"`, "g");
             const configMatches: string[] | null = curText.match(pattern);
 
+            const expandedPaths: string[] = this.resolveAndSplit([curPath], undefined, this.ExtendedEnvironment, true, true);
+            const incorrectExpandedPaths: string[] = [];
+
+            if (expandedPaths.length <= 0) {
+                continue;
+            }
+
+            if (this.rootUri) {
+                for (const [index, expandedPath] of expandedPaths.entries()) {
+                    if (expandedPath.includes("${workspaceFolder}")) {
+                        expandedPaths[index] = this.resolvePath(expandedPath, false);
+                    } else {
+                        expandedPaths[index] = this.resolvePath(expandedPath);
+                    }
+
+                    const checkPathExists: any = util.checkPathExistsSync(expandedPaths[index], this.rootUri.fsPath + path.sep, isWindows, false);
+                    if (!checkPathExists.pathExists) {
+                        // If there are multiple paths, store any non-existing paths to squiggle later on.
+                        incorrectExpandedPaths.push(expandedPaths[index]);
+                    }
+                }
+            }
+
+            const pathExists: boolean = incorrectExpandedPaths.length === 0;
+
+            for (const [index, expandedPath] of expandedPaths.entries()) {
+                // Normalize path separators.
+                if (path.sep === "/") {
+                    expandedPaths[index] = expandedPath.replace(/\\/g, path.sep);
+                } else {
+                    expandedPaths[index] = expandedPath.replace(/\//g, path.sep);
+                }
+            }
+
+            // Iterate through the text and apply squiggles.
+
             let globPath: boolean = false;
             const asteriskPosition = curPath.indexOf("*");
             if (asteriskPosition !== -1) {
@@ -2041,6 +2054,7 @@ export class CppProperties {
                     }
                 }
             }
+
             if (configMatches && !globPath) {
                 let curOffset: number = 0;
                 let endOffset: number = 0;
@@ -2050,29 +2064,57 @@ export class CppProperties {
                     if (curOffset >= compilerPathStart && curOffset <= compilerPathEnd) {
                         continue;
                     }
-                    let message: string;
+                    let message: string = "";
                     if (!pathExists) {
                         if (curOffset >= forcedIncludeStart && curOffset <= forcedeIncludeEnd
-                                && !path.isAbsolute(resolvedPath)) {
+                                && !path.isAbsolute(expandedPaths[0])) {
                             continue; // Skip the error, because it could be resolved recursively.
                         }
-                        message = localize('cannot.find2', "Cannot find \"{0}\".", resolvedPath);
+                        let badPath = "";
+                        if (incorrectExpandedPaths.length > 0) {
+                            badPath = incorrectExpandedPaths.map(s => `"${s}"`).join(', ');
+                        } else {
+                            badPath = `"${expandedPaths[0]}"`;
+                        }
+                        message = localize('cannot.find2', "Cannot find {0}", badPath);
                         newSquiggleMetrics.PathNonExistent++;
                     } else {
                         // Check for file versus path mismatches.
                         if ((curOffset >= forcedIncludeStart && curOffset <= forcedeIncludeEnd) ||
-                                (curOffset >= compileCommandsStart && curOffset <= compileCommandsEnd)) {
-                            if (util.checkFileExistsSync(resolvedPath)) {
-                                continue;
+                                    (curOffset >= compileCommandsStart && curOffset <= compileCommandsEnd)) {
+                            if (expandedPaths.length > 1) {
+                                message = localize("multiple.paths.not.allowed", "Multiple paths are not allowed.");
+                                newSquiggleMetrics.MultiplePathsNotAllowed++;
+                            } else {
+                                const resolvedPath = this.resolvePath(expandedPaths[0]);
+                                if (util.checkFileExistsSync(resolvedPath)) {
+                                    continue;
+                                }
+
+                                message = localize("path.is.not.a.file", "Path is not a file: {0}", expandedPaths[0]);
+                                newSquiggleMetrics.PathNotAFile++;
                             }
-                            message = localize("path.is.not.a.file", "Path is not a file: {0}", resolvedPath);
-                            newSquiggleMetrics.PathNotAFile++;
                         } else {
-                            if (util.checkDirectoryExistsSync(resolvedPath)) {
+                            const mismatchedPaths: string[] = [];
+                            for (const expandedPath of expandedPaths) {
+                                const resolvedPath = this.resolvePath(expandedPath);
+                                if (!util.checkDirectoryExistsSync(resolvedPath)) {
+                                    mismatchedPaths.push(expandedPath);
+                                }
+                            }
+
+                            let badPath = "";
+                            if (mismatchedPaths.length > 1) {
+                                badPath = mismatchedPaths.map(s => `"${s}"`).join(', ');
+                                message = localize('paths.are.not.directories', "Paths are not directories: {0}", badPath);
+                                newSquiggleMetrics.PathNotADirectory++;
+                            } else if (mismatchedPaths.length === 1) {
+                                badPath = `"${mismatchedPaths[0]}"`;
+                                message = localize('path.is.not.a.directory', "Path is not a directory: {0}", badPath);
+                                newSquiggleMetrics.PathNotADirectory++;
+                            } else {
                                 continue;
                             }
-                            message = localize("path.is.not.a.directory", "Path is not a directory: {0}", resolvedPath);
-                            newSquiggleMetrics.PathNotADirectory++;
                         }
                     }
                     const diagnostic: vscode.Diagnostic = new vscode.Diagnostic(
@@ -2092,7 +2134,7 @@ export class CppProperties {
                         endOffset = curOffset + curMatch.length;
                         let message: string;
                         if (!pathExists) {
-                            message = localize('cannot.find2', "Cannot find \"{0}\".", resolvedPath);
+                            message = localize('cannot.find2', "Cannot find \"{0}\".", expandedPaths[0]);
                             newSquiggleMetrics.PathNonExistent++;
                             const diagnostic: vscode.Diagnostic = new vscode.Diagnostic(
                                 new vscode.Range(document.positionAt(envTextStartOffSet + curOffset),
@@ -2128,6 +2170,9 @@ export class CppProperties {
         if (newSquiggleMetrics.CompilerModeMismatch !== this.prevSquiggleMetrics.get(currentConfiguration.name)?.CompilerModeMismatch) {
             changedSquiggleMetrics.CompilerModeMismatch = newSquiggleMetrics.CompilerModeMismatch;
         }
+        if (newSquiggleMetrics.MultiplePathsNotAllowed !== this.prevSquiggleMetrics.get(currentConfiguration.name)?.MultiplePathsNotAllowed) {
+            changedSquiggleMetrics.MultiplePathsNotAllowed = newSquiggleMetrics.MultiplePathsNotAllowed;
+        }
         if (Object.keys(changedSquiggleMetrics).length > 0) {
             telemetry.logLanguageServerEvent("ConfigSquiggles", undefined, changedSquiggleMetrics);
         }
diff --git a/Extension/src/LanguageServer/extension.ts b/Extension/src/LanguageServer/extension.ts
@@ -36,7 +36,8 @@ const localize: nls.LocalizeFunc = nls.loadMessageBundle();
 export const CppSourceStr: string = "C/C++";
 export const configPrefix: string = "C/C++: ";
 
-let prevCrashFile: string;
+let prevMacCrashFile: string;
+let prevCppCrashFile: string;
 export let clients: ClientCollection;
 let activeDocument: vscode.TextDocument | undefined;
 let ui: LanguageStatusUI;
@@ -914,7 +915,7 @@ function onShowRefCommand(arg?: TreeNode): void {
 
 function reportMacCrashes(): void {
     if (process.platform === "darwin") {
-        prevCrashFile = "";
+        prevMacCrashFile = "";
         const home: string = os.homedir();
         const crashFolder: string = path.resolve(home, "Library/Logs/DiagnosticReports");
         fs.stat(crashFolder, (err) => {
@@ -932,10 +933,10 @@ function reportMacCrashes(): void {
                     if (event !== "rename") {
                         return;
                     }
-                    if (!filename || filename === prevCrashFile) {
+                    if (!filename || filename === prevMacCrashFile) {
                         return;
                     }
-                    prevCrashFile = filename;
+                    prevMacCrashFile = filename;
                     if (!filename.startsWith("cpptools")) {
                         return;
                     }
@@ -964,7 +965,7 @@ export function usesCrashHandler(): boolean {
 
 export function watchForCrashes(crashDirectory: string): void {
     if (crashDirectory !== "") {
-        prevCrashFile = "";
+        prevCppCrashFile = "";
         fs.stat(crashDirectory, (err) => {
             const crashObject: Record<string, string> = {};
             if (err?.code) {
@@ -980,10 +981,10 @@ export function watchForCrashes(crashDirectory: string): void {
                     if (event !== "rename") {
                         return;
                     }
-                    if (!filename || filename === prevCrashFile) {
+                    if (!filename || filename === prevCppCrashFile) {
                         return;
                     }
-                    prevCrashFile = filename;
+                    prevCppCrashFile = filename;
                     if (!filename.startsWith("cpptools")) {
                         return;
                     }
@@ -1125,7 +1126,7 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, er
 
     const lines: string[] = data.split("\n");
     let addressData: string = ".\n.";
-    data = crashFile + "\n";
+    data = (crashFile.startsWith("cpptools-srv") ? "cpptools-srv.txt" : crashFile) + "\n";
     const filtPath: string | null = which.sync("c++filt", { nothrow: true });
     const isMac: boolean = process.platform === "darwin";
     const startStr: string = isMac ? " _" : "<";
@@ -1202,7 +1203,9 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, er
     logCppCrashTelemetry(data, addressData);
 
     await util.deleteFile(path.resolve(crashDirectory, crashFile)).catch(logAndReturn.undefined);
-    void util.deleteDirectory(crashDirectory).catch(logAndReturn.undefined);
+    if (crashFile === "cpptools.txt") {
+        void util.deleteDirectory(crashDirectory).catch(logAndReturn.undefined);
+    }
 }
 
 export function deactivate(): Thenable<void> {
