Fix location for diagnostics generated from macros via swiftc (#1234)

* Fix location for diagnostics generated from macros via swiftc

The format of messages generated from macro errors/warnings is different
than those generated normally. The parsing regex would capture the
location for the related info diagnostic with a backtick. The location
for the parent regex would be "#macro expansion" which when clicked
would open an error window in a VS Code tab stating that the file
"#macro expansion" could not be found.

This patch tweaks two things:
1. Makes the regex for capturing the filepaths of warnings/errors
   account for leading trivia
2. If the path for a diagnostic is invalid, wait until we parse the
   related infromation and use the URI from there.

* No Testing on Swift < 6.0

Author: plemarquand

assets/test/.vscode/settings.json

@@ -7,5 +7,4 @@
         "-DTEST_ARGUMENT_SET_VIA_TEST_BUILD_ARGUMENTS_SETTING"
     ],
     "lldb.verboseLogging": true
-
 }

assets/test/diagnostics/Sources/main.swift

@@ -12,3 +12,8 @@ repeat {
   line = readLine()
   print(line ?? "nil")
 } while line != nil;
+
+#if canImport(Testing)
+import Testing
+#expect(try myFunc() != 0)
+#endif

src/DiagnosticsManager.ts

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 import * as vscode from "vscode";
+import * as fs from "fs";
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import stripAnsi = require("strip-ansi");
 import configuration from "./configuration";
@@ -255,6 +256,7 @@ export class DiagnosticsManager implements vscode.Disposable {
             };
             let remainingData: string | undefined;
             let lastDiagnostic: vscode.Diagnostic | undefined;
+            let lastDiagnosticNeedsSaving = false;
             disposables.push(
                 swiftExecution.onDidWrite(data => {
                     const sanitizedData = (remainingData || "") + stripAnsi(data);
@@ -293,6 +295,18 @@ export class DiagnosticsManager implements vscode.Disposable {
                             lastDiagnostic.relatedInformation = (
                                 lastDiagnostic.relatedInformation || []
                             ).concat(relatedInformation);
+
+                            if (lastDiagnosticNeedsSaving) {
+                                const expandedUri = relatedInformation.location.uri.fsPath;
+                                const currentUriDiagnostics = diagnostics.get(expandedUri) || [];
+                                lastDiagnostic.range = relatedInformation.location.range;
+                                diagnostics.set(expandedUri, [
+                                    ...currentUriDiagnostics,
+                                    lastDiagnostic,
+                                ]);
+
+                                lastDiagnosticNeedsSaving = false;
+                            }
                             continue;
                         }
                         const { uri, diagnostic } = result as ParsedDiagnostic;
@@ -312,18 +326,36 @@ export class DiagnosticsManager implements vscode.Disposable {
                             continue;
                         }
                         lastDiagnostic = diagnostic;
-                        diagnostics.set(uri, [...currentUriDiagnostics, diagnostic]);
+
+                        // If the diagnostic comes from a macro expansion the URI is going to be an invalid URI.
+                        // Save the diagnostic for when we get the related information which has the macro expansion location
+                        // that should be used as the correct URI.
+                        if (this.isValidUri(uri)) {
+                            diagnostics.set(uri, [...currentUriDiagnostics, diagnostic]);
+                        } else {
+                            lastDiagnosticNeedsSaving = true;
+                        }
                     }
                 }),
                 swiftExecution.onDidClose(done)
             );
         });
     }
 
+    private isValidUri(uri: string): boolean {
+        try {
+            fs.accessSync(uri, fs.constants.F_OK);
+            return true;
+        } catch {
+            return false;
+        }
+    }
+
     private parseDiagnostic(
         line: string
     ): ParsedDiagnostic | vscode.DiagnosticRelatedInformation | undefined {
-        const diagnosticRegex = /^(.*?):(\d+)(?::(\d+))?:\s+(warning|error|note):\s+([^\\[]*)/g;
+        const diagnosticRegex =
+            /^(?:\S+\s+)?(.*?):(\d+)(?::(\d+))?:\s+(warning|error|note):\s+([^\\[]*)/g;
         const match = diagnosticRegex.exec(line);
         if (!match) {
             return;

test/integration-tests/DiagnosticsManager.test.ts

@@ -56,7 +56,7 @@ function assertHasDiagnostic(uri: vscode.Uri, expected: vscode.Diagnostic): vsco
     assert.notEqual(
         diagnostic,
         undefined,
-        `Could not find diagnostic matching:\n${JSON.stringify(expected)}`
+        `Could not find diagnostic matching:\n${JSON.stringify(expected)}\nDiagnostics:\n${JSON.stringify(diagnostics)}`
     );
     return diagnostic!;
 }
@@ -66,7 +66,7 @@ function assertWithoutDiagnostic(uri: vscode.Uri, expected: vscode.Diagnostic) {
     assert.equal(
         diagnostics.find(findDiagnostic(expected)),
         undefined,
-        `Unexpected diagnostic matching:\n${JSON.stringify(expected)}`
+        `Unexpected diagnostic matching:\n${JSON.stringify(expected)}\nDiagnostics:\n${JSON.stringify(diagnostics)}`
     );
 }
 
@@ -136,6 +136,23 @@ suite("DiagnosticsManager Test Suite", async function () {
             );
             expectedFuncErrorDiagnostic.source = "swiftc";
 
+            const expectedMacroDiagnostic = new vscode.Diagnostic(
+                new vscode.Range(new vscode.Position(17, 26), new vscode.Position(17, 26)),
+                "No calls to throwing functions occur within 'try' expression",
+                vscode.DiagnosticSeverity.Warning
+            );
+
+            expectedMacroDiagnostic.source = "swiftc";
+            expectedMacroDiagnostic.relatedInformation = [
+                {
+                    location: {
+                        uri: mainUri,
+                        range: expectedMacroDiagnostic.range,
+                    },
+                    message: "Expanded code originates here",
+                },
+            ];
+
             // SourceKit-LSP sometimes sends diagnostics
             // after first build and can cause intermittent
             // failure if `swiftc` diagnostic is fixed
@@ -161,6 +178,9 @@ suite("DiagnosticsManager Test Suite", async function () {
                 // Should have parsed correct severity
                 assertHasDiagnostic(mainUri, expectedWarningDiagnostic);
                 assertHasDiagnostic(mainUri, expectedMainErrorDiagnostic);
+                if (workspaceContext.swiftVersion.isGreaterThanOrEqual(new Version(6, 0, 0))) {
+                    assertHasDiagnostic(mainUri, expectedMacroDiagnostic);
+                }
                 // Check parsed for other file
                 assertHasDiagnostic(funcUri, expectedFuncErrorDiagnostic);
             }).timeout(2 * 60 * 1000); // Allow 2 minutes to build
@@ -183,6 +203,9 @@ suite("DiagnosticsManager Test Suite", async function () {
                 // Should have parsed severity
                 assertHasDiagnostic(mainUri, expectedWarningDiagnostic);
                 assertHasDiagnostic(mainUri, expectedMainErrorDiagnostic);
+                if (workspaceContext.swiftVersion.isGreaterThanOrEqual(new Version(6, 0, 0))) {
+                    assertHasDiagnostic(mainUri, expectedMacroDiagnostic);
+                }
                 // Check parsed for other file
                 assertHasDiagnostic(funcUri, expectedFuncErrorDiagnostic);
             }).timeout(2 * 60 * 1000); // Allow 2 minutes to build
@@ -198,6 +221,9 @@ suite("DiagnosticsManager Test Suite", async function () {
 
                 // Should have parsed severity
                 assertHasDiagnostic(mainUri, expectedWarningDiagnostic);
+
+                // llvm style doesn't do macro diagnostics
+                assertWithoutDiagnostic(mainUri, expectedMacroDiagnostic);
                 const diagnostic = assertHasDiagnostic(mainUri, expectedMainErrorDiagnostic);
                 // Should have parsed related note
                 assert.equal(diagnostic.relatedInformation?.length, 1);